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

Conditional feature activation based on profiles does not work for cyclical dependency #9245

Closed
kartva opened this issue Mar 7, 2021 · 9 comments
Labels
A-dev-dependencies Area: [dev-dependencies] A-diagnostics Area: Error and warning messages generated by Cargo itself. C-bug Category: bug

Comments

@kartva
Copy link

kartva commented Mar 7, 2021

Problem

[package]
name = "test-features"

[features]
add_two = []

[dev-dependencies]
test-features = { path = ".", features = ["add_two"] }

Does not behave as expected, as running the binary with -no-default-features and --release still activates the feature. To be fair, as far as I know, there is no expected behavior (See: #3921).

Steps

  1. Clone this repository containing a sample rust project
  2. cargo run --no-default-features --release prints both lines, even though the feature two is not supposed to be active.

Possible Solution(s)

Notes

Output of cargo version:
cargo 1.50.0 (f04e7fab7 2021-02-04)

@kartva kartva added the C-bug Category: bug label Mar 7, 2021
@ehuss
Copy link
Contributor

ehuss commented Mar 10, 2021

Can you say more about why you are trying to define a cyclical dependency like that? It is highly unusual, and I suspect it may not mean what you intend (this adds a duplicate copy of the package).

@kartva
Copy link
Author

kartva commented Mar 11, 2021

As per #2911, the motivation is to enable crate features while running cargo test.

@ehuss ehuss added A-dev-dependencies Area: [dev-dependencies] A-diagnostics Area: Error and warning messages generated by Cargo itself. labels Mar 11, 2021
@kartva
Copy link
Author

kartva commented Mar 12, 2021

Erm, Cargo doesn't generate an error or warning for this bug. A-errors label is for issues that generate a warning or error, right?

@ehuss
Copy link
Contributor

ehuss commented Mar 12, 2021

Yea. I think path="." probably shouldn't be allowed, though I'm uncertain if there are any legitimate use cases.

@kartva
Copy link
Author

kartva commented Mar 12, 2021

Agreed, as per #3921, this behavior was discussed and the conclusion was to let it go.

As for the use cases, it was to enable specific features of the current package during testing.

@sander2
Copy link

sander2 commented Mar 19, 2021

To add to this, the same problem occurs when depending on an external crate - in my case I normally depend on crate x without any features, but for testing I need crate x with feature y.

@kartva
Copy link
Author

kartva commented Mar 19, 2021

This may be indicative of a bigger bug than previously thought, since the behaviour @sander2 has brought forward is supposed to be defined, but is not followed. This is worth filing another issue over.

@kartva kartva changed the title Conditional feature activation based on profiles does not work for cyclical dependency Conditional feature activation based on test profile enables feature for all profiles Mar 19, 2021
@kartva kartva changed the title Conditional feature activation based on test profile enables feature for all profiles Conditional feature activation based on profiles does not work for cyclical dependency Mar 19, 2021
@DanielJoyce
Copy link

DanielJoyce commented May 10, 2021

Seems related to #1796 which describes this exact same behavior..?

Supposedly fixed here: #7916 <<< Funnily enough, a permutation of the first issue number!

Can you test with nightly feature and confirm it works?

hmm, that was merged in januaray. So it looks like this is still an issue for circular dependencies involving the local crate. I guess worst case I can split my integration tests into a seperate package.

@Eh2406
Copy link
Contributor

Eh2406 commented May 27, 2021

@DanielJoyce #7916 will require you to opt in with resolver="2" in your Cargo.toml. If you are still having a problem with that applied, please file a new issue.

I am closing in faver of #9518

@Eh2406 Eh2406 closed this as completed May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dev-dependencies Area: [dev-dependencies] A-diagnostics Area: Error and warning messages generated by Cargo itself. C-bug Category: bug
Projects
None yet
Development

No branches or pull requests

5 participants