Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Relax dcou dep. in order-crates-for-publishing.py #32432

Merged
merged 3 commits into from
Jul 13, 2023
Merged

Relax dcou dep. in order-crates-for-publishing.py #32432

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4f11fa8
Relax dcou dep. in order-crates-for-publishing.py
ryoqun Jul 10, 2023
15cf9c4
Comment, comment and comment...
ryoqun Jul 11, 2023
44e6513
Avoid early-abort and report all collected errors
ryoqun Jul 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion ci/order-crates-for-publishing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,36 @@ def load_metadata():
return json.loads(subprocess.Popen(
cmd, shell=True, stdout=subprocess.PIPE).communicate()[0])

no_explicit_version_specified = '*'

def is_special_cased_self_dep(package, dependency):
is_special_cased = False

# Sometimes self-dev-dep with dev-context-only-utils is needed and this dep
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of "self-dev-dep" in this comment throws me off. Maybe it can be expanded or more context added.

It looks like is_special_cased_self_dep only cares about the dev-context-only-util crate, so I'd rather it be more explicit in the function name that this is really only about that one crate

Copy link
Member Author

@ryoqun ryoqun Jul 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha, no wonder for throwing you off. the proper commenting resulted in quite a high wall of text: 15cf9c4

yeah, i needed fresh eyes to spell the hidden context around dev-context-only-utils out. thanks for review. :)

# pattern is actually legalized by cargo:
# https://releases.rs/docs/1.40.0/#cargo
# https://github.com/rust-lang/cargo/pull/7333
if (dependency['kind'] == 'dev' and
dependency['name'] == package['name'] and
'dev-context-only-utils' in dependency['features'] and
'path' in dependency):
if dependency['req'] == no_explicit_version_specified:
is_special_cased = True
else:
sys.exit(
'Error: wrong dev-context-only-utils circular dependency. try: ' +
'{} = {{ path = ".", features = {} }}\n'
.format(dependency['name'], json.dumps(dependency['features']))
)

return is_special_cased

def should_check(package, dependency):
is_related_to_solana = dependency['name'].startswith('solana')
return (
is_related_to_solana and not is_special_cased_self_dep(package, dependency)
)

def get_packages():
metadata = load_metadata()

Expand All @@ -30,7 +60,7 @@ def get_packages():
dependency_graph = dict()
for pkg in metadata['packages']:
manifest_path[pkg['name']] = pkg['manifest_path'];
dependency_graph[pkg['name']] = [x['name'] for x in pkg['dependencies'] if x['name'].startswith('solana')];
dependency_graph[pkg['name']] = [x['name'] for x in pkg['dependencies'] if should_check(pkg, x)];

# Check for direct circular dependencies
circular_dependencies = set()
Expand Down