From 45c390a2a50aaafcb32522868550d3074f8487e4 Mon Sep 17 00:00:00 2001 From: Arlo Siemsen Date: Wed, 3 Apr 2024 12:04:10 -0500 Subject: [PATCH 1/2] Switch to using gitoxide by default for listing files --- src/cargo/core/features.rs | 8 +------- src/cargo/sources/path.rs | 11 ++++++----- tests/testsuite/package.rs | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index f72797dd91f..747ebd8f338 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -909,8 +909,6 @@ fn parse_git(it: impl Iterator>) -> CargoResult Self { GitoxideFeatures { fetch: true, - list_files: true, checkout: true, internal_use_git2: false, } @@ -935,7 +932,6 @@ impl GitoxideFeatures { fn safe() -> Self { GitoxideFeatures { fetch: true, - list_files: true, checkout: true, internal_use_git2: false, } @@ -948,7 +944,6 @@ fn parse_gitoxide( let mut out = GitoxideFeatures::default(); let GitoxideFeatures { fetch, - list_files, checkout, internal_use_git2, } = &mut out; @@ -957,10 +952,9 @@ fn parse_gitoxide( match e.as_ref() { "fetch" => *fetch = true, "checkout" => *checkout = true, - "list-files" => *list_files = true, "internal-use-git2" => *internal_use_git2 = true, _ => { - bail!("unstable 'gitoxide' only takes `fetch`, `list-files` and 'checkout' as valid input, for shallow fetches see `-Zgit=shallow-index,shallow-deps`") + bail!("unstable 'gitoxide' only takes `fetch` and 'checkout' as valid input, for shallow fetches see `-Zgit=shallow-index,shallow-deps`") } } } diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index 65459127b62..cc3842ac942 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -143,13 +143,14 @@ impl<'gctx> PathSource<'gctx> { let git_repo = if no_include_option { if self .gctx - .cli_unstable() - .gitoxide - .map_or(false, |features| features.list_files) + .get_env("__CARGO_GITOXIDE_DISABLE_LIST_FILES") + .ok() + .as_deref() + == Some("1") { - self.discover_gix_repo(root)?.map(Git2OrGixRepository::Gix) - } else { self.discover_git_repo(root)?.map(Git2OrGixRepository::Git2) + } else { + self.discover_gix_repo(root)?.map(Git2OrGixRepository::Gix) } } else { None diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs index 81c8fdc1872..ab7cf99e805 100644 --- a/tests/testsuite/package.rs +++ b/tests/testsuite/package.rs @@ -3510,3 +3510,36 @@ Please update the `build` setting in the manifest at `[CWD]/Cargo.toml` and poin .with_stderr(&expect_msg) .run(); } + +#[cargo_test] +fn symlink_manifest_path() { + // Test `cargo install --manifest-path` pointing through a symlink. + 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() {}") + // 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 foo_symlink = paths::root().join("foo-symlink"); + t!(symlink(p.root(), &foo_symlink)); + + cargo_process("package --no-verify --manifest-path") + .arg(foo_symlink.join("Cargo.toml")) + .with_stderr( + "\ +warning: manifest has no description, license, license-file, documentation, homepage or repository. +See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info. +[PACKAGING] foo v1.0.0 ([..]foo-symlink) +[PACKAGED] 5 files[..] +", + ) + .run() +} From 312e2aab7fb9f1ecf3aa6dd5bc8051c1cabc6061 Mon Sep 17 00:00:00 2001 From: Arlo Siemsen Date: Wed, 3 Apr 2024 12:59:49 -0500 Subject: [PATCH 2/2] Use strip_path_canonical for getting the relative path This resolve an issue where the package path contains a symlink that's resolved by git --- src/cargo/ops/cargo_package.rs | 5 +++-- tests/testsuite/package.rs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index 99f1d4fdde7..a699e58a6b5 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -533,8 +533,9 @@ fn check_repo_state( if let Some(workdir) = repo.workdir() { debug!("found a git repo at {:?}", workdir); let path = p.manifest_path(); - let path = path.strip_prefix(workdir).unwrap_or(path); - if let Ok(status) = repo.status_file(path) { + let path = + paths::strip_prefix_canonical(path, workdir).unwrap_or_else(|_| path.to_path_buf()); + if let Ok(status) = repo.status_file(&path) { if (status & git2::Status::IGNORED).is_empty() { debug!( "found (git) Cargo.toml at {:?} in workdir {:?}", diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs index ab7cf99e805..8f3ab93ade0 100644 --- a/tests/testsuite/package.rs +++ b/tests/testsuite/package.rs @@ -3538,7 +3538,7 @@ fn symlink_manifest_path() { warning: manifest has no description, license, license-file, documentation, homepage or repository. See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info. [PACKAGING] foo v1.0.0 ([..]foo-symlink) -[PACKAGED] 5 files[..] +[PACKAGED] 6 files[..] ", ) .run()