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

Enabling features from [replace] or [patch] doesn't work #3034

Closed
jdm opened this issue Aug 23, 2016 · 6 comments · Fixed by #9681
Closed

Enabling features from [replace] or [patch] doesn't work #3034

jdm opened this issue Aug 23, 2016 · 6 comments · Fixed by #9681
Assignees
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself. A-features Area: features — conditional compilation A-overrides Area: general issues with overriding dependencies (patch, replace, paths) A-patch Area: [patch] table override A-replace Area: [replace] table override E-easy Experience: Easy

Comments

@jdm
Copy link

jdm commented Aug 23, 2016

If I add a path replacement for an existing dependency in my Cargo.toml, then add a nonexistent feature to it (eg. "js:0.1.3" = {path = "../rust-mozjs", features = ["nonexistent_feature"]}, Cargo does not complain. This implies to me that it's also impossible to enable existing features, since I'm having trouble with that as well.

@alexcrichton
Copy link
Member

Was the js crate at version 0.1.3? If the [replace] directive isn't used I don't believe it's checked for validity right now.

@jdm
Copy link
Author

jdm commented Aug 24, 2016

It was, yes.

@alexcrichton
Copy link
Member

Ok, finally got around to investigating this.

Yes it's true that Cargo today currently ignores features and default-features directives in [replace] sections. The logic currently just hooks into whenever candidates are loaded for a dependency a [replace] section will override any candidates returned to something else. This means that the feature activation logic, which happens in a separate step, ignores the actual [replace] line itself.

This raises an interesting question to me, though. Should we try to read features or default-features of [replace]? How does that interact with features requested by the original dependency edge we're replacing? My first inclination would be that we should reject features and default-features in a [replace] section (not possible today obviously, but we can migrate with a warning). The purpose of replace to me is "replace this node in the graph" rather than "tweak all incoming edges to this node in the dependency graph". Feature activations are properties of an edge, so that's where I think we should ignore features specified in [replace].

Thoughts? @wycats?

@alexcrichton alexcrichton added the A-features Area: features — conditional compilation label Jun 22, 2017
@kellda
Copy link
Contributor

kellda commented Jun 6, 2020

Isn't this the same as #3033 ?

@jdm
Copy link
Author

jdm commented Jun 6, 2020

This issue is about changing the set of features in a [replace] section. The other is about enabling a feature inside of a [patch] directive that only exists in a local override.

@ehuss ehuss changed the title Enabling features from [replace] doesn't work Enabling features from [replace] or [patch] doesn't work Jun 7, 2020
@ehuss ehuss added A-overrides Area: general issues with overriding dependencies (patch, replace, paths) A-patch Area: [patch] table override A-replace Area: [replace] table override labels Jun 7, 2020
@ehuss ehuss added A-diagnostics Area: Error and warning messages generated by Cargo itself. E-easy Experience: Easy labels Jul 2, 2021
@Rustin170506
Copy link
Member

@rustbot claim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself. A-features Area: features — conditional compilation A-overrides Area: general issues with overriding dependencies (patch, replace, paths) A-patch Area: [patch] table override A-replace Area: [replace] table override E-easy Experience: Easy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants