Skip to content

Commit

Permalink
feat(package): warn if symlinks checked out as plain text files
Browse files Browse the repository at this point in the history
`cargo package` will warn users when git `core.symlinks` is `false`
and some symlinks were checked out as plain files during packaging.

Git config [`core.symlinks`] defaults to true when unset.
In git-for-windows (and git as well),
the config should be set to false explicitly when the repo was created,
if symlink support wasn't detected [^1].

We assume the config was always set at creation time and never changed.
So, if it is true, we don't bother users with any warning.

[^1]: https://github.com/git-for-windows/git/blob/f1241afcc7956918d5da33ef74abd9cbba369247/setup.c#L2394-L2403
[`core.symlinks`]: https://git-scm.com/docs/git-config#Documentation/git-config.txt-coresymlinks
  • Loading branch information
weihanglo committed Dec 31, 2024
1 parent c8c8223 commit 72378eb
Show file tree
Hide file tree
Showing 3 changed files with 181 additions and 7 deletions.
46 changes: 46 additions & 0 deletions src/cargo/ops/cargo_package/vcs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ pub fn check_repo_state(
return Ok(None);
}

warn_symlink_checked_out_as_plain_text_file(gctx, src_files, &repo)?;

debug!(
"found (git) Cargo.toml at `{}` in workdir `{}`",
path.display(),
Expand All @@ -111,6 +113,50 @@ pub fn check_repo_state(
return Ok(Some(VcsInfo { git, path_in_vcs }));
}

/// Warns if any symlinks were checked out as plain text files.
///
/// Git config [`core.symlinks`] defaults to true when unset.
/// In git-for-windows (and git as well),
/// the config should be set to false explicitly when the repo was created,
/// if symlink support wasn't detected [^1].
///
/// We assume the config was always set at creation time and never changed.
/// So, if it is true, we don't bother users with any warning.
///
/// [^1]: https://github.com/git-for-windows/git/blob/f1241afcc7956918d5da33ef74abd9cbba369247/setup.c#L2394-L2403
///
/// [`core.symlinks`]: https://git-scm.com/docs/git-config#Documentation/git-config.txt-coresymlinks
fn warn_symlink_checked_out_as_plain_text_file(
gctx: &GlobalContext,
src_files: &[PathEntry],
repo: &git2::Repository,
) -> CargoResult<()> {
let core_symlinks_disabled = match repo.config().and_then(|c| c.get_bool("core.symlinks")) {
Ok(enabled) => !enabled,
Err(e) => {
debug!(
"no git config `core.symlinks` set for repo at `{}`: {e}",
repo.path().display(),
);
return Ok(());
}
};

if core_symlinks_disabled && src_files.iter().any(|f| f.maybe_plain_text_symlink()) {
let mut shell = gctx.shell();
shell.warn(format_args!(
"symbolic links may be checked out as regular files for git repo at `{}`",
repo.workdir().unwrap().display(),
))?;
shell.warn("this might cause `cargo package` to include incorrect or incomplete files")?;
shell.note("to avoid this, set the Git config `core.symlinks` to `true`")?;
#[cfg(windows)]
shell.note("and on Windows, enable the Developer Mode to support symlinks")?;
}

Ok(())
}

/// The real git status check starts from here.
fn git(
pkg: &Package,
Expand Down
56 changes: 49 additions & 7 deletions src/cargo/sources/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ impl<'gctx> Source for RecursivePathSource<'gctx> {
/// Type that abstracts over [`gix::dir::entry::Kind`] and [`fs::FileType`].
#[derive(Debug, Clone, Copy)]
enum FileType {
File,
File { maybe_symlink: bool },
Dir,
Symlink,
Other,
Expand All @@ -416,7 +416,9 @@ enum FileType {
impl From<fs::FileType> for FileType {
fn from(value: fs::FileType) -> Self {
if value.is_file() {
FileType::File
FileType::File {
maybe_symlink: false,
}
} else if value.is_dir() {
FileType::Dir
} else if value.is_symlink() {
Expand All @@ -432,7 +434,9 @@ impl From<gix::dir::entry::Kind> for FileType {
use gix::dir::entry::Kind;
match value {
Kind::Untrackable => FileType::Other,
Kind::File => FileType::File,
Kind::File => FileType::File {
maybe_symlink: false,
},
Kind::Symlink => FileType::Symlink,
Kind::Directory | Kind::Repository => FileType::Dir,
}
Expand All @@ -454,7 +458,7 @@ impl PathEntry {
/// Similar to [`std::path::Path::is_file`]
/// but doesn't follow the symbolic link nor make any system call
pub fn is_file(&self) -> bool {
matches!(self.ty, FileType::File)
matches!(self.ty, FileType::File { .. })
}

/// Similar to [`std::path::Path::is_dir`]
Expand All @@ -468,6 +472,19 @@ impl PathEntry {
pub fn is_symlink(&self) -> bool {
matches!(self.ty, FileType::Symlink)
}

/// Whether this path might be a plain text symlink.
///
/// Git may check out symlinks as plain text files that contain the link texts,
/// when either `core.symlinks` is `false`, or on Windows.
pub fn maybe_plain_text_symlink(&self) -> bool {
matches!(
self.ty,
FileType::File {
maybe_symlink: true
}
)
}
}

impl std::ops::Deref for PathEntry {
Expand Down Expand Up @@ -713,7 +730,24 @@ fn list_files_gix(
&& item.entry.rela_path == "Cargo.lock")
})
})
.map(|res| res.map(|item| (item.entry.rela_path, item.entry.disk_kind)))
.map(|res| {
res.map(|item| {
// Assumption: if a file tracked as a symlink in Git index, and
// the actual file type on disk is file, then it might be a
// plain text file symlink.
// There are exceptions like the file has changed from a symlink
// to a real text file, but hasn't been committed to Git index.
// Exceptions may be rare so we're okay with this now.
let maybe_plain_text_symlink = item.entry.index_kind
== Some(gix::dir::entry::Kind::Symlink)
&& item.entry.disk_kind == Some(gix::dir::entry::Kind::File);
(
item.entry.rela_path,
item.entry.disk_kind,
maybe_plain_text_symlink,
)
})
})
.chain(
// Append entries that might be tracked in `<pkg_root>/target/`.
index
Expand All @@ -731,12 +765,13 @@ fn list_files_gix(
// This traversal is not part of a `status()`, and tracking things in `target/`
// is rare.
None,
false,
)
})
.map(Ok),
)
{
let (rela_path, kind) = item?;
let (rela_path, kind, maybe_plain_text_symlink) = item?;
let file_path = root.join(gix::path::from_bstr(rela_path));
if file_path.file_name().and_then(|name| name.to_str()) == Some("Cargo.toml") {
// Keep track of all sub-packages found and also strip out all
Expand Down Expand Up @@ -781,9 +816,16 @@ fn list_files_gix(
} else if (filter)(&file_path, is_dir) {
assert!(!is_dir);
trace!(" found {}", file_path.display());
let ty = match kind.map(Into::into) {
Some(FileType::File { .. }) => FileType::File {
maybe_symlink: maybe_plain_text_symlink,
},
Some(ty) => ty,
None => FileType::Other,
};
files.push(PathEntry {
path: file_path,
ty: kind.map(Into::into).unwrap_or(FileType::Other),
ty,
});
}
}
Expand Down
86 changes: 86 additions & 0 deletions tests/testsuite/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7025,3 +7025,89 @@ src/main.rs
"#]])
.run();
}

#[cargo_test]
fn git_core_symlinks_false() {
if !symlink_supported() {
return;
}

let git_project = git::new("bar", |p| {
p.file(
"Cargo.toml",
r#"
[package]
name = "bar"
description = "bar"
license = "MIT"
edition = "2021"
documentation = "foo"
"#,
)
.file("src/lib.rs", "//! This is a module")
.symlink("src/lib.rs", "symlink-lib.rs")
.symlink_dir("src", "symlink-dir")
});

let url = git_project.root().to_url().to_string();

let p = project().build();
let root = p.root();
// Remove the default project layout,
// so we can git-fetch from git_project under the same directory
fs::remove_dir_all(&root).unwrap();
fs::create_dir_all(&root).unwrap();
let repo = git::init(&root);

let mut cfg = repo.config().unwrap();
cfg.set_bool("core.symlinks", false).unwrap();

// let's fetch from git_project so it respects our core.symlinks=false config.
repo.remote_anonymous(&url)
.unwrap()
.fetch(&["HEAD"], None, None)
.unwrap();
let rev = repo
.find_reference("FETCH_HEAD")
.unwrap()
.peel_to_commit()
.unwrap();
repo.reset(rev.as_object(), git2::ResetType::Hard, None)
.unwrap();

p.cargo("package --allow-dirty")
.with_stderr_data(str![[r#"
[WARNING] symbolic links may be checked out as regular files for git repo at `[ROOT]/foo/`
[WARNING] this might cause `cargo package` to include incorrect or incomplete files
[NOTE] to avoid this, set the Git config `core.symlinks` to `true`
...
[PACKAGING] bar v0.0.0 ([ROOT]/foo)
[PACKAGED] 7 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
[VERIFYING] bar v0.0.0 ([ROOT]/foo)
[COMPILING] bar v0.0.0 ([ROOT]/foo/target/package/bar-0.0.0)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
"#]])
.run();

let f = File::open(&p.root().join("target/package/bar-0.0.0.crate")).unwrap();
validate_crate_contents(
f,
"bar-0.0.0.crate",
&[
"Cargo.lock",
"Cargo.toml",
"Cargo.toml.orig",
"src/lib.rs",
// We're missing symlink-dir/lib.rs in the `.crate` file.
"symlink-dir",
"symlink-lib.rs",
".cargo_vcs_info.json",
],
[
// And their contents are incorrect.
("symlink-dir", str!["[ROOT]/bar/src"]),
("symlink-lib.rs", str!["[ROOT]/bar/src/lib.rs"]),
],
);
}

0 comments on commit 72378eb

Please sign in to comment.