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

Don't unify "real" and dev dependencies #262

Merged
merged 1 commit into from
Jul 7, 2022
Merged

Conversation

tgeoghegan
Copy link
Contributor

This commit moves the library and binaries targets to resolver = "2"
so that dev-dependencies don't get pulled in when building non-test
configurations of the prio library. Up until now, the crate releases
were enabling feature test-vector, unnecessarily including the
test_vector module and its dependencies. See 1 for details on
dependency resolution with the new resolver.

@tgeoghegan tgeoghegan requested a review from a team as a code owner July 7, 2022 16:14
@tgeoghegan
Copy link
Contributor Author

This one has an extra nuance that didn't turn up in divviup/janus#294: here, binaries/Cargo.toml turns on feature test-vector in its regular dependencies, not exclusively in its dev-dependencies. As a consequence, if we do cargo build --all, then prio gets built with test-vector because (AFAICT) features do get unified across the dependencies of all workspace members. However, I did verify with cargo build --verbose --lib and cargo publish --verbose that when building or publishing just the library crate, the test-vector feature does not get enabled.

@@ -5,6 +5,7 @@ edition = "2018"
description = "Prio utilities"
license = "MPL-2.0"
repository = "https://github.com/divviup/libprio-rs"
resolver = "2"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this may be extraneous because it's implied by the workspace's resolver = "2". From https://doc.rust-lang.org/edition-guide/rust-2021/default-cargo-resolver.html:

The resolver is a global setting for a workspace, and the setting is ignored in dependencies. The setting is only honored for the top-level package of the workspace.

This commit moves the library and `binaries` targets to `resolver = "2"`
so that `dev-dependencies` don't get pulled in when building non-test
configurations of the `prio` library. Up until now, the crate releases
were enabling feature `test-vector`, unnecessarily including the
`test_vector` module and its dependencies. See [1] for details on
dependency resolution with the new resolver.

[1]: rust-lang/cargo#4866
@tgeoghegan tgeoghegan merged commit d49a7ad into main Jul 7, 2022
@tgeoghegan tgeoghegan deleted the timg/resolver-2 branch October 12, 2022 19:21
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