From 5ff7eaa86bddfa94aec97355a5d6adb117045693 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 29 Aug 2021 22:10:19 +0800 Subject: [PATCH 01/13] =?UTF-8?q?[repository=20#185]=20refactor=20reposito?= =?UTF-8?q?ry=20initialization=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …so that now it's easy to create a repository without `discover()`. --- Cargo.lock | 1 + git-repository/CHANGELOG.md | 12 +++++ git-repository/Cargo.toml | 1 + git-repository/src/init.rs | 29 ++++++------ git-repository/src/lib.rs | 22 ++++++++- git-repository/src/path/mod.rs | 14 +++--- git-repository/src/repository.rs | 77 +++++++++++++++++++++++--------- git-repository/tests/init/mod.rs | 25 +++++++++++ git-repository/tests/repo.rs | 1 + gitoxide-core/src/repository.rs | 4 +- src/porcelain/main.rs | 2 +- 11 files changed, 142 insertions(+), 46 deletions(-) create mode 100644 git-repository/tests/init/mod.rs diff --git a/Cargo.lock b/Cargo.lock index a86f1150e3d..f48dea99d2b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1222,6 +1222,7 @@ dependencies = [ "parking_lot", "quick-error", "signal-hook", + "tempfile", ] [[package]] diff --git a/git-repository/CHANGELOG.md b/git-repository/CHANGELOG.md index c354049a786..e8a5921ef4e 100644 --- a/git-repository/CHANGELOG.md +++ b/git-repository/CHANGELOG.md @@ -1,3 +1,15 @@ +### v0.9.0 (2021-08-??) + +#### New + +- `init()` +- `Repository::init()` + +#### Breaking +- **renames / moves** + - `path::Path` to `Path` + - `init::repository()` -> `init::into()` + ### v0.8.1 (2021-08-28) - Introduce `EasyArcExclusive` type, now available thanks to `parking_lot` 0.11.2 diff --git a/git-repository/Cargo.toml b/git-repository/Cargo.toml index 0357b890d5e..12d000f9bce 100644 --- a/git-repository/Cargo.toml +++ b/git-repository/Cargo.toml @@ -59,3 +59,4 @@ parking_lot = { version = "0.11.2", features = ["arc_lock"] } git-testtools = { path = "../tests/tools" } signal-hook = { version = "0.3.9", default-features = false } anyhow = "1" +tempfile = "3.2.0" diff --git a/git-repository/src/init.rs b/git-repository/src/init.rs index 17df6a001fe..079781c5d47 100644 --- a/git-repository/src/init.rs +++ b/git-repository/src/init.rs @@ -1,4 +1,3 @@ -#![allow(missing_docs)] use std::{ fs::{self, OpenOptions}, io::Write, @@ -8,7 +7,9 @@ use std::{ use quick_error::quick_error; quick_error! { + /// The error used in [`into()`]. #[derive(Debug)] + #[allow(missing_docs)] pub enum Error { IoOpen(err: std::io::Error, path: PathBuf) { display("Could not open data at '{}'", path.display()) @@ -95,22 +96,24 @@ fn create_dir(p: &Path) -> Result<(), Error> { fs::create_dir_all(p).map_err(|e| Error::CreateDirectory(e, p.to_owned())) } -pub fn repository(directory: impl Into) -> Result<(), Error> { - let mut cursor = directory.into(); - cursor.push(GIT_DIR_NAME); +/// Create a new `.git` repository within the possibly non-existing `directory` +/// and return its path. +pub fn into(directory: impl Into) -> Result { + let mut dot_git = directory.into(); + dot_git.push(GIT_DIR_NAME); - if cursor.is_dir() { - return Err(Error::DirectoryExists(cursor)); + if dot_git.is_dir() { + return Err(Error::DirectoryExists(dot_git)); } - create_dir(&cursor)?; + create_dir(&dot_git)?; { - let mut cursor = NewDir(&mut cursor).at("info")?; + let mut cursor = NewDir(&mut dot_git).at("info")?; write_file(TPL_INFO_EXCLUDE, PathCursor(cursor.as_mut()).at("exclude"))?; } { - let mut cursor = NewDir(&mut cursor).at("hooks")?; + let mut cursor = NewDir(&mut dot_git).at("hooks")?; for (tpl, filename) in &[ (TPL_HOOKS_UPDATE, "update.sample"), (TPL_HOOKS_PREPARE_COMMIT_MSG, "prepare-commit-msg.sample"), @@ -130,13 +133,13 @@ pub fn repository(directory: impl Into) -> Result<(), Error> { } { - let mut cursor = NewDir(&mut cursor).at("objects")?; + let mut cursor = NewDir(&mut dot_git).at("objects")?; create_dir(PathCursor(cursor.as_mut()).at("info"))?; create_dir(PathCursor(cursor.as_mut()).at("pack"))?; } { - let mut cursor = NewDir(&mut cursor).at("refs")?; + let mut cursor = NewDir(&mut dot_git).at("refs")?; create_dir(PathCursor(cursor.as_mut()).at("heads"))?; create_dir(PathCursor(cursor.as_mut()).at("tags"))?; } @@ -146,8 +149,8 @@ pub fn repository(directory: impl Into) -> Result<(), Error> { (TPL_DESCRIPTION, "description"), (TPL_CONFIG, "config"), ] { - write_file(tpl, PathCursor(&mut cursor).at(filename))?; + write_file(tpl, PathCursor(&mut dot_git).at(filename))?; } - Ok(()) + Ok(crate::Path::from_dot_git_dir(dot_git, crate::Kind::WorkTree)) } diff --git a/git-repository/src/lib.rs b/git-repository/src/lib.rs index 6ee64eac535..ab2fa1b1e63 100644 --- a/git-repository/src/lib.rs +++ b/git-repository/src/lib.rs @@ -46,6 +46,13 @@ //! extension traits can't apply internally if if it is implemented, but must be part of the external interface. This is only //! relevant for code within `git-repository` //! +//! ### Terminology +//! +//! #### WorkingTree and WorkTree +//! +//! When reading the documentation of the canonical git-worktree program one gets the impression work tree and working tree are used +//! interchangeably. We use the term _work tree_ only and try to do so consistently as its shorter and assumed to be the same. +//! //! # Cargo-features //! //! ## With the optional "unstable" cargo feature @@ -102,7 +109,6 @@ pub use git_tempfile as tempfile; pub use git_traverse as traverse; #[cfg(all(feature = "unstable", feature = "git-url"))] pub use git_url as url; -pub use path::Path; pub mod interrupt; @@ -123,6 +129,15 @@ pub mod path; /// pub mod repository; +/// A repository path which either points to a work tree or the `.git` repository itself. +#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] +pub enum Path { + /// The currently checked out or nascent work tree of a git repository. + WorkTree(PathBuf), + /// The git repository itself + Repository(PathBuf), +} + /// A instance with access to everything a git repository entails, best imagined as container for _most_ for system resources required /// to interact with a `git` repository which are loaded in once the instance is created. /// @@ -231,3 +246,8 @@ impl Kind { pub fn discover(directory: impl AsRef) -> Result { Repository::discover(directory) } + +/// See [Repository::init()]. +pub fn init(directory: impl AsRef) -> Result { + Repository::init(directory) +} diff --git a/git-repository/src/path/mod.rs b/git-repository/src/path/mod.rs index 8cc4e548f48..dbcf49307b5 100644 --- a/git-repository/src/path/mod.rs +++ b/git-repository/src/path/mod.rs @@ -1,18 +1,12 @@ #![allow(missing_docs)] use std::path::PathBuf; -use crate::Kind; +use crate::{Kind, Path}; pub mod discover; pub mod is_git; pub use is_git::{is_bare, is_git}; -#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] -pub enum Path { - WorkTree(PathBuf), - Repository(PathBuf), -} - impl AsRef for Path { fn as_ref(&self) -> &std::path::Path { match self { @@ -42,4 +36,10 @@ impl Path { Path::Repository(path) => path, } } + pub fn into_repository_and_work_tree_directories(self) -> (PathBuf, Option) { + match self { + crate::Path::WorkTree(working_tree) => (working_tree.join(".git"), Some(working_tree)), + crate::Path::Repository(repository) => (repository, None), + } + } } diff --git a/git-repository/src/repository.rs b/git-repository/src/repository.rs index 6290dcce58e..dd06fb25846 100644 --- a/git-repository/src/repository.rs +++ b/git-repository/src/repository.rs @@ -19,17 +19,64 @@ mod access { } } -mod init { +pub mod from_path { + use crate::Path; + use std::convert::TryFrom; + + pub type Error = git_odb::linked::init::Error; + + impl TryFrom for crate::Repository { + type Error = Error; + + fn try_from(value: Path) -> Result { + let (git_dir, working_tree) = value.into_repository_and_work_tree_directories(); + Ok(crate::Repository { + odb: git_odb::linked::Store::at(git_dir.join("objects"))?, + refs: git_ref::file::Store::at( + git_dir, + if working_tree.is_none() { + git_ref::file::WriteReflog::Disable + } else { + git_ref::file::WriteReflog::Normal + }, + ), + work_tree: working_tree, + }) + } + } +} + +pub mod init { + use quick_error::quick_error; use std::path::Path; + quick_error! { + #[derive(Debug)] + pub enum Error { + Init(err: crate::init::Error) { + display("Failed to initialize a new repository") + from() + source(err) + } + ObjectStoreInitialization(err: git_odb::linked::init::Error) { + display("Could not initialize the object database") + from() + source(err) + } + } + } + use crate::Repository; + use std::convert::TryInto; impl Repository { - /// Really just a sketch at this point to help guide the API. - pub fn create_and_init(directory: impl AsRef) -> Result { - // TODO: proper error - crate::init::repository(directory.as_ref())?; - Ok(Repository::discover(directory).unwrap()) // TODO: a specialized method without discovery + /// Create a repository with work-tree within `directory`, creating intermediate directories as needed. + /// + /// Fails without action if there is already a `.git` repository inside of `directory`, but + /// won't mind if the `directory` otherwise is non-empty. + pub fn init(directory: impl AsRef) -> Result { + let path = crate::init::into(directory.as_ref())?; + Ok(path.try_into()?) } } } @@ -40,6 +87,7 @@ pub mod discover { use quick_error::quick_error; use crate::{path::discover, Repository}; + use std::convert::TryInto; quick_error! { #[derive(Debug)] @@ -60,22 +108,7 @@ pub mod discover { impl Repository { pub fn discover(directory: impl AsRef) -> Result { let path = discover::existing(directory)?; - let (git_dir, working_tree) = match path { - crate::Path::WorkTree(working_tree) => (working_tree.join(".git"), Some(working_tree)), - crate::Path::Repository(repository) => (repository, None), - }; - Ok(Repository { - odb: git_odb::linked::Store::at(git_dir.join("objects"))?, - refs: git_ref::file::Store::at( - git_dir, - if working_tree.is_none() { - git_ref::file::WriteReflog::Disable - } else { - git_ref::file::WriteReflog::Normal - }, - ), - work_tree: working_tree, - }) + Ok(path.try_into()?) } } } diff --git a/git-repository/tests/init/mod.rs b/git-repository/tests/init/mod.rs new file mode 100644 index 00000000000..0a9aab96d16 --- /dev/null +++ b/git-repository/tests/init/mod.rs @@ -0,0 +1,25 @@ +#[test] +fn init_into_empty_directory_creates_a_dot_git_dir() -> crate::Result { + let tmp = tempfile::tempdir()?; + let repo = git_repository::init(tmp.path())?; + assert_eq!( + repo.work_tree.as_deref(), + Some(tmp.path()), + "there is a work tree by default" + ); + assert_eq!( + repo.git_dir(), + tmp.path().join(".git"), + "there is a work tree by default" + ); + Ok(()) +} + +#[test] +fn init_into_non_empty_directory_is_allowed() -> crate::Result { + let tmp = tempfile::tempdir()?; + std::fs::write(tmp.path().join("existing.txt"), b"I was here before you")?; + + git_repository::init(tmp.path())?; + Ok(()) +} diff --git a/git-repository/tests/repo.rs b/git-repository/tests/repo.rs index 7f2e054a436..de538206f33 100644 --- a/git-repository/tests/repo.rs +++ b/git-repository/tests/repo.rs @@ -9,5 +9,6 @@ fn repo(name: &str) -> crate::Result { mod access; mod discover; +mod init; mod object; mod reference; diff --git a/gitoxide-core/src/repository.rs b/gitoxide-core/src/repository.rs index fab4b72a53e..65332034939 100644 --- a/gitoxide-core/src/repository.rs +++ b/gitoxide-core/src/repository.rs @@ -2,6 +2,6 @@ use std::path::PathBuf; use anyhow::{Context as AnyhowContext, Result}; -pub fn init(directory: Option) -> Result<()> { - git_repository::init::repository(directory.unwrap_or_default()).with_context(|| "Repository initialization failed") +pub fn init(directory: Option) -> Result { + git_repository::init::into(directory.unwrap_or_default()).with_context(|| "Repository initialization failed") } diff --git a/src/porcelain/main.rs b/src/porcelain/main.rs index d93bffde562..d6799b1f0bd 100644 --- a/src/porcelain/main.rs +++ b/src/porcelain/main.rs @@ -34,7 +34,7 @@ pub fn main() -> Result<()> { crate::shared::STANDARD_RANGE, move |_progress, _out, _err| panic!("something went very wrong"), ), - Subcommands::Init { directory } => core::repository::init(directory), + Subcommands::Init { directory } => core::repository::init(directory).map(|_| ()), Subcommands::Tools(tool) => match tool { ToolCommands::EstimateHours(EstimateHours { working_dir, From 7604935b12eacb26a98bedc5f77636b5583629a5 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 29 Aug 2021 22:16:04 +0800 Subject: [PATCH 02/13] [repository #185] refactor --- git-pack/src/bundle/init.rs | 3 ++- git-pack/src/bundle/mod.rs | 3 ++- git-pack/src/cache/delta/from_offsets.rs | 3 +-- git-pack/src/cache/delta/traverse/mod.rs | 6 ++++-- git-pack/src/cache/delta/traverse/resolve.rs | 6 ++++-- git-pack/src/data/entry/mod.rs | 3 ++- git-pack/src/data/object.rs | 3 ++- git-pack/src/data/output/count/mod.rs | 4 ++-- git-pack/src/index/traverse/indexed.rs | 7 ++++--- git-pack/src/index/write/mod.rs | 6 ++++-- git-repository/src/repository.rs | 12 +++++++----- 11 files changed, 34 insertions(+), 22 deletions(-) diff --git a/git-pack/src/bundle/init.rs b/git-pack/src/bundle/init.rs index e6e627c15b3..4b60c82d6f0 100644 --- a/git-pack/src/bundle/init.rs +++ b/git-pack/src/bundle/init.rs @@ -1,9 +1,10 @@ -use crate::Bundle; use std::{ convert::TryFrom, path::{Path, PathBuf}, }; +use crate::Bundle; + /// Returned by [`Bundle::at()`] #[derive(thiserror::Error, Debug)] #[allow(missing_docs)] diff --git a/git-pack/src/bundle/mod.rs b/git-pack/src/bundle/mod.rs index 684b0b27392..12bd24f1500 100644 --- a/git-pack/src/bundle/mod.rs +++ b/git-pack/src/bundle/mod.rs @@ -29,9 +29,10 @@ pub mod write; mod verify { use std::sync::{atomic::AtomicBool, Arc}; - use crate::Bundle; use git_features::progress::Progress; + use crate::Bundle; + impl Bundle { /// Similar to [`crate::index::File::verify_integrity()`] but more convenient to call as the presence of the /// pack file is a given. diff --git a/git-pack/src/cache/delta/from_offsets.rs b/git-pack/src/cache/delta/from_offsets.rs index 21b6b6d7bdf..c78af36637f 100644 --- a/git-pack/src/cache/delta/from_offsets.rs +++ b/git-pack/src/cache/delta/from_offsets.rs @@ -8,8 +8,7 @@ use std::{ use git_features::progress::{self, Progress}; -use crate::cache::delta::Tree; -use crate::index::access::PackOffset; +use crate::{cache::delta::Tree, index::access::PackOffset}; /// Returned by [`Tree::from_offsets_in_pack()`] #[derive(thiserror::Error, Debug)] diff --git a/git-pack/src/cache/delta/traverse/mod.rs b/git-pack/src/cache/delta/traverse/mod.rs index 1c5db8d2cc5..d46e1809c67 100644 --- a/git-pack/src/cache/delta/traverse/mod.rs +++ b/git-pack/src/cache/delta/traverse/mod.rs @@ -9,8 +9,10 @@ use git_features::{ progress::{self, Progress}, }; -use crate::cache::delta::{Item, Tree}; -use crate::data::EntryRange; +use crate::{ + cache::delta::{Item, Tree}, + data::EntryRange, +}; mod resolve; diff --git a/git-pack/src/cache/delta/traverse/resolve.rs b/git-pack/src/cache/delta/traverse/resolve.rs index 79bba0f5724..4e5eadfd422 100644 --- a/git-pack/src/cache/delta/traverse/resolve.rs +++ b/git-pack/src/cache/delta/traverse/resolve.rs @@ -5,8 +5,10 @@ use git_features::{ zlib, }; -use crate::cache::delta::traverse::{Context, Error}; -use crate::data::EntryRange; +use crate::{ + cache::delta::traverse::{Context, Error}, + data::EntryRange, +}; pub(crate) fn deltas( nodes: crate::cache::delta::Chunk<'_, T>, diff --git a/git-pack/src/data/entry/mod.rs b/git-pack/src/data/entry/mod.rs index e5104dc44fb..c8ae8b793fe 100644 --- a/git-pack/src/data/entry/mod.rs +++ b/git-pack/src/data/entry/mod.rs @@ -1,6 +1,7 @@ -use crate::data::Entry; use git_hash::SIZE_OF_SHA1_DIGEST as SHA1_SIZE; +use crate::data::Entry; + const _TYPE_EXT1: u8 = 0; const COMMIT: u8 = 1; const TREE: u8 = 2; diff --git a/git-pack/src/data/object.rs b/git-pack/src/data/object.rs index e3a36afe8ed..580e3812501 100644 --- a/git-pack/src/data/object.rs +++ b/git-pack/src/data/object.rs @@ -1,8 +1,9 @@ //! Contains a borrowed Object bound to a buffer holding its decompressed data. -use crate::data::Object; use git_object::{BlobRef, CommitRef, CommitRefIter, ObjectRef, TagRef, TagRefIter, TreeRef, TreeRefIter}; +use crate::data::Object; + impl<'a> Object<'a> { /// Constructs a new data object from `kind` and `data`. pub fn new(kind: git_object::Kind, data: &'a [u8]) -> Object<'a> { diff --git a/git-pack/src/data/output/count/mod.rs b/git-pack/src/data/output/count/mod.rs index ab4adc59c61..345e13c01f6 100644 --- a/git-pack/src/data/output/count/mod.rs +++ b/git-pack/src/data/output/count/mod.rs @@ -1,7 +1,7 @@ -use crate::data; -use crate::data::output::Count; use git_hash::ObjectId; +use crate::{data, data::output::Count}; + /// Specifies how the pack location was handled during counting #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] #[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] diff --git a/git-pack/src/index/traverse/indexed.rs b/git-pack/src/index/traverse/indexed.rs index 8e438be57cd..50d761c93b9 100644 --- a/git-pack/src/index/traverse/indexed.rs +++ b/git-pack/src/index/traverse/indexed.rs @@ -8,10 +8,11 @@ use std::{ use git_features::{parallel, progress::Progress}; -use crate::cache::delta::traverse::Context; -use crate::index::{self, util::index_entries_sorted_by_offset_ascending}; - use super::{Error, SafetyCheck}; +use crate::{ + cache::delta::traverse::Context, + index::{self, util::index_entries_sorted_by_offset_ascending}, +}; /// Traversal with index impl index::File { diff --git a/git-pack/src/index/write/mod.rs b/git-pack/src/index/write/mod.rs index 2a1e07aa2ba..194d3dfc6bc 100644 --- a/git-pack/src/index/write/mod.rs +++ b/git-pack/src/index/write/mod.rs @@ -3,8 +3,10 @@ use std::{convert::TryInto, io, sync::atomic::AtomicBool}; pub use error::Error; use git_features::progress::{self, Progress}; -use crate::cache::delta::{traverse::Context, Tree}; -use crate::loose; +use crate::{ + cache::delta::{traverse::Context, Tree}, + loose, +}; mod encode; mod error; diff --git a/git-repository/src/repository.rs b/git-repository/src/repository.rs index dd06fb25846..9691f1f4d0b 100644 --- a/git-repository/src/repository.rs +++ b/git-repository/src/repository.rs @@ -20,9 +20,10 @@ mod access { } pub mod from_path { - use crate::Path; use std::convert::TryFrom; + use crate::Path; + pub type Error = git_odb::linked::init::Error; impl TryFrom for crate::Repository { @@ -47,9 +48,10 @@ pub mod from_path { } pub mod init { - use quick_error::quick_error; use std::path::Path; + use quick_error::quick_error; + quick_error! { #[derive(Debug)] pub enum Error { @@ -66,9 +68,10 @@ pub mod init { } } - use crate::Repository; use std::convert::TryInto; + use crate::Repository; + impl Repository { /// Create a repository with work-tree within `directory`, creating intermediate directories as needed. /// @@ -82,12 +85,11 @@ pub mod init { } pub mod discover { - use std::path::Path; + use std::{convert::TryInto, path::Path}; use quick_error::quick_error; use crate::{path::discover, Repository}; - use std::convert::TryInto; quick_error! { #[derive(Debug)] From 63089ff356ea0f62963ae213ea0dbb09f891ada6 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 29 Aug 2021 22:22:07 +0800 Subject: [PATCH 03/13] [repository #185] refactor --- git-repository/CHANGELOG.md | 2 +- git-repository/src/lib.rs | 8 ++--- .../src/{init.rs => path/create.rs} | 32 +++++++++---------- git-repository/src/path/mod.rs | 6 +++- git-repository/src/repository.rs | 4 +-- gitoxide-core/src/repository.rs | 3 +- 6 files changed, 29 insertions(+), 26 deletions(-) rename git-repository/src/{init.rs => path/create.rs} (71%) diff --git a/git-repository/CHANGELOG.md b/git-repository/CHANGELOG.md index e8a5921ef4e..629aa2b8449 100644 --- a/git-repository/CHANGELOG.md +++ b/git-repository/CHANGELOG.md @@ -8,7 +8,7 @@ #### Breaking - **renames / moves** - `path::Path` to `Path` - - `init::repository()` -> `init::into()` + - `init::repository()` -> `path::create::into()` ### v0.8.1 (2021-08-28) diff --git a/git-repository/src/lib.rs b/git-repository/src/lib.rs index ab2fa1b1e63..db0e323384f 100644 --- a/git-repository/src/lib.rs +++ b/git-repository/src/lib.rs @@ -121,13 +121,11 @@ pub mod prelude { pub use crate::{easy::ext::*, ext::*}; } -/// -pub mod init; - /// pub mod path; -/// -pub mod repository; + +mod repository; +pub use repository::{discover, from_path, init}; /// A repository path which either points to a work tree or the `.git` repository itself. #[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] diff --git a/git-repository/src/init.rs b/git-repository/src/path/create.rs similarity index 71% rename from git-repository/src/init.rs rename to git-repository/src/path/create.rs index 079781c5d47..8303c11f95f 100644 --- a/git-repository/src/init.rs +++ b/git-repository/src/path/create.rs @@ -31,22 +31,22 @@ quick_error! { const GIT_DIR_NAME: &str = ".git"; -const TPL_INFO_EXCLUDE: &[u8] = include_bytes!("./assets/baseline-init/info/exclude"); -const TPL_HOOKS_APPLYPATCH_MSG: &[u8] = include_bytes!("./assets/baseline-init/hooks/applypatch-msg.sample"); -const TPL_HOOKS_COMMIT_MSG: &[u8] = include_bytes!("./assets/baseline-init/hooks/commit-msg.sample"); -const TPL_HOOKS_FSMONITOR_WATCHMAN: &[u8] = include_bytes!("./assets/baseline-init/hooks/fsmonitor-watchman.sample"); -const TPL_HOOKS_POST_UPDATE: &[u8] = include_bytes!("./assets/baseline-init/hooks/post-update.sample"); -const TPL_HOOKS_PRE_APPLYPATCH: &[u8] = include_bytes!("./assets/baseline-init/hooks/pre-applypatch.sample"); -const TPL_HOOKS_PRE_COMMIT: &[u8] = include_bytes!("./assets/baseline-init/hooks/pre-commit.sample"); -const TPL_HOOKS_PRE_MERGE_COMMIT: &[u8] = include_bytes!("./assets/baseline-init/hooks/pre-merge-commit.sample"); -const TPL_HOOKS_PRE_PUSH: &[u8] = include_bytes!("./assets/baseline-init/hooks/pre-push.sample"); -const TPL_HOOKS_PRE_REBASE: &[u8] = include_bytes!("./assets/baseline-init/hooks/pre-rebase.sample"); -const TPL_HOOKS_PRE_RECEIVE: &[u8] = include_bytes!("./assets/baseline-init/hooks/pre-receive.sample"); -const TPL_HOOKS_PREPARE_COMMIT_MSG: &[u8] = include_bytes!("./assets/baseline-init/hooks/prepare-commit-msg.sample"); -const TPL_HOOKS_UPDATE: &[u8] = include_bytes!("./assets/baseline-init/hooks/update.sample"); -const TPL_CONFIG: &[u8] = include_bytes!("./assets/baseline-init/config"); -const TPL_DESCRIPTION: &[u8] = include_bytes!("./assets/baseline-init/description"); -const TPL_HEAD: &[u8] = include_bytes!("./assets/baseline-init/HEAD"); +const TPL_INFO_EXCLUDE: &[u8] = include_bytes!("../assets/baseline-init/info/exclude"); +const TPL_HOOKS_APPLYPATCH_MSG: &[u8] = include_bytes!("../assets/baseline-init/hooks/applypatch-msg.sample"); +const TPL_HOOKS_COMMIT_MSG: &[u8] = include_bytes!("../assets/baseline-init/hooks/commit-msg.sample"); +const TPL_HOOKS_FSMONITOR_WATCHMAN: &[u8] = include_bytes!("../assets/baseline-init/hooks/fsmonitor-watchman.sample"); +const TPL_HOOKS_POST_UPDATE: &[u8] = include_bytes!("../assets/baseline-init/hooks/post-update.sample"); +const TPL_HOOKS_PRE_APPLYPATCH: &[u8] = include_bytes!("../assets/baseline-init/hooks/pre-applypatch.sample"); +const TPL_HOOKS_PRE_COMMIT: &[u8] = include_bytes!("../assets/baseline-init/hooks/pre-commit.sample"); +const TPL_HOOKS_PRE_MERGE_COMMIT: &[u8] = include_bytes!("../assets/baseline-init/hooks/pre-merge-commit.sample"); +const TPL_HOOKS_PRE_PUSH: &[u8] = include_bytes!("../assets/baseline-init/hooks/pre-push.sample"); +const TPL_HOOKS_PRE_REBASE: &[u8] = include_bytes!("../assets/baseline-init/hooks/pre-rebase.sample"); +const TPL_HOOKS_PRE_RECEIVE: &[u8] = include_bytes!("../assets/baseline-init/hooks/pre-receive.sample"); +const TPL_HOOKS_PREPARE_COMMIT_MSG: &[u8] = include_bytes!("../assets/baseline-init/hooks/prepare-commit-msg.sample"); +const TPL_HOOKS_UPDATE: &[u8] = include_bytes!("../assets/baseline-init/hooks/update.sample"); +const TPL_CONFIG: &[u8] = include_bytes!("../assets/baseline-init/config"); +const TPL_DESCRIPTION: &[u8] = include_bytes!("../assets/baseline-init/description"); +const TPL_HEAD: &[u8] = include_bytes!("../assets/baseline-init/HEAD"); struct PathCursor<'a>(&'a mut PathBuf); diff --git a/git-repository/src/path/mod.rs b/git-repository/src/path/mod.rs index dbcf49307b5..be5bda4c01a 100644 --- a/git-repository/src/path/mod.rs +++ b/git-repository/src/path/mod.rs @@ -1,11 +1,15 @@ #![allow(missing_docs)] + use std::path::PathBuf; +pub use is_git::{is_bare, is_git}; + use crate::{Kind, Path}; +/// +pub mod create; pub mod discover; pub mod is_git; -pub use is_git::{is_bare, is_git}; impl AsRef for Path { fn as_ref(&self) -> &std::path::Path { diff --git a/git-repository/src/repository.rs b/git-repository/src/repository.rs index 9691f1f4d0b..79051772bb4 100644 --- a/git-repository/src/repository.rs +++ b/git-repository/src/repository.rs @@ -55,7 +55,7 @@ pub mod init { quick_error! { #[derive(Debug)] pub enum Error { - Init(err: crate::init::Error) { + Init(err: crate::path::create::Error) { display("Failed to initialize a new repository") from() source(err) @@ -78,7 +78,7 @@ pub mod init { /// Fails without action if there is already a `.git` repository inside of `directory`, but /// won't mind if the `directory` otherwise is non-empty. pub fn init(directory: impl AsRef) -> Result { - let path = crate::init::into(directory.as_ref())?; + let path = crate::path::create::into(directory.as_ref())?; Ok(path.try_into()?) } } diff --git a/gitoxide-core/src/repository.rs b/gitoxide-core/src/repository.rs index 65332034939..8c7cef2a572 100644 --- a/gitoxide-core/src/repository.rs +++ b/gitoxide-core/src/repository.rs @@ -3,5 +3,6 @@ use std::path::PathBuf; use anyhow::{Context as AnyhowContext, Result}; pub fn init(directory: Option) -> Result { - git_repository::init::into(directory.unwrap_or_default()).with_context(|| "Repository initialization failed") + git_repository::path::create::into(directory.unwrap_or_default()) + .with_context(|| "Repository initialization failed") } From 48207b54b97ac1b6354f6b53c13ccc4d1d8ea98f Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 30 Aug 2021 12:10:38 +0800 Subject: [PATCH 04/13] =?UTF-8?q?[repository=20#185]=20sketch=20of=20how?= =?UTF-8?q?=20to=20open=20a=20repository=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …but it doesn't work correctly just yet as it really needs git-config --- git-repository/CHANGELOG.md | 2 + git-repository/src/lib.rs | 5 +++ git-repository/src/path/is_git.rs | 3 +- git-repository/src/path/mod.rs | 6 --- git-repository/src/repository.rs | 60 ++++++++++++++++++++++++++++ git-repository/tests/discover/mod.rs | 2 +- git-repository/tests/init/mod.rs | 3 ++ 7 files changed, 73 insertions(+), 8 deletions(-) diff --git a/git-repository/CHANGELOG.md b/git-repository/CHANGELOG.md index 629aa2b8449..5845025aa03 100644 --- a/git-repository/CHANGELOG.md +++ b/git-repository/CHANGELOG.md @@ -4,6 +4,8 @@ - `init()` - `Repository::init()` +- `open()` +- `Repository::open()` #### Breaking - **renames / moves** diff --git a/git-repository/src/lib.rs b/git-repository/src/lib.rs index db0e323384f..dcfef1a9c23 100644 --- a/git-repository/src/lib.rs +++ b/git-repository/src/lib.rs @@ -249,3 +249,8 @@ pub fn discover(directory: impl AsRef) -> Result) -> Result { Repository::init(directory) } + +/// See [Repository::open()]. +pub fn open(directory: impl Into) -> Result { + Repository::open(directory) +} diff --git a/git-repository/src/path/is_git.rs b/git-repository/src/path/is_git.rs index 44b7bd43f22..b99aec70171 100644 --- a/git-repository/src/path/is_git.rs +++ b/git-repository/src/path/is_git.rs @@ -29,7 +29,8 @@ pub fn is_bare(git_dir: impl AsRef) -> bool { !git_dir.as_ref().join("index").exists() } -/// What constitutes a valid git repository, and what's yet to be implemented. +/// What constitutes a valid git repository, and what's yet to be implemented, returning the guessed repository kind +/// purely based on the presence of files. Note that the git-config ultimately decides what's bare. /// /// * [x] a valid head /// * [ ] git common directory diff --git a/git-repository/src/path/mod.rs b/git-repository/src/path/mod.rs index be5bda4c01a..0e90ed1d3a1 100644 --- a/git-repository/src/path/mod.rs +++ b/git-repository/src/path/mod.rs @@ -34,12 +34,6 @@ impl Path { } } - pub fn into_repository_directory(self) -> PathBuf { - match self { - Path::WorkTree(path) => path.join(".git"), - Path::Repository(path) => path, - } - } pub fn into_repository_and_work_tree_directories(self) -> (PathBuf, Option) { match self { crate::Path::WorkTree(working_tree) => (working_tree.join(".git"), Some(working_tree)), diff --git a/git-repository/src/repository.rs b/git-repository/src/repository.rs index 79051772bb4..bc01dae3590 100644 --- a/git-repository/src/repository.rs +++ b/git-repository/src/repository.rs @@ -29,6 +29,8 @@ pub mod from_path { impl TryFrom for crate::Repository { type Error = Error; + // TODO: Don't use this for cases where input paths are given to save on IOPs, there may be + // duplicate file checks here. fn try_from(value: Path) -> Result { let (git_dir, working_tree) = value.into_repository_and_work_tree_directories(); Ok(crate::Repository { @@ -47,6 +49,43 @@ pub mod from_path { } } +pub mod open { + use crate::Repository; + + use quick_error::quick_error; + use std::convert::TryInto; + + quick_error! { + #[derive(Debug)] + pub enum Error { + NotARepository(err: crate::path::is_git::Error) { + display("The provided path doesn't appear to be a git repository") + from() + source(err) + } + ObjectStoreInitialization(err: git_odb::linked::init::Error) { + display("Could not initialize the object database") + from() + source(err) + } + } + } + + impl Repository { + pub fn open(path: impl Into) -> Result { + let path = path.into(); + let (path, kind) = match crate::path::is_git(&path) { + Ok(kind) => (path, kind), + Err(_) => { + let git_dir = path.join(".git"); + crate::path::is_git(&git_dir).map(|kind| (git_dir, kind))? + } + }; + crate::Path::from_dot_git_dir(path, kind).try_into().map_err(Into::into) + } + } +} + pub mod init { use std::path::Path; @@ -114,3 +153,24 @@ pub mod discover { } } } + +mod impls { + use crate::Repository; + + impl std::fmt::Debug for Repository { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "Repository(git = '{}', working_tree: {:?}", + self.git_dir().display(), + self.work_tree + ) + } + } + + impl PartialEq for Repository { + fn eq(&self, other: &Repository) -> bool { + self.git_dir() == other.git_dir() && self.work_tree == other.work_tree + } + } +} diff --git a/git-repository/tests/discover/mod.rs b/git-repository/tests/discover/mod.rs index 91d567ceca8..8f434f8f6fd 100644 --- a/git-repository/tests/discover/mod.rs +++ b/git-repository/tests/discover/mod.rs @@ -32,7 +32,7 @@ mod existing { let path = git_repository::path::discover::existing(&dir)?; assert_eq!(path.kind(), Kind::WorkTree); assert_eq!( - path.into_repository_directory(), + path.into_repository_and_work_tree_directories().0, dir, "the .git dir is directly returned if valid" ); diff --git a/git-repository/tests/init/mod.rs b/git-repository/tests/init/mod.rs index 0a9aab96d16..6ec8900687a 100644 --- a/git-repository/tests/init/mod.rs +++ b/git-repository/tests/init/mod.rs @@ -1,4 +1,5 @@ #[test] +#[ignore] fn init_into_empty_directory_creates_a_dot_git_dir() -> crate::Result { let tmp = tempfile::tempdir()?; let repo = git_repository::init(tmp.path())?; @@ -12,6 +13,8 @@ fn init_into_empty_directory_creates_a_dot_git_dir() -> crate::Result { tmp.path().join(".git"), "there is a work tree by default" ); + assert_eq!(git_repository::open(repo.git_dir())?, repo); + git_repository::open(repo.work_tree.expect("non-bare repo"))?; Ok(()) } From 8a5aac55cf62bdd7287a363fa29f12aa39d4c583 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 30 Aug 2021 14:45:05 +0800 Subject: [PATCH 05/13] [repository #185] use git-config to handle bare repos more properly --- Cargo.lock | 1 + git-repository/Cargo.toml | 1 + git-repository/src/lib.rs | 9 +++-- git-repository/src/repository.rs | 68 +++++++++++++++++++++----------- git-repository/tests/init/mod.rs | 6 ++- 5 files changed, 55 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f48dea99d2b..8784c8fd241 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1205,6 +1205,7 @@ version = "0.8.1" dependencies = [ "anyhow", "git-actor", + "git-config", "git-diff", "git-features", "git-hash", diff --git a/git-repository/Cargo.toml b/git-repository/Cargo.toml index 12d000f9bce..28461453600 100644 --- a/git-repository/Cargo.toml +++ b/git-repository/Cargo.toml @@ -39,6 +39,7 @@ git-tempfile = { version ="^1.0.0", path = "../git-tempfile" } git-lock = { version ="^1.0.0", path = "../git-lock" } git-validate = { version = "^0.5.0", path = "../git-validate" } +git-config = { version ="^0.1.0", path = "../git-config" } git-odb = { version ="^0.21.0", path = "../git-odb" } git-hash = { version = "^0.5.0", path = "../git-hash" } git-object = { version ="^0.13.0", path = "../git-object" } diff --git a/git-repository/src/lib.rs b/git-repository/src/lib.rs index dcfef1a9c23..ec392063610 100644 --- a/git-repository/src/lib.rs +++ b/git-repository/src/lib.rs @@ -125,7 +125,7 @@ pub mod prelude { pub mod path; mod repository; -pub use repository::{discover, from_path, init}; +pub use repository::{discover, from_path, init, open}; /// A repository path which either points to a work tree or the `.git` repository itself. #[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] @@ -152,11 +152,12 @@ pub struct Repository { pub odb: git_odb::linked::Store, #[cfg(not(feature = "unstable"))] pub(crate) odb: git_odb::linked::Store, - /// TODO: git-config should be here - it's read a lot but not written much in must applications, so shouldn't be in `State`. - /// Probably it's best reload it on signal (in servers) or refresh it when it's known to have been changed similar to how - /// packs are refreshed. This would be `git_config::fs::Config` when ready. /// The path to the worktree at which to find checked out files pub work_tree: Option, + // TODO: git-config should be here - it's read a lot but not written much in must applications, so shouldn't be in `State`. + // Probably it's best reload it on signal (in servers) or refresh it when it's known to have been changed similar to how + // packs are refreshed. This would be `git_config::fs::Config` when ready. + // pub(crate) config: git_config::file::GitConfig<'static>, } /// A handle to a `Repository` for use when the repository needs to be shared, providing state for one `ObjectRef` at a time, , created with [`Repository::into_easy()`]. diff --git a/git-repository/src/repository.rs b/git-repository/src/repository.rs index bc01dae3590..0fffa3d145a 100644 --- a/git-repository/src/repository.rs +++ b/git-repository/src/repository.rs @@ -24,27 +24,12 @@ pub mod from_path { use crate::Path; - pub type Error = git_odb::linked::init::Error; - impl TryFrom for crate::Repository { - type Error = Error; + type Error = crate::open::Error; - // TODO: Don't use this for cases where input paths are given to save on IOPs, there may be - // duplicate file checks here. fn try_from(value: Path) -> Result { - let (git_dir, working_tree) = value.into_repository_and_work_tree_directories(); - Ok(crate::Repository { - odb: git_odb::linked::Store::at(git_dir.join("objects"))?, - refs: git_ref::file::Store::at( - git_dir, - if working_tree.is_none() { - git_ref::file::WriteReflog::Disable - } else { - git_ref::file::WriteReflog::Normal - }, - ), - work_tree: working_tree, - }) + let (git_dir, worktree_dir) = value.into_repository_and_work_tree_directories(); + crate::Repository::open_from_paths(git_dir, worktree_dir) } } } @@ -52,12 +37,18 @@ pub mod from_path { pub mod open { use crate::Repository; + use git_config::values::Boolean; use quick_error::quick_error; - use std::convert::TryInto; + use std::path::PathBuf; quick_error! { #[derive(Debug)] pub enum Error { + Config(err: git_config::parser::ParserOrIoError<'static>) { + display("The git configuration file could not be read") + from() + source(err) + } NotARepository(err: crate::path::is_git::Error) { display("The provided path doesn't appear to be a git repository") from() @@ -81,7 +72,36 @@ pub mod open { crate::path::is_git(&git_dir).map(|kind| (git_dir, kind))? } }; - crate::Path::from_dot_git_dir(path, kind).try_into().map_err(Into::into) + let (git_dir, worktree_dir) = + crate::Path::from_dot_git_dir(path, kind).into_repository_and_work_tree_directories(); + Repository::open_from_paths(git_dir, worktree_dir) + } + + pub(in crate::repository) fn open_from_paths( + git_dir: PathBuf, + mut worktree_dir: Option, + ) -> Result { + if worktree_dir.is_none() { + let config = git_config::file::GitConfig::open(git_dir.join("config"))?; + let is_bare = config + .value::>("core", None, "bare") + .map_or(false, |b| matches!(b, Boolean::True(_))); + if !is_bare { + worktree_dir = Some(git_dir.parent().expect("parent is always available").to_owned()); + } + } + Ok(crate::Repository { + odb: git_odb::linked::Store::at(git_dir.join("objects"))?, + refs: git_ref::file::Store::at( + git_dir, + if worktree_dir.is_none() { + git_ref::file::WriteReflog::Disable + } else { + git_ref::file::WriteReflog::Normal + }, + ), + work_tree: worktree_dir, + }) } } } @@ -99,8 +119,8 @@ pub mod init { from() source(err) } - ObjectStoreInitialization(err: git_odb::linked::init::Error) { - display("Could not initialize the object database") + Open(err: crate::open::Error) { + display("Could not open repository") from() source(err) } @@ -138,8 +158,8 @@ pub mod discover { from() source(err) } - ObjectStoreInitialization(err: git_odb::linked::init::Error) { - display("Could not initialize the object database") + Open(err: crate::open::Error) { + display("Could not open repository") from() source(err) } diff --git a/git-repository/tests/init/mod.rs b/git-repository/tests/init/mod.rs index 6ec8900687a..de7f99f7f27 100644 --- a/git-repository/tests/init/mod.rs +++ b/git-repository/tests/init/mod.rs @@ -1,5 +1,4 @@ #[test] -#[ignore] fn init_into_empty_directory_creates_a_dot_git_dir() -> crate::Result { let tmp = tempfile::tempdir()?; let repo = git_repository::init(tmp.path())?; @@ -14,7 +13,10 @@ fn init_into_empty_directory_creates_a_dot_git_dir() -> crate::Result { "there is a work tree by default" ); assert_eq!(git_repository::open(repo.git_dir())?, repo); - git_repository::open(repo.work_tree.expect("non-bare repo"))?; + assert_eq!( + git_repository::open(repo.work_tree.as_ref().expect("non-bare repo"))?, + repo + ); Ok(()) } From 9e8a39e3cbd620bd48f379743df0d5783c33a86f Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 30 Aug 2021 15:08:16 +0800 Subject: [PATCH 06/13] [repository #185] support for initializing bare repositories --- git-repository/CHANGELOG.md | 7 +- .../src/assets/baseline-init/config | 1 + git-repository/src/lib.rs | 7 +- git-repository/src/path/create.rs | 45 ++++++++-- git-repository/src/repository.rs | 4 +- git-repository/tests/init/mod.rs | 85 +++++++++++++------ gitoxide-core/src/repository.rs | 2 +- tests/fixtures/baseline-init/config | 2 - tests/journey/gix.sh | 2 + 9 files changed, 113 insertions(+), 42 deletions(-) delete mode 100644 tests/fixtures/baseline-init/config diff --git a/git-repository/CHANGELOG.md b/git-repository/CHANGELOG.md index 5845025aa03..94899973d62 100644 --- a/git-repository/CHANGELOG.md +++ b/git-repository/CHANGELOG.md @@ -3,14 +3,15 @@ #### New - `init()` -- `Repository::init()` +- `init_bare()` +- `Repository::init(Kind)` - `open()` - `Repository::open()` #### Breaking -- **renames / moves** +- **renames / moves / Signature Changes** - `path::Path` to `Path` - - `init::repository()` -> `path::create::into()` + - `init::repository(dir)` -> `path::create::into(dir, **Kind**)` ### v0.8.1 (2021-08-28) diff --git a/git-repository/src/assets/baseline-init/config b/git-repository/src/assets/baseline-init/config index 9557d76e2b2..739a1b0de26 100644 --- a/git-repository/src/assets/baseline-init/config +++ b/git-repository/src/assets/baseline-init/config @@ -1,2 +1,3 @@ [core] repositoryformatversion = 0 + bare = {bare-value} diff --git a/git-repository/src/lib.rs b/git-repository/src/lib.rs index ec392063610..2e7646399a7 100644 --- a/git-repository/src/lib.rs +++ b/git-repository/src/lib.rs @@ -248,7 +248,12 @@ pub fn discover(directory: impl AsRef) -> Result) -> Result { - Repository::init(directory) + Repository::init(directory, Kind::WorkTree) +} + +/// See [Repository::init()]. +pub fn init_bare(directory: impl AsRef) -> Result { + Repository::init(directory, Kind::Bare) } /// See [Repository::open()]. diff --git a/git-repository/src/path/create.rs b/git-repository/src/path/create.rs index 8303c11f95f..0e37155327d 100644 --- a/git-repository/src/path/create.rs +++ b/git-repository/src/path/create.rs @@ -4,6 +4,7 @@ use std::{ path::{Path, PathBuf}, }; +use git_object::bstr::ByteSlice; use quick_error::quick_error; quick_error! { @@ -22,6 +23,9 @@ quick_error! { DirectoryExists(path: PathBuf) { display("Refusing to initialize the existing '{}' directory", path.display()) } + DirectoryNotEmpty(path: PathBuf) { + display("Refusing to initialize the non-empty directory as '{}'", path.display()) + } CreateDirectory(err: std::io::Error, path: PathBuf) { display("Could not create directory at '{}'", path.display()) source(err) @@ -96,14 +100,28 @@ fn create_dir(p: &Path) -> Result<(), Error> { fs::create_dir_all(p).map_err(|e| Error::CreateDirectory(e, p.to_owned())) } -/// Create a new `.git` repository within the possibly non-existing `directory` +/// Create a new `.git` repository of `kind` within the possibly non-existing `directory` /// and return its path. -pub fn into(directory: impl Into) -> Result { +pub fn into(directory: impl Into, kind: crate::Kind) -> Result { let mut dot_git = directory.into(); - dot_git.push(GIT_DIR_NAME); - if dot_git.is_dir() { - return Err(Error::DirectoryExists(dot_git)); + match kind { + crate::Kind::Bare => { + if std::fs::read_dir(&dot_git) + .map_err(|err| Error::IoOpen(err, dot_git.clone()))? + .count() + != 0 + { + return Err(Error::DirectoryNotEmpty(dot_git)); + } + } + crate::Kind::WorkTree => { + dot_git.push(GIT_DIR_NAME); + + if dot_git.is_dir() { + return Err(Error::DirectoryExists(dot_git)); + } + } } create_dir(&dot_git)?; @@ -149,8 +167,21 @@ pub fn into(directory: impl Into) -> Result { (TPL_DESCRIPTION, "description"), (TPL_CONFIG, "config"), ] { - write_file(tpl, PathCursor(&mut dot_git).at(filename))?; + if *filename == "config" { + write_file( + &tpl.replace( + "{bare-value}", + match kind { + crate::Kind::Bare => "true", + crate::Kind::WorkTree => "false", + }, + ), + PathCursor(&mut dot_git).at(filename), + )?; + } else { + write_file(tpl, PathCursor(&mut dot_git).at(filename))?; + } } - Ok(crate::Path::from_dot_git_dir(dot_git, crate::Kind::WorkTree)) + Ok(crate::Path::from_dot_git_dir(dot_git, kind)) } diff --git a/git-repository/src/repository.rs b/git-repository/src/repository.rs index 0fffa3d145a..8329dd728f4 100644 --- a/git-repository/src/repository.rs +++ b/git-repository/src/repository.rs @@ -136,8 +136,8 @@ pub mod init { /// /// Fails without action if there is already a `.git` repository inside of `directory`, but /// won't mind if the `directory` otherwise is non-empty. - pub fn init(directory: impl AsRef) -> Result { - let path = crate::path::create::into(directory.as_ref())?; + pub fn init(directory: impl AsRef, kind: crate::Kind) -> Result { + let path = crate::path::create::into(directory.as_ref(), kind)?; Ok(path.try_into()?) } } diff --git a/git-repository/tests/init/mod.rs b/git-repository/tests/init/mod.rs index de7f99f7f27..8a5020d8da2 100644 --- a/git-repository/tests/init/mod.rs +++ b/git-repository/tests/init/mod.rs @@ -1,30 +1,63 @@ -#[test] -fn init_into_empty_directory_creates_a_dot_git_dir() -> crate::Result { - let tmp = tempfile::tempdir()?; - let repo = git_repository::init(tmp.path())?; - assert_eq!( - repo.work_tree.as_deref(), - Some(tmp.path()), - "there is a work tree by default" - ); - assert_eq!( - repo.git_dir(), - tmp.path().join(".git"), - "there is a work tree by default" - ); - assert_eq!(git_repository::open(repo.git_dir())?, repo); - assert_eq!( - git_repository::open(repo.work_tree.as_ref().expect("non-bare repo"))?, - repo - ); - Ok(()) +mod bare { + #[test] + fn init_into_empty_directory_creates_a_dot_git_dir() { + let tmp = tempfile::tempdir().unwrap(); + let repo = git_repository::init_bare(tmp.path()).unwrap(); + assert_eq!(repo.kind(), git_repository::Kind::Bare); + assert!( + repo.work_tree.is_none(), + "a worktree isn't present in bare repositories" + ); + assert_eq!( + repo.git_dir(), + tmp.path(), + "the repository is placed into the directory itself" + ); + assert_eq!(git_repository::open(repo.git_dir()).unwrap(), repo); + } + + #[test] + fn init_into_non_empty_directory_is_not_allowed() { + let tmp = tempfile::tempdir().unwrap(); + std::fs::write(tmp.path().join("existing.txt"), b"I was here before you").unwrap(); + + assert_eq!( + git_repository::init_bare(tmp.path()).unwrap_err().to_string(), + "Failed to initialize a new repository" + ); + } } -#[test] -fn init_into_non_empty_directory_is_allowed() -> crate::Result { - let tmp = tempfile::tempdir()?; - std::fs::write(tmp.path().join("existing.txt"), b"I was here before you")?; +mod non_bare { + #[test] + fn init_into_empty_directory_creates_a_dot_git_dir() -> crate::Result { + let tmp = tempfile::tempdir()?; + let repo = git_repository::init(tmp.path())?; + assert_eq!(repo.kind(), git_repository::Kind::WorkTree); + assert_eq!( + repo.work_tree.as_deref(), + Some(tmp.path()), + "there is a work tree by default" + ); + assert_eq!( + repo.git_dir(), + tmp.path().join(".git"), + "there is a work tree by default" + ); + assert_eq!(git_repository::open(repo.git_dir())?, repo); + assert_eq!( + git_repository::open(repo.work_tree.as_ref().expect("non-bare repo"))?, + repo + ); + Ok(()) + } + + #[test] + fn init_into_non_empty_directory_is_allowed() -> crate::Result { + let tmp = tempfile::tempdir()?; + std::fs::write(tmp.path().join("existing.txt"), b"I was here before you")?; - git_repository::init(tmp.path())?; - Ok(()) + git_repository::init(tmp.path())?; + Ok(()) + } } diff --git a/gitoxide-core/src/repository.rs b/gitoxide-core/src/repository.rs index 8c7cef2a572..c1921f1d4fc 100644 --- a/gitoxide-core/src/repository.rs +++ b/gitoxide-core/src/repository.rs @@ -3,6 +3,6 @@ use std::path::PathBuf; use anyhow::{Context as AnyhowContext, Result}; pub fn init(directory: Option) -> Result { - git_repository::path::create::into(directory.unwrap_or_default()) + git_repository::path::create::into(directory.unwrap_or_default(), git_repository::Kind::WorkTree) .with_context(|| "Repository initialization failed") } diff --git a/tests/fixtures/baseline-init/config b/tests/fixtures/baseline-init/config deleted file mode 100644 index 9557d76e2b2..00000000000 --- a/tests/fixtures/baseline-init/config +++ /dev/null @@ -1,2 +0,0 @@ -[core] - repositoryformatversion = 0 diff --git a/tests/journey/gix.sh b/tests/journey/gix.sh index 49ea308826e..b5edd4644d6 100644 --- a/tests/journey/gix.sh +++ b/tests/journey/gix.sh @@ -145,6 +145,7 @@ title "Porcelain ${kind}" } it "matches the output of baseline git init" && { + rm .git/config # this one is altered, ignore expect_snapshot "$fixtures/baseline-init" .git } @@ -167,6 +168,7 @@ title "Porcelain ${kind}" } it "matches the output of baseline git init" && { + rm $DIR/.git/config # this one is altered, ignore expect_snapshot "$fixtures/baseline-init" $DIR/.git } From 9b9ffa3c1d5ccbea22aa38b740daa8a349494395 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 30 Aug 2021 15:37:41 +0800 Subject: [PATCH 07/13] [config #185] flyby refactor --- git-config/src/parser.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/git-config/src/parser.rs b/git-config/src/parser.rs index a88a40f4949..851c21a43b6 100644 --- a/git-config/src/parser.rs +++ b/git-config/src/parser.rs @@ -834,9 +834,7 @@ impl<'a> Parser<'a> { /// frontmatter #[inline] pub fn take_frontmatter(&mut self) -> Vec> { - let mut to_return = vec![]; - std::mem::swap(&mut self.frontmatter, &mut to_return); - to_return + std::mem::take(&mut self.frontmatter) } /// Returns the parsed sections from the parser. Consider From 6ecd431661e7ddc2f97e5a78a7932d2a7f1f27f0 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 30 Aug 2021 15:54:24 +0800 Subject: [PATCH 08/13] [repository #185] on the way to removing quick-error --- Cargo.lock | 1 + git-repository/Cargo.toml | 1 + git-repository/src/easy/borrow.rs | 19 +++----- git-repository/src/easy/object/mod.rs | 30 ++++-------- git-repository/src/easy/reference.rs | 68 ++++++++------------------- git-repository/src/path/create.rs | 65 ++++++++++++------------- git-repository/src/path/discover.rs | 33 ++++++------- git-repository/src/path/is_git.rs | 38 ++++++--------- git-repository/src/repository.rs | 28 ++++------- 9 files changed, 111 insertions(+), 172 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8784c8fd241..0bd2b8d705b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1224,6 +1224,7 @@ dependencies = [ "quick-error", "signal-hook", "tempfile", + "thiserror", ] [[package]] diff --git a/git-repository/Cargo.toml b/git-repository/Cargo.toml index 28461453600..e1609ef7348 100644 --- a/git-repository/Cargo.toml +++ b/git-repository/Cargo.toml @@ -54,6 +54,7 @@ git-features = { version = "^0.16.0", path = "../git-features", features = ["pro signal-hook = { version = "0.3.9", default-features = false } quick-error = "2.0.0" +thiserror = "1.0.26" parking_lot = { version = "0.11.2", features = ["arc_lock"] } [dev-dependencies] diff --git a/git-repository/src/easy/borrow.rs b/git-repository/src/easy/borrow.rs index 5a7bb8d5b35..05ff5157484 100644 --- a/git-repository/src/easy/borrow.rs +++ b/git-repository/src/easy/borrow.rs @@ -1,18 +1,11 @@ #![allow(missing_docs)] pub mod state { - use quick_error::quick_error; - quick_error! { - #[derive(Debug)] - pub enum Error { - Borrow(err: std::cell::BorrowError) { - display("A state member could not be borrowed") - from() - } - BorrowMut(err: std::cell::BorrowMutError) { - display("A state member could not be mutably borrowed") - from() - } - } + #[derive(Debug, thiserror::Error)] + pub enum Error { + #[error("A state member could not be borrowed")] + Borrow(#[from] std::cell::BorrowError), + #[error("A state member could not be mutably borrowed")] + BorrowMut(#[from] std::cell::BorrowMutError), } pub type Result = std::result::Result; diff --git a/git-repository/src/easy/object/mod.rs b/git-repository/src/easy/object/mod.rs index 966069df23c..455cc3ff30f 100644 --- a/git-repository/src/easy/object/mod.rs +++ b/git-repository/src/easy/object/mod.rs @@ -60,31 +60,21 @@ where pub mod find { use git_odb as odb; - use quick_error::quick_error; use crate::easy; - quick_error! { - #[derive(Debug)] - pub enum Error { - Find(err: OdbError) { - display("An error occurred while trying to find an object") - from() - source(err) - } - BorrowState(err: easy::borrow::state::Error) { - display("BUG: Part of interior state could not be borrowed.") - from() - source(err) - } - BorrowRepo(err: easy::borrow::repo::Error) { - display("BUG: The repository could not be borrowed") - from() - } - } - } pub(crate) type OdbError = odb::compound::find::Error; + #[derive(Debug, thiserror::Error)] + pub enum Error { + #[error(transparent)] + Find(#[from] OdbError), + #[error("BUG: Part of interior state could not be borrowed.")] + BorrowState(#[from] easy::borrow::state::Error), + #[error("BUG: The repository could not be borrowed")] + BorrowRepo(#[from] easy::borrow::repo::Error), + } + pub mod existing { use git_odb as odb; use quick_error::quick_error; diff --git a/git-repository/src/easy/reference.rs b/git-repository/src/easy/reference.rs index cdaec73bd19..cfe67512bbc 100644 --- a/git-repository/src/easy/reference.rs +++ b/git-repository/src/easy/reference.rs @@ -25,65 +25,37 @@ pub(crate) enum Backing { pub mod edit { use git_ref as refs; - use quick_error::quick_error; use crate::easy; - quick_error! { - #[derive(Debug)] - pub enum Error { - FileTransactionPrepare(err: refs::file::transaction::prepare::Error) { - display("Could not prepare the file transaction") - from() - source(err) - } - FileTransactionCommit(err: refs::file::transaction::commit::Error) { - display("Could not commit the file transaction") - from() - source(err) - } - NameValidation(err: git_validate::reference::name::Error) { - display("The reference name is invalid") - from() - source(err) - } - BorrowRepo(err: easy::borrow::repo::Error) { - display("BUG: The repository could not be borrowed") - from() - } - } + #[derive(Debug, thiserror::Error)] + pub enum Error { + #[error(transparent)] + FileTransactionPrepare(#[from] refs::file::transaction::prepare::Error), + #[error(transparent)] + FileTransactionCommit(#[from] refs::file::transaction::commit::Error), + #[error(transparent)] + NameValidation(#[from] git_validate::reference::name::Error), + #[error("BUG: The repository could not be borrowed")] + BorrowRepo(#[from] easy::borrow::repo::Error), } } pub mod peel_to_oid_in_place { use git_ref as refs; - use quick_error::quick_error; use crate::easy; - quick_error! { - #[derive(Debug)] - pub enum Error { - LoosePeelToId(err: refs::file::loose::reference::peel::to_id::Error) { - display("Could not peel loose reference") - from() - source(err) - } - PackedRefsOpen(err: refs::packed::buffer::open::Error) { - display("The packed-refs file could not be opened") - from() - source(err) - } - BorrowState(err: easy::borrow::state::Error) { - display("BUG: Part of interior state could not be borrowed.") - from() - source(err) - } - BorrowRepo(err: easy::borrow::repo::Error) { - display("BUG: The repository could not be borrowed") - from() - } - } + #[derive(Debug, thiserror::Error)] + pub enum Error { + #[error(transparent)] + LoosePeelToId(#[from] refs::file::loose::reference::peel::to_id::Error), + #[error(transparent)] + PackedRefsOpen(#[from] refs::packed::buffer::open::Error), + #[error("BUG: Part of interior state could not be borrowed.")] + BorrowState(#[from] easy::borrow::state::Error), + #[error("BUG: The repository could not be borrowed")] + BorrowRepo(#[from] easy::borrow::repo::Error), } } diff --git a/git-repository/src/path/create.rs b/git-repository/src/path/create.rs index 0e37155327d..2aa5f6e4d98 100644 --- a/git-repository/src/path/create.rs +++ b/git-repository/src/path/create.rs @@ -5,32 +5,21 @@ use std::{ }; use git_object::bstr::ByteSlice; -use quick_error::quick_error; - -quick_error! { - /// The error used in [`into()`]. - #[derive(Debug)] - #[allow(missing_docs)] - pub enum Error { - IoOpen(err: std::io::Error, path: PathBuf) { - display("Could not open data at '{}'", path.display()) - source(err) - } - IoWrite(err: std::io::Error, path: PathBuf) { - display("Could not write data at '{}'", path.display()) - source(err) - } - DirectoryExists(path: PathBuf) { - display("Refusing to initialize the existing '{}' directory", path.display()) - } - DirectoryNotEmpty(path: PathBuf) { - display("Refusing to initialize the non-empty directory as '{}'", path.display()) - } - CreateDirectory(err: std::io::Error, path: PathBuf) { - display("Could not create directory at '{}'", path.display()) - source(err) - } - } + +/// The error used in [`into()`]. +#[derive(Debug, thiserror::Error)] +#[allow(missing_docs)] +pub enum Error { + #[error("Could not open data at '{}'", .path.display())] + IoOpen { source: std::io::Error, path: PathBuf }, + #[error("Could not write data at '{}'", .path.display())] + IoWrite { source: std::io::Error, path: PathBuf }, + #[error("Refusing to initialize the existing '{}' directory", .path.display())] + DirectoryExists { path: PathBuf }, + #[error("Refusing to initialize the non-empty directory as '{}'", .path.display())] + DirectoryNotEmpty { path: PathBuf }, + #[error("Could not create directory at '{}'", .path.display())] + CreateDirectory { source: std::io::Error, path: PathBuf }, } const GIT_DIR_NAME: &str = ".git"; @@ -92,12 +81,21 @@ fn write_file(data: &[u8], path: &Path) -> Result<(), Error> { .create(true) .append(false) .open(path) - .map_err(|e| Error::IoOpen(e, path.to_owned()))?; - file.write_all(data).map_err(|e| Error::IoWrite(e, path.to_owned())) + .map_err(|e| Error::IoOpen { + source: e, + path: path.to_owned(), + })?; + file.write_all(data).map_err(|e| Error::IoWrite { + source: e, + path: path.to_owned(), + }) } fn create_dir(p: &Path) -> Result<(), Error> { - fs::create_dir_all(p).map_err(|e| Error::CreateDirectory(e, p.to_owned())) + fs::create_dir_all(p).map_err(|e| Error::CreateDirectory { + source: e, + path: p.to_owned(), + }) } /// Create a new `.git` repository of `kind` within the possibly non-existing `directory` @@ -108,18 +106,21 @@ pub fn into(directory: impl Into, kind: crate::Kind) -> Result { if std::fs::read_dir(&dot_git) - .map_err(|err| Error::IoOpen(err, dot_git.clone()))? + .map_err(|err| Error::IoOpen { + source: err, + path: dot_git.clone(), + })? .count() != 0 { - return Err(Error::DirectoryNotEmpty(dot_git)); + return Err(Error::DirectoryNotEmpty { path: dot_git }); } } crate::Kind::WorkTree => { dot_git.push(GIT_DIR_NAME); if dot_git.is_dir() { - return Err(Error::DirectoryExists(dot_git)); + return Err(Error::DirectoryExists { path: dot_git }); } } } diff --git a/git-repository/src/path/discover.rs b/git-repository/src/path/discover.rs index 75ec002cefe..4a6f35a42f8 100644 --- a/git-repository/src/path/discover.rs +++ b/git-repository/src/path/discover.rs @@ -8,18 +8,12 @@ use crate::path; pub mod existing { use std::path::PathBuf; - use quick_error::quick_error; - - quick_error! { - #[derive(Debug)] - pub enum Error { - InaccessibleDirectory(path: PathBuf) { - display("Failed to access a directory, or path is not a direectory") - } - NoGitRepository(path: PathBuf) { - display("Could find a git repository in '{}' or in any of its parents", path.display()) - } - } + #[derive(Debug, thiserror::Error)] + pub enum Error { + #[error("Failed to access a directory, or path is not a directory: '{}'", .path.display())] + InaccessibleDirectory { path: PathBuf }, + #[error("Could find a git repository in '{}' or in any of its parents", .path.display())] + NoGitRepository { path: PathBuf }, } } @@ -30,10 +24,13 @@ pub fn existing(directory: impl AsRef) -> Result) -> Result cursor = parent, - None => break Err(existing::Error::NoGitRepository(directory.into_owned())), + None => { + break Err(existing::Error::NoGitRepository { + path: directory.into_owned(), + }) + } } } } diff --git a/git-repository/src/path/is_git.rs b/git-repository/src/path/is_git.rs index b99aec70171..d643587c1f1 100644 --- a/git-repository/src/path/is_git.rs +++ b/git-repository/src/path/is_git.rs @@ -1,25 +1,15 @@ use std::path::{Path, PathBuf}; -use quick_error::quick_error; - -quick_error! { - #[derive(Debug)] - pub enum Error { - FindHeadRef(err: git_ref::file::find::existing::Error) { - display("Could not find a valid HEAD reference") - from() - source(err) - } - MisplacedHead(name: git_object::bstr::BString) { - display("Expected HEAD at '.git/HEAD', got '.git/{}'", name) - } - MissingObjectsDirectory(missing: PathBuf) { - display("Expected an objects directory at '{}'", missing.display()) - } - MissingRefsDirectory(missing: PathBuf) { - display("Expected a refs directory at '{}'", missing.display()) - } - } +#[derive(Debug, thiserror::Error)] +pub enum Error { + #[error("Could not find a valid HEAD reference")] + FindHeadRef(#[from] git_ref::file::find::existing::Error), + #[error("Expected HEAD at '.git/HEAD', got '.git/{}'", .name)] + MisplacedHead { name: git_object::bstr::BString }, + #[error("Expected an objects directory at '{}'", .missing.display())] + MissingObjectsDirectory { missing: PathBuf }, + #[error("Expected a refs directory at '{}'", .missing.display())] + MissingRefsDirectory { missing: PathBuf }, } /// Returns true if the given `git_dir` seems to be a bare repository. @@ -45,7 +35,9 @@ pub fn is_git(git_dir: impl AsRef) -> Result { let refs = git_ref::file::Store::at(&dot_git, Default::default()); let head = refs.find_loose("HEAD")?; if head.name.as_bstr() != "HEAD" { - return Err(Error::MisplacedHead(head.name.into_inner())); + return Err(Error::MisplacedHead { + name: head.name.into_inner(), + }); } } @@ -54,13 +46,13 @@ pub fn is_git(git_dir: impl AsRef) -> Result { .map(PathBuf::from) .unwrap_or_else(|_| dot_git.join("objects")); if !objects_path.is_dir() { - return Err(Error::MissingObjectsDirectory(objects_path)); + return Err(Error::MissingObjectsDirectory { missing: objects_path }); } } { let refs_path = dot_git.join("refs"); if !refs_path.is_dir() { - return Err(Error::MissingRefsDirectory(refs_path)); + return Err(Error::MissingRefsDirectory { missing: refs_path }); } } diff --git a/git-repository/src/repository.rs b/git-repository/src/repository.rs index 8329dd728f4..23e6d37b99e 100644 --- a/git-repository/src/repository.rs +++ b/git-repository/src/repository.rs @@ -38,28 +38,16 @@ pub mod open { use crate::Repository; use git_config::values::Boolean; - use quick_error::quick_error; use std::path::PathBuf; - quick_error! { - #[derive(Debug)] - pub enum Error { - Config(err: git_config::parser::ParserOrIoError<'static>) { - display("The git configuration file could not be read") - from() - source(err) - } - NotARepository(err: crate::path::is_git::Error) { - display("The provided path doesn't appear to be a git repository") - from() - source(err) - } - ObjectStoreInitialization(err: git_odb::linked::init::Error) { - display("Could not initialize the object database") - from() - source(err) - } - } + #[derive(Debug, thiserror::Error)] + pub enum Error { + #[error(transparent)] + Config(#[from] git_config::parser::ParserOrIoError<'static>), + #[error(transparent)] + NotARepository(#[from] crate::path::is_git::Error), + #[error(transparent)] + ObjectStoreInitialization(#[from] git_odb::linked::init::Error), } impl Repository { From 212c44c84b903681f6d35d934ee5f7ad6e1da791 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 30 Aug 2021 19:37:23 +0800 Subject: [PATCH 09/13] [repository #185] remove quick-error infavor of thiserror --- Cargo.lock | 1 - git-repository/Cargo.toml | 1 - git-repository/src/easy/object/mod.rs | 53 ++++++++++----------------- git-repository/src/easy/reference.rs | 53 ++++++++------------------- git-repository/src/repository.rs | 53 ++++++++------------------- git-repository/tests/init/mod.rs | 8 ++-- 6 files changed, 54 insertions(+), 115 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0bd2b8d705b..568f8eec179 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1221,7 +1221,6 @@ dependencies = [ "git-url", "git-validate", "parking_lot", - "quick-error", "signal-hook", "tempfile", "thiserror", diff --git a/git-repository/Cargo.toml b/git-repository/Cargo.toml index e1609ef7348..efa6d4d6de1 100644 --- a/git-repository/Cargo.toml +++ b/git-repository/Cargo.toml @@ -53,7 +53,6 @@ git-diff = { version ="^0.9.0", path = "../git-diff", optional = true } git-features = { version = "^0.16.0", path = "../git-features", features = ["progress"] } signal-hook = { version = "0.3.9", default-features = false } -quick-error = "2.0.0" thiserror = "1.0.26" parking_lot = { version = "0.11.2", features = ["arc_lock"] } diff --git a/git-repository/src/easy/object/mod.rs b/git-repository/src/easy/object/mod.rs index 455cc3ff30f..9b71006d2f9 100644 --- a/git-repository/src/easy/object/mod.rs +++ b/git-repository/src/easy/object/mod.rs @@ -77,31 +77,20 @@ pub mod find { pub mod existing { use git_odb as odb; - use quick_error::quick_error; use crate::easy; - quick_error! { - #[derive(Debug)] - pub enum Error { - FindExisting(err: OdbError) { - display("Could not find a supposedly existing object") - from() - source(err) - } - BorrowState(err: easy::borrow::state::Error) { - display("BUG: Part of interior state could not be borrowed.") - from() - source(err) - } - BorrowRepo(err: easy::borrow::repo::Error) { - display("BUG: The repository could not be borrowed") - from() - } - } - } - pub(crate) type OdbError = odb::pack::find::existing::Error; + + #[derive(Debug, thiserror::Error)] + pub enum Error { + #[error(transparent)] + FindExisting(#[from] OdbError), + #[error("BUG: Part of interior state could not be borrowed.")] + BorrowState(#[from] easy::borrow::state::Error), + #[error("BUG: The repository could not be borrowed")] + BorrowRepo(#[from] easy::borrow::repo::Error), + } } } @@ -186,22 +175,18 @@ pub mod peel_to_kind { } mod error { - use quick_error::quick_error; use crate::easy::{object, object::find}; - quick_error! { - #[derive(Debug)] - pub enum Error { - FindExisting(err: find::existing::Error) { - display("A non existing object was encountered during object peeling") - from() - source(err) - } - NotFound{actual: object::Kind, expected: object::Kind} { - display("Last encountered object kind was {} while trying to peel to {}", actual, expected) - } - } + #[derive(Debug, thiserror::Error)] + pub enum Error { + #[error(transparent)] + FindExisting(#[from] find::existing::Error), + #[error("Last encountered object kind was {} while trying to peel to {}", .actual, .expected)] + NotFound { + actual: object::Kind, + expected: object::Kind, + }, } } } diff --git a/git-repository/src/easy/reference.rs b/git-repository/src/easy/reference.rs index cfe67512bbc..40c5c4e5155 100644 --- a/git-repository/src/easy/reference.rs +++ b/git-repository/src/easy/reference.rs @@ -137,52 +137,31 @@ where pub mod find { use git_ref as refs; - use quick_error::quick_error; use crate::easy; pub mod existing { - use quick_error::quick_error; use crate::easy::reference::find; - quick_error! { - #[derive(Debug)] - pub enum Error { - Find(err: find::Error) { - display("An error occurred when trying to find a reference") - from() - source(err) - } - NotFound { - display("The reference did not exist even though that was expected") - } - } + #[derive(Debug, thiserror::Error)] + pub enum Error { + #[error(transparent)] + Find(#[from] find::Error), + #[error("The reference did not exist even though that was expected")] + NotFound, } } - quick_error! { - #[derive(Debug)] - pub enum Error { - Find(err: refs::file::find::Error) { - display("An error occurred when trying to find a reference") - from() - source(err) - } - PackedRefsOpen(err: refs::packed::buffer::open::Error) { - display("The packed-refs file could not be opened") - from() - source(err) - } - BorrowState(err: easy::borrow::state::Error) { - display("BUG: Part of interior state could not be borrowed.") - from() - source(err) - } - BorrowRepo(err: easy::borrow::repo::Error) { - display("BUG: The repository could not be borrowed") - from() - } - } + #[derive(Debug, thiserror::Error)] + pub enum Error { + #[error(transparent)] + Find(#[from] refs::file::find::Error), + #[error(transparent)] + PackedRefsOpen(#[from] refs::packed::buffer::open::Error), + #[error("BUG: Part of interior state could not be borrowed.")] + BorrowState(#[from] easy::borrow::state::Error), + #[error("BUG: The repository could not be borrowed")] + BorrowRepo(#[from] easy::borrow::repo::Error), } } diff --git a/git-repository/src/repository.rs b/git-repository/src/repository.rs index 23e6d37b99e..6626d9db933 100644 --- a/git-repository/src/repository.rs +++ b/git-repository/src/repository.rs @@ -95,30 +95,18 @@ pub mod open { } pub mod init { + use crate::Repository; + use std::convert::TryInto; use std::path::Path; - use quick_error::quick_error; - - quick_error! { - #[derive(Debug)] - pub enum Error { - Init(err: crate::path::create::Error) { - display("Failed to initialize a new repository") - from() - source(err) - } - Open(err: crate::open::Error) { - display("Could not open repository") - from() - source(err) - } - } + #[derive(Debug, thiserror::Error)] + pub enum Error { + #[error(transparent)] + Init(#[from] crate::path::create::Error), + #[error(transparent)] + Open(#[from] crate::open::Error), } - use std::convert::TryInto; - - use crate::Repository; - impl Repository { /// Create a repository with work-tree within `directory`, creating intermediate directories as needed. /// @@ -132,26 +120,15 @@ pub mod init { } pub mod discover { - use std::{convert::TryInto, path::Path}; - - use quick_error::quick_error; - use crate::{path::discover, Repository}; + use std::{convert::TryInto, path::Path}; - quick_error! { - #[derive(Debug)] - pub enum Error { - Discover(err: discover::existing::Error) { - display("Could not find a valid git repository directory") - from() - source(err) - } - Open(err: crate::open::Error) { - display("Could not open repository") - from() - source(err) - } - } + #[derive(Debug, thiserror::Error)] + pub enum Error { + #[error(transparent)] + Discover(#[from] discover::existing::Error), + #[error(transparent)] + Open(#[from] crate::open::Error), } impl Repository { diff --git a/git-repository/tests/init/mod.rs b/git-repository/tests/init/mod.rs index 8a5020d8da2..954613d29f7 100644 --- a/git-repository/tests/init/mod.rs +++ b/git-repository/tests/init/mod.rs @@ -21,10 +21,10 @@ mod bare { let tmp = tempfile::tempdir().unwrap(); std::fs::write(tmp.path().join("existing.txt"), b"I was here before you").unwrap(); - assert_eq!( - git_repository::init_bare(tmp.path()).unwrap_err().to_string(), - "Failed to initialize a new repository" - ); + assert!(git_repository::init_bare(tmp.path()) + .unwrap_err() + .to_string() + .starts_with("Refusing to initialize the non-empty directory as"),); } } From 2a2a89f68cc45e27a1cf0d33fc644ebabc762302 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 30 Aug 2021 20:04:42 +0800 Subject: [PATCH 10/13] =?UTF-8?q?[config=20#185]=20add=20test=20for=20hand?= =?UTF-8?q?ling=20windows=20formatted=20files=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …which occour implicitly when checking out the repository fixtures on windows, which are then windows formatted. From there the unholy carriage return \r enters the compiled binary using include_bytes! as a template, which then is used to initialize a repository. In case of bare repositories, the implementation has to check if it really is bare (i.e. bare = true) or just looks bare (because there is no index, but that's the case in a new repository as well). This check would use the config parser, which wouldn't consider carriage returns and fail to parse the config file, making the opening of repositories impossible on windows. --- git-config/src/parser.rs | 2 +- git-config/tests/fixtures/repo-config.crlf | 14 ++++++++++++++ .../integration_tests/file_integeration_test.rs | 8 ++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 git-config/tests/fixtures/repo-config.crlf diff --git a/git-config/src/parser.rs b/git-config/src/parser.rs index 851c21a43b6..6c409a8cb90 100644 --- a/git-config/src/parser.rs +++ b/git-config/src/parser.rs @@ -1367,7 +1367,7 @@ fn take_spaces(i: &[u8]) -> IResult<&[u8], &str> { fn take_newline(i: &[u8]) -> IResult<&[u8], (&str, usize)> { let mut counter = 0; - let (i, v) = take_while(|c| (c as char).is_ascii() && is_newline(c))(i)?; + let (i, v) = take_while(|c| (c as char).is_ascii() && (c == b'\r' || is_newline(c)))(i)?; counter += v.len(); if v.is_empty() { Err(nom::Err::Error(NomError { diff --git a/git-config/tests/fixtures/repo-config.crlf b/git-config/tests/fixtures/repo-config.crlf new file mode 100644 index 00000000000..96672b93484 --- /dev/null +++ b/git-config/tests/fixtures/repo-config.crlf @@ -0,0 +1,14 @@ +; hello +# world + +[core] + repositoryformatversion = 0 + bare = true +[other] + repositoryformatversion = 0 + + bare = true + +# before section with newline +[foo] + repositoryformatversion = 0 diff --git a/git-config/tests/integration_tests/file_integeration_test.rs b/git-config/tests/integration_tests/file_integeration_test.rs index 6814ee2ffd6..3ef610653a7 100644 --- a/git-config/tests/integration_tests/file_integeration_test.rs +++ b/git-config/tests/integration_tests/file_integeration_test.rs @@ -1,6 +1,14 @@ use std::{borrow::Cow, convert::TryFrom}; use git_config::{file::GitConfig, values::*}; +use std::path::Path; + +#[test] +fn parse_config_with_windows_line_endings_successfully() { + GitConfig::open(Path::new("tests").join("fixtures").join("repo-config.crlf")) + .map_err(|err| err.to_string()) + .unwrap(); +} /// Asserts we can cast into all variants of our type #[test] From 57203ce5d5e3c481b69c3ca173e4b00f11aaf7d7 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 30 Aug 2021 20:25:31 +0800 Subject: [PATCH 11/13] =?UTF-8?q?[config=20#185]=20Count=20lines=20correct?= =?UTF-8?q?ly=20on=20windows=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …previously each line would count doubly, now CRLF is counted as one line, and NL is counted as one line too. --- git-config/src/parser.rs | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/git-config/src/parser.rs b/git-config/src/parser.rs index 6c409a8cb90..b74acbdef6c 100644 --- a/git-config/src/parser.rs +++ b/git-config/src/parser.rs @@ -16,7 +16,7 @@ use nom::{ bytes::complete::{escaped, tag, take_till, take_while}, character::{ complete::{char, none_of, one_of}, - is_newline, is_space, + is_space, }, combinator::{map, opt}, error::{Error as NomError, ErrorKind}, @@ -946,7 +946,7 @@ pub fn parse_from_bytes(input: &[u8]) -> Result { let (i, frontmatter) = many0(alt(( map(comment, Event::Comment), map(take_spaces, |whitespace| Event::Whitespace(Cow::Borrowed(whitespace))), - map(take_newline, |(newline, counter)| { + map(take_newlines, |(newline, counter)| { newlines += counter; Event::Newline(Cow::Borrowed(newline)) }), @@ -1015,7 +1015,7 @@ pub fn parse_from_bytes_owned(input: &[u8]) -> Result, Error<'st let (i, frontmatter) = many0(alt(( map(comment, Event::Comment), map(take_spaces, |whitespace| Event::Whitespace(Cow::Borrowed(whitespace))), - map(take_newline, |(newline, counter)| { + map(take_newlines, |(newline, counter)| { newlines += counter; Event::Newline(Cow::Borrowed(newline)) }), @@ -1094,7 +1094,7 @@ fn section<'a, 'b>(i: &'a [u8], node: &'b mut ParserNode) -> IResult<&'a [u8], ( } } - if let Ok((new_i, (v, new_newlines))) = take_newline(i) { + if let Ok((new_i, (v, new_newlines))) = take_newlines(i) { if old_i != new_i { i = new_i; newlines += new_newlines; @@ -1365,10 +1365,30 @@ fn take_spaces(i: &[u8]) -> IResult<&[u8], &str> { } } -fn take_newline(i: &[u8]) -> IResult<&[u8], (&str, usize)> { +fn take_newlines(i: &[u8]) -> IResult<&[u8], (&str, usize)> { let mut counter = 0; - let (i, v) = take_while(|c| (c as char).is_ascii() && (c == b'\r' || is_newline(c)))(i)?; - counter += v.len(); + let mut consumed_bytes = 0; + let mut next_must_be_newline = false; + for b in i.iter() { + if !(*b as char).is_ascii() { + break; + }; + if *b == b'\r' { + if next_must_be_newline { + break; + } + next_must_be_newline = true; + continue; + }; + if *b == b'\n' { + counter += 1; + consumed_bytes += if next_must_be_newline { 2 } else { 1 }; + next_must_be_newline = false; + } else { + break; + } + } + let (v, i) = i.split_at(consumed_bytes); if v.is_empty() { Err(nom::Err::Error(NomError { input: i, From 509c938dd061060141756ee791cdcb6017934fe2 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 30 Aug 2021 20:31:00 +0800 Subject: [PATCH 12/13] [config #185] refactor --- .../integration_tests/file_integeration_test.rs | 15 +++++++-------- git-config/tests/integration_tests/main.rs | 2 ++ .../integration_tests/parser_integration_tests.rs | 15 ++++++++++++--- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/git-config/tests/integration_tests/file_integeration_test.rs b/git-config/tests/integration_tests/file_integeration_test.rs index 3ef610653a7..4c6957ebf9e 100644 --- a/git-config/tests/integration_tests/file_integeration_test.rs +++ b/git-config/tests/integration_tests/file_integeration_test.rs @@ -4,15 +4,14 @@ use git_config::{file::GitConfig, values::*}; use std::path::Path; #[test] -fn parse_config_with_windows_line_endings_successfully() { - GitConfig::open(Path::new("tests").join("fixtures").join("repo-config.crlf")) - .map_err(|err| err.to_string()) - .unwrap(); +fn parse_config_with_windows_line_endings_successfully() -> crate::Result { + GitConfig::open(Path::new("tests").join("fixtures").join("repo-config.crlf"))?; + Ok(()) } /// Asserts we can cast into all variants of our type #[test] -fn get_value_for_all_provided_values() -> Result<(), Box> { +fn get_value_for_all_provided_values() -> crate::Result { let config = r#" [core] bool-explicit = false @@ -80,7 +79,7 @@ fn get_value_for_all_provided_values() -> Result<(), Box> /// There was a regression where lookup would fail because we only checked the /// last section entry for any given section and subsection #[test] -fn get_value_looks_up_all_sections_before_failing() -> Result<(), Box> { +fn get_value_looks_up_all_sections_before_failing() -> crate::Result { let config = r#" [core] bool-explicit = false @@ -106,7 +105,7 @@ fn get_value_looks_up_all_sections_before_failing() -> Result<(), Box Result<(), Box> { +fn section_names_are_case_insensitive() -> crate::Result { let config = "[core] bool-implicit"; let file = GitConfig::try_from(config)?; assert!(file.value::("core", None, "bool-implicit").is_ok()); @@ -119,7 +118,7 @@ fn section_names_are_case_insensitive() -> Result<(), Box } #[test] -fn value_names_are_case_insensitive() -> Result<(), Box> { +fn value_names_are_case_insensitive() -> crate::Result { let config = "[core] a = true A = false"; diff --git a/git-config/tests/integration_tests/main.rs b/git-config/tests/integration_tests/main.rs index 93ed91f9c6a..7f67ccff61a 100644 --- a/git-config/tests/integration_tests/main.rs +++ b/git-config/tests/integration_tests/main.rs @@ -4,5 +4,7 @@ // TL;DR single mod makes integration tests faster to compile, test, and with // less build artifacts. +type Result = std::result::Result<(), Box>; + mod file_integeration_test; mod parser_integration_tests; diff --git a/git-config/tests/integration_tests/parser_integration_tests.rs b/git-config/tests/integration_tests/parser_integration_tests.rs index 205c204fe7b..09c8735d8b5 100644 --- a/git-config/tests/integration_tests/parser_integration_tests.rs +++ b/git-config/tests/integration_tests/parser_integration_tests.rs @@ -211,9 +211,18 @@ fn newline_events_are_merged() { #[test] fn error() { let input = "[core] a=b\n 4a=3"; - println!("{}", parse_from_str(input).unwrap_err()); + assert_eq!( + parse_from_str(input).unwrap_err().to_string(), + "Got an unexpected token on line 2 while trying to parse a config name: '4a=3'" + ); let input = "[core] a=b\n =3"; - println!("{}", parse_from_str(input).unwrap_err()); + assert_eq!( + parse_from_str(input).unwrap_err().to_string(), + "Got an unexpected token on line 2 while trying to parse a config name: '=3'" + ); let input = "[core"; - println!("{}", parse_from_str(input).unwrap_err()); + assert_eq!( + parse_from_str(input).unwrap_err().to_string(), + "Got an unexpected token on line 1 while trying to parse a section header: '[core'" + ); } From dfbb015a89db47c79015135870013ecc384c4aea Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 30 Aug 2021 20:32:28 +0800 Subject: [PATCH 13/13] [repository #185] rustfmt --- .../integration_tests/file_integeration_test.rs | 3 +-- git-repository/src/repository.rs | 12 +++++++----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/git-config/tests/integration_tests/file_integeration_test.rs b/git-config/tests/integration_tests/file_integeration_test.rs index 4c6957ebf9e..efca33693e0 100644 --- a/git-config/tests/integration_tests/file_integeration_test.rs +++ b/git-config/tests/integration_tests/file_integeration_test.rs @@ -1,7 +1,6 @@ -use std::{borrow::Cow, convert::TryFrom}; +use std::{borrow::Cow, convert::TryFrom, path::Path}; use git_config::{file::GitConfig, values::*}; -use std::path::Path; #[test] fn parse_config_with_windows_line_endings_successfully() -> crate::Result { diff --git a/git-repository/src/repository.rs b/git-repository/src/repository.rs index 6626d9db933..44d9c3249f8 100644 --- a/git-repository/src/repository.rs +++ b/git-repository/src/repository.rs @@ -35,10 +35,11 @@ pub mod from_path { } pub mod open { - use crate::Repository; + use std::path::PathBuf; use git_config::values::Boolean; - use std::path::PathBuf; + + use crate::Repository; #[derive(Debug, thiserror::Error)] pub enum Error { @@ -95,9 +96,9 @@ pub mod open { } pub mod init { + use std::{convert::TryInto, path::Path}; + use crate::Repository; - use std::convert::TryInto; - use std::path::Path; #[derive(Debug, thiserror::Error)] pub enum Error { @@ -120,9 +121,10 @@ pub mod init { } pub mod discover { - use crate::{path::discover, Repository}; use std::{convert::TryInto, path::Path}; + use crate::{path::discover, Repository}; + #[derive(Debug, thiserror::Error)] pub enum Error { #[error(transparent)]