-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Feature that affects dev dependencies can't be enabled #6915
Comments
Without this: error: failed to select a version for `serde_test_suite`. ... required by package `serde_test_suite-tests v0.0.0` versions that meet the requirements `= 0.0.0` are: 0.0.0 the package `serde_test_suite-tests` depends on `serde_test_suite`, with features: `serde` but `serde_test_suite` does not have these features. failed to select a version for `serde_test_suite` which could resolve this conflict Seems like a Cargo bug -- I will minimize and report.
This should finally fix the nightly configuration on travis
This PR breaks the circle dependencies between grpcio and grpcio-proto by adding a new crate named tests and examples. We can bring things back once rust-lang/cargo#6915 is fixed. The new structure also fixes a problem that crates.io denies to accept circle dependencies in dev-dependencies. More see rust-lang/cargo#4242. This PR also updates protobuf-build to the latest version. Note that until protobuf-build is released, we can't release grpcio-proto.
This PR breaks the circle dependencies between grpcio and grpcio-proto by adding a new crate named tests and examples. We can bring things back once rust-lang/cargo#6915 is fixed. The new structure also fixes a problem that crates.io denies to accept circle dependencies in dev-dependencies. More see rust-lang/cargo#4242. This PR also updates protobuf-build to the latest version.
This PR breaks the circle dependencies between grpcio and grpcio-proto by adding a new crate named tests and examples. We can bring things back once rust-lang/cargo#6915 is fixed. The new structure also fixes a problem that crates.io denies to accept circle dependencies in dev-dependencies. More see rust-lang/cargo#4242. This PR also updates protobuf-build to the latest version.
Is there a known workaround for this except moving the deps to the |
Known workaround for rust-lang/cargo#6915
We're sort of in a weird spot as it is, we have features within `nearcore` that are required but can't be enabled if they depend on crates listed under `dev-dependencies` (bug in cargo: rust-lang/cargo#6915). Temporarily listing them under `dependencies`, fixed the CI testing failures we had while working on #4292 (same error @ailisp highlighted: #4333 (comment)), but as pointed out in #4331 (comment), this method should be out of the question. If we remove it, however, we can't work with the tests that depend on those features. This PR moves the _previously top-level_ tests into a new crate to have better dependency and feature handling as @matklad suggested > * [move tests from `/test` folder somewhere -- either to some of the existing packages (neard perhaps?) or into a new `integration-tests` package](#4292 (comment)) This would ensure we have dependencies like `testlib`, `runtime-params-estimator` and `restored-receipts-verifier` that are only needed for testing and depend on nearcore without cyclic dependency issues while having all features relevant to testing in order. The features within `neard` have been reduced to proxies to `nearcore` features, which partially resolves #4325 (though the intent for that has morphed a bit). This also makes for a cleaner dependency graph from `neard` perspective, removing extraneous dependencies that were previously required. I also noticed removed the `old_tests` feature that should've been removed along with the rest of it in #928. Updated some docs and code comments referencing the old `neard` path too.
As a workaround for rust-lang/cargo#6915
I ran $ cargo new --lib a
$ cargo new --lib b
$ cargo new --lib c And replaced the I then ran $ cargo check
$ cargo check -F f And it worked. Seems like this was fixed (maybe when weak/namespaced features were added?). Closing. If there is something I missed, let us know! |
Thank you! The fix bisects to nightly-2021-11-24. The commit range is rust-lang/rust@936f260...65c55bf, in which the only Cargo update is rust-lang/rust#91144. In there, I bet it was #10103, although the PR description does not directly describe the scenario in this issue. |
Add regression test for issue 6915: features and transitive dev deps Closes #6915. That issue was fixed without realizing by #10103. In search of test coverage, I looked through `rg '[a-z0-9_-]+ = \["[a-z0-9_-]+/[a-z0-9_-]+"\]' tests/testsuite/` and did not find any existing test that exercises this situation. The tests added by #10103 are focused on cycles and `cargo tree`, which is substantially different from #6915. Tested by `cargo test --test testsuite -- features::activating_feature_does_not_activate_transitive_dev_dependency`.
Ref: rust-lang/cargo#6915. This pr is used to format relevant `cargo.toml` files and tidy the test crates in the `[dependencies]` section, by moving them to `[dev-dependencies]`. By introducing this pr, the unnecessary compilation on test crates when building the release output can be ignored. Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>
Problem
I have crates A, B, C with the following manifests:
Expected behavior: I believe it should be possible to build crate A. Running
cargo check
in crate A should build crate A without feature f and crate B without feature f. Runningcargo check --features f
in crate A should build crate A with feature f and crate B with feature f, but not C because that is a dev dependency of a dependency.Actual behavior: crate A cannot be built, with or without f.
Possible objection: "but B/f requires C/f, we can't enable B/f without building C!" I don't think this is valid because
cargo check --features f
in crate B will already build crate B with feature f enabled without building the dev dependency C.Notes
cargo 1.36.0-nightly (beb8fcb 2019-04-30)
The text was updated successfully, but these errors were encountered: