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

cargo install always re-installs sccache even if the newest version is already present #8513

Closed
jonasbb opened this issue Jul 19, 2020 · 4 comments
Labels

Comments

@jonasbb
Copy link

jonasbb commented Jul 19, 2020

Problem

Running cargo install sccache --no-default-features twice compiles sccache twice.
However, given that the first install installs the most recent version I would expect the second install to be a no-op, since no update is required.

Steps

  1. Run cargo install sccache --no-default-features
  2. Run cargo install sccache --no-default-features again

Possible Solution(s)
I instrumented the version f84f3f8 with some dbg! statements to pinpoint the problem.
As best as I can tell, the problem is that exe_names in src/cargo/ops/common_for_install_and_uninstall.rs does not take into account feature flags.
By default only the sccache binary will be installed, but the sccache crate can also install sccache-dist when given the correct feature flags (Lines common_for_install_and_uninstall.rs:498).
This leads the check in common_for_install_and_uninstall.rs:218 to fail, which marks sccache as dirty.

Here is the debug output:

[src/cargo/ops/common_for_install_and_uninstall.rs:636] filter = Default {
    required_features_filterable: true, 
}                 
[src/cargo/ops/common_for_install_and_uninstall.rs:215] dupe_pkg_id.version() == pkg.version() = true
[src/cargo/ops/common_for_install_and_uninstall.rs:216] dupe_pkg_id.source_id() == source_id = true
[src/cargo/ops/common_for_install_and_uninstall.rs:217] precise_equal = true
[src/cargo/ops/common_for_install_and_uninstall.rs:493] self.features == feature_set(&opts.features) = true
[src/cargo/ops/common_for_install_and_uninstall.rs:494] self.all_features == opts.all_features = true
[src/cargo/ops/common_for_install_and_uninstall.rs:495] self.no_default_features == opts.no_default_features = true
[src/cargo/ops/common_for_install_and_uninstall.rs:496] self.profile.as_str() == opts.build_config.requested_profile.as_str() = true                          
[src/cargo/ops/common_for_install_and_uninstall.rs:497] (self.target.is_none() || self.target.as_deref() == Some(target)) = true
[src/cargo/ops/common_for_install_and_uninstall.rs:498] &self.bins = {
    "sccache",
}
[src/cargo/ops/common_for_install_and_uninstall.rs:498] exes = {
    "sccache",
    "sccache-dist",
}
[src/cargo/ops/common_for_install_and_uninstall.rs:498] dbg!(& self . bins) == dbg!(exes) = false
[src/cargo/ops/common_for_install_and_uninstall.rs:218] info.is_up_to_date(opts, target, &exes) = false
[src/cargo/ops/common_for_install_and_uninstall.rs:223] Ok((Freshness::Dirty, duplicates)) = Ok(
    (
        Dirty,
        {
            "sccache": Some(
                PackageId {
                    name: "sccache",
                    version: "0.2.13",
                    source: "registry `https://github.com/rust-lang/crates.io-index`",
                },
            ),
        },
    ),
)
[src/cargo/ops/cargo_install.rs:544] freshness = Dirty

Notes

Output of cargo version:

Revision f84f3f8, cargo 1.46.0
also reproduces on stable cargo 1.45.0 (744bd1f 2020-06-15)

@jonasbb jonasbb added the C-bug Category: bug label Jul 19, 2020
@ChrisJefferson
Copy link

ChrisJefferson commented Jul 21, 2020

I don't have to give ``--no-default-features, I get the same thing just running cargo install sccache` (which I was coming to report, but then found this bug.

@jonasbb
Copy link
Author

jonasbb commented Sep 14, 2020

@ehuss Do you know where a good place is to start solving this bug? The cargo codebase is quite large and I'm not familiar with it.

This might be the same issue as #8703.

@hampuslidin
Copy link

hampuslidin commented Sep 14, 2020

I missed this issue when looking for a similar issue that of #8703. I have made a quite rough POC of a fix for the bug, which resolves the dependency graph of the package before checking if the package is fresh or dirty, to get the activated features for the package, and I can confirm that this solves the issue fornushell. The diff can be found in this gist. You can try it out on sccache locally as well, to see if it solves it for you.

@weihanglo
Copy link
Member

This is effectively the same as #8703. Given a Cargo team member commented there with some insights, let's close this in favor of that one. Thank you!

@weihanglo weihanglo closed this as not planned Won't fix, can't repro, duplicate, stale May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants