search

rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.56k stars 2.38k forks source link

Issue with Workspace and Crate features #14362

Open gwbres opened 1 month ago

gwbres commented 1 month ago

Problem

Hello, we're running into severe troubles in the RINEX workspace, which has quite a few build options and integrates inner dependencies (path = "../" within the workspace).

My explanation is that Cargo picks up the worst case scenario, in terms of dependencies and it kills the capabilities to detect issues that exist with for example a simple build -r while actually running build -r within the workspace.

The worst case scenario (in terms of dependencies) in our case, is the top level -cli application, which requires pretty much --all-features in the local libs.

Steps

Download the RINEX workspace commit 70242aa09551f12ea511ea36b528210ee33a022a for example.

The CI did pass, because building within the workspace does pass.

Now try this

[package]
name = "tester"

[dependencies]
rinex = { git = "https://github.com/georust/rinex", rev = "70242aa09551f12ea511ea36b528210ee33a022a" }
fn main() {}

Possible Solution(s)

No response

Notes

No response

Version

No response

weihanglo commented 1 month ago

Could you provide

Thank you.

gwbres commented 1 month ago

Hello

  • The Rust toolchain version you use

rustc 1.77.2, but I doubt the toolchain impacts this problem, it's more of a Cargo problem. The version of cargo seems tied to the toolchain, exact same version

  • The expected output

The build should have failed when building within the workspace. When importing the rinex lib as an externaly dependency, the build is "detached" from the workspace, and actually fails.

  • The actual error you've got

We have code portions that should only exist within some features. When building within the workspace, it looks like Cargo resolves the "worst case scenario" in terms of dependency, which would be "all-features" for all libs, within this very workspace, due to our high level applications. Builds with more "basic setup" , like build -r should not have passed because we have errors in that very repo state. build -r in the workspace does not apply build -r to all crates within said workspace

  • The minimal complete reproduction (I don't know what command you've run)

Any build command will do. If you run build -r within the workspace, it falsely passes. If you build the example demo (simple import) it rightfully fails

gwbres commented 1 month ago

cargo build -r -p $pkg does not work either, because any job at the root of the workspace, shares unique build products and has the same dependency solving "issue".

One answer to our problem, is to do cd $pkg && cargo build -r to trigger a default build within $pkg correctly. I reworked our CI scripts.

gwbres commented 1 month ago

I'm not sure this actually falls in a Cargo Issue. The conclusion is the Cargo Workspace does not behave like I thought it would, and that our CI script solely relied on build at the root of the workspace. But i'm pretty sure this behavior is kind of problematic or at least, needs to be emphasized somewhere. This behavior only impacts complex workspaces, like ours, tokio and other similar

epage commented 1 month ago

cargo build -r -p $pkg

cd $pkg && cargo build -r

Should behave the same except for loading of .cargo/config.toml / rustup toolchain files.

Its still not quite clear what problem you are having but it sounds like its related to feature unification. When Cargo builds, it takes all selected packages (whether implicit or explicitly selected) and merges the enabled features together to do a single build of every dependency rather than a build per selected package. People tend to work around this by either selecting specific packages or by having a "cargo workspace hack" which is a code-genned workspace member that every package depends on to force on a set of dependency features in a consistent way across the entire dependency tree.

See also

gwbres commented 1 month ago

Should behave the same except for loading of .cargo/config.toml / rustup toolchain files.

Clearly does not ! I reworked the CI script and cd in each individual packages

it sounds like its related to feature unification

totally ! 100 %. The problem is we have a -cli app (high level) that requires "--all-features" to local packages. The workspace picks up "--all-features" as the requirement to fulfill and we wind up not being able to deploy a simpler build.

It is problematic though that -p $pkg does not trigger a simpler build anyways

it takes all selected packages (whether implicit or explicitly selected) and merges the enabled features together to do a single build of every dependency rather than a build per selected package

Which is totally understandable and kind of answers the question. When we request a Default build in our workspace, the -cli application Default is the most demanding Default case, and winds up activating --all-features in other packages. Bottom line, we're unable to test "Default builds" without moving to inner packages, in our scenario.

This is totally tied to our very setup, if we did not have -cli application, we would not have such an issue for example

See also

Thank you very much !

epage commented 1 month ago

Clearly does not ! I reworked the CI script and cd in each individual packages

I can't add much more without concrete reproduction steps.

totally ! 100 %. The problem is we have a -cli app (high level) that requires "--all-features" to local packages.

As in you are doing cargo build --all-features and it only works then? I don't see anything about that in the original Issue description. Are there steps that are missing?

Bottom line, we're unable to test "Default builds" without moving to inner packages, in our scenario.

The cargo plugin cargo hack is a great utility for working around feature unification when it is undesired.

gwbres commented 1 month ago

The reproduction steps, consists in creating a basic Main application from the code I gave, and running cargo build

As in you are doing cargo build --all-features and it only works then? I don't see anything about that in the original Issue description. Are there steps that are missing?

not exactly, doing cargo build within our workspace, actually winds up doing cargo build --all-features due to the dependencies of our highest level application

The cargo plugin cargo hack is a great utility for working around feature unification when it is undesired

need to take a lookt at this

epage commented 1 month ago

cargo build -r -p $pkg

cd $pkg && cargo build -r

Should behave the same except for loading of .cargo/config.toml / rustup toolchain files.

Clearly does not ! I reworked the CI script and cd in each individual packages

I can't add much more without concrete reproduction steps.

The reproduction steps, consists in creating a basic Main application from the code I gave, and running cargo build

I put in all the quotes to make sure context is fully there. I'm not seeing how creating a basic "main" is relevant to those steps. Please write out explicit reproduction steps of a git clone operation, the build commands, and their output.

not exactly, doing cargo build within our workspace, actually winds up doing cargo build --all-features due to the dependencies of our highest level application

I'm not guess its not literally doing all-features then but instead "its almost as if --all-features".

So then this sounds like its just a matter of feature unifcation for the packages you are selecting to build which is covered by some of those other issues I linked earlier, right?

gwbres commented 1 month ago

I put in all the quotes to make sure context is fully there. I'm not seeing how creating a basic "main" is relevant to those steps. Please write out explicit reproduction steps of a git clone operation, the build commands, and their output.

No git cloning involved. Everything is proposed in the reproducible example above there.

It's about using one of our libs, which is said to build fine in default mode (because it does pass within the workspace) but not when you actually use it.

Here's a reproducible example, that only requires to "copy pasting"

echo "[package]" >> Cargo.toml
echo "name = \"tester\"" >> Cargo.toml

echo "[dependencies]" >> Cargo.toml
echo "rinex = { git = \"https://github.com/georust/rinex\", rev = \"70242aa09551f12ea511ea36b528210ee33a022a\" }" >> Cargo.toml

mkdir src
touch src/main.rs
echo "fn main() {}" >> src/main.rs
cargo build
epage commented 1 month ago

That is, I assume, reproduction steps for the issue. I was asking about reproduction steps for cargo build -p pkgand cd pkg && cargo build behaving differently. As those steps you gave do not include a workspace, i can't see how they can reproduce that issue.

weihanglo commented 1 month ago

Just fly-by. Two issues with clear reproduction steps for reference：

Since this seems to be a comparison between two different invocations of cargo, if you could provide their steps and outputs side-by-side, and show what the expected outputs are verbatimly（surely can omit unimportant parts), that would be helpful.