Skip to content

Commit 059fe16

Browse files
committed
fix(package): warn if symlinks checked out as plain text files
`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
1 parent 871b17f commit 059fe16

File tree

3 files changed

+101
-7
lines changed

3 files changed

+101
-7
lines changed

src/cargo/ops/cargo_package/vcs.rs

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,8 @@ pub fn check_repo_state(
9292
return Ok(None);
9393
}
9494

95+
warn_symlink_checked_out_as_plain_text_file(gctx, src_files, &repo)?;
96+
9597
debug!(
9698
"found (git) Cargo.toml at `{}` in workdir `{}`",
9799
path.display(),
@@ -111,6 +113,52 @@ pub fn check_repo_state(
111113
return Ok(Some(VcsInfo { git, path_in_vcs }));
112114
}
113115

116+
/// Warns if any symlinks were checked out as plain text files.
117+
///
118+
/// Git config [`core.symlinks`] defaults to true when unset.
119+
/// In git-for-windows (and git as well),
120+
/// the config should be set to false explicitly when the repo was created,
121+
/// if symlink support wasn't detected [^1].
122+
///
123+
/// We assume the config was always set at creation time and never changed.
124+
/// So, if it is true, we don't bother users with any warning.
125+
///
126+
/// [^1]: <https://github.com/git-for-windows/git/blob/f1241afcc7956918d5da33ef74abd9cbba369247/setup.c#L2394-L2403>
127+
///
128+
/// [`core.symlinks`]: https://git-scm.com/docs/git-config#Documentation/git-config.txt-coresymlinks
129+
fn warn_symlink_checked_out_as_plain_text_file(
130+
gctx: &GlobalContext,
131+
src_files: &[PathEntry],
132+
repo: &git2::Repository,
133+
) -> CargoResult<()> {
134+
if repo
135+
.config()
136+
.and_then(|c| c.get_bool("core.symlinks"))
137+
.unwrap_or(true)
138+
{
139+
return Ok(());
140+
}
141+
142+
if src_files.iter().any(|f| f.maybe_plain_text_symlink()) {
143+
let mut shell = gctx.shell();
144+
shell.warn(format_args!(
145+
"found symbolic links that may be checked out as regular files for git repo at `{}`\n\
146+
This might cause the `.crate` file to include incorrect or incomplete files",
147+
repo.workdir().unwrap().display(),
148+
))?;
149+
let extra_note = if cfg!(windows) {
150+
"\nAnd on Windows, enable the Developer Mode to support symlinks"
151+
} else {
152+
""
153+
};
154+
shell.note(format_args!(
155+
"to avoid this, set the Git config `core.symlinks` to `true`{extra_note}",
156+
))?;
157+
}
158+
159+
Ok(())
160+
}
161+
114162
/// The real git status check starts from here.
115163
fn git(
116164
pkg: &Package,

src/cargo/sources/path.rs

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ impl<'gctx> Source for RecursivePathSource<'gctx> {
407407
/// Type that abstracts over [`gix::dir::entry::Kind`] and [`fs::FileType`].
408408
#[derive(Debug, Clone, Copy)]
409409
enum FileType {
410-
File,
410+
File { maybe_symlink: bool },
411411
Dir,
412412
Symlink,
413413
Other,
@@ -416,7 +416,9 @@ enum FileType {
416416
impl From<fs::FileType> for FileType {
417417
fn from(value: fs::FileType) -> Self {
418418
if value.is_file() {
419-
FileType::File
419+
FileType::File {
420+
maybe_symlink: false,
421+
}
420422
} else if value.is_dir() {
421423
FileType::Dir
422424
} else if value.is_symlink() {
@@ -432,7 +434,9 @@ impl From<gix::dir::entry::Kind> for FileType {
432434
use gix::dir::entry::Kind;
433435
match value {
434436
Kind::Untrackable => FileType::Other,
435-
Kind::File => FileType::File,
437+
Kind::File => FileType::File {
438+
maybe_symlink: false,
439+
},
436440
Kind::Symlink => FileType::Symlink,
437441
Kind::Directory | Kind::Repository => FileType::Dir,
438442
}
@@ -454,7 +458,7 @@ impl PathEntry {
454458
/// Similar to [`std::path::Path::is_file`]
455459
/// but doesn't follow the symbolic link nor make any system call
456460
pub fn is_file(&self) -> bool {
457-
matches!(self.ty, FileType::File)
461+
matches!(self.ty, FileType::File { .. })
458462
}
459463

460464
/// Similar to [`std::path::Path::is_dir`]
@@ -468,6 +472,19 @@ impl PathEntry {
468472
pub fn is_symlink(&self) -> bool {
469473
matches!(self.ty, FileType::Symlink)
470474
}
475+
476+
/// Whether this path might be a plain text symlink.
477+
///
478+
/// Git may check out symlinks as plain text files that contain the link texts,
479+
/// when either `core.symlinks` is `false`, or on Windows.
480+
pub fn maybe_plain_text_symlink(&self) -> bool {
481+
matches!(
482+
self.ty,
483+
FileType::File {
484+
maybe_symlink: true
485+
}
486+
)
487+
}
471488
}
472489

473490
impl std::ops::Deref for PathEntry {
@@ -713,7 +730,24 @@ fn list_files_gix(
713730
&& item.entry.rela_path == "Cargo.lock")
714731
})
715732
})
716-
.map(|res| res.map(|item| (item.entry.rela_path, item.entry.disk_kind)))
733+
.map(|res| {
734+
res.map(|item| {
735+
// Assumption: if a file tracked as a symlink in Git index, and
736+
// the actual file type on disk is file, then it might be a
737+
// plain text file symlink.
738+
// There are exceptions like the file has changed from a symlink
739+
// to a real text file, but hasn't been committed to Git index.
740+
// Exceptions may be rare so we're okay with this now.
741+
let maybe_plain_text_symlink = item.entry.index_kind
742+
== Some(gix::dir::entry::Kind::Symlink)
743+
&& item.entry.disk_kind == Some(gix::dir::entry::Kind::File);
744+
(
745+
item.entry.rela_path,
746+
item.entry.disk_kind,
747+
maybe_plain_text_symlink,
748+
)
749+
})
750+
})
717751
.chain(
718752
// Append entries that might be tracked in `<pkg_root>/target/`.
719753
index
@@ -731,12 +765,13 @@ fn list_files_gix(
731765
// This traversal is not part of a `status()`, and tracking things in `target/`
732766
// is rare.
733767
None,
768+
false,
734769
)
735770
})
736771
.map(Ok),
737772
)
738773
{
739-
let (rela_path, kind) = item?;
774+
let (rela_path, kind, maybe_plain_text_symlink) = item?;
740775
let file_path = root.join(gix::path::from_bstr(rela_path));
741776
if file_path.file_name().and_then(|name| name.to_str()) == Some("Cargo.toml") {
742777
// Keep track of all sub-packages found and also strip out all
@@ -781,9 +816,16 @@ fn list_files_gix(
781816
} else if (filter)(&file_path, is_dir) {
782817
assert!(!is_dir);
783818
trace!(" found {}", file_path.display());
819+
let ty = match kind.map(Into::into) {
820+
Some(FileType::File { .. }) => FileType::File {
821+
maybe_symlink: maybe_plain_text_symlink,
822+
},
823+
Some(ty) => ty,
824+
None => FileType::Other,
825+
};
784826
files.push(PathEntry {
785827
path: file_path,
786-
ty: kind.map(Into::into).unwrap_or(FileType::Other),
828+
ty,
787829
});
788830
}
789831
}

tests/testsuite/package.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7077,6 +7077,10 @@ fn git_core_symlinks_false() {
70777077

70787078
p.cargo("package --allow-dirty")
70797079
.with_stderr_data(str![[r#"
7080+
[WARNING] found symbolic links that may be checked out as regular files for git repo at `[ROOT]/foo/`
7081+
This might cause the `.crate` file to include incorrect or incomplete files
7082+
[NOTE] to avoid this, set the Git config `core.symlinks` to `true`
7083+
...
70807084
[PACKAGING] bar v0.0.0 ([ROOT]/foo)
70817085
[PACKAGED] 7 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
70827086
[VERIFYING] bar v0.0.0 ([ROOT]/foo)

0 commit comments

Comments
 (0)