From 004394ad04a965b631c5d75a7eced632540d9e1e Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Thu, 24 Feb 2022 17:50:16 +0800 Subject: [PATCH 01/19] refactor (#301) --- git-worktree/src/index.rs | 171 +++++++++++++++++++++++++++++++++++++ git-worktree/src/lib.rs | 174 +------------------------------------- 2 files changed, 172 insertions(+), 173 deletions(-) create mode 100644 git-worktree/src/index.rs diff --git a/git-worktree/src/index.rs b/git-worktree/src/index.rs new file mode 100644 index 00000000000..e67775c193f --- /dev/null +++ b/git-worktree/src/index.rs @@ -0,0 +1,171 @@ +use git_hash::oid; + +pub mod checkout { + use bstr::BString; + use quick_error::quick_error; + + #[derive(Clone, Copy)] + pub struct Options { + pub symlinks: bool, + } + + impl Default for Options { + fn default() -> Self { + Options { symlinks: true } + } + } + + quick_error! { + #[derive(Debug)] + pub enum Error { + IllformedUtf8{ path: BString } { + display("Could not convert path to UTF8: {}", path) + } + Time(err: std::time::SystemTimeError) { + from() + source(err) + display("The clock was off when reading file related metadata after updating a file on disk") + } + Io(err: std::io::Error) { + from() + source(err) + display("IO error while writing blob or reading file metadata or changing filetype") + } + ObjectNotFound{ oid: git_hash::ObjectId, path: std::path::PathBuf } { + display("object {} for checkout at {} not found in object database", oid.to_hex(), path.display()) + } + } + } +} + +pub fn checkout( + index: &mut git_index::State, + path: impl AsRef, + mut find: Find, + options: checkout::Options, +) -> Result<(), checkout::Error> +where + Find: for<'a> FnMut(&oid, &'a mut Vec) -> Option>, +{ + let root = path.as_ref(); + let mut buf = Vec::new(); + for (entry, entry_path) in index.entries_mut_with_paths() { + // TODO: write test for that + if entry.flags.contains(git_index::entry::Flags::SKIP_WORKTREE) { + continue; + } + + entry::checkout(entry, entry_path, &mut find, root, options, &mut buf)?; + } + Ok(()) +} + +pub(crate) mod entry { + use std::{ + convert::TryInto, + fs::{create_dir_all, OpenOptions}, + io::Write, + time::Duration, + }; + + use bstr::BStr; + use git_hash::oid; + use git_index::Entry; + + use crate::index; + + pub fn checkout( + entry: &mut Entry, + entry_path: &BStr, + find: &mut Find, + root: &std::path::Path, + index::checkout::Options { symlinks }: index::checkout::Options, + buf: &mut Vec, + ) -> Result<(), index::checkout::Error> + where + Find: for<'a> FnMut(&oid, &'a mut Vec) -> Option>, + { + let dest = root.join(git_features::path::from_byte_slice(entry_path).map_err(|_| { + index::checkout::Error::IllformedUtf8 { + path: entry_path.to_owned(), + } + })?); + create_dir_all(dest.parent().expect("entry paths are never empty"))?; // TODO: can this be avoided to create dirs when needed only? + + match entry.mode { + git_index::entry::Mode::FILE | git_index::entry::Mode::FILE_EXECUTABLE => { + let obj = find(&entry.id, buf).ok_or_else(|| index::checkout::Error::ObjectNotFound { + oid: entry.id, + path: root.to_path_buf(), + })?; + let mut options = OpenOptions::new(); + options.write(true).create_new(true); + #[cfg(unix)] + if entry.mode == git_index::entry::Mode::FILE_EXECUTABLE { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o777); + } + let mut file = options.open(&dest)?; + file.write_all(obj.data)?; + let met = file.metadata()?; + let ctime = met + .created() + .map_or(Ok(Duration::default()), |x| x.duration_since(std::time::UNIX_EPOCH))?; + let mtime = met + .modified() + .map_or(Ok(Duration::default()), |x| x.duration_since(std::time::UNIX_EPOCH))?; + + update_fstat(entry, ctime, mtime)?; + } + git_index::entry::Mode::SYMLINK => { + let obj = find(&entry.id, buf).ok_or_else(|| index::checkout::Error::ObjectNotFound { + oid: entry.id, + path: root.to_path_buf(), + })?; + let symlink_destination = git_features::path::from_byte_slice(obj.data) + .map_err(|_| index::checkout::Error::IllformedUtf8 { path: obj.data.into() })?; + if symlinks { + #[cfg(unix)] + std::os::unix::fs::symlink(symlink_destination, &dest)?; + #[cfg(windows)] + if dest.exists() { + if dest.is_file() { + std::os::windows::fs::symlink_file(symlink_destination, &dest)?; + } else { + std::os::windows::fs::symlink_dir(symlink_destination, &dest)?; + } + } + } else { + std::fs::write(&dest, obj.data)?; + } + let met = std::fs::symlink_metadata(&dest)?; + let ctime = met + .created() + .map_or(Ok(Duration::default()), |x| x.duration_since(std::time::UNIX_EPOCH))?; + let mtime = met + .modified() + .map_or(Ok(Duration::default()), |x| x.duration_since(std::time::UNIX_EPOCH))?; + update_fstat(entry, ctime, mtime)?; + } + git_index::entry::Mode::DIR => todo!(), + git_index::entry::Mode::COMMIT => todo!(), + _ => unreachable!(), + } + Ok(()) + } + + fn update_fstat(entry: &mut Entry, ctime: Duration, mtime: Duration) -> Result<(), index::checkout::Error> { + let stat = &mut entry.stat; + stat.mtime.secs = mtime + .as_secs() + .try_into() + .expect("by 2038 we found a solution for this"); + stat.mtime.nsecs = mtime.subsec_nanos(); + stat.ctime.secs = ctime + .as_secs() + .try_into() + .expect("by 2038 we found a solution for this"); + stat.ctime.nsecs = ctime.subsec_nanos(); + Ok(()) + } +} diff --git a/git-worktree/src/lib.rs b/git-worktree/src/lib.rs index 11901252e1b..ccd3937c4a6 100644 --- a/git-worktree/src/lib.rs +++ b/git-worktree/src/lib.rs @@ -1,175 +1,3 @@ #![forbid(unsafe_code, rust_2018_idioms)] -pub mod index { - use git_hash::oid; - - pub mod checkout { - use bstr::BString; - use quick_error::quick_error; - - #[derive(Clone, Copy)] - pub struct Options { - pub symlinks: bool, - } - - impl Default for Options { - fn default() -> Self { - Options { symlinks: true } - } - } - - quick_error! { - #[derive(Debug)] - pub enum Error { - IllformedUtf8{ path: BString } { - display("Could not convert path to UTF8: {}", path) - } - Time(err: std::time::SystemTimeError) { - from() - source(err) - display("The clock was off when reading file related metadata after updating a file on disk") - } - Io(err: std::io::Error) { - from() - source(err) - display("IO error while writing blob or reading file metadata or changing filetype") - } - ObjectNotFound{ oid: git_hash::ObjectId, path: std::path::PathBuf } { - display("object {} for checkout at {} not found in object database", oid.to_hex(), path.display()) - } - } - } - } - - pub fn checkout( - index: &mut git_index::State, - path: impl AsRef, - mut find: Find, - options: checkout::Options, - ) -> Result<(), checkout::Error> - where - Find: for<'a> FnMut(&oid, &'a mut Vec) -> Option>, - { - let root = path.as_ref(); - let mut buf = Vec::new(); - for (entry, entry_path) in index.entries_mut_with_paths() { - // TODO: write test for that - if entry.flags.contains(git_index::entry::Flags::SKIP_WORKTREE) { - continue; - } - - entry::checkout(entry, entry_path, &mut find, root, options, &mut buf)?; - } - Ok(()) - } - - pub(crate) mod entry { - use std::{ - convert::TryInto, - fs::{create_dir_all, OpenOptions}, - io::Write, - time::Duration, - }; - - use bstr::BStr; - use git_hash::oid; - use git_index::Entry; - - use crate::index; - - pub fn checkout( - entry: &mut Entry, - entry_path: &BStr, - find: &mut Find, - root: &std::path::Path, - index::checkout::Options { symlinks }: index::checkout::Options, - buf: &mut Vec, - ) -> Result<(), index::checkout::Error> - where - Find: for<'a> FnMut(&oid, &'a mut Vec) -> Option>, - { - let dest = root.join(git_features::path::from_byte_slice(entry_path).map_err(|_| { - index::checkout::Error::IllformedUtf8 { - path: entry_path.to_owned(), - } - })?); - create_dir_all(dest.parent().expect("entry paths are never empty"))?; // TODO: can this be avoided to create dirs when needed only? - - match entry.mode { - git_index::entry::Mode::FILE | git_index::entry::Mode::FILE_EXECUTABLE => { - let obj = find(&entry.id, buf).ok_or_else(|| index::checkout::Error::ObjectNotFound { - oid: entry.id, - path: root.to_path_buf(), - })?; - let mut options = OpenOptions::new(); - options.write(true).create_new(true); - #[cfg(unix)] - if entry.mode == git_index::entry::Mode::FILE_EXECUTABLE { - use std::os::unix::fs::OpenOptionsExt; - options.mode(0o777); - } - let mut file = options.open(&dest)?; - file.write_all(obj.data)?; - let met = file.metadata()?; - let ctime = met - .created() - .map_or(Ok(Duration::default()), |x| x.duration_since(std::time::UNIX_EPOCH))?; - let mtime = met - .modified() - .map_or(Ok(Duration::default()), |x| x.duration_since(std::time::UNIX_EPOCH))?; - - update_fstat(entry, ctime, mtime)?; - } - git_index::entry::Mode::SYMLINK => { - let obj = find(&entry.id, buf).ok_or_else(|| index::checkout::Error::ObjectNotFound { - oid: entry.id, - path: root.to_path_buf(), - })?; - let symlink_destination = git_features::path::from_byte_slice(obj.data) - .map_err(|_| index::checkout::Error::IllformedUtf8 { path: obj.data.into() })?; - if symlinks { - #[cfg(unix)] - std::os::unix::fs::symlink(symlink_destination, &dest)?; - #[cfg(windows)] - if dest.exists() { - if dest.is_file() { - std::os::windows::fs::symlink_file(symlink_destination, &dest)?; - } else { - std::os::windows::fs::symlink_dir(symlink_destination, &dest)?; - } - } - } else { - std::fs::write(&dest, obj.data)?; - } - let met = std::fs::symlink_metadata(&dest)?; - let ctime = met - .created() - .map_or(Ok(Duration::default()), |x| x.duration_since(std::time::UNIX_EPOCH))?; - let mtime = met - .modified() - .map_or(Ok(Duration::default()), |x| x.duration_since(std::time::UNIX_EPOCH))?; - update_fstat(entry, ctime, mtime)?; - } - git_index::entry::Mode::DIR => todo!(), - git_index::entry::Mode::COMMIT => todo!(), - _ => unreachable!(), - } - Ok(()) - } - - fn update_fstat(entry: &mut Entry, ctime: Duration, mtime: Duration) -> Result<(), index::checkout::Error> { - let stat = &mut entry.stat; - stat.mtime.secs = mtime - .as_secs() - .try_into() - .expect("by 2038 we found a solution for this"); - stat.mtime.nsecs = mtime.subsec_nanos(); - stat.ctime.secs = ctime - .as_secs() - .try_into() - .expect("by 2038 we found a solution for this"); - stat.ctime.nsecs = ctime.subsec_nanos(); - Ok(()) - } - } -} +pub mod index; From de3749e1426d48a1d31a0ddc1fddfdb394a01078 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 27 Feb 2022 10:57:33 +0800 Subject: [PATCH 02/19] sketch filesystem context, without probing for now (#301) --- git-worktree/src/lib.rs | 58 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/git-worktree/src/lib.rs b/git-worktree/src/lib.rs index ccd3937c4a6..f3c7df0ac3b 100644 --- a/git-worktree/src/lib.rs +++ b/git-worktree/src/lib.rs @@ -1,3 +1,61 @@ #![forbid(unsafe_code, rust_2018_idioms)] +/// file system related utilities +pub mod fs { + /// Common knowledge about the worktree that is needed across most interactions with the work tree + pub struct Context { + /// If true, the filesystem will store paths as decomposed unicode, i.e. `ä` becomes `"a\u{308}"`, which means that + /// we have to turn these forms back from decomposed to precomposed unicode before storing it in the index or generally + /// using it. This also applies to input received from the command-line, so callers may have to be aware of this and + /// perform conversions accordingly. + /// If false, no conversions will be performed. + pub precompose_unicode: bool, + /// If true, the filesystem ignores the case of input, which makes `A` the same file as `a`. + /// This is also called case-folding. + pub ignore_case: bool, + /// If true, we assume the the executable bit is honored as part of the files mode. If false, we assume the file system + /// ignores the executable bit, hence it will be reported as 'off' even though we just tried to set it to be on. + pub file_mode: bool, + /// If true, the file system supports symbolic links and we should try to create them. Otherwise symbolic links will be checked + /// out as files which contain the link as text. + pub symlink: bool, + } + + #[cfg(windows)] + impl Default for Context { + fn default() -> Self { + Context { + precompose_unicode: false, + ignore_case: true, + file_mode: false, + symlink: false, + } + } + } + + #[cfg(target_os = "macos")] + impl Default for Context { + fn default() -> Self { + Context { + precompose_unicode: true, + ignore_case: true, + file_mode: true, + symlink: true, + } + } + } + + #[cfg(all(unix, not(target_os = "macos")))] + impl Default for Context { + fn default() -> Self { + Context { + precompose_unicode: false, + ignore_case: false, + file_mode: true, + symlink: true, + } + } + } +} + pub mod index; From f11929c9652b2f414029f2ad02dacee238a138d1 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 27 Feb 2022 11:02:41 +0800 Subject: [PATCH 03/19] Support for 'serde1' feature in git-worktree (#301) --- Cargo.lock | 2 ++ Makefile | 1 + git-worktree/Cargo.toml | 12 ++++++++++++ git-worktree/src/lib.rs | 2 ++ 4 files changed, 17 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 07445725a14..8a30d16c5b6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1779,6 +1779,7 @@ name = "git-worktree" version = "0.0.0" dependencies = [ "bstr", + "document-features", "git-features 0.19.1", "git-hash 0.9.2", "git-index", @@ -1786,6 +1787,7 @@ dependencies = [ "git-odb 0.27.0", "git-testtools", "quick-error", + "serde", "tempfile", "walkdir", ] diff --git a/Makefile b/Makefile index ab069ec3cbb..0b499addcac 100644 --- a/Makefile +++ b/Makefile @@ -89,6 +89,7 @@ check: ## Build all code in suitable configurations cd git-object && cargo check --all-features \ && cargo check --features verbose-object-parsing-errors cd git-index && cargo check --features serde1 + cd git-worktree && cargo check --features serde1 cd git-actor && cargo check --features serde1 cd git-pack && cargo check --features serde1 \ && cargo check --features pack-cache-lru-static \ diff --git a/git-worktree/Cargo.toml b/git-worktree/Cargo.toml index b9dca5753e5..48f8a1ba771 100644 --- a/git-worktree/Cargo.toml +++ b/git-worktree/Cargo.toml @@ -12,18 +12,30 @@ doctest = false # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html +[features] +## Data structures implement `serde::Serialize` and `serde::Deserialize`. +serde1 = [ "serde", "bstr/serde1", "git-index/serde1", "git-hash/serde1", "git-object/serde1" ] + [dependencies] git-index = { version = "^0.1.0", path = "../git-index" } git-hash = { version = "^0.9.0", path = "../git-hash" } git-object = { version = "^0.17.0", path = "../git-object" } git-features = { version = "^0.19.1", path = "../git-features" } +serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"]} + quick-error = "2.0.1" bstr = { version = "0.2.13", default-features = false } +document-features = { version = "0.2.0", optional = true } + [dev-dependencies] git-testtools = { path = "../tests/tools" } git-odb = { path = "../git-odb" } walkdir = "2.3.2" tempfile = "3.2.0" + +[package.metadata.docs.rs] +features = ["document-features"] +all-features = true diff --git a/git-worktree/src/lib.rs b/git-worktree/src/lib.rs index f3c7df0ac3b..150f2e80064 100644 --- a/git-worktree/src/lib.rs +++ b/git-worktree/src/lib.rs @@ -3,6 +3,8 @@ /// file system related utilities pub mod fs { /// Common knowledge about the worktree that is needed across most interactions with the work tree + #[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] + #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)] pub struct Context { /// If true, the filesystem will store paths as decomposed unicode, i.e. `ä` becomes `"a\u{308}"`, which means that /// we have to turn these forms back from decomposed to precomposed unicode before storing it in the index or generally From 1367cf5bc5908639e67e12f78f57835c5fd68a90 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 27 Feb 2022 11:06:34 +0800 Subject: [PATCH 04/19] document-features support for git-index and git-worktree (#301) --- Cargo.lock | 1 + git-index/Cargo.toml | 6 ++++++ git-index/src/lib.rs | 5 +++++ git-worktree/Cargo.toml | 3 +-- git-worktree/src/lib.rs | 5 +++++ 5 files changed, 18 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8a30d16c5b6..6100ddd7a24 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1315,6 +1315,7 @@ dependencies = [ "atoi", "bitflags", "bstr", + "document-features", "filetime", "git-bitmap", "git-features 0.19.1", diff --git a/git-index/Cargo.toml b/git-index/Cargo.toml index 35fbdbf69a3..44eac69f185 100644 --- a/git-index/Cargo.toml +++ b/git-index/Cargo.toml @@ -46,5 +46,11 @@ smallvec = "1.7.0" atoi = "1.0.0" bitflags = "1.3.2" +document-features = { version = "0.2.0", optional = true } + [dev-dependencies] git-testtools = { path = "../tests/tools"} + +[package.metadata.docs.rs] +features = ["document-features", "serde1"] + diff --git a/git-index/src/lib.rs b/git-index/src/lib.rs index 1f6ff98a791..8151618edc6 100644 --- a/git-index/src/lib.rs +++ b/git-index/src/lib.rs @@ -1,3 +1,8 @@ +//! ## Feature Flags +#![cfg_attr( + feature = "document-features", + cfg_attr(doc, doc = ::document_features::document_features!()) +)] #![deny(unsafe_code, missing_docs, rust_2018_idioms)] #![allow(missing_docs)] diff --git a/git-worktree/Cargo.toml b/git-worktree/Cargo.toml index 48f8a1ba771..1ac36e3df15 100644 --- a/git-worktree/Cargo.toml +++ b/git-worktree/Cargo.toml @@ -37,5 +37,4 @@ walkdir = "2.3.2" tempfile = "3.2.0" [package.metadata.docs.rs] -features = ["document-features"] -all-features = true +features = ["document-features", "serde1"] diff --git a/git-worktree/src/lib.rs b/git-worktree/src/lib.rs index 150f2e80064..2883e3cd2df 100644 --- a/git-worktree/src/lib.rs +++ b/git-worktree/src/lib.rs @@ -1,3 +1,8 @@ +//! ## Feature Flags +#![cfg_attr( + feature = "document-features", + cfg_attr(doc, doc = ::document_features::document_features!()) +)] #![forbid(unsafe_code, rust_2018_idioms)] /// file system related utilities From 8914fcc114cdf920f2f4162e71d4d390007f6f3b Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 28 Feb 2022 13:58:24 +0800 Subject: [PATCH 05/19] refactor for checkout to use fs::Context (#301) The latter should be useful when fully implementing all required baseline capabilities of doing a correct checkout --- git-worktree/src/fs.rs | 56 ++++++++++++++++++ git-worktree/src/index.rs | 17 +++--- git-worktree/src/lib.rs | 59 +------------------ .../fixtures/make_ignorecase_collisions.sh | 11 ++++ ...po.sh => make_mixed_without_submodules.sh} | 0 git-worktree/tests/index/mod.rs | 29 ++++++--- 6 files changed, 96 insertions(+), 76 deletions(-) create mode 100644 git-worktree/src/fs.rs create mode 100644 git-worktree/tests/fixtures/make_ignorecase_collisions.sh rename git-worktree/tests/fixtures/{make_repo.sh => make_mixed_without_submodules.sh} (100%) diff --git a/git-worktree/src/fs.rs b/git-worktree/src/fs.rs new file mode 100644 index 00000000000..7fb7e19469c --- /dev/null +++ b/git-worktree/src/fs.rs @@ -0,0 +1,56 @@ +/// Common knowledge about the worktree that is needed across most interactions with the work tree +#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] +#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)] +pub struct Context { + /// If true, the filesystem will store paths as decomposed unicode, i.e. `ä` becomes `"a\u{308}"`, which means that + /// we have to turn these forms back from decomposed to precomposed unicode before storing it in the index or generally + /// using it. This also applies to input received from the command-line, so callers may have to be aware of this and + /// perform conversions accordingly. + /// If false, no conversions will be performed. + pub precompose_unicode: bool, + /// If true, the filesystem ignores the case of input, which makes `A` the same file as `a`. + /// This is also called case-folding. + pub ignore_case: bool, + /// If true, we assume the the executable bit is honored as part of the files mode. If false, we assume the file system + /// ignores the executable bit, hence it will be reported as 'off' even though we just tried to set it to be on. + pub file_mode: bool, + /// If true, the file system supports symbolic links and we should try to create them. Otherwise symbolic links will be checked + /// out as files which contain the link as text. + pub symlink: bool, +} + +#[cfg(windows)] +impl Default for Context { + fn default() -> Self { + Context { + precompose_unicode: false, + ignore_case: true, + file_mode: false, + symlink: false, + } + } +} + +#[cfg(target_os = "macos")] +impl Default for Context { + fn default() -> Self { + Context { + precompose_unicode: true, + ignore_case: true, + file_mode: true, + symlink: true, + } + } +} + +#[cfg(all(unix, not(target_os = "macos")))] +impl Default for Context { + fn default() -> Self { + Context { + precompose_unicode: false, + ignore_case: false, + file_mode: true, + symlink: true, + } + } +} diff --git a/git-worktree/src/index.rs b/git-worktree/src/index.rs index e67775c193f..8b674dd0dd3 100644 --- a/git-worktree/src/index.rs +++ b/git-worktree/src/index.rs @@ -4,15 +4,10 @@ pub mod checkout { use bstr::BString; use quick_error::quick_error; - #[derive(Clone, Copy)] + #[derive(Default, Clone, Copy)] pub struct Options { - pub symlinks: bool, - } - - impl Default for Options { - fn default() -> Self { - Options { symlinks: true } - } + /// capabilities of the file system + pub fs: crate::fs::Context, } quick_error! { @@ -79,7 +74,9 @@ pub(crate) mod entry { entry_path: &BStr, find: &mut Find, root: &std::path::Path, - index::checkout::Options { symlinks }: index::checkout::Options, + index::checkout::Options { + fs: crate::fs::Context { symlink, .. }, + }: index::checkout::Options, buf: &mut Vec, ) -> Result<(), index::checkout::Error> where @@ -124,7 +121,7 @@ pub(crate) mod entry { })?; let symlink_destination = git_features::path::from_byte_slice(obj.data) .map_err(|_| index::checkout::Error::IllformedUtf8 { path: obj.data.into() })?; - if symlinks { + if symlink { #[cfg(unix)] std::os::unix::fs::symlink(symlink_destination, &dest)?; #[cfg(windows)] diff --git a/git-worktree/src/lib.rs b/git-worktree/src/lib.rs index 2883e3cd2df..e0bf42a64dd 100644 --- a/git-worktree/src/lib.rs +++ b/git-worktree/src/lib.rs @@ -6,63 +6,6 @@ #![forbid(unsafe_code, rust_2018_idioms)] /// file system related utilities -pub mod fs { - /// Common knowledge about the worktree that is needed across most interactions with the work tree - #[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] - #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)] - pub struct Context { - /// If true, the filesystem will store paths as decomposed unicode, i.e. `ä` becomes `"a\u{308}"`, which means that - /// we have to turn these forms back from decomposed to precomposed unicode before storing it in the index or generally - /// using it. This also applies to input received from the command-line, so callers may have to be aware of this and - /// perform conversions accordingly. - /// If false, no conversions will be performed. - pub precompose_unicode: bool, - /// If true, the filesystem ignores the case of input, which makes `A` the same file as `a`. - /// This is also called case-folding. - pub ignore_case: bool, - /// If true, we assume the the executable bit is honored as part of the files mode. If false, we assume the file system - /// ignores the executable bit, hence it will be reported as 'off' even though we just tried to set it to be on. - pub file_mode: bool, - /// If true, the file system supports symbolic links and we should try to create them. Otherwise symbolic links will be checked - /// out as files which contain the link as text. - pub symlink: bool, - } - - #[cfg(windows)] - impl Default for Context { - fn default() -> Self { - Context { - precompose_unicode: false, - ignore_case: true, - file_mode: false, - symlink: false, - } - } - } - - #[cfg(target_os = "macos")] - impl Default for Context { - fn default() -> Self { - Context { - precompose_unicode: true, - ignore_case: true, - file_mode: true, - symlink: true, - } - } - } - - #[cfg(all(unix, not(target_os = "macos")))] - impl Default for Context { - fn default() -> Self { - Context { - precompose_unicode: false, - ignore_case: false, - file_mode: true, - symlink: true, - } - } - } -} +pub mod fs; pub mod index; diff --git a/git-worktree/tests/fixtures/make_ignorecase_collisions.sh b/git-worktree/tests/fixtures/make_ignorecase_collisions.sh new file mode 100644 index 00000000000..4921fff70da --- /dev/null +++ b/git-worktree/tests/fixtures/make_ignorecase_collisions.sh @@ -0,0 +1,11 @@ +#!/bin/bash +set -eu -o pipefail + +git init -q +git config commit.gpgsign false + +touch a A +git add a +echo A | git update-index --add --stdin + +git commit -m "init" diff --git a/git-worktree/tests/fixtures/make_repo.sh b/git-worktree/tests/fixtures/make_mixed_without_submodules.sh similarity index 100% rename from git-worktree/tests/fixtures/make_repo.sh rename to git-worktree/tests/fixtures/make_mixed_without_submodules.sh diff --git a/git-worktree/tests/index/mod.rs b/git-worktree/tests/index/mod.rs index a572dbe9ccc..6585332c758 100644 --- a/git-worktree/tests/index/mod.rs +++ b/git-worktree/tests/index/mod.rs @@ -15,19 +15,29 @@ mod checkout { #[test] fn allow_symlinks() -> crate::Result { - let opts = Default::default(); - let (source_tree, destination) = setup_fixture_with_options(opts)?; + let opts = index::checkout::Options { + fs: git_worktree::fs::Context { + symlink: true, + ..Default::default() + }, + }; + let (source_tree, destination) = setup_fixture_with_options(opts, "make_mixed_without_submodules")?; - assert_equality(&source_tree, &destination, opts.symlinks)?; + assert_equality(&source_tree, &destination, opts.fs.symlink)?; Ok(()) } #[test] fn symlinks_become_files_if_disabled() -> crate::Result { - let opts = index::checkout::Options { symlinks: false }; - let (source_tree, destination) = setup_fixture_with_options(opts)?; + let opts = index::checkout::Options { + fs: git_worktree::fs::Context { + symlink: false, + ..Default::default() + }, + }; + let (source_tree, destination) = setup_fixture_with_options(opts, "make_mixed_without_submodules")?; - assert_equality(&source_tree, &destination, opts.symlinks)?; + assert_equality(&source_tree, &destination, opts.fs.symlink)?; Ok(()) } @@ -71,8 +81,11 @@ mod checkout { files } - fn setup_fixture_with_options(opts: git_worktree::index::checkout::Options) -> crate::Result<(PathBuf, TempDir)> { - let source_tree = fixture_path("make_repo"); + fn setup_fixture_with_options( + opts: git_worktree::index::checkout::Options, + name: &str, + ) -> crate::Result<(PathBuf, TempDir)> { + let source_tree = fixture_path(name); let git_dir = source_tree.join(".git"); let mut index = git_index::File::at(git_dir.join("index"), Default::default())?; let odb = git_odb::at(git_dir.join("objects"))?; From 178beb42eaf1112143299eafa7fc93106eb9fc5b Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 28 Feb 2022 14:07:32 +0800 Subject: [PATCH 06/19] make clear that we are currently only dealing with checkout during clone (#301) --- git-worktree/src/index.rs | 7 +++++++ git-worktree/tests/index/mod.rs | 23 ++++++++++++----------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/git-worktree/src/index.rs b/git-worktree/src/index.rs index 8b674dd0dd3..db0fa0937f3 100644 --- a/git-worktree/src/index.rs +++ b/git-worktree/src/index.rs @@ -8,6 +8,9 @@ pub mod checkout { pub struct Options { /// capabilities of the file system pub fs: crate::fs::Context, + /// If true, we assume no file to exist in the target directory, and want exclusive access to it. + /// This should be enabled when cloning. + pub destination_is_initially_empty: bool, } quick_error! { @@ -42,6 +45,9 @@ pub fn checkout( where Find: for<'a> FnMut(&oid, &'a mut Vec) -> Option>, { + if !options.destination_is_initially_empty { + todo!("non-clone logic isn't implemented or vetted yet"); + } let root = path.as_ref(); let mut buf = Vec::new(); for (entry, entry_path) in index.entries_mut_with_paths() { @@ -76,6 +82,7 @@ pub(crate) mod entry { root: &std::path::Path, index::checkout::Options { fs: crate::fs::Context { symlink, .. }, + .. }: index::checkout::Options, buf: &mut Vec, ) -> Result<(), index::checkout::Error> diff --git a/git-worktree/tests/index/mod.rs b/git-worktree/tests/index/mod.rs index 6585332c758..b3e8b7a2718 100644 --- a/git-worktree/tests/index/mod.rs +++ b/git-worktree/tests/index/mod.rs @@ -9,32 +9,33 @@ mod checkout { use git_object::bstr::ByteSlice; use git_odb::FindExt; use git_worktree::index; + use git_worktree::index::checkout::Options; use tempfile::TempDir; use crate::fixture_path; #[test] fn allow_symlinks() -> crate::Result { - let opts = index::checkout::Options { - fs: git_worktree::fs::Context { - symlink: true, - ..Default::default() - }, - }; + let opts = opts_with_symlink(true); let (source_tree, destination) = setup_fixture_with_options(opts, "make_mixed_without_submodules")?; assert_equality(&source_tree, &destination, opts.fs.symlink)?; Ok(()) } - #[test] - fn symlinks_become_files_if_disabled() -> crate::Result { - let opts = index::checkout::Options { + fn opts_with_symlink(symlink: bool) -> Options { + index::checkout::Options { fs: git_worktree::fs::Context { - symlink: false, + symlink, ..Default::default() }, - }; + destination_is_initially_empty: true, + } + } + + #[test] + fn symlinks_become_files_if_disabled() -> crate::Result { + let opts = opts_with_symlink(false); let (source_tree, destination) = setup_fixture_with_options(opts, "make_mixed_without_submodules")?; assert_equality(&source_tree, &destination, opts.fs.symlink)?; From 1bfbf1d120e31474367cd2008e1715c50af19071 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 28 Feb 2022 16:29:47 +0800 Subject: [PATCH 07/19] symlink probing (#301) --- Cargo.lock | 7 ++++++ git-worktree/Cargo.toml | 1 + git-worktree/src/fs.rs | 43 +++++++++++++++++++++++++++++++++ git-worktree/tests/index/mod.rs | 5 ++++ 4 files changed, 56 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 6100ddd7a24..352882b0508 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1789,6 +1789,7 @@ dependencies = [ "git-testtools", "quick-error", "serde", + "symlink", "tempfile", "walkdir", ] @@ -3010,6 +3011,12 @@ version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" +[[package]] +name = "symlink" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a7973cce6668464ea31f176d85b13c7ab3bba2cb3b77a2ed26abd7801688010a" + [[package]] name = "syn" version = "1.0.86" diff --git a/git-worktree/Cargo.toml b/git-worktree/Cargo.toml index 1ac36e3df15..9f93704ff69 100644 --- a/git-worktree/Cargo.toml +++ b/git-worktree/Cargo.toml @@ -28,6 +28,7 @@ quick-error = "2.0.1" bstr = { version = "0.2.13", default-features = false } document-features = { version = "0.2.0", optional = true } +symlink = "0.1.0" [dev-dependencies] git-testtools = { path = "../tests/tools" } diff --git a/git-worktree/src/fs.rs b/git-worktree/src/fs.rs index 7fb7e19469c..8336031afd4 100644 --- a/git-worktree/src/fs.rs +++ b/git-worktree/src/fs.rs @@ -19,6 +19,49 @@ pub struct Context { pub symlink: bool, } +impl Context { + /// try to determine all values in this context by probing them in the current working directory, which should be on the file system + /// the git repository is located on. + /// + /// All errors are ignored and interpreted on top of the default for the platform the binary is compiled for. + pub fn from_probing_cwd() -> Self { + let ctx = Context::default(); + Context { + symlink: Self::probe_symlink().unwrap_or(ctx.symlink), + ..ctx + } + } + + fn probe_symlink() -> Option { + let src_path = "__link_src_file"; + std::fs::File::options() + .create_new(true) + .write(true) + .open(src_path) + .ok()?; + let link_path = "__file_link"; + if symlink::symlink_file(src_path, link_path).is_err() { + std::fs::remove_file(src_path).ok(); + return None; + } + let cleanup_all = || { + std::fs::remove_file(src_path).ok(); + symlink::remove_symlink_file(link_path) + .or_else(|_| std::fs::remove_file(link_path)) + .ok(); + }; + + let res = Some( + std::fs::symlink_metadata(link_path) + .map_err(|_| cleanup_all()) + .ok()? + .is_symlink(), + ); + cleanup_all(); + res + } +} + #[cfg(windows)] impl Default for Context { fn default() -> Self { diff --git a/git-worktree/tests/index/mod.rs b/git-worktree/tests/index/mod.rs index b3e8b7a2718..aa9de364477 100644 --- a/git-worktree/tests/index/mod.rs +++ b/git-worktree/tests/index/mod.rs @@ -17,6 +17,11 @@ mod checkout { #[test] fn allow_symlinks() -> crate::Result { let opts = opts_with_symlink(true); + if !git_worktree::fs::Context::from_probing_cwd().symlink { + eprintln!("IGNORING symlink test on file system without symlink support"); + // skip if symlinks aren't supported anyway. + return Ok(()); + }; let (source_tree, destination) = setup_fixture_with_options(opts, "make_mixed_without_submodules")?; assert_equality(&source_tree, &destination, opts.fs.symlink)?; From adbed121f969a05b622d0325b434b3c6d44ae248 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 28 Feb 2022 16:48:14 +0800 Subject: [PATCH 08/19] basic test for filesystem probing (#301) --- git-worktree/src/fs.rs | 51 ++++++++++++++++----------------- git-worktree/tests/fs/mod.rs | 16 +++++++++++ git-worktree/tests/index/mod.rs | 2 +- git-worktree/tests/worktree.rs | 1 + 4 files changed, 42 insertions(+), 28 deletions(-) create mode 100644 git-worktree/tests/fs/mod.rs diff --git a/git-worktree/src/fs.rs b/git-worktree/src/fs.rs index 8336031afd4..04237791fc6 100644 --- a/git-worktree/src/fs.rs +++ b/git-worktree/src/fs.rs @@ -1,3 +1,5 @@ +use std::path::Path; + /// Common knowledge about the worktree that is needed across most interactions with the work tree #[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)] @@ -20,45 +22,40 @@ pub struct Context { } impl Context { - /// try to determine all values in this context by probing them in the current working directory, which should be on the file system - /// the git repository is located on. + /// try to determine all values in this context by probing them in the given `directory`, which + /// should be on the file system the git repository is located on. /// /// All errors are ignored and interpreted on top of the default for the platform the binary is compiled for. - pub fn from_probing_cwd() -> Self { + pub fn probe(directory: impl AsRef) -> Self { + let root = directory.as_ref(); let ctx = Context::default(); Context { - symlink: Self::probe_symlink().unwrap_or(ctx.symlink), + symlink: Self::probe_symlink(root).unwrap_or(ctx.symlink), ..ctx } } - fn probe_symlink() -> Option { - let src_path = "__link_src_file"; - std::fs::File::options() - .create_new(true) - .write(true) - .open(src_path) - .ok()?; - let link_path = "__file_link"; - if symlink::symlink_file(src_path, link_path).is_err() { - std::fs::remove_file(src_path).ok(); - return None; + fn probe_symlink(root: &Path) -> std::io::Result { + let src_path = root.join("__link_src_file"); + std::fs::File::options().create_new(true).write(true).open(&src_path)?; + let link_path = root.join("__file_link"); + if symlink::symlink_file(&src_path, &link_path).is_err() { + std::fs::remove_file(&src_path)?; + return Ok(false); } let cleanup_all = || { - std::fs::remove_file(src_path).ok(); - symlink::remove_symlink_file(link_path) - .or_else(|_| std::fs::remove_file(link_path)) - .ok(); + let res = std::fs::remove_file(&src_path); + symlink::remove_symlink_file(&link_path) + .or_else(|_| std::fs::remove_file(&link_path)) + .and(res) }; - let res = Some( - std::fs::symlink_metadata(link_path) - .map_err(|_| cleanup_all()) - .ok()? - .is_symlink(), - ); - cleanup_all(); - res + let res = std::fs::symlink_metadata(&link_path) + .or_else(|err| cleanup_all().and(Err(err)))? + .is_symlink(); + + cleanup_all()?; + Ok(res) } } diff --git a/git-worktree/tests/fs/mod.rs b/git-worktree/tests/fs/mod.rs new file mode 100644 index 00000000000..cfa15bede2f --- /dev/null +++ b/git-worktree/tests/fs/mod.rs @@ -0,0 +1,16 @@ +#[test] +fn from_probing_cwd() { + let dir = tempfile::tempdir().unwrap(); + let _ctx = git_worktree::fs::Context::probe(dir.path()); + let entries: Vec<_> = std::fs::read_dir(dir.path()) + .unwrap() + .filter_map(Result::ok) + .map(|e| e.path().to_owned()) + .collect(); + assert_eq!( + entries.len(), + 0, + "there should be no left-over files after probing, found {:?}", + entries + ); +} diff --git a/git-worktree/tests/index/mod.rs b/git-worktree/tests/index/mod.rs index aa9de364477..fd1661f53ac 100644 --- a/git-worktree/tests/index/mod.rs +++ b/git-worktree/tests/index/mod.rs @@ -17,7 +17,7 @@ mod checkout { #[test] fn allow_symlinks() -> crate::Result { let opts = opts_with_symlink(true); - if !git_worktree::fs::Context::from_probing_cwd().symlink { + if !git_worktree::fs::Context::probe(std::env::current_dir()?).symlink { eprintln!("IGNORING symlink test on file system without symlink support"); // skip if symlinks aren't supported anyway. return Ok(()); diff --git a/git-worktree/tests/worktree.rs b/git-worktree/tests/worktree.rs index 830e8787f43..ad199f619be 100644 --- a/git-worktree/tests/worktree.rs +++ b/git-worktree/tests/worktree.rs @@ -1,5 +1,6 @@ use std::path::{Path, PathBuf}; +mod fs; mod index; type Result = std::result::Result>; From f8e1de0dc031ad73084b2da6a6d39960b9b78b4b Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 28 Feb 2022 17:02:37 +0800 Subject: [PATCH 09/19] determine filesystem case (#301) --- git-worktree/src/fs.rs | 23 +++++++++++++++++++---- git-worktree/tests/fs/mod.rs | 5 ++++- git-worktree/tests/index/mod.rs | 2 +- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/git-worktree/src/fs.rs b/git-worktree/src/fs.rs index 04237791fc6..ce0ffa84255 100644 --- a/git-worktree/src/fs.rs +++ b/git-worktree/src/fs.rs @@ -22,22 +22,37 @@ pub struct Context { } impl Context { - /// try to determine all values in this context by probing them in the given `directory`, which + /// try to determine all values in this context by probing them in the given `git_dir`, which /// should be on the file system the git repository is located on. + /// `git_dir` is a typical git repository, expected to be populated with the typical files like `config`. /// /// All errors are ignored and interpreted on top of the default for the platform the binary is compiled for. - pub fn probe(directory: impl AsRef) -> Self { - let root = directory.as_ref(); + pub fn probe(git_dir: impl AsRef) -> Self { + let root = git_dir.as_ref(); let ctx = Context::default(); Context { symlink: Self::probe_symlink(root).unwrap_or(ctx.symlink), + ignore_case: Self::probe_ignore_case(root).unwrap_or(ctx.ignore_case), ..ctx } } + fn probe_ignore_case(git_dir: &Path) -> std::io::Result { + std::fs::metadata(git_dir.join("cOnFiG")).map(|_| true).or_else(|err| { + if err.kind() == std::io::ErrorKind::NotFound { + Ok(false) + } else { + Err(err) + } + }) + } + fn probe_symlink(root: &Path) -> std::io::Result { let src_path = root.join("__link_src_file"); - std::fs::File::options().create_new(true).write(true).open(&src_path)?; + std::fs::OpenOptions::new() + .create_new(true) + .write(true) + .open(&src_path)?; let link_path = root.join("__file_link"); if symlink::symlink_file(&src_path, &link_path).is_err() { std::fs::remove_file(&src_path)?; diff --git a/git-worktree/tests/fs/mod.rs b/git-worktree/tests/fs/mod.rs index cfa15bede2f..e4c3dac7bea 100644 --- a/git-worktree/tests/fs/mod.rs +++ b/git-worktree/tests/fs/mod.rs @@ -1,10 +1,13 @@ #[test] fn from_probing_cwd() { let dir = tempfile::tempdir().unwrap(); - let _ctx = git_worktree::fs::Context::probe(dir.path()); + std::fs::File::create(dir.path().join("config")).unwrap(); + let ctx = git_worktree::fs::Context::probe(dir.path()); + dbg!(ctx); let entries: Vec<_> = std::fs::read_dir(dir.path()) .unwrap() .filter_map(Result::ok) + .filter(|e| e.file_name().to_str() != Some("config")) .map(|e| e.path().to_owned()) .collect(); assert_eq!( diff --git a/git-worktree/tests/index/mod.rs b/git-worktree/tests/index/mod.rs index fd1661f53ac..f3b103e9f2d 100644 --- a/git-worktree/tests/index/mod.rs +++ b/git-worktree/tests/index/mod.rs @@ -17,7 +17,7 @@ mod checkout { #[test] fn allow_symlinks() -> crate::Result { let opts = opts_with_symlink(true); - if !git_worktree::fs::Context::probe(std::env::current_dir()?).symlink { + if !git_worktree::fs::Context::probe(std::env::current_dir()?.join("..").join(".git")).symlink { eprintln!("IGNORING symlink test on file system without symlink support"); // skip if symlinks aren't supported anyway. return Ok(()); From fc816bd12f142d1df4d10429ee5b56e9eb5fbf4d Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 28 Feb 2022 17:14:55 +0800 Subject: [PATCH 10/19] refactor (#301) --- git-worktree/src/index.rs | 48 ++++++++++++++++----------------------- 1 file changed, 19 insertions(+), 29 deletions(-) diff --git a/git-worktree/src/index.rs b/git-worktree/src/index.rs index db0fa0937f3..0df607eab2d 100644 --- a/git-worktree/src/index.rs +++ b/git-worktree/src/index.rs @@ -109,17 +109,14 @@ pub(crate) mod entry { use std::os::unix::fs::OpenOptionsExt; options.mode(0o777); } - let mut file = options.open(&dest)?; - file.write_all(obj.data)?; - let met = file.metadata()?; - let ctime = met - .created() - .map_or(Ok(Duration::default()), |x| x.duration_since(std::time::UNIX_EPOCH))?; - let mtime = met - .modified() - .map_or(Ok(Duration::default()), |x| x.duration_since(std::time::UNIX_EPOCH))?; - update_fstat(entry, ctime, mtime)?; + { + let mut file = options.open(&dest)?; + file.write_all(obj.data)?; + // NOTE: we don't call `file.sync_all()` here knowing that some filesystems don't handle this well. + // revisit this once there is a bug to fix. + } + update_fstat(entry, dest.symlink_metadata()?)?; } git_index::entry::Mode::SYMLINK => { let obj = find(&entry.id, buf).ok_or_else(|| index::checkout::Error::ObjectNotFound { @@ -128,28 +125,14 @@ pub(crate) mod entry { })?; let symlink_destination = git_features::path::from_byte_slice(obj.data) .map_err(|_| index::checkout::Error::IllformedUtf8 { path: obj.data.into() })?; + if symlink { - #[cfg(unix)] - std::os::unix::fs::symlink(symlink_destination, &dest)?; - #[cfg(windows)] - if dest.exists() { - if dest.is_file() { - std::os::windows::fs::symlink_file(symlink_destination, &dest)?; - } else { - std::os::windows::fs::symlink_dir(symlink_destination, &dest)?; - } - } + symlink::symlink_auto(symlink_destination, &dest)?; } else { std::fs::write(&dest, obj.data)?; } - let met = std::fs::symlink_metadata(&dest)?; - let ctime = met - .created() - .map_or(Ok(Duration::default()), |x| x.duration_since(std::time::UNIX_EPOCH))?; - let mtime = met - .modified() - .map_or(Ok(Duration::default()), |x| x.duration_since(std::time::UNIX_EPOCH))?; - update_fstat(entry, ctime, mtime)?; + + update_fstat(entry, std::fs::symlink_metadata(&dest)?)?; } git_index::entry::Mode::DIR => todo!(), git_index::entry::Mode::COMMIT => todo!(), @@ -158,7 +141,14 @@ pub(crate) mod entry { Ok(()) } - fn update_fstat(entry: &mut Entry, ctime: Duration, mtime: Duration) -> Result<(), index::checkout::Error> { + fn update_fstat(entry: &mut Entry, meta: std::fs::Metadata) -> Result<(), index::checkout::Error> { + let ctime = meta + .created() + .map_or(Ok(Duration::default()), |x| x.duration_since(std::time::UNIX_EPOCH))?; + let mtime = meta + .modified() + .map_or(Ok(Duration::default()), |x| x.duration_since(std::time::UNIX_EPOCH))?; + let stat = &mut entry.stat; stat.mtime.secs = mtime .as_secs() From 0c1c00689000dfc943ed25cd52eac42e3642a78c Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 28 Feb 2022 17:24:10 +0800 Subject: [PATCH 11/19] probe precompose unicode (#301) --- git-worktree/src/fs.rs | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/git-worktree/src/fs.rs b/git-worktree/src/fs.rs index ce0ffa84255..4d641ef7680 100644 --- a/git-worktree/src/fs.rs +++ b/git-worktree/src/fs.rs @@ -33,6 +33,7 @@ impl Context { Context { symlink: Self::probe_symlink(root).unwrap_or(ctx.symlink), ignore_case: Self::probe_ignore_case(root).unwrap_or(ctx.ignore_case), + precompose_unicode: Self::probe_precompose_unicode(root).unwrap_or(ctx.precompose_unicode), ..ctx } } @@ -47,6 +48,20 @@ impl Context { }) } + fn probe_precompose_unicode(root: &Path) -> std::io::Result { + let precomposed = "ä"; + let decomposed = "a\u{308}"; + + let precomposed = root.join(precomposed); + std::fs::OpenOptions::new() + .create_new(true) + .write(true) + .open(&precomposed)?; + let res = root.join(decomposed).symlink_metadata().map(|_| true); + std::fs::remove_file(precomposed)?; + res + } + fn probe_symlink(root: &Path) -> std::io::Result { let src_path = root.join("__link_src_file"); std::fs::OpenOptions::new() @@ -58,19 +73,13 @@ impl Context { std::fs::remove_file(&src_path)?; return Ok(false); } - let cleanup_all = || { - let res = std::fs::remove_file(&src_path); - symlink::remove_symlink_file(&link_path) - .or_else(|_| std::fs::remove_file(&link_path)) - .and(res) - }; - - let res = std::fs::symlink_metadata(&link_path) - .or_else(|err| cleanup_all().and(Err(err)))? - .is_symlink(); - cleanup_all()?; - Ok(res) + let res = std::fs::symlink_metadata(&link_path).map(|m| m.is_symlink()); + let cleanup = std::fs::remove_file(&src_path); + symlink::remove_symlink_file(&link_path) + .or_else(|_| std::fs::remove_file(&link_path)) + .and(cleanup)?; + res } } From 322924037a1710f35e4134e5a35c82b3d4266a1f Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 28 Feb 2022 17:24:52 +0800 Subject: [PATCH 12/19] thanks clippy --- git-worktree/tests/fs/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-worktree/tests/fs/mod.rs b/git-worktree/tests/fs/mod.rs index e4c3dac7bea..4c513d5f8a3 100644 --- a/git-worktree/tests/fs/mod.rs +++ b/git-worktree/tests/fs/mod.rs @@ -8,7 +8,7 @@ fn from_probing_cwd() { .unwrap() .filter_map(Result::ok) .filter(|e| e.file_name().to_str() != Some("config")) - .map(|e| e.path().to_owned()) + .map(|e| e.path()) .collect(); assert_eq!( entries.len(), From 267e3a7f4718c8f724e3e4488dd24dcebfc69413 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 28 Feb 2022 17:53:49 +0800 Subject: [PATCH 13/19] support for executable bit check (#301) --- git-worktree/src/fs.rs | 53 +++++++++++++++++++++++---------- git-worktree/src/index.rs | 25 +++++++++------- git-worktree/tests/fs/mod.rs | 2 +- git-worktree/tests/index/mod.rs | 4 +-- 4 files changed, 54 insertions(+), 30 deletions(-) diff --git a/git-worktree/src/fs.rs b/git-worktree/src/fs.rs index 4d641ef7680..2db575ba6c4 100644 --- a/git-worktree/src/fs.rs +++ b/git-worktree/src/fs.rs @@ -3,7 +3,7 @@ use std::path::Path; /// Common knowledge about the worktree that is needed across most interactions with the work tree #[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)] -pub struct Context { +pub struct Capabilities { /// If true, the filesystem will store paths as decomposed unicode, i.e. `ä` becomes `"a\u{308}"`, which means that /// we have to turn these forms back from decomposed to precomposed unicode before storing it in the index or generally /// using it. This also applies to input received from the command-line, so callers may have to be aware of this and @@ -15,29 +15,50 @@ pub struct Context { pub ignore_case: bool, /// If true, we assume the the executable bit is honored as part of the files mode. If false, we assume the file system /// ignores the executable bit, hence it will be reported as 'off' even though we just tried to set it to be on. - pub file_mode: bool, + pub executable_bit: bool, /// If true, the file system supports symbolic links and we should try to create them. Otherwise symbolic links will be checked /// out as files which contain the link as text. pub symlink: bool, } -impl Context { +impl Capabilities { /// try to determine all values in this context by probing them in the given `git_dir`, which /// should be on the file system the git repository is located on. /// `git_dir` is a typical git repository, expected to be populated with the typical files like `config`. /// /// All errors are ignored and interpreted on top of the default for the platform the binary is compiled for. - pub fn probe(git_dir: impl AsRef) -> Self { + pub fn probe(git_dir: impl AsRef) -> Self { let root = git_dir.as_ref(); - let ctx = Context::default(); - Context { + let ctx = Capabilities::default(); + Capabilities { symlink: Self::probe_symlink(root).unwrap_or(ctx.symlink), ignore_case: Self::probe_ignore_case(root).unwrap_or(ctx.ignore_case), precompose_unicode: Self::probe_precompose_unicode(root).unwrap_or(ctx.precompose_unicode), - ..ctx + executable_bit: Self::probe_file_mode(root).unwrap_or(ctx.executable_bit), } } + #[cfg(unix)] + fn probe_file_mode(root: &Path) -> std::io::Result { + use std::os::unix::fs::{MetadataExt, OpenOptionsExt}; + + // test it exactly as we typically create executable files, not using chmod. + let test_path = root.join("_test_executable_bit"); + let res = std::fs::OpenOptions::new() + .create_new(true) + .write(true) + .mode(0o777) + .open(&test_path) + .and_then(|f| f.metadata().map(|m| m.mode() & 0o100 == 0o100)); + std::fs::remove_file(test_path)?; + res + } + + #[cfg(not(unix))] + fn probe_file_mode(root: &Path) -> std::io::Result { + Ok(false) + } + fn probe_ignore_case(git_dir: &Path) -> std::io::Result { std::fs::metadata(git_dir.join("cOnFiG")).map(|_| true).or_else(|err| { if err.kind() == std::io::ErrorKind::NotFound { @@ -84,36 +105,36 @@ impl Context { } #[cfg(windows)] -impl Default for Context { +impl Default for Capabilities { fn default() -> Self { - Context { + Capabilities { precompose_unicode: false, ignore_case: true, - file_mode: false, + executable_bit: false, symlink: false, } } } #[cfg(target_os = "macos")] -impl Default for Context { +impl Default for Capabilities { fn default() -> Self { - Context { + Capabilities { precompose_unicode: true, ignore_case: true, - file_mode: true, + executable_bit: true, symlink: true, } } } #[cfg(all(unix, not(target_os = "macos")))] -impl Default for Context { +impl Default for Capabilities { fn default() -> Self { - Context { + Capabilities { precompose_unicode: false, ignore_case: false, - file_mode: true, + executable_bit: true, symlink: true, } } diff --git a/git-worktree/src/index.rs b/git-worktree/src/index.rs index 0df607eab2d..211567714ce 100644 --- a/git-worktree/src/index.rs +++ b/git-worktree/src/index.rs @@ -7,7 +7,7 @@ pub mod checkout { #[derive(Default, Clone, Copy)] pub struct Options { /// capabilities of the file system - pub fs: crate::fs::Context, + pub fs: crate::fs::Capabilities, /// If true, we assume no file to exist in the target directory, and want exclusive access to it. /// This should be enabled when cloning. pub destination_is_initially_empty: bool, @@ -81,7 +81,12 @@ pub(crate) mod entry { find: &mut Find, root: &std::path::Path, index::checkout::Options { - fs: crate::fs::Context { symlink, .. }, + fs: + crate::fs::Capabilities { + symlink, + executable_bit, + .. + }, .. }: index::checkout::Options, buf: &mut Vec, @@ -103,20 +108,18 @@ pub(crate) mod entry { path: root.to_path_buf(), })?; let mut options = OpenOptions::new(); - options.write(true).create_new(true); + options.create_new(true).write(true); #[cfg(unix)] - if entry.mode == git_index::entry::Mode::FILE_EXECUTABLE { + if executable_bit && entry.mode == git_index::entry::Mode::FILE_EXECUTABLE { use std::os::unix::fs::OpenOptionsExt; options.mode(0o777); } - { - let mut file = options.open(&dest)?; - file.write_all(obj.data)?; - // NOTE: we don't call `file.sync_all()` here knowing that some filesystems don't handle this well. - // revisit this once there is a bug to fix. - } - update_fstat(entry, dest.symlink_metadata()?)?; + let mut file = options.open(&dest)?; + file.write_all(obj.data)?; + // NOTE: we don't call `file.sync_all()` here knowing that some filesystems don't handle this well. + // revisit this once there is a bug to fix. + update_fstat(entry, file.metadata()?)?; } git_index::entry::Mode::SYMLINK => { let obj = find(&entry.id, buf).ok_or_else(|| index::checkout::Error::ObjectNotFound { diff --git a/git-worktree/tests/fs/mod.rs b/git-worktree/tests/fs/mod.rs index 4c513d5f8a3..b7cd91c0be9 100644 --- a/git-worktree/tests/fs/mod.rs +++ b/git-worktree/tests/fs/mod.rs @@ -2,7 +2,7 @@ fn from_probing_cwd() { let dir = tempfile::tempdir().unwrap(); std::fs::File::create(dir.path().join("config")).unwrap(); - let ctx = git_worktree::fs::Context::probe(dir.path()); + let ctx = git_worktree::fs::Capabilities::probe(dir.path()); dbg!(ctx); let entries: Vec<_> = std::fs::read_dir(dir.path()) .unwrap() diff --git a/git-worktree/tests/index/mod.rs b/git-worktree/tests/index/mod.rs index f3b103e9f2d..96f39de29e7 100644 --- a/git-worktree/tests/index/mod.rs +++ b/git-worktree/tests/index/mod.rs @@ -17,7 +17,7 @@ mod checkout { #[test] fn allow_symlinks() -> crate::Result { let opts = opts_with_symlink(true); - if !git_worktree::fs::Context::probe(std::env::current_dir()?.join("..").join(".git")).symlink { + if !git_worktree::fs::Capabilities::probe(std::env::current_dir()?.join("..").join(".git")).symlink { eprintln!("IGNORING symlink test on file system without symlink support"); // skip if symlinks aren't supported anyway. return Ok(()); @@ -30,7 +30,7 @@ mod checkout { fn opts_with_symlink(symlink: bool) -> Options { index::checkout::Options { - fs: git_worktree::fs::Context { + fs: git_worktree::fs::Capabilities { symlink, ..Default::default() }, From 09fecd9687cf3271f7138bca9214ba99c17b5ef7 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 1 Mar 2022 16:43:06 +0800 Subject: [PATCH 14/19] validate that colliding files are checked out (#301) --- git-worktree/src/index.rs | 2 +- git-worktree/tests/index/mod.rs | 89 +++++++++++++++++++++++++-------- 2 files changed, 69 insertions(+), 22 deletions(-) diff --git a/git-worktree/src/index.rs b/git-worktree/src/index.rs index 211567714ce..abd08da7785 100644 --- a/git-worktree/src/index.rs +++ b/git-worktree/src/index.rs @@ -108,7 +108,7 @@ pub(crate) mod entry { path: root.to_path_buf(), })?; let mut options = OpenOptions::new(); - options.create_new(true).write(true); + options.create(true).write(true); #[cfg(unix)] if executable_bit && entry.mode == git_index::entry::Mode::FILE_EXECUTABLE { use std::os::unix::fs::OpenOptionsExt; diff --git a/git-worktree/tests/index/mod.rs b/git-worktree/tests/index/mod.rs index 96f39de29e7..79ccba58bd3 100644 --- a/git-worktree/tests/index/mod.rs +++ b/git-worktree/tests/index/mod.rs @@ -8,27 +8,19 @@ mod checkout { use git_object::bstr::ByteSlice; use git_odb::FindExt; + use git_worktree::fs::Capabilities; use git_worktree::index; - use git_worktree::index::checkout::Options; use tempfile::TempDir; use crate::fixture_path; - #[test] - fn allow_symlinks() -> crate::Result { - let opts = opts_with_symlink(true); - if !git_worktree::fs::Capabilities::probe(std::env::current_dir()?.join("..").join(".git")).symlink { - eprintln!("IGNORING symlink test on file system without symlink support"); - // skip if symlinks aren't supported anyway. - return Ok(()); - }; - let (source_tree, destination) = setup_fixture_with_options(opts, "make_mixed_without_submodules")?; - - assert_equality(&source_tree, &destination, opts.fs.symlink)?; - Ok(()) + fn probe_gitoxide_dir() -> crate::Result { + Ok(git_worktree::fs::Capabilities::probe( + std::env::current_dir()?.join("..").join(".git"), + )) } - fn opts_with_symlink(symlink: bool) -> Options { + fn opts_with_symlink(symlink: bool) -> index::checkout::Options { index::checkout::Options { fs: git_worktree::fs::Capabilities { symlink, @@ -41,14 +33,67 @@ mod checkout { #[test] fn symlinks_become_files_if_disabled() -> crate::Result { let opts = opts_with_symlink(false); - let (source_tree, destination) = setup_fixture_with_options(opts, "make_mixed_without_submodules")?; + let (source_tree, destination, _index) = checkout_index_in_tmp_dir(opts, "make_mixed_without_submodules")?; assert_equality(&source_tree, &destination, opts.fs.symlink)?; Ok(()) } - fn assert_equality(source_tree: &Path, destination: &TempDir, allow_symlinks: bool) -> crate::Result { + #[test] + fn allow_symlinks() -> crate::Result { + let opts = opts_with_symlink(true); + if !probe_gitoxide_dir()?.symlink { + eprintln!("IGNORING symlink test on file system without symlink support"); + // skip if symlinks aren't supported anyway. + return Ok(()); + }; + let (source_tree, destination, _index) = checkout_index_in_tmp_dir(opts, "make_mixed_without_submodules")?; + + assert_equality(&source_tree, &destination, opts.fs.symlink)?; + Ok(()) + } + + #[test] + fn no_case_related_collisions_on_case_sensitive_filesystem() { + if probe_gitoxide_dir().unwrap().ignore_case { + eprintln!("Skipping case-sensitive testing on what would be a case-insenstive file system"); + return; + } + let opts = opts_with_symlink(true); + let (source_tree, destination, index) = checkout_index_in_tmp_dir(opts, "make_ignorecase_collisions").unwrap(); + assert_eq!(index.entries().len(), 2, "there is just one colliding item"); + + let num_files = assert_equality(&source_tree, &destination, opts.fs.symlink).unwrap(); + assert_eq!(num_files, index.entries().len(), "it checks out all files"); + } + + #[test] + fn collisions_are_detected_on_a_case_sensitive_filesystem() { + if !probe_gitoxide_dir().unwrap().ignore_case { + eprintln!("Skipping case-insensitive testing on what would be a case-senstive file system"); + return; + } + let opts = opts_with_symlink(true); + let (source_tree, destination, index) = checkout_index_in_tmp_dir(opts, "make_ignorecase_collisions").unwrap(); + assert_eq!(index.entries().len(), 2, "there is just one colliding item"); + + let source_files = dir_structure(&source_tree); + assert_eq!( + stripped_prefix(&source_tree, &source_files), + vec![PathBuf::from("a")], + "the source also only contains the first created file" + ); + + let dest_files = dir_structure(&destination); + assert_eq!( + stripped_prefix(&destination, &dest_files), + vec![PathBuf::from("A")], + "it only creates the first file of a collision" + ); + } + + fn assert_equality(source_tree: &Path, destination: &TempDir, allow_symlinks: bool) -> crate::Result { let source_files = dir_structure(source_tree); let worktree_files = dir_structure(&destination); @@ -57,7 +102,9 @@ mod checkout { stripped_prefix(&destination, &worktree_files), ); + let mut count = 0; for (source_file, worktree_file) in source_files.iter().zip(worktree_files.iter()) { + count += 1; if !allow_symlinks && source_file.is_symlink() { assert!(!worktree_file.is_symlink()); assert_eq!(fs::read(worktree_file)?.to_path()?, fs::read_link(source_file)?); @@ -71,7 +118,7 @@ mod checkout { ); } } - Ok(()) + Ok(count) } pub fn dir_structure>(path: P) -> Vec { @@ -87,10 +134,10 @@ mod checkout { files } - fn setup_fixture_with_options( - opts: git_worktree::index::checkout::Options, + fn checkout_index_in_tmp_dir( + opts: index::checkout::Options, name: &str, - ) -> crate::Result<(PathBuf, TempDir)> { + ) -> crate::Result<(PathBuf, TempDir, git_index::File)> { let source_tree = fixture_path(name); let git_dir = source_tree.join(".git"); let mut index = git_index::File::at(git_dir.join("index"), Default::default())?; @@ -103,7 +150,7 @@ mod checkout { move |oid, buf| odb.find_blob(oid, buf).ok(), opts, )?; - Ok((source_tree, destination)) + Ok((source_tree, destination, index)) } fn stripped_prefix(prefix: impl AsRef, source_files: &[PathBuf]) -> Vec<&Path> { From 9b9f10ad862b5e097c836c51df1eb98607df5ae1 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 1 Mar 2022 17:19:49 +0800 Subject: [PATCH 15/19] refactor: remove unnecessary unsafe by using `chunks_mut()` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was probably a left-over from times where there was a static requirement on the chunks processing. Maybe… . --- README.md | 2 +- .../src/data/output/entry/iter_from_counts.rs | 17 ++++------------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index f30ad388b6a..6b8c324080b 100644 --- a/README.md +++ b/README.md @@ -90,10 +90,10 @@ Follow linked crate name for detailed status. Please note that all crates follow * `gitoxide-core` * **very early** * [git-index](https://github.com/Byron/gitoxide/blob/main/crate-status.md#git-index) + * [git-worktree](https://github.com/Byron/gitoxide/blob/main/crate-status.md#git-worktree) * [git-bitmap](https://github.com/Byron/gitoxide/blob/main/crate-status.md#git-bitmap) * **idea** * [git-revision](https://github.com/Byron/gitoxide/blob/main/crate-status.md#git-revision) - * [git-worktree](https://github.com/Byron/gitoxide/blob/main/crate-status.md#git-worktree) * [git-tui](https://github.com/Byron/gitoxide/blob/main/crate-status.md#git-tui) * [git-bundle](https://github.com/Byron/gitoxide/blob/main/crate-status.md#git-bundle) diff --git a/git-pack/src/data/output/entry/iter_from_counts.rs b/git-pack/src/data/output/entry/iter_from_counts.rs index 2071212cad8..d5f6176236a 100644 --- a/git-pack/src/data/output/entry/iter_from_counts.rs +++ b/git-pack/src/data/output/entry/iter_from_counts.rs @@ -56,7 +56,6 @@ where ); let (chunk_size, thread_limit, _) = parallel::optimize_chunk_size_and_thread_limit(chunk_size, Some(counts.len()), thread_limit, None); - let chunks = util::ChunkRanges::new(chunk_size, counts.len()); { let progress = Arc::new(parking_lot::Mutex::new(progress.add_child("resolving"))); progress.lock().init(None, git_features::progress::count("counts")); @@ -64,23 +63,13 @@ where let start = std::time::Instant::now(); parallel::in_parallel_if( || enough_counts_present, - chunks.clone(), + counts.chunks_mut(chunk_size), thread_limit, |_n| Vec::::new(), { let progress = Arc::clone(&progress); - let counts = &counts; let db = db.clone(); - move |chunk_range, buf| { - let chunk = { - let c = &counts[chunk_range]; - let mut_ptr = c.as_ptr() as *mut output::Count; - // SAFETY: We know that 'chunks' is only non-overlapping slices, and this function owns `counts`. - #[allow(unsafe_code)] - unsafe { - std::slice::from_raw_parts_mut(mut_ptr, c.len()) - } - }; + move |chunk, buf| { let chunk_size = chunk.len(); for count in chunk { use crate::data::output::count::PackLocation::*; @@ -135,8 +124,10 @@ where index } }; + let counts = Arc::new(counts); let progress = Arc::new(parking_lot::Mutex::new(progress)); + let chunks = util::ChunkRanges::new(chunk_size, counts.len()); parallel::reduce::Stepwise::new( chunks.enumerate(), From 1a502c7e456a191d8639b799648ea33eb5a7dac2 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 2 Mar 2022 10:26:07 +0800 Subject: [PATCH 16/19] Add check_stat and trust_ctime options to index checkout (#301) For now they are unused but that should change when doing collision checks. --- git-worktree/src/index.rs | 24 +++++++++++++++++++++++- git-worktree/tests/index/mod.rs | 1 + 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/git-worktree/src/index.rs b/git-worktree/src/index.rs index abd08da7785..90392baa6e6 100644 --- a/git-worktree/src/index.rs +++ b/git-worktree/src/index.rs @@ -4,13 +4,35 @@ pub mod checkout { use bstr::BString; use quick_error::quick_error; - #[derive(Default, Clone, Copy)] + #[derive(Clone, Copy)] pub struct Options { /// capabilities of the file system pub fs: crate::fs::Capabilities, /// If true, we assume no file to exist in the target directory, and want exclusive access to it. /// This should be enabled when cloning. pub destination_is_initially_empty: bool, + /// If true, a files creation time is taken into consideration when checking if a file changed. + /// Can be set to false in case other tools alter the creation time in ways that interfere with our operation. + /// + /// Default true. + pub trust_ctime: bool, + /// If true, all stat fields will be used when checking for up-to-date'ness of the entry. Otherwise + /// nano-second parts of mtime and ctime,uid, gid, inode and device number won't be used, leaving only + /// the whole-second part of ctime and mtime and the file size to be checked. + /// + /// Default true. + pub check_stat: bool, + } + + impl Default for Options { + fn default() -> Self { + Options { + fs: Default::default(), + destination_is_initially_empty: false, + trust_ctime: true, + check_stat: true, + } + } } quick_error! { diff --git a/git-worktree/tests/index/mod.rs b/git-worktree/tests/index/mod.rs index 79ccba58bd3..e9db67d5ee0 100644 --- a/git-worktree/tests/index/mod.rs +++ b/git-worktree/tests/index/mod.rs @@ -27,6 +27,7 @@ mod checkout { ..Default::default() }, destination_is_initially_empty: true, + ..Default::default() } } From 0454e4a6f039541255728c4c8e076578236f0d86 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 2 Mar 2022 11:56:50 +0800 Subject: [PATCH 17/19] finally an understanding on collision checking (#301) Let's not try to imitate git here but instead solve the problem our way while learning from bugs git ran into in the process. This will be particularly interesting when dealing with parallel checkouts, something we need to implement for sure. On the bright side, there seems to be opportunity for a performance boost due to less iops during clone. --- git-worktree/src/index.rs | 61 +++++++++++++++--- .../fixtures/make_ignorecase_collisions.sh | 15 ++++- git-worktree/tests/index/mod.rs | 63 ++++++++++++++----- 3 files changed, 113 insertions(+), 26 deletions(-) diff --git a/git-worktree/src/index.rs b/git-worktree/src/index.rs index 90392baa6e6..58c598e3b11 100644 --- a/git-worktree/src/index.rs +++ b/git-worktree/src/index.rs @@ -1,16 +1,35 @@ use git_hash::oid; +use crate::{index, index::checkout::Collision}; + pub mod checkout { use bstr::BString; use quick_error::quick_error; + #[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] + pub struct Collision { + /// the path that collided with something already present on disk. + pub path: BString, + /// The io error we encountered when checking out `path`. + pub error_kind: std::io::ErrorKind, + } + + pub struct Outcome { + pub collisions: Vec, + } + #[derive(Clone, Copy)] pub struct Options { /// capabilities of the file system pub fs: crate::fs::Capabilities, /// If true, we assume no file to exist in the target directory, and want exclusive access to it. - /// This should be enabled when cloning. + /// This should be enabled when cloning to avoid checks for freshness of files. This also enables + /// detection of collisions based on whether or not exclusive file creation succeeds or fails. pub destination_is_initially_empty: bool, + /// If true, default false, try to checkout as much as possible and don't abort on first error which isn't + /// due to a conflict. + /// The operation will never fail, but count the encountered errors instead along with their paths. + pub keep_going: bool, /// If true, a files creation time is taken into consideration when checking if a file changed. /// Can be set to false in case other tools alter the creation time in ways that interfere with our operation. /// @@ -29,6 +48,7 @@ pub mod checkout { Options { fs: Default::default(), destination_is_initially_empty: false, + keep_going: false, trust_ctime: true, check_stat: true, } @@ -63,24 +83,43 @@ pub fn checkout( path: impl AsRef, mut find: Find, options: checkout::Options, -) -> Result<(), checkout::Error> +) -> Result where Find: for<'a> FnMut(&oid, &'a mut Vec) -> Option>, { - if !options.destination_is_initially_empty { - todo!("non-clone logic isn't implemented or vetted yet"); - } let root = path.as_ref(); let mut buf = Vec::new(); + let mut collisions = Vec::new(); for (entry, entry_path) in index.entries_mut_with_paths() { // TODO: write test for that if entry.flags.contains(git_index::entry::Flags::SKIP_WORKTREE) { continue; } - entry::checkout(entry, entry_path, &mut find, root, options, &mut buf)?; + let res = entry::checkout(entry, entry_path, &mut find, root, options, &mut buf); + match res { + Ok(()) => {} + // TODO: use ::IsDirectory as well when stabilized instead of raw_os_error() + Err(index::checkout::Error::Io(err)) + if err.kind() == std::io::ErrorKind::AlreadyExists || err.raw_os_error() == Some(21) => + { + // We are here because a file existed or was blocked by a directory which shouldn't be possible unless + // we are on a file insensitive file system. + collisions.push(Collision { + path: entry_path.into(), + error_kind: err.kind(), + }); + } + Err(err) => { + if options.keep_going { + todo!("keep going") + } else { + return Err(err); + } + } + } } - Ok(()) + Ok(checkout::Outcome { collisions }) } pub(crate) mod entry { @@ -109,6 +148,7 @@ pub(crate) mod entry { executable_bit, .. }, + destination_is_initially_empty, .. }: index::checkout::Options, buf: &mut Vec, @@ -130,7 +170,10 @@ pub(crate) mod entry { path: root.to_path_buf(), })?; let mut options = OpenOptions::new(); - options.create(true).write(true); + options + .create_new(destination_is_initially_empty) + .create(!destination_is_initially_empty) + .write(true); #[cfg(unix)] if executable_bit && entry.mode == git_index::entry::Mode::FILE_EXECUTABLE { use std::os::unix::fs::OpenOptionsExt; @@ -151,6 +194,8 @@ pub(crate) mod entry { let symlink_destination = git_features::path::from_byte_slice(obj.data) .map_err(|_| index::checkout::Error::IllformedUtf8 { path: obj.data.into() })?; + // TODO: how to deal with mode changes? Maybe this info can be passed once we check for whether + // a checkout is needed at all. if symlink { symlink::symlink_auto(symlink_destination, &dest)?; } else { diff --git a/git-worktree/tests/fixtures/make_ignorecase_collisions.sh b/git-worktree/tests/fixtures/make_ignorecase_collisions.sh index 4921fff70da..9fb772681e0 100644 --- a/git-worktree/tests/fixtures/make_ignorecase_collisions.sh +++ b/git-worktree/tests/fixtures/make_ignorecase_collisions.sh @@ -4,8 +4,17 @@ set -eu -o pipefail git init -q git config commit.gpgsign false -touch a A -git add a -echo A | git update-index --add --stdin +empty_oid=$(git hash-object -w --stdin crate::Result { let opts = opts_with_symlink(false); - let (source_tree, destination, _index) = checkout_index_in_tmp_dir(opts, "make_mixed_without_submodules")?; + let (source_tree, destination, _index, outcome) = + checkout_index_in_tmp_dir(opts, "make_mixed_without_submodules")?; assert_equality(&source_tree, &destination, opts.fs.symlink)?; + assert!(outcome.collisions.is_empty()); Ok(()) } @@ -49,9 +51,11 @@ mod checkout { // skip if symlinks aren't supported anyway. return Ok(()); }; - let (source_tree, destination, _index) = checkout_index_in_tmp_dir(opts, "make_mixed_without_submodules")?; + let (source_tree, destination, _index, outcome) = + checkout_index_in_tmp_dir(opts, "make_mixed_without_submodules")?; assert_equality(&source_tree, &destination, opts.fs.symlink)?; + assert!(outcome.collisions.is_empty()); Ok(()) } @@ -62,11 +66,12 @@ mod checkout { return; } let opts = opts_with_symlink(true); - let (source_tree, destination, index) = checkout_index_in_tmp_dir(opts, "make_ignorecase_collisions").unwrap(); - assert_eq!(index.entries().len(), 2, "there is just one colliding item"); + let (source_tree, destination, index, outcome) = + checkout_index_in_tmp_dir(opts, "make_ignorecase_collisions").unwrap(); let num_files = assert_equality(&source_tree, &destination, opts.fs.symlink).unwrap(); assert_eq!(num_files, index.entries().len(), "it checks out all files"); + assert!(outcome.collisions.is_empty()); } #[test] @@ -76,21 +81,44 @@ mod checkout { return; } let opts = opts_with_symlink(true); - let (source_tree, destination, index) = checkout_index_in_tmp_dir(opts, "make_ignorecase_collisions").unwrap(); - assert_eq!(index.entries().len(), 2, "there is just one colliding item"); + let (source_tree, destination, _index, outcome) = + checkout_index_in_tmp_dir(opts, "make_ignorecase_collisions").unwrap(); + + assert_eq!( + outcome.collisions, + vec![ + Collision { + path: "FILE_x".into(), + error_kind: ErrorKind::AlreadyExists, + }, + Collision { + path: "d".into(), + error_kind: ErrorKind::AlreadyExists, + }, + Collision { + path: "file_X".into(), + error_kind: ErrorKind::AlreadyExists, + }, + Collision { + path: "file_x".into(), + error_kind: ErrorKind::AlreadyExists, + }, + ], + "these files couldn't be checked out" + ); let source_files = dir_structure(&source_tree); assert_eq!( stripped_prefix(&source_tree, &source_files), - vec![PathBuf::from("a")], - "the source also only contains the first created file" + vec![PathBuf::from("d"), PathBuf::from("file_x")], + "plenty of collisions prevent a checkout" ); let dest_files = dir_structure(&destination); assert_eq!( stripped_prefix(&destination, &dest_files), - vec![PathBuf::from("A")], - "it only creates the first file of a collision" + vec![PathBuf::from("D/B"), PathBuf::from("D/C"), PathBuf::from("FILE_X")], + "we checkout files in order and generally handle collision detection differently, hence the difference" ); } @@ -138,20 +166,25 @@ mod checkout { fn checkout_index_in_tmp_dir( opts: index::checkout::Options, name: &str, - ) -> crate::Result<(PathBuf, TempDir, git_index::File)> { + ) -> crate::Result<( + PathBuf, + TempDir, + git_index::File, + git_worktree::index::checkout::Outcome, + )> { let source_tree = fixture_path(name); let git_dir = source_tree.join(".git"); let mut index = git_index::File::at(git_dir.join("index"), Default::default())?; let odb = git_odb::at(git_dir.join("objects"))?; let destination = tempfile::tempdir()?; - index::checkout( + let outcome = index::checkout( &mut index, &destination, move |oid, buf| odb.find_blob(oid, buf).ok(), opts, )?; - Ok((source_tree, destination, index)) + Ok((source_tree, destination, index, outcome)) } fn stripped_prefix(prefix: impl AsRef, source_files: &[PathBuf]) -> Vec<&Path> { From 5c1e727a1af4b9a0b5b7dcfca0d1ef5a533a66b6 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 2 Mar 2022 13:01:56 +0800 Subject: [PATCH 18/19] try to fix windows (#301) --- git-worktree/src/fs.rs | 2 +- git-worktree/src/index.rs | 12 +++++++++++- git-worktree/tests/index/mod.rs | 14 ++++++++++---- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/git-worktree/src/fs.rs b/git-worktree/src/fs.rs index 2db575ba6c4..7c71001160b 100644 --- a/git-worktree/src/fs.rs +++ b/git-worktree/src/fs.rs @@ -55,7 +55,7 @@ impl Capabilities { } #[cfg(not(unix))] - fn probe_file_mode(root: &Path) -> std::io::Result { + fn probe_file_mode(_root: &Path) -> std::io::Result { Ok(false) } diff --git a/git-worktree/src/index.rs b/git-worktree/src/index.rs index 58c598e3b11..d8afb59df46 100644 --- a/git-worktree/src/index.rs +++ b/git-worktree/src/index.rs @@ -87,6 +87,7 @@ pub fn checkout( where Find: for<'a> FnMut(&oid, &'a mut Vec) -> Option>, { + use std::io::ErrorKind::AlreadyExists; let root = path.as_ref(); let mut buf = Vec::new(); let mut collisions = Vec::new(); @@ -100,9 +101,17 @@ where match res { Ok(()) => {} // TODO: use ::IsDirectory as well when stabilized instead of raw_os_error() + #[cfg(windows)] Err(index::checkout::Error::Io(err)) - if err.kind() == std::io::ErrorKind::AlreadyExists || err.raw_os_error() == Some(21) => + if err.kind() == AlreadyExists || err.kind() == std::io::ErrorKind::PermissionDenied => { + collisions.push(Collision { + path: entry_path.into(), + error_kind: err.kind(), + }); + } + #[cfg(not(windows))] + Err(index::checkout::Error::Io(err)) if err.kind() == AlreadyExists || err.raw_os_error() == Some(21) => { // We are here because a file existed or was blocked by a directory which shouldn't be possible unless // we are on a file insensitive file system. collisions.push(Collision { @@ -136,6 +145,7 @@ pub(crate) mod entry { use crate::index; + #[cfg_attr(not(unix), allow(unused_variables))] pub fn checkout( entry: &mut Entry, entry_path: &BStr, diff --git a/git-worktree/tests/index/mod.rs b/git-worktree/tests/index/mod.rs index 5ed6b0077c4..35c51bc08a8 100644 --- a/git-worktree/tests/index/mod.rs +++ b/git-worktree/tests/index/mod.rs @@ -84,24 +84,30 @@ mod checkout { let (source_tree, destination, _index, outcome) = checkout_index_in_tmp_dir(opts, "make_ignorecase_collisions").unwrap(); + let error_kind = ErrorKind::AlreadyExists; + #[cfg(windows)] + let error_kind_dir = ErrorKind::PermissionDenied; + #[cfg(not(windows))] + let error_kind_dir = error_kind; + assert_eq!( outcome.collisions, vec![ Collision { path: "FILE_x".into(), - error_kind: ErrorKind::AlreadyExists, + error_kind, }, Collision { path: "d".into(), - error_kind: ErrorKind::AlreadyExists, + error_kind: error_kind_dir, }, Collision { path: "file_X".into(), - error_kind: ErrorKind::AlreadyExists, + error_kind, }, Collision { path: "file_x".into(), - error_kind: ErrorKind::AlreadyExists, + error_kind, }, ], "these files couldn't be checked out" From 8ca400c8647e0e59a96a936d41c2dc2d07c6bf2d Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 2 Mar 2022 13:48:44 +0800 Subject: [PATCH 19/19] speed up git-pack testing on windows (#301) It takes more than 60 seconds to run this test, which is odd but for now it seems ok to just dial down the numbers a little. --- git-ref/tests/packed/find.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/git-ref/tests/packed/find.rs b/git-ref/tests/packed/find.rs index fc8f5f16045..ea965ebd782 100644 --- a/git-ref/tests/packed/find.rs +++ b/git-ref/tests/packed/find.rs @@ -175,7 +175,11 @@ fn find_speed() -> crate::Result { let packed = store.open_packed_buffer()?.expect("packed-refs present"); let start = std::time::Instant::now(); let mut num_refs = 0; - for r in packed.iter()?.take(10_000) { + #[cfg(windows)] + let count = 500; + #[cfg(not(windows))] + let count = 10_000; + for r in packed.iter()?.take(count) { num_refs += 1; let r = r?; assert_eq!(