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

Canonicalize src path for fingerprint #7078

Open
nmattia opened this issue Jun 28, 2019 · 0 comments
Open

Canonicalize src path for fingerprint #7078

nmattia opened this issue Jun 28, 2019 · 0 comments
Labels
A-rebuild-detection Area: rebuild detection and fingerprinting C-enhancement Category: enhancement S-needs-team-input Status: Needs input from team on whether/how to proceed.

Comments

@nmattia
Copy link
Contributor

nmattia commented Jun 28, 2019

Describe the problem you are trying to solve

When cargo registers information used later on to figure out whether or not to recompile a unit, the path to the source is tracked in the fingerprint:

fn path_args(bcx: &BuildContext<'_, '_>, unit: &Unit<'_>) -> (PathBuf, PathBuf) {
let ws_root = bcx.ws.root();
let src = match unit.target.src_path() {
TargetSourcePath::Path(path) => path.to_path_buf(),
TargetSourcePath::Metabuild => unit.pkg.manifest().metabuild_path(bcx.ws.target_dir()),
};
assert!(src.is_absolute());
if unit.pkg.package_id().source_id().is_path() {
if let Ok(path) = src.strip_prefix(ws_root) {
return (path.to_path_buf(), ws_root.to_path_buf());
}
}
(src, unit.pkg.root().to_path_buf())
}

However this path is not canonicalized, meaning that e.g. symlinks aren't resolved before the source paths are added to the fingerprint.

This causes two kinds of issues:

  1. It's convenient to use symlinks to point to a local registry when using source replacement. But since the paths are not canonicalized this can cause spurious rebuilds if the symlink source changes (while the symlink target doesn't).
  2. Same use case as above, but say the symlink source doesn't change, while the symlink target changes. In this case cargo will reuse outdated build artifacts.

Describe the solution you'd like

The paths can be canonicalized before being returned:

diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs
index 628ac8bb..dcc668e8 100644
--- a/src/cargo/core/compiler/mod.rs
+++ b/src/cargo/core/compiler/mod.rs
@@ -688,6 +688,7 @@ fn path_args(bcx: &BuildContext<'_, '_>, unit: &Unit<'_>) -> (PathBuf, PathBuf)
         TargetSourcePath::Metabuild => unit.pkg.manifest().metabuild_path(bcx.ws.target_dir()),
     };
     assert!(src.is_absolute());
+    let src = fs::canonicalize(&src).unwrap_or(src);
     if unit.pkg.package_id().source_id().is_path() {
         if let Ok(path) = src.strip_prefix(ws_root) {
             return (path.to_path_buf(), ws_root.to_path_buf());

One drawback is that cache miss info (with CARGO_LOG=cargo::core::compiler::fingerprint=trace) may confuse the user in the presence of symlinks.

@nmattia nmattia added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Jun 28, 2019
@ehuss ehuss added A-rebuild-detection Area: rebuild detection and fingerprinting and removed C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` labels Aug 20, 2019
@epage epage added C-enhancement Category: enhancement S-needs-team-input Status: Needs input from team on whether/how to proceed. labels Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rebuild-detection Area: rebuild detection and fingerprinting C-enhancement Category: enhancement S-needs-team-input Status: Needs input from team on whether/how to proceed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants