-
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
Cargo sometimes chooses incorrect features #3812
Comments
Thanks for the report, something definitely sounds fishy! Note that cargo's resolver is slightly nondeterminstic (due to usage of hashmap somewhere), but in general that shouldn't affect this. It's normal to take different paths each time to reach the same resolution graph. Can you try running your builds with Also, if you've got a full log of |
I had modified diff --git a/src/cargo/ops/cargo_rustc/fingerprint.rs b/src/cargo/ops/cargo_rustc/fingerprint.rs
index 25c734a..ff47558 100644
--- a/src/cargo/ops/cargo_rustc/fingerprint.rs
+++ b/src/cargo/ops/cargo_rustc/fingerprint.rs
@@ -127,7 +127,7 @@ pub fn prepare_target<'a, 'cfg>(cx: &mut Context<'a, 'cfg>,
/// `DependencyQueue`, but it also needs to be retained here because Cargo can
/// be interrupted while executing, losing the state of the `DependencyQueue`
/// graph.
-#[derive(Serialize, Deserialize)]
+#[derive(Serialize, Deserialize, Debug)]
pub struct Fingerprint {
rustc: u64,
features: String,
@@ -168,12 +168,13 @@ fn deserialize_deps<D>(d: D) -> Result<Vec<(String, Arc<Fingerprint>)>, D::Error
}).collect())
}
-#[derive(Serialize, Deserialize, Hash)]
+#[derive(Serialize, Deserialize, Hash, Debug)]
enum LocalFingerprint {
Precalculated(String),
MtimeBased(MtimeSlot, PathBuf),
}
+#[derive(Debug)]
struct MtimeSlot(Mutex<Option<FileTime>>);
impl Fingerprint {
@@ -251,7 +252,7 @@ impl Fingerprint {
}
for (a, b) in self.deps.iter().zip(old.deps.iter()) {
if a.1.hash() != b.1.hash() {
- bail!("new ({}) != old ({})", a.0, b.0)
+ bail!("new ({} {:#?}) != old ({} {:#?})", a.0, a.1, b.0, b.1)
}
}
Ok(()) This is the line you're looking for:
I do have the outputs in both cases with 11580094398159781064 is pretty much the same fingerprint except with |
Yeah if you can gist that between two runs that's most helpful. We don't save off the entire fingerprint (as it'd literally take up gigabytes of data) so just the memoized hash is restored (which is all we need for a comparison). The two runs should have different "new" fingerprints where we can confirm it's different feature selection. The full If you'd prefer you can send me links privately on IRC if you'd prefer to avoid posting here as well! |
Going the other way:
As I said, the root difference is in the features (and further dependencies selected based on that). I e-mailed you the trace logs. |
@jethrogb to confirm, you're using an up-to-date Cargo right? |
I've tried several versions including the latest nightly as of last Friday when I filed this issue. |
Ok cool, I'm still digging into this. If you can get me access to the various Cargo.tomls (at least around the problematic ones) that may make debugging easier, but I'm not certain I'll need that yet (sorry haven't had a chance to really dive in yet) |
@jethrogb can gist the
|
Aha nevermind, I've minimized it! Working on a fix. |
Previously Cargo had a bug where if a crate *without* a default feature was overridden with one that did indeed have a default feature then the default may not end up getting activated by accident. The fix was to avoid returning too quickly hen activating dependencies until after we've inspected and learned about replacements. Closes rust-lang#3812
Fix: #3843 |
Fantastic! Will try right away. |
…atklad Fix overriding mixing with default features Previously Cargo had a bug where if a crate *without* a default feature was overridden with one that did indeed have a default feature then the default may not end up getting activated by accident. The fix was to avoid returning too quickly hen activating dependencies until after we've inspected and learned about replacements. Closes #3812
I've been experiencing “random rebuilds”: sometimes when running
cargo build
twice in a row, the second time it rebuilds some crates even though nothing changed.I've spent some time debugging this issue and it seems to be caused by a low but non-negligible probability of incorrect feature selection for one of the crates in my dependency graph. This will then lead to a different fingerprint being computed and a rebuild happening.
When I run
RUST_LOG=cargo::core::resolver=trace cargo build --lib --verbose|&egrep 'activating (tempdir|rand)'
, most of the time I get output like this:or this:
But sometimes, I get this:
or this:
When that happens, the incorrect features are selected for the rand crate. As far as I can tell, the resolver logic in cargo is run twice. If during the first time tempdir was activated after rand but during the second time tempdir is activated before rand, then the features that tempdir specifies for the rand crate are being used.
Note that tempdir is not actually ever built since it is a build-dependency of a crate that is a dependency of another crate, but not on my platform.
I suspect the non-determinism is coming from HashMaps or ReadDir. I have no clue where to go from here and could use some debugging help. I unfortunately can't share my Cargo.toml and have not been able to reproduce the issue in a smaller test case. My dependency graph includes about 400 crates and I'm using crates.io, git, and path dependencies as well as a
[replace]
section.You may ignore the following, putting it here for my records:
The text was updated successfully, but these errors were encountered: