Skip to content

improvements for Cargo #1323

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

Merged
merged 11 commits into from
Mar 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions gitoxide-core/src/repository/clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ pub(crate) mod function {
} else {
Vec::new()
},
&gix::interrupt::IS_INTERRUPTED,
options,
&mut collect,
)?;
Expand Down
9 changes: 7 additions & 2 deletions gix-actor/src/signature/decode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,14 @@ pub(crate) mod function {
take_while(1..=2, AsChar::is_dec_digit)
.verify_map(|v| to_signed::<OffsetInSeconds>(v).ok())
.context(StrContext::Expected("MM".into())),
take_while(0.., AsChar::is_dec_digit).map(|v: &[u8]| v),
)
.map(|(time, sign, hours, minutes)| {
let offset = (hours * 3600 + minutes * 60) * if sign == Sign::Minus { -1 } else { 1 };
.map(|(time, sign, hours, minutes, trailing_digits)| {
let offset = if trailing_digits.is_empty() {
(hours * 3600 + minutes * 60) * if sign == Sign::Minus { -1 } else { 1 }
} else {
0
};
Time {
seconds: time,
offset,
Expand Down
27 changes: 26 additions & 1 deletion gix-actor/tests/signature/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ mod write_to {
}

use bstr::ByteSlice;
use gix_actor::Signature;
use gix_actor::{Signature, SignatureRef};

#[test]
fn trim() {
Expand All @@ -80,3 +80,28 @@ fn round_trip() -> Result<(), Box<dyn std::error::Error>> {
}
Ok(())
}

#[test]
fn parse_timestamp_with_trailing_digits() {
let signature = gix_actor::SignatureRef::from_bytes::<()>(b"first last <name@example.com> 1312735823 +051800")
.expect("deal with trailing zeroes in timestamp by discarding it");
assert_eq!(
signature,
SignatureRef {
name: "first last".into(),
email: "name@example.com".into(),
time: gix_actor::date::Time::new(1312735823, 0),
}
);

let signature = gix_actor::SignatureRef::from_bytes::<()>(b"first last <name@example.com> 1312735823 +0518")
.expect("this naturally works as the timestamp does not have trailing zeroes");
assert_eq!(
signature,
SignatureRef {
name: "first last".into(),
email: "name@example.com".into(),
time: gix_actor::date::Time::new(1312735823, 19080),
}
);
}
12 changes: 11 additions & 1 deletion gix-dir/src/walk/classify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ pub fn path(
emit_ignored,
for_deletion,
classify_untracked_bare_repositories,
symlinks_to_directories_are_ignored_like_directories,
..
}: Options,
ctx: &mut Context<'_>,
Expand Down Expand Up @@ -199,6 +200,15 @@ pub fn path(
.pattern_matching_relative_path(rela_path.as_bstr(), kind.map(|ft| ft.is_dir()), ctx.pathspec_attributes)
.map(Into::into);

let is_dir = if symlinks_to_directories_are_ignored_like_directories
&& ctx.excludes.is_some()
&& kind.map_or(false, |ft| ft == entry::Kind::Symlink)
{
path.metadata().ok().map(|md| md.is_dir()).or(Some(false))
} else {
kind.map(|ft| ft.is_dir())
};

let mut maybe_upgrade_to_repository = |current_kind, find_harder: bool| {
if recurse_repositories {
return current_kind;
Expand Down Expand Up @@ -245,7 +255,7 @@ pub fn path(
.as_mut()
.map_or(Ok(None), |stack| {
stack
.at_entry(rela_path.as_bstr(), kind.map(|ft| ft.is_dir()), ctx.objects)
.at_entry(rela_path.as_bstr(), is_dir, ctx.objects)
.map(|platform| platform.excluded_kind())
})
.map_err(Error::ExcludesAccess)?
Expand Down
18 changes: 13 additions & 5 deletions gix-dir/src/walk/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,16 @@ pub fn walk(
)?;
if !can_recurse(
buf.as_bstr(),
root_info,
if root == worktree_root && root_info.disk_kind == Some(entry::Kind::Symlink) && current.is_dir() {
classify::Outcome {
disk_kind: Some(entry::Kind::Directory),
..root_info
}
} else {
root_info
},
options.for_deletion,
worktree_root_is_repository, /* is root */
worktree_root_is_repository,
delegate,
) {
if buf.is_empty() && !root_info.disk_kind.map_or(false, |kind| kind.is_dir()) {
Expand Down Expand Up @@ -147,16 +154,17 @@ pub(super) fn can_recurse(
rela_path: &BStr,
info: classify::Outcome,
for_deletion: Option<ForDeletionMode>,
is_root: bool,
worktree_root_is_repository: bool,
delegate: &mut dyn Delegate,
) -> bool {
if info.disk_kind.map_or(true, |k| !k.is_dir()) {
let is_dir = info.disk_kind.map_or(false, |k| k.is_dir());
if !is_dir {
return false;
}
delegate.can_recurse(
EntryRef::from_outcome(Cow::Borrowed(rela_path), info),
for_deletion,
is_root,
worktree_root_is_repository,
)
}

Expand Down
15 changes: 15 additions & 0 deletions gix-dir/src/walk/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::{entry, EntryRef};
use bstr::BStr;
use std::path::PathBuf;
use std::sync::atomic::AtomicBool;

/// A type returned by the [`Delegate::emit()`] as passed to [`walk()`](function::walk()).
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
Expand Down Expand Up @@ -183,10 +184,22 @@ pub struct Options {
pub emit_empty_directories: bool,
/// If `None`, no entries inside of collapsed directories are emitted. Otherwise, act as specified by `Some(mode)`.
pub emit_collapsed: Option<CollapsedEntriesEmissionMode>,
/// This is a `libgit2` compatibility flag, and if enabled, symlinks that point to directories will be considered a directory
/// when checking for exclusion.
///
/// This is relevant if `src2` points to `src`, and is excluded with `src2/`. If `false`, `src2` will not be excluded,
/// if `true` it will be excluded as the symlink is considered a directory.
///
/// In other words, for Git compatibility this flag should be `false`, the default, for `git2` compatibility it should be `true`.
pub symlinks_to_directories_are_ignored_like_directories: bool,
}

/// All information that is required to perform a dirwalk, and classify paths properly.
pub struct Context<'a> {
/// If not `None`, it will be checked before entering any directory to trigger early interruption.
///
/// If this flag is `true` at any point in the iteration, it will abort with an error.
pub should_interrupt: Option<&'a AtomicBool>,
/// The `git_dir` of the parent repository, after a call to [`gix_path::realpath()`].
///
/// It's used to help us differentiate our own `.git` directory from nested unrelated repositories,
Expand Down Expand Up @@ -261,6 +274,8 @@ pub struct Outcome {
#[derive(Debug, thiserror::Error)]
#[allow(missing_docs)]
pub enum Error {
#[error("Interrupted")]
Interrupted,
#[error("Worktree root at '{}' is not a directory", root.display())]
WorktreeRootIsFile { root: PathBuf },
#[error("Traversal root '{}' contains relative path components and could not be normalized", root.display())]
Expand Down
4 changes: 4 additions & 0 deletions gix-dir/src/walk/readdir.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use bstr::{BStr, BString, ByteSlice};
use std::borrow::Cow;
use std::path::{Path, PathBuf};
use std::sync::atomic::Ordering;

use crate::entry::{PathspecMatch, Status};
use crate::walk::function::{can_recurse, emit_entry};
Expand All @@ -23,6 +24,9 @@ pub(super) fn recursive(
out: &mut Outcome,
state: &mut State,
) -> Result<(Action, bool), Error> {
if ctx.should_interrupt.map_or(false, |flag| flag.load(Ordering::Relaxed)) {
return Err(Error::Interrupted);
}
out.read_dir_calls += 1;
let entries = gix_fs::read_dir(current, opts.precompose_unicode).map_err(|err| Error::ReadDir {
path: current.to_owned(),
Expand Down
26 changes: 26 additions & 0 deletions gix-dir/tests/fixtures/many-symlinks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,29 @@ git init immediate-breakout-symlink
(cd immediate-breakout-symlink
ln -s .. breakout
)

git init excluded-symlinks-to-dir
(cd excluded-symlinks-to-dir
cat <<EOF >.gitignore
src1
src2/
file1
file2/
ignored
ignored-must-be-dir/
EOF
git add .gitignore && git commit -m "init"

mkdir src
>src/file

mkdir ignored-must-be-dir ignored
touch ignored-must-be-dir/file ignored/file

ln -s src src1
ln -s src src2
ln -s src/file file1
ln -s src/file file2
)

ln -s excluded-symlinks-to-dir worktree-root-is-symlink
127 changes: 127 additions & 0 deletions gix-dir/tests/walk/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use gix_dir::{walk, EntryRef};
use pretty_assertions::assert_eq;
use std::sync::atomic::AtomicBool;

use crate::walk_utils::{
collect, collect_filtered, collect_filtered_with_cwd, entry, entry_dirstat, entry_nokind, entry_nomatch, entryps,
Expand All @@ -16,6 +17,81 @@ use gix_dir::walk::EmissionMode::*;
use gix_dir::walk::ForDeletionMode;
use gix_ignore::Kind::*;

#[test]
#[cfg_attr(windows, ignore = "symlinks the way they are organized don't yet work on windows")]
fn symlink_to_dir_can_be_excluded() -> crate::Result {
let root = fixture_in("many-symlinks", "excluded-symlinks-to-dir");
let ((out, _root), entries) = collect(&root, None, |keep, ctx| {
walk(
&root,
ctx,
gix_dir::walk::Options {
emit_ignored: Some(Matching),
..options()
},
keep,
)
});
assert_eq!(
out,
walk::Outcome {
read_dir_calls: 2,
returned_entries: entries.len(),
seen_entries: 9,
}
);

assert_eq!(
entries,
&[
entry("file1", Ignored(Expendable), Symlink),
entry("file2", Untracked, Symlink),
entry("ignored", Ignored(Expendable), Directory),
entry("ignored-must-be-dir", Ignored(Expendable), Directory),
entry("src/file", Untracked, File),
entry("src1", Ignored(Expendable), Symlink),
entry("src2", Untracked, Symlink), /* marked as src2/ in .gitignore */
],
"by default, symlinks are counted as files only, even if they point to a directory, when handled by the exclude machinery"
);

let ((out, _root), entries) = collect(&root, None, |keep, ctx| {
walk(
&root,
ctx,
gix_dir::walk::Options {
emit_ignored: Some(Matching),
symlinks_to_directories_are_ignored_like_directories: true,
..options()
},
keep,
)
});
assert_eq!(
out,
walk::Outcome {
read_dir_calls: 2,
returned_entries: entries.len(),
seen_entries: 9,
}
);

assert_eq!(
entries,
&[
entry("file1", Ignored(Expendable), Symlink),
entry("file2", Untracked, Symlink),
entry("ignored", Ignored(Expendable), Directory),
entry("ignored-must-be-dir", Ignored(Expendable), Directory),
entry("src/file", Untracked, File),
entry("src1", Ignored(Expendable), Symlink),
entry("src2", Ignored(Expendable), Symlink), /* marked as src2/ in .gitignore */
],
"with libgit2 compatibility enabled, symlinks to directories are treated like a directory, not symlink"
);
Ok(())
}

#[test]
#[cfg_attr(windows, ignore = "symlinks the way they are organized don't yet work on windows")]
fn root_may_not_lead_through_symlinks() -> crate::Result {
Expand Down Expand Up @@ -43,6 +119,57 @@ fn root_may_not_lead_through_symlinks() -> crate::Result {
Ok(())
}

#[test]
#[cfg_attr(windows, ignore = "symlinks the way they are organized don't yet work on windows")]
fn root_may_be_a_symlink_if_it_is_the_worktree() -> crate::Result {
let root = fixture_in("many-symlinks", "worktree-root-is-symlink");
let ((_out, _root), entries) = collect(&root, None, |keep, ctx| {
walk(
&root,
ctx,
gix_dir::walk::Options {
emit_ignored: Some(Matching),
symlinks_to_directories_are_ignored_like_directories: true,
..options()
},
keep,
)
});

assert_eq!(
entries,
&[
entry("file1", Ignored(Expendable), Symlink),
entry("file2", Untracked, Symlink),
entry("ignored", Ignored(Expendable), Directory),
entry("ignored-must-be-dir", Ignored(Expendable), Directory),
entry("src/file", Untracked, File),
entry("src1", Ignored(Expendable), Symlink),
entry("src2", Ignored(Expendable), Symlink), /* marked as src2/ in .gitignore */
],
"it traversed the directory normally - without this capability, symlinked repos can't be traversed"
);
Ok(())
}

#[test]
fn should_interrupt_works_even_in_empty_directories() {
let root = fixture("empty");
let should_interrupt = AtomicBool::new(true);
let err = try_collect_filtered_opts_collect(
&root,
None,
|keep, ctx| walk(&root, ctx, gix_dir::walk::Options { ..options() }, keep),
None::<&str>,
Options {
should_interrupt: Some(&should_interrupt),
..Default::default()
},
)
.unwrap_err();
assert!(matches!(err, gix_dir::walk::Error::Interrupted));
}

#[test]
fn empty_root() -> crate::Result {
let root = fixture("empty");
Expand Down
Loading