From edf73dd4db5b0f5d9309c95bf366e11ea6723885 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 13 Aug 2022 08:15:43 +0800 Subject: [PATCH 01/18] change!: `ein tools` to `ein tool` for as it's more intuitive --- src/porcelain/main.rs | 2 +- src/porcelain/options.rs | 2 +- tests/journey/ein.sh | 30 +++++++++---------- .../{tools => tool}/find/no-args-success | 0 .../snapshots/porcelain/tool/no-args-failure | 6 ++++ .../directory-structure-after-organize | 0 .../{tools => tool}/organize/execute-success | 0 .../organize/initial-directory-structure | 0 .../{tools => tool}/organize/no-args-success | 0 .../snapshots/porcelain/tools/no-args-failure | 6 ---- 10 files changed, 23 insertions(+), 23 deletions(-) rename tests/snapshots/porcelain/{tools => tool}/find/no-args-success (100%) create mode 100644 tests/snapshots/porcelain/tool/no-args-failure rename tests/snapshots/porcelain/{tools => tool}/organize/directory-structure-after-organize (100%) rename tests/snapshots/porcelain/{tools => tool}/organize/execute-success (100%) rename tests/snapshots/porcelain/{tools => tool}/organize/initial-directory-structure (100%) rename tests/snapshots/porcelain/{tools => tool}/organize/no-args-success (100%) delete mode 100644 tests/snapshots/porcelain/tools/no-args-failure diff --git a/src/porcelain/main.rs b/src/porcelain/main.rs index af1c7d8b9a5..dd155077972 100644 --- a/src/porcelain/main.rs +++ b/src/porcelain/main.rs @@ -35,7 +35,7 @@ pub fn main() -> Result<()> { ), Subcommands::Init { directory } => core::repository::init(directory).map(|_| ()), #[cfg(feature = "gitoxide-core-tools")] - Subcommands::Tools(tool) => match tool { + Subcommands::Tool(tool) => match tool { crate::porcelain::options::ToolCommands::EstimateHours(crate::porcelain::options::EstimateHours { working_dir, refname, diff --git a/src/porcelain/options.rs b/src/porcelain/options.rs index 9d3a25aaab6..af008a87371 100644 --- a/src/porcelain/options.rs +++ b/src/porcelain/options.rs @@ -35,7 +35,7 @@ pub enum Subcommands { #[cfg(feature = "gitoxide-core-tools")] /// A selection of useful tools #[clap(subcommand)] - Tools(ToolCommands), + Tool(ToolCommands), #[cfg(debug_assertions)] Panic, } diff --git a/tests/journey/ein.sh b/tests/journey/ein.sh index b5edd4644d6..a0d2f7d8e49 100644 --- a/tests/journey/ein.sh +++ b/tests/journey/ein.sh @@ -30,35 +30,35 @@ title "Porcelain ${kind}" ) snapshot="$snapshot/porcelain" (with_program tree - (when "using the 'tools' subcommand" - title "gix tools…" + (when "using the 'tool' subcommand" + title "ein tool" (with "a repo with a tiny commit history" (small-repo-in-sandbox - title "gix tools estimate-hours" + title "ein tool estimate-hours" (when "running 'estimate-hours'" snapshot="$snapshot/estimate-hours" (with "no arguments" it "succeeds and prints only a summary" && { WITH_SNAPSHOT="$snapshot/no-args-success" \ - expect_run_sh $SUCCESSFULLY "$exe tools estimate-hours 2>/dev/null" + expect_run_sh $SUCCESSFULLY "$exe tool estimate-hours 2>/dev/null" } ) (with "the show-pii argument" it "succeeds and shows information identifying people before the summary" && { WITH_SNAPSHOT="$snapshot/show-pii-success" \ - expect_run_sh $SUCCESSFULLY "$exe tools estimate-hours --show-pii 2>/dev/null" + expect_run_sh $SUCCESSFULLY "$exe tool estimate-hours --show-pii 2>/dev/null" } ) (with "the omit-unify-identities argument" it "succeeds and doesn't show unified identities (in this case there is only one author anyway)" && { WITH_SNAPSHOT="$snapshot/no-unify-identities-success" \ - expect_run_sh $SUCCESSFULLY "$exe tools estimate-hours --omit-unify-identities 2>/dev/null" + expect_run_sh $SUCCESSFULLY "$exe t estimate-hours --omit-unify-identities 2>/dev/null" } ) (with "a branch name that doesn't exist" it "fails and shows a decent enough error message" && { WITH_SNAPSHOT="$snapshot/invalid-branch-name-failure" \ - expect_run_sh $WITH_FAILURE "$exe -q tools estimate-hours . foobar" + expect_run_sh $WITH_FAILURE "$exe -q t estimate-hours . foobar" } ) ) @@ -71,25 +71,25 @@ title "Porcelain ${kind}" repo-with-remotes special-origin special-name https://example.com/special-origin repo-with-remotes no-origin repo-with-remotes a-non-bare-repo-with-extension.git origin https://example.com/a-repo-with-extension.git - snapshot="$snapshot/tools" + snapshot="$snapshot/tool" - title "gix tools find" + title "ein tool find" (when "running 'find'" snapshot="$snapshot/find" (with "no arguments" it "succeeds and prints a list of repository work directories" && { WITH_SNAPSHOT="$snapshot/no-args-success" \ - expect_run_sh $SUCCESSFULLY "$exe tools find 2>/dev/null" + expect_run_sh $SUCCESSFULLY "$exe tool find 2>/dev/null" } ) ) - title "gix tools organize" + title "ein tool organize" (when "running 'organize'" snapshot="$snapshot/organize" (with "no arguments" it "succeeds and informs about the operations that it WOULD do" && { WITH_SNAPSHOT="$snapshot/no-args-success" \ - expect_run_sh $SUCCESSFULLY "$exe tools organize 2>/dev/null" + expect_run_sh $SUCCESSFULLY "$exe tool organize 2>/dev/null" } it "does not change the directory structure at all" && { @@ -101,7 +101,7 @@ title "Porcelain ${kind}" (with "--execute" it "succeeds" && { WITH_SNAPSHOT="$snapshot/execute-success" \ - expect_run_sh $SUCCESSFULLY "$exe tools organize --execute 2>/dev/null" + expect_run_sh $SUCCESSFULLY "$exe tool organize --execute 2>/dev/null" } it "changes the directory structure" && { @@ -113,7 +113,7 @@ title "Porcelain ${kind}" (with "--execute again" it "succeeds" && { WITH_SNAPSHOT="$snapshot/execute-success" \ - expect_run_sh $SUCCESSFULLY "$exe tools organize --execute 2>/dev/null" + expect_run_sh $SUCCESSFULLY "$exe tool organize --execute 2>/dev/null" } it "does not alter the directory structure as these are already in place" && { @@ -133,7 +133,7 @@ title "Porcelain ${kind}" ) ) - title "gix init" + title "ein init" (when "running 'init'" snapshot="$snapshot/init" (with "no argument" diff --git a/tests/snapshots/porcelain/tools/find/no-args-success b/tests/snapshots/porcelain/tool/find/no-args-success similarity index 100% rename from tests/snapshots/porcelain/tools/find/no-args-success rename to tests/snapshots/porcelain/tool/find/no-args-success diff --git a/tests/snapshots/porcelain/tool/no-args-failure b/tests/snapshots/porcelain/tool/no-args-failure new file mode 100644 index 00000000000..6ed2fe8d656 --- /dev/null +++ b/tests/snapshots/porcelain/tool/no-args-failure @@ -0,0 +1,6 @@ +error: 'ein tool' requires a subcommand but one was not provided + +USAGE: + ein tool + +For more information try --help \ No newline at end of file diff --git a/tests/snapshots/porcelain/tools/organize/directory-structure-after-organize b/tests/snapshots/porcelain/tool/organize/directory-structure-after-organize similarity index 100% rename from tests/snapshots/porcelain/tools/organize/directory-structure-after-organize rename to tests/snapshots/porcelain/tool/organize/directory-structure-after-organize diff --git a/tests/snapshots/porcelain/tools/organize/execute-success b/tests/snapshots/porcelain/tool/organize/execute-success similarity index 100% rename from tests/snapshots/porcelain/tools/organize/execute-success rename to tests/snapshots/porcelain/tool/organize/execute-success diff --git a/tests/snapshots/porcelain/tools/organize/initial-directory-structure b/tests/snapshots/porcelain/tool/organize/initial-directory-structure similarity index 100% rename from tests/snapshots/porcelain/tools/organize/initial-directory-structure rename to tests/snapshots/porcelain/tool/organize/initial-directory-structure diff --git a/tests/snapshots/porcelain/tools/organize/no-args-success b/tests/snapshots/porcelain/tool/organize/no-args-success similarity index 100% rename from tests/snapshots/porcelain/tools/organize/no-args-success rename to tests/snapshots/porcelain/tool/organize/no-args-success diff --git a/tests/snapshots/porcelain/tools/no-args-failure b/tests/snapshots/porcelain/tools/no-args-failure deleted file mode 100644 index 38568530515..00000000000 --- a/tests/snapshots/porcelain/tools/no-args-failure +++ /dev/null @@ -1,6 +0,0 @@ -error: 'ein tools' requires a subcommand but one was not provided - -USAGE: - ein tools - -For more information try --help \ No newline at end of file From a5b2ef9a33720312a6b30b7cdae564bf759b0218 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 13 Aug 2022 08:55:33 +0800 Subject: [PATCH 02/18] refactor prepare eoie extension for a more generalized writing. --- .../decode.rs} | 19 +++++++++++++------ .../src/extension/end_of_index_entry/mod.rs | 8 ++++++++ 2 files changed, 21 insertions(+), 6 deletions(-) rename git-index/src/extension/{end_of_index_entry.rs => end_of_index_entry/decode.rs} (65%) create mode 100644 git-index/src/extension/end_of_index_entry/mod.rs diff --git a/git-index/src/extension/end_of_index_entry.rs b/git-index/src/extension/end_of_index_entry/decode.rs similarity index 65% rename from git-index/src/extension/end_of_index_entry.rs rename to git-index/src/extension/end_of_index_entry/decode.rs index c44d15b295c..3cf08ec62d7 100644 --- a/git-index/src/extension/end_of_index_entry.rs +++ b/git-index/src/extension/end_of_index_entry/decode.rs @@ -1,9 +1,16 @@ -use crate::{decode::header, extension, extension::Signature, util::from_be_u32}; - -pub const SIGNATURE: Signature = *b"EOIE"; -pub const SIZE: usize = 4 /* offset to extensions */ + git_hash::Kind::Sha1.len_in_bytes(); -pub const SIZE_WITH_HEADER: usize = crate::extension::MIN_SIZE + SIZE; - +use crate::decode::header; +use crate::extension; +use crate::extension::end_of_index_entry::{SIGNATURE, SIZE, SIZE_WITH_HEADER}; +use crate::util::from_be_u32; + +/// Decode the end of index entry extension, which is no more than a glorified offset to the first byte of all extensions to allow +/// loading entries and extensions in parallel. +/// +/// Itself it's located at the end of the index file, which allows its location to be known and thus addressable. +/// From there it's possible to traverse the chunks of all set extensions, hash them, and compare that hash with all extensions +/// stored prior to this one to assure they are correct. +/// +/// If the checksum wasn't matched, we will ignoree this extension entirely. pub fn decode(data: &[u8], object_hash: git_hash::Kind) -> Option { let hash_len = object_hash.len_in_bytes(); if data.len() < SIZE_WITH_HEADER + hash_len { diff --git a/git-index/src/extension/end_of_index_entry/mod.rs b/git-index/src/extension/end_of_index_entry/mod.rs new file mode 100644 index 00000000000..4a16dbecc9d --- /dev/null +++ b/git-index/src/extension/end_of_index_entry/mod.rs @@ -0,0 +1,8 @@ +use crate::{extension, extension::Signature}; + +pub const SIGNATURE: Signature = *b"EOIE"; +pub const SIZE: usize = 4 /* offset to extensions */ + git_hash::Kind::Sha1.len_in_bytes(); +pub const SIZE_WITH_HEADER: usize = extension::MIN_SIZE + SIZE; + +mod decode; +pub use decode::decode; From 7ca297af400e50d42cffcaab54b1684f6810eb4f Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 13 Aug 2022 09:08:44 +0800 Subject: [PATCH 03/18] provide a stand-alone way of writing end-of-index extensions --- .../extension/end_of_index_entry/decode.rs | 10 ++--- .../src/extension/end_of_index_entry/mod.rs | 37 ++++++++++++++++++- git-index/src/extension/mod.rs | 3 +- 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/git-index/src/extension/end_of_index_entry/decode.rs b/git-index/src/extension/end_of_index_entry/decode.rs index 3cf08ec62d7..282091dbed2 100644 --- a/git-index/src/extension/end_of_index_entry/decode.rs +++ b/git-index/src/extension/end_of_index_entry/decode.rs @@ -1,6 +1,6 @@ use crate::decode::header; use crate::extension; -use crate::extension::end_of_index_entry::{SIGNATURE, SIZE, SIZE_WITH_HEADER}; +use crate::extension::end_of_index_entry::{MIN_SIZE, MIN_SIZE_WITH_HEADER, SIGNATURE}; use crate::util::from_be_u32; /// Decode the end of index entry extension, which is no more than a glorified offset to the first byte of all extensions to allow @@ -13,15 +13,15 @@ use crate::util::from_be_u32; /// If the checksum wasn't matched, we will ignoree this extension entirely. pub fn decode(data: &[u8], object_hash: git_hash::Kind) -> Option { let hash_len = object_hash.len_in_bytes(); - if data.len() < SIZE_WITH_HEADER + hash_len { + if data.len() < MIN_SIZE_WITH_HEADER + hash_len { return None; } - let start_of_eoie = data.len() - SIZE_WITH_HEADER - hash_len; + let start_of_eoie = data.len() - MIN_SIZE_WITH_HEADER - hash_len; let ext_data = &data[start_of_eoie..data.len() - hash_len]; let (signature, ext_size, ext_data) = extension::decode::header(ext_data); - if signature != SIGNATURE || ext_size as usize != SIZE { + if signature != SIGNATURE || ext_size as usize != MIN_SIZE { return None; } @@ -33,7 +33,7 @@ pub fn decode(data: &[u8], object_hash: git_hash::Kind) -> Option { let mut hasher = git_features::hash::hasher(git_hash::Kind::Sha1); let mut last_chunk = None; - for (signature, chunk) in extension::Iter::new(&data[offset..data.len() - SIZE_WITH_HEADER - hash_len]) { + for (signature, chunk) in extension::Iter::new(&data[offset..data.len() - MIN_SIZE_WITH_HEADER - hash_len]) { hasher.update(&signature); hasher.update(&(chunk.len() as u32).to_be_bytes()); last_chunk = Some(chunk); diff --git a/git-index/src/extension/end_of_index_entry/mod.rs b/git-index/src/extension/end_of_index_entry/mod.rs index 4a16dbecc9d..40db5819f38 100644 --- a/git-index/src/extension/end_of_index_entry/mod.rs +++ b/git-index/src/extension/end_of_index_entry/mod.rs @@ -1,8 +1,41 @@ use crate::{extension, extension::Signature}; +/// The signature of the end-of-index-entry extension pub const SIGNATURE: Signature = *b"EOIE"; -pub const SIZE: usize = 4 /* offset to extensions */ + git_hash::Kind::Sha1.len_in_bytes(); -pub const SIZE_WITH_HEADER: usize = extension::MIN_SIZE + SIZE; +/// The minimal size of the extension, depending on the shortest hash. +pub const MIN_SIZE: usize = 4 /* offset to extensions */ + git_hash::Kind::shortest().len_in_bytes(); +/// The smallest size of the extension varying by hash kind, along with the standard extension header. +pub const MIN_SIZE_WITH_HEADER: usize = extension::MIN_SIZE + MIN_SIZE; mod decode; pub use decode::decode; + +mod write { + use crate::extension::end_of_index_entry::SIGNATURE; + use crate::extension::Signature; + + /// Write this extension to out and generate a hash of `hash_kind` over all `prior_extensions` which are specified as `(signature, size)` + /// pair. `one_past_entries` is the offset to the first byte past the entries, which is also the first byte of the signature of the + /// first extension in `prior_extensions`. Note that `prior_extensions` must have been written prior to this one, as the name suggests, + /// allowing this extension to be the last one in the index file. + pub fn write_to( + out: &mut impl std::io::Write, + hash_kind: git_hash::Kind, + one_past_entries: u32, + prior_extensions: &[(Signature, u32)], + ) -> Result<(), std::io::Error> { + out.write_all(&SIGNATURE)?; + out.write_all(&(4 + hash_kind.len_in_bytes()).to_be_bytes())?; + out.write_all(&one_past_entries.to_be_bytes())?; + + let mut hasher = git_features::hash::hasher(hash_kind); + for (signature, size) in prior_extensions { + hasher.update(signature); + hasher.update(&size.to_be_bytes()); + } + out.write_all(&hasher.digest())?; + + Ok(()) + } +} +pub use write::write_to; diff --git a/git-index/src/extension/mod.rs b/git-index/src/extension/mod.rs index 3cd6ad77a31..f44ca0f25e3 100644 --- a/git-index/src/extension/mod.rs +++ b/git-index/src/extension/mod.rs @@ -77,7 +77,8 @@ pub(crate) mod decode; /// pub mod tree; -pub(crate) mod end_of_index_entry; +/// +pub mod end_of_index_entry; pub(crate) mod index_entry_offset_table; From 18b722e06bfb8bbbf0ada7438266e31a4317f2d4 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 13 Aug 2022 09:59:22 +0800 Subject: [PATCH 04/18] generalize EOIE exstension writing --- .../src/extension/end_of_index_entry/mod.rs | 29 +----------- .../src/extension/end_of_index_entry/write.rs | 30 +++++++++++++ git-index/src/extension/mod.rs | 3 +- git-index/src/write.rs | 44 ++++++------------- git-index/tests/index/file/write.rs | 4 +- 5 files changed, 48 insertions(+), 62 deletions(-) create mode 100644 git-index/src/extension/end_of_index_entry/write.rs diff --git a/git-index/src/extension/end_of_index_entry/mod.rs b/git-index/src/extension/end_of_index_entry/mod.rs index 40db5819f38..8cef0dacfe3 100644 --- a/git-index/src/extension/end_of_index_entry/mod.rs +++ b/git-index/src/extension/end_of_index_entry/mod.rs @@ -10,32 +10,5 @@ pub const MIN_SIZE_WITH_HEADER: usize = extension::MIN_SIZE + MIN_SIZE; mod decode; pub use decode::decode; -mod write { - use crate::extension::end_of_index_entry::SIGNATURE; - use crate::extension::Signature; - - /// Write this extension to out and generate a hash of `hash_kind` over all `prior_extensions` which are specified as `(signature, size)` - /// pair. `one_past_entries` is the offset to the first byte past the entries, which is also the first byte of the signature of the - /// first extension in `prior_extensions`. Note that `prior_extensions` must have been written prior to this one, as the name suggests, - /// allowing this extension to be the last one in the index file. - pub fn write_to( - out: &mut impl std::io::Write, - hash_kind: git_hash::Kind, - one_past_entries: u32, - prior_extensions: &[(Signature, u32)], - ) -> Result<(), std::io::Error> { - out.write_all(&SIGNATURE)?; - out.write_all(&(4 + hash_kind.len_in_bytes()).to_be_bytes())?; - out.write_all(&one_past_entries.to_be_bytes())?; - - let mut hasher = git_features::hash::hasher(hash_kind); - for (signature, size) in prior_extensions { - hasher.update(signature); - hasher.update(&size.to_be_bytes()); - } - out.write_all(&hasher.digest())?; - - Ok(()) - } -} +mod write; pub use write::write_to; diff --git a/git-index/src/extension/end_of_index_entry/write.rs b/git-index/src/extension/end_of_index_entry/write.rs new file mode 100644 index 00000000000..a1850624256 --- /dev/null +++ b/git-index/src/extension/end_of_index_entry/write.rs @@ -0,0 +1,30 @@ +use crate::extension::end_of_index_entry::SIGNATURE; +use crate::extension::Signature; + +/// Write this extension to out and generate a hash of `hash_kind` over all `prior_extensions` which are specified as `(signature, size)` +/// pair. `one_past_entries` is the offset to the first byte past the entries, which is also the first byte of the signature of the +/// first extension in `prior_extensions`. Note that `prior_extensions` must have been written prior to this one, as the name suggests, +/// allowing this extension to be the last one in the index file. +/// +/// Even if there are no `prior_extensions`, this extension will be written unconditionally. +pub fn write_to( + out: &mut impl std::io::Write, + hash_kind: git_hash::Kind, + offset_to_extensions: u32, + prior_extensions: &[(Signature, u32)], +) -> Result<(), std::io::Error> { + out.write_all(&SIGNATURE)?; + let extension_size: u32 = 4 + hash_kind.len_in_bytes() as u32; + out.write_all(&extension_size.to_be_bytes())?; + + out.write_all(&offset_to_extensions.to_be_bytes())?; + + let mut hasher = git_features::hash::hasher(hash_kind); + for (signature, size) in prior_extensions { + hasher.update(signature); + hasher.update(&size.to_be_bytes()); + } + out.write_all(&hasher.digest())?; + + Ok(()) +} diff --git a/git-index/src/extension/mod.rs b/git-index/src/extension/mod.rs index f44ca0f25e3..391dcb574bc 100644 --- a/git-index/src/extension/mod.rs +++ b/git-index/src/extension/mod.rs @@ -1,7 +1,8 @@ use bstr::BString; use smallvec::SmallVec; -const MIN_SIZE: usize = 4 /* signature */ + 4 /* size */; +/// The size of the smallest possible exstension, which is no more than a signature and a 0 indicating its size. +pub const MIN_SIZE: usize = 4 /* signature */ + 4 /* size */; /// The kind of index extension. pub type Signature = [u8; 4]; diff --git a/git-index/src/write.rs b/git-index/src/write.rs index 9986c0e95d0..0f19bbda4c8 100644 --- a/git-index/src/write.rs +++ b/git-index/src/write.rs @@ -60,17 +60,24 @@ impl State { .try_into() .expect("definitely not 4billion entries"); - let header_offset = header(&mut write, version, num_entries)?; - let entries_offset = entries(&mut write, self, header_offset)?; - let tree_offset = tree_cache_extension + let offset_to_entries = header(&mut write, version, num_entries)?; + let offset_to_extensions = entries(&mut write, self, offset_to_entries)?; + + let mut extensions = Vec::with_capacity(5); + if let Some(offset_past_tree_ext) = tree_cache_extension .then(|| self.tree()) .flatten() .map(|tree| tree.write_to(&mut write).map(|_| write.count)) .transpose()? - .unwrap_or(entries_offset); + { + extensions.push(( + extension::tree::SIGNATURE, + offset_past_tree_ext - offset_to_extensions - (extension::MIN_SIZE as u32), + )); + } - if num_entries > 0 && end_of_index_entry_extension { - end_of_index_entry_ext(write.inner, hash_kind, entries_offset, tree_offset)?; + if num_entries > 0 && end_of_index_entry_extension && !extensions.is_empty() { + extension::end_of_index_entry::write_to(write.inner, hash_kind, offset_to_extensions, &extensions)? } Ok(()) @@ -116,31 +123,6 @@ fn entries( Ok(out.count) } -fn end_of_index_entry_ext( - out: &mut impl std::io::Write, - hash_kind: git_hash::Kind, - entries_offset: u32, - tree_offset: u32, -) -> Result<(), std::io::Error> { - let signature = extension::end_of_index_entry::SIGNATURE; - let extension_size = 4 + hash_kind.len_in_bytes() as u32; - - let mut hasher = git_features::hash::hasher(hash_kind); - let tree_size = (tree_offset - entries_offset).saturating_sub(8); - if tree_size > 0 { - hasher.update(&extension::tree::SIGNATURE); - hasher.update(&tree_size.to_be_bytes()); - } - let hash = hasher.digest(); - - out.write_all(&signature)?; - out.write_all(&extension_size.to_be_bytes())?; - out.write_all(&entries_offset.to_be_bytes())?; - out.write_all(&hash)?; - - Ok(()) -} - mod util { use std::convert::TryFrom; diff --git a/git-index/tests/index/file/write.rs b/git-index/tests/index/file/write.rs index fcaae3396f9..5546f82e530 100644 --- a/git-index/tests/index/file/write.rs +++ b/git-index/tests/index/file/write.rs @@ -6,8 +6,8 @@ use std::cmp::{max, min}; #[test] fn roundtrips() { let input = [ - ("V2_empty", write::Options::default()), ("v2", write::Options::default()), + ("V2_empty", write::Options::default()), ( "v2_more_files", write::Options { @@ -21,7 +21,7 @@ fn roundtrips() { let path = crate::fixture_index_path(fixture); let expected_index = git_index::File::at(&path, decode::Options::default()).unwrap(); let expected_bytes = std::fs::read(&path).unwrap(); - let mut out_bytes = Vec::::new(); + let mut out_bytes = Vec::new(); expected_index.write_to(&mut out_bytes, options).unwrap(); let (out_index, _) = State::from_bytes(&out_bytes, FileTime::now(), decode::Options::default()).unwrap(); From 8ef5378dfaefe2d562d16b861fb4bb0fa4fdfe93 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 13 Aug 2022 10:31:40 +0800 Subject: [PATCH 05/18] generalize extension writing so that writing more will be easier --- .../src/extension/end_of_index_entry/write.rs | 4 +- git-index/src/write.rs | 40 +++++++++++++------ 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/git-index/src/extension/end_of_index_entry/write.rs b/git-index/src/extension/end_of_index_entry/write.rs index a1850624256..e73be681096 100644 --- a/git-index/src/extension/end_of_index_entry/write.rs +++ b/git-index/src/extension/end_of_index_entry/write.rs @@ -11,7 +11,7 @@ pub fn write_to( out: &mut impl std::io::Write, hash_kind: git_hash::Kind, offset_to_extensions: u32, - prior_extensions: &[(Signature, u32)], + prior_extensions: impl Iterator, ) -> Result<(), std::io::Error> { out.write_all(&SIGNATURE)?; let extension_size: u32 = 4 + hash_kind.len_in_bytes() as u32; @@ -21,7 +21,7 @@ pub fn write_to( let mut hasher = git_features::hash::hasher(hash_kind); for (signature, size) in prior_extensions { - hasher.update(signature); + hasher.update(&signature); hasher.update(&size.to_be_bytes()); } out.write_all(&hasher.digest())?; diff --git a/git-index/src/write.rs b/git-index/src/write.rs index 0f19bbda4c8..9f3654d8a69 100644 --- a/git-index/src/write.rs +++ b/git-index/src/write.rs @@ -63,21 +63,35 @@ impl State { let offset_to_entries = header(&mut write, version, num_entries)?; let offset_to_extensions = entries(&mut write, self, offset_to_entries)?; - let mut extensions = Vec::with_capacity(5); - if let Some(offset_past_tree_ext) = tree_cache_extension - .then(|| self.tree()) - .flatten() - .map(|tree| tree.write_to(&mut write).map(|_| write.count)) - .transpose()? - { - extensions.push(( - extension::tree::SIGNATURE, - offset_past_tree_ext - offset_to_extensions - (extension::MIN_SIZE as u32), - )); - } + let extensions = { + let extensions: &[&dyn Fn(&mut dyn std::io::Write) -> Option>] = + &[&|write| { + tree_cache_extension + .then(|| self.tree()) + .flatten() + .map(|tree| tree.write_to(write).map(|_| extension::tree::SIGNATURE)) + }]; + + let mut offset_to_previous_ext = offset_to_extensions; + let mut out = Vec::with_capacity(5); + for write_ext in extensions { + if let Some(signature) = write_ext(&mut write).transpose()? { + let offset_past_ext = write.count; + let ext_size = offset_past_ext - offset_to_previous_ext - (extension::MIN_SIZE as u32); + offset_to_previous_ext = offset_past_ext; + out.push((signature, ext_size)); + } + } + out + }; if num_entries > 0 && end_of_index_entry_extension && !extensions.is_empty() { - extension::end_of_index_entry::write_to(write.inner, hash_kind, offset_to_extensions, &extensions)? + extension::end_of_index_entry::write_to( + write.inner, + hash_kind, + offset_to_extensions, + extensions.into_iter(), + )? } Ok(()) From 9b3a940d9f4694912f32cb86752f3f7507882010 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 13 Aug 2022 10:36:45 +0800 Subject: [PATCH 06/18] thanks clippy --- .../src/extension/end_of_index_entry/write.rs | 4 ++-- git-index/src/write.rs | 21 +++++++------------ 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/git-index/src/extension/end_of_index_entry/write.rs b/git-index/src/extension/end_of_index_entry/write.rs index e73be681096..a1850624256 100644 --- a/git-index/src/extension/end_of_index_entry/write.rs +++ b/git-index/src/extension/end_of_index_entry/write.rs @@ -11,7 +11,7 @@ pub fn write_to( out: &mut impl std::io::Write, hash_kind: git_hash::Kind, offset_to_extensions: u32, - prior_extensions: impl Iterator, + prior_extensions: &[(Signature, u32)], ) -> Result<(), std::io::Error> { out.write_all(&SIGNATURE)?; let extension_size: u32 = 4 + hash_kind.len_in_bytes() as u32; @@ -21,7 +21,7 @@ pub fn write_to( let mut hasher = git_features::hash::hasher(hash_kind); for (signature, size) in prior_extensions { - hasher.update(&signature); + hasher.update(signature); hasher.update(&size.to_be_bytes()); } out.write_all(&hasher.digest())?; diff --git a/git-index/src/write.rs b/git-index/src/write.rs index 9f3654d8a69..13f4f45d93f 100644 --- a/git-index/src/write.rs +++ b/git-index/src/write.rs @@ -64,13 +64,13 @@ impl State { let offset_to_extensions = entries(&mut write, self, offset_to_entries)?; let extensions = { - let extensions: &[&dyn Fn(&mut dyn std::io::Write) -> Option>] = - &[&|write| { - tree_cache_extension - .then(|| self.tree()) - .flatten() - .map(|tree| tree.write_to(write).map(|_| extension::tree::SIGNATURE)) - }]; + type WriteExtFn<'a> = &'a dyn Fn(&mut dyn std::io::Write) -> Option>; + let extensions: &[WriteExtFn<'_>] = &[&|write| { + tree_cache_extension + .then(|| self.tree()) + .flatten() + .map(|tree| tree.write_to(write).map(|_| extension::tree::SIGNATURE)) + }]; let mut offset_to_previous_ext = offset_to_extensions; let mut out = Vec::with_capacity(5); @@ -86,12 +86,7 @@ impl State { }; if num_entries > 0 && end_of_index_entry_extension && !extensions.is_empty() { - extension::end_of_index_entry::write_to( - write.inner, - hash_kind, - offset_to_extensions, - extensions.into_iter(), - )? + extension::end_of_index_entry::write_to(write.inner, hash_kind, offset_to_extensions, &extensions)? } Ok(()) From 834be93e6db84bb9160dd4677b7e9d63213c23cd Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 13 Aug 2022 10:37:38 +0800 Subject: [PATCH 07/18] thanks clippy --- git-index/src/extension/end_of_index_entry/write.rs | 4 ++-- git-index/src/write.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/git-index/src/extension/end_of_index_entry/write.rs b/git-index/src/extension/end_of_index_entry/write.rs index a1850624256..e4fb6ba7902 100644 --- a/git-index/src/extension/end_of_index_entry/write.rs +++ b/git-index/src/extension/end_of_index_entry/write.rs @@ -11,7 +11,7 @@ pub fn write_to( out: &mut impl std::io::Write, hash_kind: git_hash::Kind, offset_to_extensions: u32, - prior_extensions: &[(Signature, u32)], + prior_extensions: impl IntoIterator, ) -> Result<(), std::io::Error> { out.write_all(&SIGNATURE)?; let extension_size: u32 = 4 + hash_kind.len_in_bytes() as u32; @@ -21,7 +21,7 @@ pub fn write_to( let mut hasher = git_features::hash::hasher(hash_kind); for (signature, size) in prior_extensions { - hasher.update(signature); + hasher.update(&signature); hasher.update(&size.to_be_bytes()); } out.write_all(&hasher.digest())?; diff --git a/git-index/src/write.rs b/git-index/src/write.rs index 13f4f45d93f..ae368144ec4 100644 --- a/git-index/src/write.rs +++ b/git-index/src/write.rs @@ -86,7 +86,7 @@ impl State { }; if num_entries > 0 && end_of_index_entry_extension && !extensions.is_empty() { - extension::end_of_index_entry::write_to(write.inner, hash_kind, offset_to_extensions, &extensions)? + extension::end_of_index_entry::write_to(write.inner, hash_kind, offset_to_extensions, extensions)? } Ok(()) From 9861a6ce8abc438a1e0739aa6d55ced450a4465b Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 13 Aug 2022 10:39:50 +0800 Subject: [PATCH 08/18] fix docs --- git-index/src/extension/tree/verify.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-index/src/extension/tree/verify.rs b/git-index/src/extension/tree/verify.rs index d384a7f04bd..000763d28b2 100644 --- a/git-index/src/extension/tree/verify.rs +++ b/git-index/src/extension/tree/verify.rs @@ -2,7 +2,7 @@ use crate::extension::Tree; use bstr::{BString, ByteSlice}; use std::cmp::Ordering; -/// The error returned by [Tree::verify()][super::Tree::verify()]. +/// The error returned by [Tree::verify()][crate::extension::Tree::verify()]. #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { From fbe981519446e55c4020e95841e7bff7e54e358e Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 13 Aug 2022 14:20:13 +0800 Subject: [PATCH 09/18] Make it more explicit to write all available extensions by default --- git-index/src/write.rs | 75 ++++++++++++++++++++++------- git-index/tests/index/file/write.rs | 30 +++++++----- 2 files changed, 75 insertions(+), 30 deletions(-) diff --git a/git-index/src/write.rs b/git-index/src/write.rs index ae368144ec4..663f150099e 100644 --- a/git-index/src/write.rs +++ b/git-index/src/write.rs @@ -3,10 +3,51 @@ use crate::{extension, State, Version}; use std::convert::TryInto; use std::io::Write; +/// A way to specify which extensions to write. +#[derive(Debug, Copy, Clone)] +pub enum Extensions { + /// Writes all available extensions to avoid loosing any information, and to allow accelerated reading of the index file. + All, + /// Only write the given extensions, with each extension being marked by a boolean flag. + Given { + /// Write the tree-cache extension, if present. + tree_cache: bool, + /// Write the end-of-index-entry extension. + end_of_index_entry: bool, + }, + /// Write no extension at all for what should be the smallest possible index + None, +} + +impl Default for Extensions { + fn default() -> Self { + Extensions::All + } +} + +impl Extensions { + /// Returns `Some(signature)` if it should be written out. + pub fn should_write(&self, signature: extension::Signature) -> Option { + match self { + Extensions::None => None, + Extensions::All => Some(signature), + Extensions::Given { + tree_cache, + end_of_index_entry, + } => match signature { + extension::tree::SIGNATURE => tree_cache, + extension::end_of_index_entry::SIGNATURE => end_of_index_entry, + _ => &false, + } + .then(|| signature), + } + } +} + /// The options for use when [writing an index][State::write_to()]. /// /// Note that default options write -#[derive(Debug, PartialEq, Eq, Clone, Copy)] +#[derive(Debug, Clone, Copy)] pub struct Options { /// The hash kind to use when writing the index file. /// @@ -15,12 +56,8 @@ pub struct Options { /// The index version to write. Note that different versions affect the format and ultimately the size. pub version: Version, - /// If true, write the tree-cache extension, if present. - // TODO: should we not write all we have by default to be lossless, but provide options to those who seek them? - pub tree_cache_extension: bool, - /// If true, write the end-of-index-entry extension. - // TODO: figure out if this is implied by other options, for instance multi-threading. - pub end_of_index_entry_extension: bool, + /// Configures which extensions to write + pub extensions: Extensions, } impl Default for Options { @@ -29,8 +66,7 @@ impl Default for Options { hash_kind: git_hash::Kind::default(), /// TODO: make this 'automatic' by default to determine the correct index version - not all versions can represent all in-memory states. version: Version::V2, - tree_cache_extension: true, - end_of_index_entry_extension: true, + extensions: Default::default(), } } } @@ -43,8 +79,7 @@ impl State { Options { hash_kind, version, - tree_cache_extension, - end_of_index_entry_extension, + extensions, }: Options, ) -> std::io::Result<()> { assert_eq!( @@ -63,13 +98,12 @@ impl State { let offset_to_entries = header(&mut write, version, num_entries)?; let offset_to_extensions = entries(&mut write, self, offset_to_entries)?; - let extensions = { + let extension_toc = { type WriteExtFn<'a> = &'a dyn Fn(&mut dyn std::io::Write) -> Option>; let extensions: &[WriteExtFn<'_>] = &[&|write| { - tree_cache_extension - .then(|| self.tree()) - .flatten() - .map(|tree| tree.write_to(write).map(|_| extension::tree::SIGNATURE)) + extensions + .should_write(extension::tree::SIGNATURE) + .and_then(|signature| self.tree().map(|tree| tree.write_to(write).map(|_| signature))) }]; let mut offset_to_previous_ext = offset_to_extensions; @@ -85,8 +119,13 @@ impl State { out }; - if num_entries > 0 && end_of_index_entry_extension && !extensions.is_empty() { - extension::end_of_index_entry::write_to(write.inner, hash_kind, offset_to_extensions, extensions)? + if num_entries > 0 + && extensions + .should_write(extension::end_of_index_entry::SIGNATURE) + .is_some() + && !extension_toc.is_empty() + { + extension::end_of_index_entry::write_to(write.inner, hash_kind, offset_to_extensions, extension_toc)? } Ok(()) diff --git a/git-index/tests/index/file/write.rs b/git-index/tests/index/file/write.rs index 5546f82e530..c78b1992234 100644 --- a/git-index/tests/index/file/write.rs +++ b/git-index/tests/index/file/write.rs @@ -1,6 +1,6 @@ use filetime::FileTime; use git_index::verify::extensions::no_find; -use git_index::{decode, write, State, Version}; +use git_index::{decode, extension, write, State, Version}; use std::cmp::{max, min}; #[test] @@ -11,7 +11,10 @@ fn roundtrips() { ( "v2_more_files", write::Options { - end_of_index_entry_extension: false, + extensions: write::Extensions::Given { + end_of_index_entry: false, + tree_cache: true, + }, ..write::Options::default() }, ), @@ -49,8 +52,7 @@ fn v2_index_no_extensions() { let options = write::Options { hash_kind: git_hash::Kind::Sha1, version: Version::V2, - tree_cache_extension: false, - end_of_index_entry_extension: false, + extensions: write::Extensions::None, }; expected.write_to(&mut out, options).unwrap(); @@ -78,8 +80,10 @@ fn v2_index_tree_extensions() { let options = write::Options { hash_kind: git_hash::Kind::Sha1, version: Version::V2, - tree_cache_extension: true, - end_of_index_entry_extension: false, + extensions: write::Extensions::Given { + tree_cache: true, + end_of_index_entry: false, + }, }; expected.write_to(&mut out, options).unwrap(); @@ -107,8 +111,10 @@ fn v2_index_eoie_extensions() { let options = write::Options { hash_kind: git_hash::Kind::Sha1, version: Version::V2, - tree_cache_extension: false, - end_of_index_entry_extension: true, + extensions: write::Extensions::Given { + tree_cache: false, + end_of_index_entry: true, + }, }; expected.write_to(&mut out, options).unwrap(); @@ -124,10 +130,10 @@ fn compare_states(generated: &State, expected: &State, options: write::Options, assert_eq!(generated.version(), options.version, "version mismatch in {}", fixture); assert_eq!( generated.tree(), - match options.tree_cache_extension { - true => expected.tree(), - false => None, - }, + options + .extensions + .should_write(extension::tree::SIGNATURE) + .and_then(|_| expected.tree()), "tree extension mismatch in {}", fixture ); From d7a8a7dfe3089e35fca249af7a3482a893f91111 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 13 Aug 2022 14:56:02 +0800 Subject: [PATCH 10/18] add tests to run into long-paths special case (#XXX) --- git-index/src/entry/flags.rs | 8 +++++--- git-index/src/entry/tests.rs | 3 +-- git-index/src/entry/write.rs | 7 +++---- git-index/tests/index/file/read.rs | 8 ++++++-- git-index/tests/index/file/write.rs | 19 +++++++++++++++---- 5 files changed, 30 insertions(+), 15 deletions(-) diff --git a/git-index/src/entry/flags.rs b/git-index/src/entry/flags.rs index dec04baeddc..623f07ce946 100644 --- a/git-index/src/entry/flags.rs +++ b/git-index/src/entry/flags.rs @@ -1,5 +1,4 @@ use crate::entry::Stage; -use crate::Version; use bitflags::bitflags; bitflags! { @@ -65,8 +64,11 @@ impl Flags { /// Transform ourselves to a storage representation to keep all flags which are to be persisted, /// with the caller intending to write `version`. - pub fn to_storage(&self, version: Version) -> at_rest::Flags { - assert_eq!(version, Version::V2, "Can only encode V2 flags at the moment"); + pub fn to_storage(&self) -> at_rest::Flags { + assert!( + !self.contains(Self::EXTENDED_FLAGS), + "Cannot currently encode extended flags" + ); at_rest::Flags::from_bits(self.bits() as u16).unwrap() } } diff --git a/git-index/src/entry/tests.rs b/git-index/src/entry/tests.rs index 8765e58eb88..2247db22bf4 100644 --- a/git-index/src/entry/tests.rs +++ b/git-index/src/entry/tests.rs @@ -1,5 +1,4 @@ use crate::entry::at_rest; -use crate::Version; #[test] fn in_mem_flags_to_storage_flags_v2() { @@ -7,7 +6,7 @@ fn in_mem_flags_to_storage_flags_v2() { let flags_at_rest = at_rest::Flags::from_bits(flag_bytes).unwrap(); let in_memory_flags = flags_at_rest.to_memory(); - let output = in_memory_flags.to_storage(Version::V2); + let output = in_memory_flags.to_storage(); assert_eq!(output.bits(), flag_bytes); } diff --git a/git-index/src/entry/write.rs b/git-index/src/entry/write.rs index 07c36f83701..0925a5f18f9 100644 --- a/git-index/src/entry/write.rs +++ b/git-index/src/entry/write.rs @@ -1,8 +1,8 @@ -use crate::{Entry, State, Version}; +use crate::{Entry, State}; use std::convert::TryInto; impl Entry { - /// Serialize ourselves to `out` with path access via `state`. + /// Serialize ourselves to `out` with path access via `state`, without padding. pub fn write_to(&self, mut out: impl std::io::Write, state: &State) -> std::io::Result<()> { let stat = self.stat; out.write_all(&stat.ctime.secs.to_be_bytes())?; @@ -25,8 +25,7 @@ impl Entry { path_len <= 0xFFF, "Paths can't be longer than 12 bits as they share space with bit flags in a u16" ); - let version = Version::V2; // TODO: don't hardcode once `to_storage()` can do its work without assertion - out.write_all(&(self.flags.to_storage(version).bits() | path_len).to_be_bytes())?; + out.write_all(&(self.flags.to_storage().bits() | path_len).to_be_bytes())?; out.write_all(path)?; out.write_all(b"\0") } diff --git a/git-index/tests/index/file/read.rs b/git-index/tests/index/file/read.rs index 02356f4ce75..9ed11f61ba2 100644 --- a/git-index/tests/index/file/read.rs +++ b/git-index/tests/index/file/read.rs @@ -13,8 +13,12 @@ fn verify(index: git_index::File) -> git_index::File { index } -fn loose_file(name: &str) -> git_index::File { - let path = git_testtools::fixture_path(Path::new("loose_index").join(name).with_extension("git-index")); +pub(crate) fn loose_file_path(name: &str) -> PathBuf { + git_testtools::fixture_path(Path::new("loose_index").join(name).with_extension("git-index")) +} + +pub(crate) fn loose_file(name: &str) -> git_index::File { + let path = loose_file_path(name); let file = git_index::File::at(path, git_index::decode::Options::default()).unwrap(); verify(file) } diff --git a/git-index/tests/index/file/write.rs b/git-index/tests/index/file/write.rs index c78b1992234..6013099ac9f 100644 --- a/git-index/tests/index/file/write.rs +++ b/git-index/tests/index/file/write.rs @@ -1,15 +1,23 @@ +use crate::index::file::read::loose_file_path; use filetime::FileTime; use git_index::verify::extensions::no_find; use git_index::{decode, extension, write, State, Version}; use std::cmp::{max, min}; #[test] +#[ignore] fn roundtrips() { + enum Kind { + Generated(&'static str), + Loose(&'static str), + } + use Kind::*; let input = [ - ("v2", write::Options::default()), - ("V2_empty", write::Options::default()), + (Loose("very-long-path"), write::Options::default()), + (Generated("v2"), write::Options::default()), + (Generated("V2_empty"), write::Options::default()), ( - "v2_more_files", + Generated("v2_more_files"), write::Options { extensions: write::Extensions::Given { end_of_index_entry: false, @@ -21,7 +29,10 @@ fn roundtrips() { ]; for (fixture, options) in input { - let path = crate::fixture_index_path(fixture); + let (path, fixture) = match fixture { + Generated(name) => (crate::fixture_index_path(name), name), + Loose(name) => (loose_file_path(name), name), + }; let expected_index = git_index::File::at(&path, decode::Options::default()).unwrap(); let expected_bytes = std::fs::read(&path).unwrap(); let mut out_bytes = Vec::new(); From 581cbd7afeac0f7654611c83deacae802ef5da6f Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 13 Aug 2022 19:58:32 +0800 Subject: [PATCH 11/18] first PoC for writing long paths, even though it doens't produce the entire file yet --- git-index/src/entry/write.rs | 17 ++++---- git-index/tests/index-multi-threaded.rs | 2 + git-index/tests/index-single-threaded.rs | 2 + git-index/tests/index/file/write.rs | 49 +++++++++++------------ git-repository/src/repository/identity.rs | 9 ++--- 5 files changed, 39 insertions(+), 40 deletions(-) diff --git a/git-index/src/entry/write.rs b/git-index/src/entry/write.rs index 0925a5f18f9..6594ae33df8 100644 --- a/git-index/src/entry/write.rs +++ b/git-index/src/entry/write.rs @@ -1,4 +1,4 @@ -use crate::{Entry, State}; +use crate::{entry, Entry, State}; use std::convert::TryInto; impl Entry { @@ -17,14 +17,13 @@ impl Entry { out.write_all(&stat.size.to_be_bytes())?; out.write_all(self.id.as_bytes())?; let path = self.path(state); - let path_len: u16 = path - .len() - .try_into() - .expect("Cannot handle paths longer than 16bits ever"); - assert!( - path_len <= 0xFFF, - "Paths can't be longer than 12 bits as they share space with bit flags in a u16" - ); + let path_len: u16 = if path.len() >= entry::Flags::PATH_LEN.bits() as usize { + entry::Flags::PATH_LEN.bits() as u16 + } else { + path.len() + .try_into() + .expect("we just checked that the length is smaller than 0xfff") + }; out.write_all(&(self.flags.to_storage().bits() | path_len).to_be_bytes())?; out.write_all(path)?; out.write_all(b"\0") diff --git a/git-index/tests/index-multi-threaded.rs b/git-index/tests/index-multi-threaded.rs index 936bcb7f428..fd3abfb9ce5 100644 --- a/git-index/tests/index-multi-threaded.rs +++ b/git-index/tests/index-multi-threaded.rs @@ -1,2 +1,4 @@ +pub use git_testtools::Result; + mod index; pub use index::*; diff --git a/git-index/tests/index-single-threaded.rs b/git-index/tests/index-single-threaded.rs index 4dcec528c1e..4a90ca77939 100644 --- a/git-index/tests/index-single-threaded.rs +++ b/git-index/tests/index-single-threaded.rs @@ -1,3 +1,5 @@ +pub use git_testtools::Result; + #[cfg(not(feature = "internal-testing-git-features-parallel"))] mod index; #[cfg(not(feature = "internal-testing-git-features-parallel"))] diff --git a/git-index/tests/index/file/write.rs b/git-index/tests/index/file/write.rs index 6013099ac9f..2508fe32db8 100644 --- a/git-index/tests/index/file/write.rs +++ b/git-index/tests/index/file/write.rs @@ -5,17 +5,16 @@ use git_index::{decode, extension, write, State, Version}; use std::cmp::{max, min}; #[test] -#[ignore] -fn roundtrips() { +fn roundtrips() -> crate::Result { enum Kind { Generated(&'static str), Loose(&'static str), } use Kind::*; let input = [ - (Loose("very-long-path"), write::Options::default()), - (Generated("v2"), write::Options::default()), - (Generated("V2_empty"), write::Options::default()), + (Loose("very-long-path"), write::Options::default(), false), // unclear why the file is smaller when written back + (Generated("v2"), write::Options::default(), true), + (Generated("V2_empty"), write::Options::default(), true), ( Generated("v2_more_files"), write::Options { @@ -25,24 +24,28 @@ fn roundtrips() { }, ..write::Options::default() }, + true, ), ]; - for (fixture, options) in input { + for (fixture, options, compare_byte_by_byte) in input { let (path, fixture) = match fixture { Generated(name) => (crate::fixture_index_path(name), name), Loose(name) => (loose_file_path(name), name), }; - let expected_index = git_index::File::at(&path, decode::Options::default()).unwrap(); - let expected_bytes = std::fs::read(&path).unwrap(); + let expected = git_index::File::at(&path, decode::Options::default())?; + let expected_bytes = std::fs::read(&path)?; let mut out_bytes = Vec::new(); - expected_index.write_to(&mut out_bytes, options).unwrap(); - let (out_index, _) = State::from_bytes(&out_bytes, FileTime::now(), decode::Options::default()).unwrap(); + expected.write_to(&mut out_bytes, options)?; + let (actual, _) = State::from_bytes(&out_bytes, FileTime::now(), decode::Options::default())?; - compare_states(&out_index, &expected_index, options, fixture); - compare_raw_bytes(&out_bytes, &expected_bytes, fixture); + compare_states(&actual, &expected, options, fixture); + if compare_byte_by_byte { + compare_raw_bytes(&out_bytes, &expected_bytes, fixture); + } } + Ok(()) } #[test] @@ -135,12 +138,13 @@ fn v2_index_eoie_extensions() { } } -fn compare_states(generated: &State, expected: &State, options: write::Options, fixture: &str) { - generated.verify_entries().expect("valid"); - generated.verify_extensions(false, no_find).expect("valid"); - assert_eq!(generated.version(), options.version, "version mismatch in {}", fixture); +fn compare_states(actual: &State, expected: &State, options: write::Options, fixture: &str) { + actual.verify_entries().expect("valid"); + actual.verify_extensions(false, no_find).expect("valid"); + + assert_eq!(actual.version(), options.version, "version mismatch in {}", fixture); assert_eq!( - generated.tree(), + actual.tree(), options .extensions .should_write(extension::tree::SIGNATURE) @@ -149,19 +153,14 @@ fn compare_states(generated: &State, expected: &State, options: write::Options, fixture ); assert_eq!( - generated.entries().len(), + actual.entries().len(), expected.entries().len(), "entry count mismatch in {}", fixture ); + assert_eq!(actual.entries(), expected.entries(), "entries mismatch in {}", fixture); assert_eq!( - generated.entries(), - expected.entries(), - "entries mismatch in {}", - fixture - ); - assert_eq!( - generated.path_backing(), + actual.path_backing(), expected.path_backing(), "path_backing mismatch in {}", fixture diff --git a/git-repository/src/repository/identity.rs b/git-repository/src/repository/identity.rs index a5f9207fe7f..baa9684d393 100644 --- a/git-repository/src/repository/identity.rs +++ b/git-repository/src/repository/identity.rs @@ -1,8 +1,5 @@ use std::borrow::Cow; -use git_actor::SignatureRef; -use git_config::File; - use crate::{bstr::BString, permission}; /// Identity handling. @@ -15,8 +12,8 @@ impl crate::Repository { /// # Note /// /// The values are cached when the repository is instantiated. - pub fn user_default(&self) -> SignatureRef<'_> { - SignatureRef { + pub fn user_default(&self) -> git_actor::SignatureRef<'_> { + git_actor::SignatureRef { name: "gitoxide".into(), email: "gitoxide@localhost".into(), time: git_date::Time::now_local_or_utc(), @@ -105,7 +102,7 @@ impl Personas { fn env_var(name: &str) -> Option { std::env::var_os(name).map(|value| git_path::into_bstr(Cow::Owned(value.into())).into_owned()) } - fn entity_in_section(name: &str, config: &File<'_>) -> (Option, Option) { + fn entity_in_section(name: &str, config: &git_config::File<'_>) -> (Option, Option) { config .section(name, None) .map(|section| { From f93febe2d2c55938ac8f698b57144583caab54ef Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 13 Aug 2022 21:26:20 +0800 Subject: [PATCH 12/18] fix tree ext reading and writing; round-trip with long path works now --- Cargo.lock | 17 +++++++++-------- git-index/Cargo.toml | 1 + git-index/src/extension/mod.rs | 3 ++- git-index/src/extension/tree/decode.rs | 16 ++++++++++++---- git-index/src/extension/tree/mod.rs | 2 +- git-index/src/extension/tree/verify.rs | 14 +++++++------- git-index/src/extension/tree/write.rs | 18 ++++++++++++------ git-index/tests/index/file/read.rs | 17 +++++++++++++---- git-index/tests/index/file/write.rs | 24 +++++++++++++++--------- gitoxide-core/src/index/information.rs | 2 +- 10 files changed, 73 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 871d0702344..fd9a248dbfe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -477,7 +477,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e33b5c3ee2b4ffa00ac2b00d1645cd9229ade668139bccf95f15fadcf374127b" dependencies = [ "castaway", - "itoa 1.0.2", + "itoa 1.0.3", "ryu", "serde", ] @@ -1069,7 +1069,7 @@ dependencies = [ "git-date", "git-features", "git-testtools", - "itoa 1.0.2", + "itoa 1.0.3", "nom", "pretty_assertions", "quick-error", @@ -1179,7 +1179,7 @@ dependencies = [ "bstr", "document-features", "git-testtools", - "itoa 1.0.2", + "itoa 1.0.3", "serde", "time", ] @@ -1276,6 +1276,7 @@ dependencies = [ "git-hash", "git-object", "git-testtools", + "itoa 1.0.3", "memmap2", "serde", "smallvec", @@ -1324,7 +1325,7 @@ dependencies = [ "git-testtools", "git-validate", "hex", - "itoa 1.0.2", + "itoa 1.0.3", "nom", "pretty_assertions", "quick-error", @@ -1925,9 +1926,9 @@ checksum = "b71991ff56294aa922b450139ee08b3bfc70982c6b2c7562771375cf73542dd4" [[package]] name = "itoa" -version = "1.0.2" +version = "1.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "112c678d4050afce233f4f2852bb2eb519230b3cf12f33585275537d7e41578d" +checksum = "6c8af84674fe1f223a982c933a0ee1086ac4d4052aa0fb8060c12c6ad838e754" [[package]] name = "jobserver" @@ -2640,7 +2641,7 @@ version = "1.0.82" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "82c2c1fdcd807d1098552c5b9a36e425e42e9fbd7c6a37a8425f390f781f7fa7" dependencies = [ - "itoa 1.0.2", + "itoa 1.0.3", "ryu", "serde", ] @@ -2921,7 +2922,7 @@ version = "0.3.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72c91f41dcb2f096c05f0873d667dceec1087ce5bcf984ec8ffb19acddbb3217" dependencies = [ - "itoa 1.0.2", + "itoa 1.0.3", "libc", "num_threads", ] diff --git a/git-index/Cargo.toml b/git-index/Cargo.toml index b32a2984d2f..e4931817ed4 100644 --- a/git-index/Cargo.toml +++ b/git-index/Cargo.toml @@ -45,6 +45,7 @@ bstr = { version = "0.2.13", default-features = false } serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"] } smallvec = "1.7.0" atoi = "1.0.0" +itoa = "1.0.3" bitflags = "1.3.2" document-features = { version = "0.2.0", optional = true } diff --git a/git-index/src/extension/mod.rs b/git-index/src/extension/mod.rs index 391dcb574bc..bf71119c734 100644 --- a/git-index/src/extension/mod.rs +++ b/git-index/src/extension/mod.rs @@ -26,7 +26,8 @@ pub struct Tree { pub id: git_hash::ObjectId, /// The amount of non-tree items in this directory tree, including sub-trees, recursively. /// The value of the top-level tree is thus equal to the value of the total amount of entries. - pub num_entries: u32, + /// If `None`, the tree is considered invalid and needs to be refreshed + pub num_entries: Option, /// The child-trees below the current tree. pub children: Vec, } diff --git a/git-index/src/extension/tree/decode.rs b/git-index/src/extension/tree/decode.rs index ca8f42cf774..09f5dc267d6 100644 --- a/git-index/src/extension/tree/decode.rs +++ b/git-index/src/extension/tree/decode.rs @@ -1,6 +1,7 @@ use crate::extension::Tree; use crate::util::{split_at_byte_exclusive, split_at_pos}; use git_hash::ObjectId; +use std::convert::TryInto; /// A recursive data structure pub fn decode(data: &[u8], object_hash: git_hash::Kind) -> Option { @@ -17,13 +18,20 @@ fn one_recursive(data: &[u8], hash_len: usize) -> Option<(Tree, &[u8])> { let (path, data) = split_at_byte_exclusive(data, 0)?; let (entry_count, data) = split_at_byte_exclusive(data, b' ')?; - let num_entries: u32 = atoi::atoi(entry_count)?; + let num_entries: i32 = atoi::atoi(entry_count)?; let (subtree_count, data) = split_at_byte_exclusive(data, b'\n')?; let subtree_count: usize = atoi::atoi(subtree_count)?; - let (hash, mut data) = split_at_pos(data, hash_len)?; - let id = ObjectId::from(hash); + let (id, mut data) = if num_entries >= 0 { + let (hash, data) = split_at_pos(data, hash_len)?; + (ObjectId::from(hash), data) + } else { + ( + ObjectId::null(git_hash::Kind::from_hex_len(hash_len * 2).expect("valid hex_len")), + data, + ) + }; let mut subtrees = Vec::with_capacity(subtree_count); for _ in 0..subtree_count { @@ -42,7 +50,7 @@ fn one_recursive(data: &[u8], hash_len: usize) -> Option<(Tree, &[u8])> { Some(( Tree { id, - num_entries, + num_entries: num_entries.try_into().ok(), name: path.into(), children: subtrees, }, diff --git a/git-index/src/extension/tree/mod.rs b/git-index/src/extension/tree/mod.rs index b0e17be23cf..f2075939978 100644 --- a/git-index/src/extension/tree/mod.rs +++ b/git-index/src/extension/tree/mod.rs @@ -16,6 +16,6 @@ mod tests { #[test] fn size_of_tree() { - assert_eq!(std::mem::size_of::(), 80); + assert_eq!(std::mem::size_of::(), 88); } } diff --git a/git-index/src/extension/tree/verify.rs b/git-index/src/extension/tree/verify.rs index 000763d28b2..15ff4565365 100644 --- a/git-index/src/extension/tree/verify.rs +++ b/git-index/src/extension/tree/verify.rs @@ -57,7 +57,7 @@ impl Tree { let mut entries = 0; let mut prev = None::<&Tree>; for child in children { - entries += child.num_entries; + entries += child.num_entries.unwrap_or(0); if let Some(prev) = prev { if prev.name.cmp(&child.name) != Ordering::Less { return Err(Error::OutOfOrder { @@ -98,11 +98,11 @@ impl Tree { // This is actually needed here as it's a mut ref, which isn't copy. We do a re-borrow here. #[allow(clippy::needless_option_as_deref)] let actual_num_entries = verify_recursive(child.id, &child.children, find_buf.as_deref_mut(), find)?; - if let Some(actual) = actual_num_entries { - if actual > child.num_entries { + if let Some((actual, num_entries)) = actual_num_entries.zip(child.num_entries) { + if actual > num_entries { return Err(Error::EntriesCount { actual, - expected: child.num_entries, + expected: num_entries, }); } } @@ -118,11 +118,11 @@ impl Tree { let mut buf = Vec::new(); let declared_entries = verify_recursive(self.id, &self.children, use_find.then(|| &mut buf), &mut find)?; - if let Some(actual) = declared_entries { - if actual > self.num_entries { + if let Some((actual, num_entries)) = declared_entries.zip(self.num_entries) { + if actual > num_entries { return Err(Error::EntriesCount { actual, - expected: self.num_entries, + expected: num_entries, }); } } diff --git a/git-index/src/extension/tree/write.rs b/git-index/src/extension/tree/write.rs index 4effaec1454..08dc47b4d32 100644 --- a/git-index/src/extension/tree/write.rs +++ b/git-index/src/extension/tree/write.rs @@ -5,16 +5,22 @@ impl Tree { /// Serialize this instance to `out`. pub fn write_to(&self, mut out: impl std::io::Write) -> Result<(), std::io::Error> { fn tree_entry(out: &mut impl std::io::Write, tree: &Tree) -> Result<(), std::io::Error> { - let num_entries_ascii = tree.num_entries.to_string(); - let num_children_ascii = tree.children.len().to_string(); + let mut buf = itoa::Buffer::new(); + let num_entries = match tree.num_entries { + Some(num_entries) => buf.format(num_entries), + None => buf.format(-1), + }; out.write_all(tree.name.as_slice())?; out.write_all(b"\0")?; - out.write_all(num_entries_ascii.as_bytes())?; + out.write_all(num_entries.as_bytes())?; out.write_all(b" ")?; - out.write_all(num_children_ascii.as_bytes())?; + let num_children = buf.format(tree.children.len()); + out.write_all(num_children.as_bytes())?; out.write_all(b"\n")?; - out.write_all(tree.id.as_bytes())?; + if tree.num_entries.is_some() { + out.write_all(tree.id.as_bytes())?; + } for child in &tree.children { tree_entry(out, child)?; @@ -25,7 +31,7 @@ impl Tree { let signature = tree::SIGNATURE; - let estimated_size = self.num_entries * (300 + 3 + 1 + 3 + 1 + 20); + let estimated_size = self.num_entries.unwrap_or(0) * (300 + 3 + 1 + 3 + 1 + 20); let mut entries: Vec = Vec::with_capacity(estimated_size as usize); tree_entry(&mut entries, self)?; diff --git a/git-index/tests/index/file/read.rs b/git-index/tests/index/file/read.rs index 9ed11f61ba2..4d983a77f11 100644 --- a/git-index/tests/index/file/read.rs +++ b/git-index/tests/index/file/read.rs @@ -52,7 +52,7 @@ fn v2_with_single_entry_tree_and_eoie_ext() { assert_eq!(entry.path(&file.state), "a"); let tree = file.tree().unwrap(); - assert_eq!(tree.num_entries, 1); + assert_eq!(tree.num_entries.unwrap_or_default(), 1); assert_eq!(tree.id, hex_to_id("496d6428b9cf92981dc9495211e6e1120fb6f2ba")); assert!(tree.name.is_empty()); assert!(tree.children.is_empty()); @@ -64,7 +64,7 @@ fn v2_empty() { assert_eq!(file.version(), Version::V2); assert_eq!(file.entries().len(), 0); let tree = file.tree().unwrap(); - assert_eq!(tree.num_entries, 0); + assert_eq!(tree.num_entries.unwrap_or_default(), 0); assert!(tree.name.is_empty()); assert!(tree.children.is_empty()); assert_eq!(tree.id, hex_to_id("4b825dc642cb6eb9a060e54bf8d69288fbee4904")); @@ -86,13 +86,13 @@ fn v2_with_multiple_entries_without_eoie_ext() { let tree = file.tree().unwrap(); assert_eq!(tree.id, hex_to_id("c9b29c3168d8e677450cc650238b23d9390801fb")); - assert_eq!(tree.num_entries, 6); + assert_eq!(tree.num_entries.unwrap_or_default(), 6); assert!(tree.name.is_empty()); assert_eq!(tree.children.len(), 1); let tree = &tree.children[0]; assert_eq!(tree.id, hex_to_id("765b32c65d38f04c4f287abda055818ec0f26912")); - assert_eq!(tree.num_entries, 3); + assert_eq!(tree.num_entries.unwrap_or_default(), 3); assert_eq!(tree.name.as_bstr(), "d"); } @@ -143,6 +143,15 @@ fn v2_very_long_path() { .chain(std::iter::once('q')) .collect::() ); + assert!( + file.tree().is_some(), + "Tree has invalid entries, but that shouldn't prevent us from loading it" + ); + let tree = file.tree().expect("present"); + assert_eq!(tree.num_entries, None, "root tree has invalid entries actually"); + assert_eq!(tree.name.as_bstr(), ""); + assert_eq!(tree.num_entries, None, "it's marked invalid actually"); + assert!(tree.id.is_null(), "there is no id for the root") } #[test] diff --git a/git-index/tests/index/file/write.rs b/git-index/tests/index/file/write.rs index 2508fe32db8..7e3f0a6fa98 100644 --- a/git-index/tests/index/file/write.rs +++ b/git-index/tests/index/file/write.rs @@ -12,9 +12,18 @@ fn roundtrips() -> crate::Result { } use Kind::*; let input = [ - (Loose("very-long-path"), write::Options::default(), false), // unclear why the file is smaller when written back - (Generated("v2"), write::Options::default(), true), - (Generated("V2_empty"), write::Options::default(), true), + ( + Loose("very-long-path"), + write::Options { + extensions: write::Extensions::Given { + end_of_index_entry: false, + tree_cache: true, + }, + ..write::Options::default() + }, + ), // tree extension can't be read + (Generated("v2"), write::Options::default()), + (Generated("V2_empty"), write::Options::default()), ( Generated("v2_more_files"), write::Options { @@ -24,11 +33,10 @@ fn roundtrips() -> crate::Result { }, ..write::Options::default() }, - true, ), ]; - for (fixture, options, compare_byte_by_byte) in input { + for (fixture, options) in input { let (path, fixture) = match fixture { Generated(name) => (crate::fixture_index_path(name), name), Loose(name) => (loose_file_path(name), name), @@ -41,9 +49,7 @@ fn roundtrips() -> crate::Result { let (actual, _) = State::from_bytes(&out_bytes, FileTime::now(), decode::Options::default())?; compare_states(&actual, &expected, options, fixture); - if compare_byte_by_byte { - compare_raw_bytes(&out_bytes, &expected_bytes, fixture); - } + compare_raw_bytes(&out_bytes, &expected_bytes, fixture); } Ok(()) } @@ -179,7 +185,7 @@ fn compare_raw_bytes(generated: &[u8], expected: &[u8], fixture: &str) { let expected = &expected[range_left..range_right]; panic! {"\n\nRoundtrip failed for index in fixture {:?} at position {:?}\n\ - \t Input: ... {:?} ...\n\ + \t Actual: ... {:?} ...\n\ \tExpected: ... {:?} ...\n\n\ ", &fixture, index, generated, expected} } diff --git a/gitoxide-core/src/index/information.rs b/gitoxide-core/src/index/information.rs index a364eb5ed5c..eddfad2ec36 100644 --- a/gitoxide-core/src/index/information.rs +++ b/gitoxide-core/src/index/information.rs @@ -14,7 +14,7 @@ mod serde_only { /// The id of the directory tree of the associated tree object. id: String, /// The amount of non-tree entries contained within, and definitely not zero. - num_entries: u32, + num_entries: Option, children: Vec, } From 27993c01a1533d561629635336c5cedf53dd0efc Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 14 Aug 2022 12:18:22 +0800 Subject: [PATCH 13/18] refcator --- git-index/tests/index/file/write.rs | 42 ++++++++++------------------- 1 file changed, 14 insertions(+), 28 deletions(-) diff --git a/git-index/tests/index/file/write.rs b/git-index/tests/index/file/write.rs index 7e3f0a6fa98..a53587a74c2 100644 --- a/git-index/tests/index/file/write.rs +++ b/git-index/tests/index/file/write.rs @@ -1,6 +1,7 @@ use crate::index::file::read::loose_file_path; use filetime::FileTime; use git_index::verify::extensions::no_find; +use git_index::write::Options; use git_index::{decode, extension, write, State, Version}; use std::cmp::{max, min}; @@ -12,28 +13,10 @@ fn roundtrips() -> crate::Result { } use Kind::*; let input = [ - ( - Loose("very-long-path"), - write::Options { - extensions: write::Extensions::Given { - end_of_index_entry: false, - tree_cache: true, - }, - ..write::Options::default() - }, - ), // tree extension can't be read + (Loose("very-long-path"), v2_all_ext_but_eoie()), (Generated("v2"), write::Options::default()), (Generated("V2_empty"), write::Options::default()), - ( - Generated("v2_more_files"), - write::Options { - extensions: write::Extensions::Given { - end_of_index_entry: false, - tree_cache: true, - }, - ..write::Options::default() - }, - ), + (Generated("v2_more_files"), v2_all_ext_but_eoie()), ]; for (fixture, options) in input { @@ -97,14 +80,7 @@ fn v2_index_tree_extensions() { let expected = git_index::File::at(&path, decode::Options::default()).unwrap(); let mut out = Vec::::new(); - let options = write::Options { - hash_kind: git_hash::Kind::Sha1, - version: Version::V2, - extensions: write::Extensions::Given { - tree_cache: true, - end_of_index_entry: false, - }, - }; + let options = v2_all_ext_but_eoie(); expected.write_to(&mut out, options).unwrap(); @@ -191,3 +167,13 @@ fn compare_raw_bytes(generated: &[u8], expected: &[u8], fixture: &str) { } } } + +fn v2_all_ext_but_eoie() -> Options { + write::Options { + extensions: write::Extensions::Given { + end_of_index_entry: false, + tree_cache: true, + }, + ..write::Options::default() + } +} From 417d90eb267dd74a5372f1c7d8feb7cb4e98d2a1 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 14 Aug 2022 15:07:29 +0800 Subject: [PATCH 14/18] Support for extended flags, and V3 as it's a requirements. We need to find a way to prevent accidentally writing extended flags as V2 though. --- git-index/src/decode/header.rs | 3 ++- git-index/src/entry/flags.rs | 30 +++++++++++++++++------------ git-index/src/entry/mod.rs | 3 --- git-index/src/entry/tests.rs | 12 ------------ git-index/src/entry/write.rs | 7 +++++++ git-index/src/write.rs | 10 ++++------ git-index/tests/index/file/write.rs | 17 ++++++++++++---- 7 files changed, 44 insertions(+), 38 deletions(-) delete mode 100644 git-index/src/entry/tests.rs diff --git a/git-index/src/decode/header.rs b/git-index/src/decode/header.rs index eed8595f738..94631006b3b 100644 --- a/git-index/src/decode/header.rs +++ b/git-index/src/decode/header.rs @@ -2,6 +2,8 @@ pub(crate) const SIZE: usize = 4 /*signature*/ + 4 /*version*/ + 4 /* num entrie use crate::{util::from_be_u32, Version}; +pub(crate) const SIGNATURE: &[u8] = b"DIRC"; + mod error { /// The error produced when failing to decode an index header. @@ -23,7 +25,6 @@ pub(crate) fn decode(data: &[u8], object_hash: git_hash::Kind) -> Result<(Versio )); } - const SIGNATURE: &[u8] = b"DIRC"; let (signature, data) = data.split_at(4); if signature != SIGNATURE { return Err(Error::Corrupt( diff --git a/git-index/src/entry/flags.rs b/git-index/src/entry/flags.rs index 623f07ce946..7d690c06fea 100644 --- a/git-index/src/entry/flags.rs +++ b/git-index/src/entry/flags.rs @@ -6,6 +6,8 @@ bitflags! { pub struct Flags: u32 { /// The mask to apply to obtain the stage number of an entry. const STAGE_MASK = 0x3000; + /// If set, additional bits need to be written to storage. + const EXTENDED = 0x4000; // TODO: could we use the pathlen ourselves to save 8 bytes? And how to handle longer paths than that? 0 as sentinel maybe? /// The mask to obtain the length of the path associated with this entry. const PATH_LEN = 0x0fff; @@ -48,9 +50,6 @@ bitflags! { /// Stored at rest const SKIP_WORKTREE = 1 << 30; - /// flags that need to be stored on disk in a V3 formatted index. - const EXTENDED_FLAGS = 1 << 29 | 1 << 30; - /// For future extension const EXTENDED_2 = 1 << 31; } @@ -63,13 +62,17 @@ impl Flags { } /// Transform ourselves to a storage representation to keep all flags which are to be persisted, - /// with the caller intending to write `version`. - pub fn to_storage(&self) -> at_rest::Flags { - assert!( - !self.contains(Self::EXTENDED_FLAGS), - "Cannot currently encode extended flags" - ); - at_rest::Flags::from_bits(self.bits() as u16).unwrap() + /// skipping all extended flags. Note that the caller has to check for the `EXTENDED` bit to be present + /// and write extended flags as well if so. + pub fn to_storage(mut self) -> at_rest::Flags { + at_rest::Flags::from_bits( + { + self.remove(Self::PATH_LEN); + self + } + .bits() as u16, + ) + .unwrap() } } @@ -91,8 +94,7 @@ pub(crate) mod at_rest { impl Flags { pub fn to_memory(self) -> super::Flags { - super::Flags::from_bits((self & (Flags::PATH_LEN | Flags::STAGE_MASK | Flags::ASSUME_VALID)).bits as u32) - .expect("PATHLEN is part of memory representation") + super::Flags::from_bits(self.bits as u32).expect("PATHLEN is part of memory representation") } } @@ -105,6 +107,10 @@ pub(crate) mod at_rest { } impl FlagsExtended { + pub fn from_flags(flags: super::Flags) -> Self { + Self::from_bits(((flags & (super::Flags::INTENT_TO_ADD | super::Flags::SKIP_WORKTREE)).bits >> 16) as u16) + .expect("valid") + } pub fn to_flags(self) -> Option { super::Flags::from_bits((self.bits as u32) << 16) } diff --git a/git-index/src/entry/mod.rs b/git-index/src/entry/mod.rs index 05e9ffc197e..0e8ffe9403b 100644 --- a/git-index/src/entry/mod.rs +++ b/git-index/src/entry/mod.rs @@ -82,6 +82,3 @@ mod _impls { } } } - -#[cfg(test)] -mod tests; diff --git a/git-index/src/entry/tests.rs b/git-index/src/entry/tests.rs deleted file mode 100644 index 2247db22bf4..00000000000 --- a/git-index/src/entry/tests.rs +++ /dev/null @@ -1,12 +0,0 @@ -use crate::entry::at_rest; - -#[test] -fn in_mem_flags_to_storage_flags_v2() { - let flag_bytes = u16::from_be_bytes(*b"\x00\x01"); - let flags_at_rest = at_rest::Flags::from_bits(flag_bytes).unwrap(); - let in_memory_flags = flags_at_rest.to_memory(); - - let output = in_memory_flags.to_storage(); - - assert_eq!(output.bits(), flag_bytes); -} diff --git a/git-index/src/entry/write.rs b/git-index/src/entry/write.rs index 6594ae33df8..8a8b00bd987 100644 --- a/git-index/src/entry/write.rs +++ b/git-index/src/entry/write.rs @@ -25,6 +25,13 @@ impl Entry { .expect("we just checked that the length is smaller than 0xfff") }; out.write_all(&(self.flags.to_storage().bits() | path_len).to_be_bytes())?; + if self.flags.contains(entry::Flags::EXTENDED) { + out.write_all( + &entry::at_rest::FlagsExtended::from_flags(self.flags) + .bits() + .to_be_bytes(), + )?; + } out.write_all(path)?; out.write_all(b"\0") } diff --git a/git-index/src/write.rs b/git-index/src/write.rs index 663f150099e..94c7d7bdd48 100644 --- a/git-index/src/write.rs +++ b/git-index/src/write.rs @@ -82,10 +82,10 @@ impl State { extensions, }: Options, ) -> std::io::Result<()> { - assert_eq!( + assert_ne!( version, - Version::V2, - "can only write V2 at the moment, please come back later" + Version::V4, + "can only write V2/3 at the moment, please come back later" ); let mut write = CountBytes::new(out); @@ -137,15 +137,13 @@ fn header( version: Version, num_entries: u32, ) -> Result { - let signature = b"DIRC"; - let version = match version { Version::V2 => 2_u32.to_be_bytes(), Version::V3 => 3_u32.to_be_bytes(), Version::V4 => 4_u32.to_be_bytes(), }; - out.write_all(signature)?; + out.write_all(crate::decode::header::SIGNATURE)?; out.write_all(&version)?; out.write_all(&num_entries.to_be_bytes())?; diff --git a/git-index/tests/index/file/write.rs b/git-index/tests/index/file/write.rs index a53587a74c2..6a310edf2df 100644 --- a/git-index/tests/index/file/write.rs +++ b/git-index/tests/index/file/write.rs @@ -3,7 +3,6 @@ use filetime::FileTime; use git_index::verify::extensions::no_find; use git_index::write::Options; use git_index::{decode, extension, write, State, Version}; -use std::cmp::{max, min}; #[test] fn roundtrips() -> crate::Result { @@ -13,6 +12,7 @@ fn roundtrips() -> crate::Result { } use Kind::*; let input = [ + (Loose("extended-flags"), v3_all_ext_but_eoie()), (Loose("very-long-path"), v2_all_ext_but_eoie()), (Generated("v2"), write::Options::default()), (Generated("V2_empty"), write::Options::default()), @@ -155,8 +155,8 @@ fn compare_raw_bytes(generated: &[u8], expected: &[u8], fixture: &str) { let print_range = 10; for (index, (a, b)) in generated.iter().zip(expected.iter()).enumerate() { if a != b { - let range_left = max(index - print_range, 0); - let range_right = min(index + print_range, generated.len()); + let range_left = index.saturating_sub(print_range); + let range_right = (index + print_range).min(generated.len()); let generated = &generated[range_left..range_right]; let expected = &expected[range_left..range_right]; @@ -168,8 +168,9 @@ fn compare_raw_bytes(generated: &[u8], expected: &[u8], fixture: &str) { } } -fn v2_all_ext_but_eoie() -> Options { +fn all_ext_but_eoie(version: git_index::Version) -> Options { write::Options { + version, extensions: write::Extensions::Given { end_of_index_entry: false, tree_cache: true, @@ -177,3 +178,11 @@ fn v2_all_ext_but_eoie() -> Options { ..write::Options::default() } } + +fn v2_all_ext_but_eoie() -> Options { + all_ext_but_eoie(Version::V2) +} + +fn v3_all_ext_but_eoie() -> Options { + all_ext_but_eoie(Version::V3) +} From 6d810a135eeb71b8b04f7d9cb6c5f115587c2a63 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 14 Aug 2022 16:02:48 +0800 Subject: [PATCH 15/18] Assure that extended flags receive version 3; make `version` an implementation detail --- git-index/src/file/write.rs | 12 +++-- git-index/src/write.rs | 33 ++++++------- git-index/tests/index/file/write.rs | 77 ++++++++++++++++------------- 3 files changed, 66 insertions(+), 56 deletions(-) diff --git a/git-index/src/file/write.rs b/git-index/src/file/write.rs index 39467ce1ca7..04682a176fa 100644 --- a/git-index/src/file/write.rs +++ b/git-index/src/file/write.rs @@ -1,13 +1,15 @@ -use crate::{write, File}; +use crate::{write, File, Version}; use git_features::hash; impl File { - /// Write the index to `out` with `options`, to be readable by [`File::at()`]. - pub fn write_to(&self, mut out: &mut impl std::io::Write, options: write::Options) -> std::io::Result<()> { + /// Write the index to `out` with `options`, to be readable by [`File::at()`], returning the version that was actually written + /// to retain all information of this index. + pub fn write_to(&self, mut out: &mut impl std::io::Write, options: write::Options) -> std::io::Result { let mut hasher = hash::Write::new(&mut out, options.hash_kind); - self.state.write_to(&mut hasher, options)?; + let version = self.state.write_to(&mut hasher, options)?; let hash = hasher.hash.digest(); - out.write_all(&hash) + out.write_all(&hash)?; + Ok(version) } } diff --git a/git-index/src/write.rs b/git-index/src/write.rs index 94c7d7bdd48..775717cc70b 100644 --- a/git-index/src/write.rs +++ b/git-index/src/write.rs @@ -1,5 +1,5 @@ use crate::write::util::CountBytes; -use crate::{extension, State, Version}; +use crate::{entry, extension, State, Version}; use std::convert::TryInto; use std::io::Write; @@ -46,15 +46,13 @@ impl Extensions { /// The options for use when [writing an index][State::write_to()]. /// -/// Note that default options write +/// Note that default options write either index V2 or V3 depending on the content of the entries. #[derive(Debug, Clone, Copy)] pub struct Options { /// The hash kind to use when writing the index file. /// /// It is not always possible to infer the hash kind when reading an index, so this is required. pub hash_kind: git_hash::Kind, - /// The index version to write. Note that different versions affect the format and ultimately the size. - pub version: Version, /// Configures which extensions to write pub extensions: Extensions, @@ -64,8 +62,6 @@ impl Default for Options { fn default() -> Self { Self { hash_kind: git_hash::Kind::default(), - /// TODO: make this 'automatic' by default to determine the correct index version - not all versions can represent all in-memory states. - version: Version::V2, extensions: Default::default(), } } @@ -76,17 +72,9 @@ impl State { pub fn write_to( &self, out: &mut impl std::io::Write, - Options { - hash_kind, - version, - extensions, - }: Options, - ) -> std::io::Result<()> { - assert_ne!( - version, - Version::V4, - "can only write V2/3 at the moment, please come back later" - ); + Options { hash_kind, extensions }: Options, + ) -> std::io::Result { + let version = self.detect_required_version(); let mut write = CountBytes::new(out); let num_entries = self @@ -128,7 +116,16 @@ impl State { extension::end_of_index_entry::write_to(write.inner, hash_kind, offset_to_extensions, extension_toc)? } - Ok(()) + Ok(version) + } +} + +impl State { + fn detect_required_version(&self) -> Version { + self.entries + .iter() + .find_map(|e| e.flags.contains(entry::Flags::EXTENDED).then(|| Version::V3)) + .unwrap_or(Version::V2) } } diff --git a/git-index/tests/index/file/write.rs b/git-index/tests/index/file/write.rs index 6a310edf2df..9ef73444271 100644 --- a/git-index/tests/index/file/write.rs +++ b/git-index/tests/index/file/write.rs @@ -1,9 +1,11 @@ +use crate::fixture_index_path; use crate::index::file::read::loose_file_path; use filetime::FileTime; use git_index::verify::extensions::no_find; use git_index::write::Options; -use git_index::{decode, extension, write, State, Version}; +use git_index::{decode, entry, extension, write, State, Version}; +/// Round-trips should eventually be possible for all files we have, as we write them back exactly as they were read. #[test] fn roundtrips() -> crate::Result { enum Kind { @@ -12,31 +14,51 @@ fn roundtrips() -> crate::Result { } use Kind::*; let input = [ - (Loose("extended-flags"), v3_all_ext_but_eoie()), - (Loose("very-long-path"), v2_all_ext_but_eoie()), + (Loose("extended-flags"), all_ext_but_eoie()), + (Loose("conflicting-file"), all_ext_but_eoie()), + (Loose("very-long-path"), all_ext_but_eoie()), (Generated("v2"), write::Options::default()), (Generated("V2_empty"), write::Options::default()), - (Generated("v2_more_files"), v2_all_ext_but_eoie()), + (Generated("v2_more_files"), all_ext_but_eoie()), ]; for (fixture, options) in input { let (path, fixture) = match fixture { - Generated(name) => (crate::fixture_index_path(name), name), + Generated(name) => (fixture_index_path(name), name), Loose(name) => (loose_file_path(name), name), }; let expected = git_index::File::at(&path, decode::Options::default())?; let expected_bytes = std::fs::read(&path)?; let mut out_bytes = Vec::new(); - expected.write_to(&mut out_bytes, options)?; + let actual_version = expected.write_to(&mut out_bytes, options)?; + assert_eq!( + actual_version, + expected.version(), + "{} didn't write the expected version", + fixture + ); let (actual, _) = State::from_bytes(&out_bytes, FileTime::now(), decode::Options::default())?; - compare_states(&actual, &expected, options, fixture); + compare_states(&actual, actual_version, &expected, options, fixture); compare_raw_bytes(&out_bytes, &expected_bytes, fixture); } Ok(()) } +#[test] +fn extended_flags_automatically_upgrade_the_version_to_avoid_data_loss() -> crate::Result { + let mut expected = git_index::File::at(fixture_index_path("V2"), Default::default())?; + assert_eq!(expected.version(), Version::V2); + expected.entries_mut()[0].flags.insert(entry::Flags::EXTENDED); + + let mut buf = Vec::new(); + let actual_version = expected.write_to(&mut buf, Default::default())?; + assert_eq!(actual_version, Version::V3, "extended flags need V3"); + + Ok(()) +} + #[test] fn v2_index_no_extensions() { let input = [ @@ -48,20 +70,19 @@ fn v2_index_no_extensions() { ]; for fixture in input { - let path = crate::fixture_index_path(fixture); + let path = fixture_index_path(fixture); let expected = git_index::File::at(&path, decode::Options::default()).unwrap(); let mut out = Vec::::new(); let options = write::Options { hash_kind: git_hash::Kind::Sha1, - version: Version::V2, extensions: write::Extensions::None, }; - expected.write_to(&mut out, options).unwrap(); + let actual_version = expected.write_to(&mut out, options).unwrap(); - let (generated, _) = State::from_bytes(&out, FileTime::now(), decode::Options::default()).unwrap(); - compare_states(&generated, &expected, options, fixture); + let (actual, _) = State::from_bytes(&out, FileTime::now(), decode::Options::default()).unwrap(); + compare_states(&actual, actual_version, &expected, options, fixture); } } @@ -76,16 +97,16 @@ fn v2_index_tree_extensions() { ]; for fixture in input { - let path = crate::fixture_index_path(fixture); + let path = fixture_index_path(fixture); let expected = git_index::File::at(&path, decode::Options::default()).unwrap(); let mut out = Vec::::new(); - let options = v2_all_ext_but_eoie(); + let options = all_ext_but_eoie(); - expected.write_to(&mut out, options).unwrap(); + let actual_version = expected.write_to(&mut out, options).unwrap(); - let (generated, _) = State::from_bytes(&out, FileTime::now(), decode::Options::default()).unwrap(); - compare_states(&generated, &expected, options, fixture); + let (actual, _) = State::from_bytes(&out, FileTime::now(), decode::Options::default()).unwrap(); + compare_states(&actual, actual_version, &expected, options, fixture); } } @@ -100,31 +121,30 @@ fn v2_index_eoie_extensions() { ]; for fixture in input { - let path = crate::fixture_index_path(fixture); + let path = fixture_index_path(fixture); let expected = git_index::File::at(&path, decode::Options::default()).unwrap(); let mut out = Vec::::new(); let options = write::Options { hash_kind: git_hash::Kind::Sha1, - version: Version::V2, extensions: write::Extensions::Given { tree_cache: false, end_of_index_entry: true, }, }; - expected.write_to(&mut out, options).unwrap(); + let version = expected.write_to(&mut out, options).unwrap(); let (generated, _) = State::from_bytes(&out, FileTime::now(), decode::Options::default()).unwrap(); - compare_states(&generated, &expected, options, fixture); + compare_states(&generated, version, &expected, options, fixture); } } -fn compare_states(actual: &State, expected: &State, options: write::Options, fixture: &str) { +fn compare_states(actual: &State, actual_version: Version, expected: &State, options: write::Options, fixture: &str) { actual.verify_entries().expect("valid"); actual.verify_extensions(false, no_find).expect("valid"); - assert_eq!(actual.version(), options.version, "version mismatch in {}", fixture); + assert_eq!(actual.version(), actual_version, "version mismatch in {}", fixture); assert_eq!( actual.tree(), options @@ -168,9 +188,8 @@ fn compare_raw_bytes(generated: &[u8], expected: &[u8], fixture: &str) { } } -fn all_ext_but_eoie(version: git_index::Version) -> Options { +fn all_ext_but_eoie() -> Options { write::Options { - version, extensions: write::Extensions::Given { end_of_index_entry: false, tree_cache: true, @@ -178,11 +197,3 @@ fn all_ext_but_eoie(version: git_index::Version) -> Options { ..write::Options::default() } } - -fn v2_all_ext_but_eoie() -> Options { - all_ext_but_eoie(Version::V2) -} - -fn v3_all_ext_but_eoie() -> Options { - all_ext_but_eoie(Version::V3) -} From 933ad9e8ff0d58ad2590907cf84b43bc424e3219 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 14 Aug 2022 16:12:35 +0800 Subject: [PATCH 16/18] combine more tests into one to reduce duplication --- git-index/tests/index/file/write.rs | 119 +++++++++------------------- 1 file changed, 39 insertions(+), 80 deletions(-) diff --git a/git-index/tests/index/file/write.rs b/git-index/tests/index/file/write.rs index 9ef73444271..76a72f51312 100644 --- a/git-index/tests/index/file/write.rs +++ b/git-index/tests/index/file/write.rs @@ -17,8 +17,8 @@ fn roundtrips() -> crate::Result { (Loose("extended-flags"), all_ext_but_eoie()), (Loose("conflicting-file"), all_ext_but_eoie()), (Loose("very-long-path"), all_ext_but_eoie()), - (Generated("v2"), write::Options::default()), - (Generated("V2_empty"), write::Options::default()), + (Generated("v2"), Options::default()), + (Generated("V2_empty"), Options::default()), (Generated("v2_more_files"), all_ext_but_eoie()), ]; @@ -47,100 +47,59 @@ fn roundtrips() -> crate::Result { } #[test] -fn extended_flags_automatically_upgrade_the_version_to_avoid_data_loss() -> crate::Result { - let mut expected = git_index::File::at(fixture_index_path("V2"), Default::default())?; - assert_eq!(expected.version(), Version::V2); - expected.entries_mut()[0].flags.insert(entry::Flags::EXTENDED); - - let mut buf = Vec::new(); - let actual_version = expected.write_to(&mut buf, Default::default())?; - assert_eq!(actual_version, Version::V3, "extended flags need V3"); - - Ok(()) -} - -#[test] -fn v2_index_no_extensions() { - let input = [ - "V2_empty", - "v2", - "v2_more_files", - "v2_split_index", - "v4_more_files_IEOT", - ]; - - for fixture in input { - let path = fixture_index_path(fixture); - let expected = git_index::File::at(&path, decode::Options::default()).unwrap(); - - let mut out = Vec::::new(); - let options = write::Options { +fn state_comparisons_with_various_extension_configurations() { + fn options_with(extensions: write::Extensions) -> Options { + Options { hash_kind: git_hash::Kind::Sha1, - extensions: write::Extensions::None, - }; - - let actual_version = expected.write_to(&mut out, options).unwrap(); - - let (actual, _) = State::from_bytes(&out, FileTime::now(), decode::Options::default()).unwrap(); - compare_states(&actual, actual_version, &expected, options, fixture); + extensions, + } } -} -#[test] -fn v2_index_tree_extensions() { - let input = [ + for fixture in [ "V2_empty", "v2", "v2_more_files", "v2_split_index", "v4_more_files_IEOT", - ]; - - for fixture in input { - let path = fixture_index_path(fixture); - let expected = git_index::File::at(&path, decode::Options::default()).unwrap(); - - let mut out = Vec::::new(); - let options = all_ext_but_eoie(); + ] { + for options in [ + options_with(write::Extensions::None), + options_with(write::Extensions::All), + options_with(write::Extensions::Given { + tree_cache: true, + end_of_index_entry: true, + }), + options_with(write::Extensions::Given { + tree_cache: false, + end_of_index_entry: true, + }), + ] { + let path = fixture_index_path(fixture); + let expected = git_index::File::at(&path, Default::default()).unwrap(); - let actual_version = expected.write_to(&mut out, options).unwrap(); + let mut out = Vec::::new(); + let actual_version = expected.write_to(&mut out, options).unwrap(); - let (actual, _) = State::from_bytes(&out, FileTime::now(), decode::Options::default()).unwrap(); - compare_states(&actual, actual_version, &expected, options, fixture); + let (actual, _) = State::from_bytes(&out, FileTime::now(), decode::Options::default()).unwrap(); + compare_states(&actual, actual_version, &expected, options, fixture); + } } } #[test] -fn v2_index_eoie_extensions() { - let input = [ - "V2_empty", - "v2", - "v2_more_files", - "v2_split_index", - "v4_more_files_IEOT", - ]; - - for fixture in input { - let path = fixture_index_path(fixture); - let expected = git_index::File::at(&path, decode::Options::default()).unwrap(); - - let mut out = Vec::::new(); - let options = write::Options { - hash_kind: git_hash::Kind::Sha1, - extensions: write::Extensions::Given { - tree_cache: false, - end_of_index_entry: true, - }, - }; +fn extended_flags_automatically_upgrade_the_version_to_avoid_data_loss() -> crate::Result { + let mut expected = git_index::File::at(fixture_index_path("V2"), Default::default())?; + assert_eq!(expected.version(), Version::V2); + expected.entries_mut()[0].flags.insert(entry::Flags::EXTENDED); - let version = expected.write_to(&mut out, options).unwrap(); + let mut buf = Vec::new(); + let actual_version = expected.write_to(&mut buf, Default::default())?; + assert_eq!(actual_version, Version::V3, "extended flags need V3"); - let (generated, _) = State::from_bytes(&out, FileTime::now(), decode::Options::default()).unwrap(); - compare_states(&generated, version, &expected, options, fixture); - } + Ok(()) } -fn compare_states(actual: &State, actual_version: Version, expected: &State, options: write::Options, fixture: &str) { +fn compare_states(actual: &State, actual_version: Version, expected: &State, options: Options, fixture: &str) { actual.verify_entries().expect("valid"); actual.verify_extensions(false, no_find).expect("valid"); @@ -189,11 +148,11 @@ fn compare_raw_bytes(generated: &[u8], expected: &[u8], fixture: &str) { } fn all_ext_but_eoie() -> Options { - write::Options { + Options { extensions: write::Extensions::Given { end_of_index_entry: false, tree_cache: true, }, - ..write::Options::default() + ..Options::default() } } From de8abe6923b01563db812ba007ea65b7f193082d Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 14 Aug 2022 16:16:38 +0800 Subject: [PATCH 17/18] run tests against all input files we have --- git-index/tests/index/file/write.rs | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/git-index/tests/index/file/write.rs b/git-index/tests/index/file/write.rs index 76a72f51312..03e791cdcb5 100644 --- a/git-index/tests/index/file/write.rs +++ b/git-index/tests/index/file/write.rs @@ -55,12 +55,25 @@ fn state_comparisons_with_various_extension_configurations() { } } + enum Kind { + Generated(&'static str), + Loose(&'static str), + } + use Kind::*; + for fixture in [ - "V2_empty", - "v2", - "v2_more_files", - "v2_split_index", - "v4_more_files_IEOT", + Loose("extended-flags"), + Loose("conflicting-file"), + Loose("very-long-path"), + Loose("FSMN"), + Loose("REUC"), + Loose("UNTR-with-oids"), + Loose("UNTR"), + Generated("V2_empty"), + Generated("v2"), + Generated("v2_more_files"), + Generated("v2_split_index"), + Generated("v4_more_files_IEOT"), ] { for options in [ options_with(write::Extensions::None), @@ -74,7 +87,10 @@ fn state_comparisons_with_various_extension_configurations() { end_of_index_entry: true, }), ] { - let path = fixture_index_path(fixture); + let (path, fixture) = match fixture { + Generated(name) => (fixture_index_path(name), name), + Loose(name) => (loose_file_path(name), name), + }; let expected = git_index::File::at(&path, Default::default()).unwrap(); let mut out = Vec::::new(); From 4390c32f9ea0683561a78349456c87329fef3b41 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 14 Aug 2022 16:17:38 +0800 Subject: [PATCH 18/18] thanks clippy --- git-index/src/write.rs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/git-index/src/write.rs b/git-index/src/write.rs index 775717cc70b..6de3298d386 100644 --- a/git-index/src/write.rs +++ b/git-index/src/write.rs @@ -47,7 +47,7 @@ impl Extensions { /// The options for use when [writing an index][State::write_to()]. /// /// Note that default options write either index V2 or V3 depending on the content of the entries. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Default, Clone, Copy)] pub struct Options { /// The hash kind to use when writing the index file. /// @@ -58,15 +58,6 @@ pub struct Options { pub extensions: Extensions, } -impl Default for Options { - fn default() -> Self { - Self { - hash_kind: git_hash::Kind::default(), - extensions: Default::default(), - } - } -} - impl State { /// Serialize this instance to `out` with [`options`][Options]. pub fn write_to(