Non-deterministic builds with new resolver

[kylefleming]

Problem
When using the new resolver, I'm encountering a bug where cargo seems to be unifying features inconsistently when a feature is enable on a build-dependency that is also the dependency of another dependency, when building with multiple roots (e.g. cargo build --bin bin1 --bin bin2).

It's a bit hard to narrow down the exact circumstances under which this seems to be occurring since the configuration is somewhat complex, however, I've noticed it in several other configurations, not just the one below.

The essentials seem to be:

• Crate A defines a bin, bin1
• Crate B defines a bin, bin2
• Both crates are in the same workspace
• Crate A declares a dependency on a lib, dep
• Crate B declares a build-dependency on the same lib, but doesn't also declare it as a normal dependency
• The lib, dep, is in the same configuration when declared as crate A's dependency as it is when declared as crate B's build-dependency
• The lib, dep, declares another dependenncy, subdep
• This other dependency, subdep, is configured differently between crate A's dependency tree and crate B's build-dependency tree (in the example below, crate B's build-dependency declaration enables subdep's feat feature)
• When building both bins in the same cargo build invocation, crate B's build-dependency on dep is built against a build of subdep that is sometimes compiled in the configuration of crate A's dependency tree and sometimes compiled in the configuration of crate B's build-dependency tree.

Note: In this explanation of what's going on, I'm making the assumption that -Zfeatures=host_dep is intended to completely separate the feature unification of build-dependencies from normal dependencies. However, even if this assumption is incorrect, there is still a bug related to build determinism, since no matter the intended behavior of feature unification, I'm assuming cargo is not meant to unify features arbitrarily between successive invocations.

Steps

The following script illustrates the issue:

echo '[workspace]' > Cargo.toml
echo 'members = ["bin1", "bin2"]' >> Cargo.toml

cargo new --bin bin1
echo 'dep = { path = "../dep" }' >> bin1/Cargo.toml

cargo new --bin bin2
echo '[build-dependencies]' >> bin2/Cargo.toml
echo 'dep = { path = "../dep" }' >> bin2/Cargo.toml
echo 'subdep = { path = "../subdep", features = ["feat"] }' >> bin2/Cargo.toml
echo 'fn main() { dep::feat_func(); }' > bin2/build.rs

cargo new --lib dep
echo 'subdep = { path = "../subdep" }' >> dep/Cargo.toml
echo 'pub fn feat_func() { subdep::feat_func(); }' > dep/src/lib.rs

cargo new --lib subdep
echo '[features]' >> subdep/Cargo.toml
echo 'feat = []' >> subdep/Cargo.toml
echo 'pub fn feat_func() {' > subdep/src/lib.rs
echo '    #[cfg(feature = "feat")] println!("feat: enabled");' >> subdep/src/lib.rs
echo '    #[cfg(not(feature = "feat"))] println!("feat: not enabled");' >> subdep/src/lib.rs
echo '}' >> subdep/src/lib.rs

for i in {1..6}; do
    cargo build -vv --bin bin1 --bin bin2 -Zunstable-options -Zfeatures=host_dep 2>&1 | grep -v '     Running'
done

Sample output (the relevant parts only):

     Created binary (application) `bin1` package
     Created binary (application) `bin2` package
     Created library `dep` package
     Created library `subdep` package
   Compiling subdep v0.1.0 (/Users/kyle/code/experiments/resolver-test/subdep)
   Compiling dep v0.1.0 (/Users/kyle/code/experiments/resolver-test/dep)
   Compiling bin2 v0.1.0 (/Users/kyle/code/experiments/resolver-test/bin2)
   Compiling bin1 v0.1.0 (/Users/kyle/code/experiments/resolver-test/bin1)
[bin2 0.1.0] feat: enabled
    Finished dev [unoptimized + debuginfo] target(s) in 2.43s
       Fresh subdep v0.1.0 (/Users/kyle/code/experiments/resolver-test/subdep)
       Fresh dep v0.1.0 (/Users/kyle/code/experiments/resolver-test/dep)
       Fresh bin1 v0.1.0 (/Users/kyle/code/experiments/resolver-test/bin1)
       Fresh bin2 v0.1.0 (/Users/kyle/code/experiments/resolver-test/bin2)
    Finished dev [unoptimized + debuginfo] target(s) in 0.01s
   Compiling subdep v0.1.0 (/Users/kyle/code/experiments/resolver-test/subdep)
   Compiling dep v0.1.0 (/Users/kyle/code/experiments/resolver-test/dep)
   Compiling bin2 v0.1.0 (/Users/kyle/code/experiments/resolver-test/bin2)
   Compiling bin1 v0.1.0 (/Users/kyle/code/experiments/resolver-test/bin1)
[bin2 0.1.0] feat: not enabled
    Finished dev [unoptimized + debuginfo] target(s) in 0.80s
       Fresh subdep v0.1.0 (/Users/kyle/code/experiments/resolver-test/subdep)
       Fresh dep v0.1.0 (/Users/kyle/code/experiments/resolver-test/dep)
       Fresh bin1 v0.1.0 (/Users/kyle/code/experiments/resolver-test/bin1)
       Fresh bin2 v0.1.0 (/Users/kyle/code/experiments/resolver-test/bin2)
    Finished dev [unoptimized + debuginfo] target(s) in 0.01s
       Fresh subdep v0.1.0 (/Users/kyle/code/experiments/resolver-test/subdep)
       Fresh dep v0.1.0 (/Users/kyle/code/experiments/resolver-test/dep)
   Compiling bin1 v0.1.0 (/Users/kyle/code/experiments/resolver-test/bin1)
   Compiling bin2 v0.1.0 (/Users/kyle/code/experiments/resolver-test/bin2)
    Finished dev [unoptimized + debuginfo] target(s) in 0.25s
       Fresh subdep v0.1.0 (/Users/kyle/code/experiments/resolver-test/subdep)
       Fresh dep v0.1.0 (/Users/kyle/code/experiments/resolver-test/dep)
       Fresh bin1 v0.1.0 (/Users/kyle/code/experiments/resolver-test/bin1)
       Fresh bin2 v0.1.0 (/Users/kyle/code/experiments/resolver-test/bin2)
    Finished dev [unoptimized + debuginfo] target(s) in 0.01s

Notes

% cargo version
cargo 1.47.0-nightly (aa6872140 2020-07-23)
% rustc --version
rustc 1.47.0-nightly (6c8927b0c 2020-07-26)
% rustup show active-toolchain
nightly-x86_64-apple-darwin (default)
% uname -rs
Darwin 19.6.0

[ehuss]

Thanks for the detailed report! I see what is wrong, it is being over-eager to unify transitive dependencies (dep in this case, which should be built twice with the exact same settings except for changing one dependency).

I'll try to fix this soonish, but it might take a while. In the meantime, you should be able to use the --target flag to circumvent this bug.

[kylefleming]

Thanks for the detailed report! I see what is wrong, it is being over-eager to unify transitive dependencies (dep in this case, which should be built twice with the exact same settings except for changing one dependency).

I'll try to fix this soonish, but it might take a while.

Wonderful, thank you!

In the meantime, you should be able to use the --target flag to circumvent this bug.

This works great, thank you!

[kylefleming]

In the meantime, you should be able to use the --target flag to circumvent this bug.

@ehuss I've been encountering more situations where this is occurring, even with the --target flag set.

I have a situation that's possibly similar to #8563:

There's a crate (A) and a derive proc-macro companion crate (A-derive). The main crate, A, declares a dependency on the derive proc-macro, A-derive, in order to re-export it. In turn, the derive proc-macro crate, A-derive, declares a dev-dependency on the main crate, A, in order to test it.

This seems to create a "fork" in the dependency tree when building with dev-deps (e.g. for testing): A-derive requires A as a dev dep, but because A requires A-derive as a normal dep, A-derive must be compiled without dev dependencies first, then A can be compiled, and finally A-derive can be compiled again with dev dependencies. The "fork" being the 2 dependency trees that stem from A-derive.

The issue seems to cause this same "cache jumping" so-to-speak, where the resolver incorrectly thinks that it found the crate it was looking for already in the cache, but in reality, there should have been separation between these 2 dependency trees.

An alternative explaination is not that the 2 dependency trees should have been separated, but that a given dependency, in the context of the cache, should reflect the total cargo feature unification that is occurring deeper in the dependency tree of that crate, not just the features that the crate in question has enabled.

In the original example in this bug report, the issue was side-stepped by specifying --target so that the resolver separates the 2 dependency trees for caching purposes.

However, in the circular dev-dependency situation, --target is already being specified. I couldn't find a way to resolve the issue purely by tweaking how the build is built. I did come up with a temporary solution though, so for anyone reading this with the same problem, the interim solution I came up with was just to move the tests into their own crate.

(Edit: disclaimer: this is my best guess for what is going on, but not necessarily what is actually going on)

[ehuss]

@kylefleming Do you think you can put together a reproduction with the circular dev-dependency similar to the original one you made? It was very useful, and I'm uncertain if your situation is much different from #8563.

[kylefleming]

@ehuss I noticed that this issue was auto-closed. Would you still like a reproduction? Alternatively, I can just report back whether the fix fixes the issue for my project.

[ehuss]

Yea, if you can put together a repro with a circular dev-dependency, that would be helpful. I'm uncertain if the fix will address what you ran into, but I'd have a bit more confidence if I could try it out. Thanks!

(BTW, it takes at least a few days for changes to land in nightly. If I remember, I'll try to ping this issue to let you know when it is available.)

[kylefleming]

I'm not sure I'm going to be able to create a reproduction. I can't seem to get it to occur it in a demo environment.

---

It occurred to me however that the project is open source. I fully expect you probably won't want to go down this route, but I'll post it here anyways.

Here's an example of a commit where it happens intermittently. I'm not 100% positive that the issue is directly caused by the circular dependency, but I am positive that cargo is building and unifying features non-deterministically, despite using --target.
Repo: https://github.com/mobilecoinofficial/mobilecoin
Failing commit: mobilecoinfoundation/mobilecoin@864d76f
Failing CI run: https://app.circleci.com/pipelines/github/mobilecoinofficial/mobilecoin/1981/workflows/f068399b-c654-4933-b2ca-32058abe0e4b/jobs/4082/parallel-runs/0/steps/0-110
PR implementing a workaround: mobilecoinfoundation/mobilecoin#387
To run it locally using docker, clone the repo, checkout the commit, make sure docker is running, then run:

$ ./mob prompt
$ cargo test --target x86_64-unknown-linux-gnu --tests --no-run