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.

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

Fix ignoring optional deps when namespaced deps are used. #241

Merged
merged 1 commit into from
Mar 27, 2024

Conversation

xStrom
Copy link
Contributor

@xStrom xStrom commented Mar 23, 2024

Thanks for making cargo-hack!

I've been lately incorporating it into the Linebender CI that we use across a bunch of projects. Its --workspace and --each-feature functionalities have been especially useful.

When I was trying out the new CI script for our Kurbo project I found that cargo-hack does not handle Kurbo's optional dependecies (e.g. libm) correctly. That is with --optional-deps and even with --features libm. Upon looking closer I identified that cargo-hack is not behaving correctly because Kurbo makes use of namespaced dependencies.

In this PR I redesigned the namespaced_features test to be more comprehensive and better match what Cargo does.

The old test incorrectly stated:

When namespace dependency is used, other optional dependencies are also not treated as implicit features.

Cargo does not treat optional dependencies as implicit features only when that specific dependency has been referenced with the dep: prefix in an explicit feature. All other optional dependencies remain implicit features as before.

The actual code changes to get the test to pass were luckily minimal.

Copy link
Owner

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

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

Thanks!

@taiki-e taiki-e merged commit 0c409b7 into taiki-e:main Mar 27, 2024
27 checks passed
@taiki-e
Copy link
Owner

taiki-e commented Mar 27, 2024

Published in v0.6.23.

@xStrom xStrom deleted the namespaced branch March 27, 2024 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants