Skip to content

Commit

Permalink
Auto merge of #8186 - ehuss:fix-git-strip-prefix, r=alexcrichton
Browse files Browse the repository at this point in the history
Fix error with git repo discovery and symlinks.

There are some cases where Cargo would generate an error when attempting to discover if a package is inside a git repo when the git repo has a symlink somewhere in its ancestor paths. One way this manifests is with `cargo install --git ...` where the given repo has a `build.rs` script. Another scenario is `cargo build --manifest-path somelink/Cargo.toml` where `somelink` is a symlink to the real thing.

The issue is that libgit2 is normalizing paths and removing symlinks, but the path Cargo uses is the path with symlinks. This was introduced in #8095.

The solution is to try to canonicalize both paths when trying to get a repo-relative path. If that fails for whatever reason, it shouldn't generate an error since this is just a "best effort" attempt to use git to list package files.

Fixes #8183
  • Loading branch information
bors committed Apr 30, 2020
2 parents ba832ac + 5bd74c4 commit 4a61d1c
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 8 deletions.
19 changes: 12 additions & 7 deletions src/cargo/sources/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,13 +197,18 @@ impl<'cfg> PathSource<'cfg> {
repo.path().display()
)
})?;
let repo_relative_path = root.strip_prefix(repo_root).chain_err(|| {
format!(
"expected git repo {} to be parent of package {}",
repo.path().display(),
root.display()
)
})?;
let repo_relative_path = match paths::strip_prefix_canonical(root, repo_root) {
Ok(p) => p,
Err(e) => {
log::warn!(
"cannot determine if path `{:?}` is in git repo `{:?}`: {:?}",
root,
repo_root,
e
);
return Ok(None);
}
};
let manifest_path = repo_relative_path.join("Cargo.toml");
if index.get_path(&manifest_path, 0).is_some() {
return Ok(Some(self.list_files_git(pkg, &repo, filter)?));
Expand Down
22 changes: 22 additions & 0 deletions src/cargo/util/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,3 +411,25 @@ pub fn set_file_time_no_err<P: AsRef<Path>>(path: P, time: FileTime) {
),
}
}

/// Strips `base` from `path`.
///
/// This canonicalizes both paths before stripping. This is useful if the
/// paths are obtained in different ways, and one or the other may or may not
/// have been normalized in some way.
pub fn strip_prefix_canonical<P: AsRef<Path>>(
path: P,
base: P,
) -> Result<PathBuf, std::path::StripPrefixError> {
// Not all filesystems support canonicalize. Just ignore if it doesn't work.
let safe_canonicalize = |path: &Path| match path.canonicalize() {
Ok(p) => p,
Err(e) => {
log::warn!("cannot canonicalize {:?}: {:?}", path, e);
path.to_path_buf()
}
};
let canon_path = safe_canonicalize(path.as_ref());
let canon_base = safe_canonicalize(base.as_ref());
canon_path.strip_prefix(canon_base).map(|p| p.to_path_buf())
}
41 changes: 40 additions & 1 deletion tests/testsuite/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ use cargo_test_support::install::{
};
use cargo_test_support::paths;
use cargo_test_support::registry::Package;
use cargo_test_support::{basic_manifest, cargo_process, project, NO_SUCH_FILE_ERR_MSG};
use cargo_test_support::{
basic_manifest, cargo_process, project, symlink_supported, t, NO_SUCH_FILE_ERR_MSG,
};

fn pkg(name: &str, vers: &str) {
Package::new(name, vers)
Expand Down Expand Up @@ -1458,3 +1460,40 @@ fn git_install_reads_workspace_manifest() {
.with_stderr_contains(" invalid type: integer `3`[..]")
.run();
}

#[cargo_test]
fn install_git_with_symlink_home() {
// Ensure that `cargo install` with a git repo is OK when CARGO_HOME is a
// symlink, and uses an build script.
if !symlink_supported() {
return;
}
let p = git::new("foo", |p| {
p.file("Cargo.toml", &basic_manifest("foo", "1.0.0"))
.file("src/main.rs", "fn main() {}")
// This triggers discover_git_and_list_files for detecting changed files.
.file("build.rs", "fn main() {}")
});
#[cfg(unix)]
use std::os::unix::fs::symlink;
#[cfg(windows)]
use std::os::windows::fs::symlink_dir as symlink;

let actual = paths::root().join("actual-home");
t!(std::fs::create_dir(&actual));
t!(symlink(&actual, paths::home().join(".cargo")));
cargo_process("install --git")
.arg(p.url().to_string())
.with_stderr(
"\
[UPDATING] git repository [..]
[INSTALLING] foo v1.0.0 [..]
[COMPILING] foo v1.0.0 [..]
[FINISHED] [..]
[INSTALLING] [..]home/.cargo/bin/foo[..]
[INSTALLED] package `foo [..]
[WARNING] be sure to add [..]
",
)
.run();
}

0 comments on commit 4a61d1c

Please sign in to comment.