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

Profile overrides warning #7378

Closed
est31 opened this issue Sep 18, 2019 · 3 comments · Fixed by #7536
Closed

Profile overrides warning #7378

est31 opened this issue Sep 18, 2019 · 3 comments · Fixed by #7536
Labels
A-profile-overrides Area: profile-overrides C-bug Category: bug

Comments

@est31
Copy link
Member

est31 commented Sep 18, 2019

I'm using the profile-overrides feature to optimize one specific crate in my workspace, like this:

[workspace]
members = ["base-crate", "user-crate", "opt-pkg"]

# This allows us to optimize dependencies
# If you aren't on nightly, you can just comment this out.
[profile.dev.overrides."*"]
opt-level = 3

# Also optimize opt-pkg
# Per default, workspace members are not included by "*".
[profile.dev.overrides.opt-pkg]
opt-level = 3

base-crate depends on no crate from the workspace, while the other two depend on base-crate and user-crate depends on opt-pkg. base-crate also has a binary target.

This works nicely so far. When I do cargo run -p base-crate though, I get this warning:

warning: profile override spec `opt-pkg` did not match any packages

Nothing bad happens but IMO the warning is redundant. You don't get the warning when doing cargo run -p user-crate as it depends on opt-pkg. I think this is because only the resolution DAG of the package built is used. The warning should only fire if the crate can't be found in all of Cargo.lock (combined resolution DAG for all workspace crates).

cc rust-lang/rust#48683

@est31 est31 added the C-bug Category: bug label Sep 18, 2019
@alexcrichton
Copy link
Member

I agree that the warning shouldn't be emitted here and we're probably doing so a little too eagerly.

cc @ehuss as well

@ehuss ehuss added the A-profile-overrides Area: profile-overrides label Sep 21, 2019
@ehuss
Copy link
Contributor

ehuss commented Sep 26, 2019

The problem is that it needs the full workspace resolve from this code path. I don't see a way to access that from the existing resolve.

Maybe that function should return two resolvers (the "full" one, and the minimal one)?

Or maybe merge_from could add a new field to Resolve to track the "workspace-wide" set of packages (it just needs PackageIds)? I'm not sure how fragile that will be with options like -Zavoid-dev-deps.

cc @Eh2406 for question about the resolver.

@Eh2406
Copy link
Contributor

Eh2406 commented Sep 27, 2019

I don't have a good sense of how workspaces interact with the resolver, but I think we have several bugs related to different folders in a workspace seeing different resolutions. So having a workspace-resolution sounds like it makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-profile-overrides Area: profile-overrides C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants