From bd36ce1b00ade1d77932efa845579af2445ee566 Mon Sep 17 00:00:00 2001 From: kevinheavey Date: Wed, 7 Aug 2024 16:34:20 +0400 Subject: [PATCH 1/5] ignore path dev deps in circular deps check --- ci/order-crates-for-publishing.py | 48 +++++++------------------------ 1 file changed, 11 insertions(+), 37 deletions(-) diff --git a/ci/order-crates-for-publishing.py b/ci/order-crates-for-publishing.py index 855f0e89d17407..5af816547cad0a 100755 --- a/ci/order-crates-for-publishing.py +++ b/ci/order-crates-for-publishing.py @@ -21,52 +21,26 @@ def load_metadata(): return json.loads(subprocess.Popen( cmd, shell=True, stdout=subprocess.PIPE).communicate()[0]) -# Consider a situation where a crate now wants to use already existing -# developing-oriented library code for their integration tests and benchmarks, -# like creating malformed data or omitting signature verifications. Ideally, -# the code should have been guarded under the special feature -# `dev-context-only-utils` to avoid accidental misuse for production code path. -# -# In this case, the feature needs to be defined then activated for the crate -# itself. To that end, the crate actually needs to depend on itself as a -# dev-dependency with `dev-context-only-utils` activated, so that the feature -# is conditionally activated only for integration tests and benchmarks. In this -# way, other crates won't see the feature activated even if they normal-depend -# on the crate. -# -# This self-referencing dev-dependency can be thought of a variant of -# dev-dependency cycles and it's well supported by cargo. The only exception is -# when publishing. In general, cyclic dev-dependency doesn't work nicely with -# publishing: https://github.com/rust-lang/cargo/issues/4242 . -# -# However, there's a work around supported by cargo. Namely, it will ignore and -# strip these cyclic dev-dependencies when publishing, if explicit version -# isn't specified: https://github.com/rust-lang/cargo/pull/7333 (Released in -# rust 1.40.0: https://releases.rs/docs/1.40.0/#cargo ) -# -# This script follows the same safe discarding logic to exclude these -# special-cased dev dependencies from its `dependency_graph` and further -# processing. -def is_self_dev_dep_with_dev_context_only_utils(package, dependency, wrong_self_dev_dependencies): +# Cargo publish if fine with circular dev-dependencies if +# they are path deps. +def is_path_dev_dep(package, dependency, wrong_path_dev_dependencies): no_explicit_version = '*' - is_special_cased = False if (dependency['kind'] == 'dev' and dependency['name'] == package['name'] and - 'dev-context-only-utils' in dependency['features'] and 'path' in dependency): is_special_cased = True if dependency['req'] != no_explicit_version: # it's likely `{ workspace = true, ... }` is used, which implicitly pulls the # version in... - wrong_self_dev_dependencies.append(dependency) + wrong_path_dev_dependencies.append(dependency) return is_special_cased -def should_add(package, dependency, wrong_self_dev_dependencies): +def should_add(package, dependency, wrong_path_dev_dependencies): related_to_solana = dependency['name'].startswith('solana') - self_dev_dep_with_dev_context_only_utils = is_self_dev_dep_with_dev_context_only_utils( - package, dependency, wrong_self_dev_dependencies + self_dev_dep_with_dev_context_only_utils = is_path_dev_dep( + package, dependency, wrong_path_dev_dependencies ) return related_to_solana and not self_dev_dep_with_dev_context_only_utils @@ -78,12 +52,12 @@ def get_packages(): # Build dictionary of packages and their immediate solana-only dependencies dependency_graph = dict() - wrong_self_dev_dependencies = list() + wrong_path_dev_dependencies = list() for pkg in metadata['packages']: manifest_path[pkg['name']] = pkg['manifest_path']; dependency_graph[pkg['name']] = [ - x['name'] for x in pkg['dependencies'] if should_add(pkg, x, wrong_self_dev_dependencies) + x['name'] for x in pkg['dependencies'] if should_add(pkg, x, wrong_path_dev_dependencies) ]; # Check for direct circular dependencies @@ -95,13 +69,13 @@ def get_packages(): for dependency in circular_dependencies: sys.stderr.write('Error: Circular dependency: {}\n'.format(dependency)) - for dependency in wrong_self_dev_dependencies: + for dependency in wrong_path_dev_dependencies: sys.stderr.write('Error: wrong dev-context-only-utils circular dependency. try: ' + '{} = {{ path = ".", features = {} }}\n' .format(dependency['name'], json.dumps(dependency['features'])) ) - if len(circular_dependencies) != 0 or len(wrong_self_dev_dependencies) != 0: + if len(circular_dependencies) != 0 or len(wrong_path_dev_dependencies) != 0: sys.exit(1) # Order dependencies From 7288c74ea01fec507681cdeb460402569700833b Mon Sep 17 00:00:00 2001 From: kevinheavey Date: Tue, 13 Aug 2024 13:44:07 +0400 Subject: [PATCH 2/5] update error message --- ci/order-crates-for-publishing.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ci/order-crates-for-publishing.py b/ci/order-crates-for-publishing.py index 5af816547cad0a..2bf45072afb279 100755 --- a/ci/order-crates-for-publishing.py +++ b/ci/order-crates-for-publishing.py @@ -70,9 +70,9 @@ def get_packages(): for dependency in circular_dependencies: sys.stderr.write('Error: Circular dependency: {}\n'.format(dependency)) for dependency in wrong_path_dev_dependencies: - sys.stderr.write('Error: wrong dev-context-only-utils circular dependency. try: ' + - '{} = {{ path = ".", features = {} }}\n' - .format(dependency['name'], json.dumps(dependency['features'])) + sys.stderr.write('Error: wrong circular dev dependency. try using a path dependency: ' + + '{} = {{ path = "$path_to_crate" }}\n' + .format(dependency['name']) ) if len(circular_dependencies) != 0 or len(wrong_path_dev_dependencies) != 0: From 5e18eaf084cecb580064f4cb8f48831460f445f6 Mon Sep 17 00:00:00 2001 From: kevinheavey Date: Tue, 13 Aug 2024 13:44:40 +0400 Subject: [PATCH 3/5] update variable name --- ci/order-crates-for-publishing.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/order-crates-for-publishing.py b/ci/order-crates-for-publishing.py index 2bf45072afb279..443170e746bb0f 100755 --- a/ci/order-crates-for-publishing.py +++ b/ci/order-crates-for-publishing.py @@ -39,11 +39,11 @@ def is_path_dev_dep(package, dependency, wrong_path_dev_dependencies): def should_add(package, dependency, wrong_path_dev_dependencies): related_to_solana = dependency['name'].startswith('solana') - self_dev_dep_with_dev_context_only_utils = is_path_dev_dep( + path_dev_dep = is_path_dev_dep( package, dependency, wrong_path_dev_dependencies ) - return related_to_solana and not self_dev_dep_with_dev_context_only_utils + return related_to_solana and not path_dev_dep def get_packages(): metadata = load_metadata() From fe695c83d33fb068c0608617b962af600627d7fa Mon Sep 17 00:00:00 2001 From: kevinheavey Date: Tue, 13 Aug 2024 13:46:55 +0400 Subject: [PATCH 4/5] include agave-* crates --- ci/order-crates-for-publishing.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ci/order-crates-for-publishing.py b/ci/order-crates-for-publishing.py index 443170e746bb0f..17b4163ab75e68 100755 --- a/ci/order-crates-for-publishing.py +++ b/ci/order-crates-for-publishing.py @@ -38,7 +38,8 @@ def is_path_dev_dep(package, dependency, wrong_path_dev_dependencies): return is_special_cased def should_add(package, dependency, wrong_path_dev_dependencies): - related_to_solana = dependency['name'].startswith('solana') + name = dependency['name'] + related_to_solana = name.startswith('solana') or name.startswith('agave') path_dev_dep = is_path_dev_dep( package, dependency, wrong_path_dev_dependencies ) From bd78d32bb5c77b4f9f50755097b50043d2d2c16b Mon Sep 17 00:00:00 2001 From: kevinheavey Date: Tue, 13 Aug 2024 13:48:11 +0400 Subject: [PATCH 5/5] update comment --- ci/order-crates-for-publishing.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ci/order-crates-for-publishing.py b/ci/order-crates-for-publishing.py index 17b4163ab75e68..fd763472934605 100755 --- a/ci/order-crates-for-publishing.py +++ b/ci/order-crates-for-publishing.py @@ -21,8 +21,11 @@ def load_metadata(): return json.loads(subprocess.Popen( cmd, shell=True, stdout=subprocess.PIPE).communicate()[0]) -# Cargo publish if fine with circular dev-dependencies if +# Cargo publish is fine with circular dev-dependencies if # they are path deps. +# However, cargo still fails if deps are path deps with versions +# (this when you use `workspace = true`): https://github.com/rust-lang/cargo/issues/4242 +# This function checks for these good and bad uses of dev dependencies. def is_path_dev_dep(package, dependency, wrong_path_dev_dependencies): no_explicit_version = '*' is_special_cased = False