diff --git a/README.md b/README.md index 0e66e28b1f4..3620566c01d 100644 --- a/README.md +++ b/README.md @@ -52,7 +52,10 @@ Please see _'Development Status'_ for a listing of all crates and their capabili * **remote** * [x] **refs** - list all references available on the remote based on the current remote configuration. * [x] **ref-map** - show how remote references relate to their local tracking branches as mapped by refspecs. - * **fetch** - fetch the current remote or the given one, optionally just as dry-run. + * [x] **fetch** - fetch the current remote or the given one, optionally just as dry-run. + * **clone** + * [ ] initialize a new **bare** repository and fetch all objects. + * [ ] initialize a new repository, fetch all objects and checkout the main worktree. * **credential** * [x] **fill/approve/reject** - The same as `git credential`, but implemented in Rust, calling helpers only when from trusted configuration. * **free** - no git repository necessary diff --git a/etc/check-package-size.sh b/etc/check-package-size.sh index 3770e5f9110..17b6eb74163 100755 --- a/etc/check-package-size.sh +++ b/etc/check-package-size.sh @@ -57,6 +57,6 @@ echo "in root: gitoxide CLI" (enter git-odb && indent cargo diet -n --package-size-limit 120KB) (enter git-protocol && indent cargo diet -n --package-size-limit 50KB) (enter git-packetline && indent cargo diet -n --package-size-limit 35KB) -(enter git-repository && indent cargo diet -n --package-size-limit 185KB) +(enter git-repository && indent cargo diet -n --package-size-limit 200KB) (enter git-transport && indent cargo diet -n --package-size-limit 55KB) (enter gitoxide-core && indent cargo diet -n --package-size-limit 90KB) diff --git a/git-config/src/file/access/mutate.rs b/git-config/src/file/access/mutate.rs index b70930eca1d..ff8a88e8b95 100644 --- a/git-config/src/file/access/mutate.rs +++ b/git-config/src/file/access/mutate.rs @@ -2,6 +2,7 @@ use std::borrow::Cow; use git_features::threading::OwnShared; +use crate::file::SectionBodyIdsLut; use crate::{ file::{self, rename_section, write::ends_with_newline, MetadataFilter, SectionId, SectionMut}, lookup, @@ -11,7 +12,7 @@ use crate::{ /// Mutating low-level access methods. impl<'event> File<'event> { - /// Returns an mutable section with a given `name` and optional `subsection_name`, _if it exists_. + /// Returns the last mutable section with a given `name` and optional `subsection_name`, _if it exists_. pub fn section_mut<'a>( &'a mut self, name: impl AsRef, @@ -29,7 +30,16 @@ impl<'event> File<'event> { .expect("BUG: Section did not have id from lookup") .to_mut(nl)) } - /// Returns an mutable section with a given `name` and optional `subsection_name`, _if it exists_, or create a new section. + + /// Return the mutable section identified by `id`, or `None` if it didn't exist. + /// + /// Note that `id` is stable across deletions and insertions. + pub fn section_mut_by_id<'a>(&'a mut self, id: SectionId) -> Option> { + let nl = self.detect_newline_style_smallvec(); + self.sections.get_mut(&id).map(|s| s.to_mut(nl)) + } + + /// Returns the last mutable section with a given `name` and optional `subsection_name`, _if it exists_, or create a new section. pub fn section_mut_or_create_new<'a>( &'a mut self, name: impl AsRef, @@ -182,13 +192,39 @@ impl<'event> File<'event> { .ok()? .rev() .next()?; - self.section_order.remove( - self.section_order - .iter() - .position(|v| *v == id) - .expect("known section id"), - ); - self.sections.remove(&id) + self.remove_section_by_id(id) + } + + /// Remove the section identified by `id` if it exists and return it, or return `None` if no such section was present. + /// + /// Note that section ids are unambiguous even in the face of removals and additions of sections. + pub fn remove_section_by_id(&mut self, id: SectionId) -> Option> { + self.section_order + .remove(self.section_order.iter().position(|v| *v == id)?); + let section = self.sections.remove(&id)?; + let lut = self + .section_lookup_tree + .get_mut(§ion.header.name) + .expect("lookup cache still has name to be deleted"); + for entry in lut { + match section.header.subsection_name.as_deref() { + Some(subsection_name) => { + if let SectionBodyIdsLut::NonTerminal(map) = entry { + if let Some(ids) = map.get_mut(subsection_name) { + ids.remove(ids.iter().position(|v| *v == id).expect("present")); + break; + } + } + } + None => { + if let SectionBodyIdsLut::Terminal(ids) = entry { + ids.remove(ids.iter().position(|v| *v == id).expect("present")); + break; + } + } + } + } + Some(section) } /// Removes the section with `name` and `subsection_name` that passed `filter`, returning the removed section diff --git a/git-config/src/file/access/read_only.rs b/git-config/src/file/access/read_only.rs index 56e274d7b05..a50d60137f2 100644 --- a/git-config/src/file/access/read_only.rs +++ b/git-config/src/file/access/read_only.rs @@ -1,9 +1,10 @@ -use std::{borrow::Cow, convert::TryFrom, iter::FromIterator}; +use std::{borrow::Cow, convert::TryFrom}; use bstr::BStr; use git_features::threading::OwnShared; use smallvec::SmallVec; +use crate::file::SectionId; use crate::{ file, file::{ @@ -206,6 +207,25 @@ impl<'event> File<'event> { }) } + /// Similar to [`sections_by_name()`][Self::sections_by_name()], but returns an identifier for this section as well to allow + /// referring to it unambiguously even in the light of deletions. + #[must_use] + pub fn sections_and_ids_by_name<'a>( + &'a self, + name: &'a str, + ) -> Option, SectionId)> + '_> { + self.section_ids_by_name(name).ok().map(move |ids| { + ids.map(move |id| { + ( + self.sections + .get(&id) + .expect("section doesn't have id from from lookup"), + id, + ) + }) + }) + } + /// Gets all sections that match the provided `name`, ignoring any subsections, and pass the `filter`. #[must_use] pub fn sections_by_name_and_filter<'a>( @@ -258,6 +278,11 @@ impl<'event> File<'event> { self.section_order.iter().map(move |id| &self.sections[id]) } + /// Return an iterator over all sections and their ids, in order of occurrence in the file itself. + pub fn sections_and_ids(&self) -> impl Iterator, SectionId)> + '_ { + self.section_order.iter().map(move |id| (&self.sections[id], *id)) + } + /// Return an iterator over all sections along with non-section events that are placed right after them, /// in order of occurrence in the file itself. /// @@ -296,6 +321,6 @@ impl<'event> File<'event> { } pub(crate) fn detect_newline_style_smallvec(&self) -> SmallVec<[u8; 2]> { - SmallVec::from_iter(self.detect_newline_style().iter().copied()) + self.detect_newline_style().as_ref().into() } } diff --git a/git-config/src/file/mod.rs b/git-config/src/file/mod.rs index e799b79b494..16209772ead 100644 --- a/git-config/src/file/mod.rs +++ b/git-config/src/file/mod.rs @@ -110,7 +110,7 @@ impl AddAssign for Size { /// words, it's possible that a section may have an ID of 3 but the next section /// has an ID of 5 as 4 was deleted. #[derive(PartialEq, Eq, Hash, Copy, Clone, PartialOrd, Ord, Debug)] -pub(crate) struct SectionId(pub(crate) usize); +pub struct SectionId(pub(crate) usize); /// All section body ids referred to by a section name. /// diff --git a/git-config/src/file/mutable/section.rs b/git-config/src/file/mutable/section.rs index 38fd3ec6f1c..26b1abacc76 100644 --- a/git-config/src/file/mutable/section.rs +++ b/git-config/src/file/mutable/section.rs @@ -95,7 +95,7 @@ impl<'a, 'event> SectionMut<'a, 'event> { Some((key_range, value_range)) => { let value_range = value_range.unwrap_or(key_range.end - 1..key_range.end); let range_start = value_range.start; - let ret = self.remove_internal(value_range); + let ret = self.remove_internal(value_range, false); self.section .body .0 @@ -109,7 +109,7 @@ impl<'a, 'event> SectionMut<'a, 'event> { pub fn remove(&mut self, key: impl AsRef) -> Option> { let key = Key::from_str_unchecked(key.as_ref()); let (key_range, _value_range) = self.key_and_value_range_by(&key)?; - Some(self.remove_internal(key_range)) + Some(self.remove_internal(key_range, true)) } /// Adds a new line event. Note that you don't need to call this unless @@ -231,17 +231,33 @@ impl<'a, 'event> SectionMut<'a, 'event> { } /// Performs the removal, assuming the range is valid. - fn remove_internal(&mut self, range: Range) -> Cow<'event, BStr> { - self.section - .body - .0 - .drain(range) - .fold(Cow::Owned(BString::default()), |mut acc, e| { + fn remove_internal(&mut self, range: Range, fix_whitespace: bool) -> Cow<'event, BStr> { + let events = &mut self.section.body.0; + if fix_whitespace + && events + .get(range.end) + .map_or(false, |ev| matches!(ev, Event::Newline(_))) + { + events.remove(range.end); + } + let value = events + .drain(range.clone()) + .fold(Cow::Owned(BString::default()), |mut acc: Cow<'_, BStr>, e| { if let Event::Value(v) | Event::ValueNotDone(v) | Event::ValueDone(v) = e { acc.to_mut().extend(&**v); } acc - }) + }); + if fix_whitespace + && range + .start + .checked_sub(1) + .and_then(|pos| events.get(pos)) + .map_or(false, |ev| matches!(ev, Event::Whitespace(_))) + { + events.remove(range.start - 1); + } + value } } diff --git a/git-config/src/parse/section/header.rs b/git-config/src/parse/section/header.rs index fdedb27e00d..c4c13a97365 100644 --- a/git-config/src/parse/section/header.rs +++ b/git-config/src/parse/section/header.rs @@ -41,9 +41,13 @@ impl<'a> Header<'a> { } } +/// Return true if `name` is valid as subsection name, like `origin` in `[remote "origin"]`. +pub fn is_valid_subsection(name: &BStr) -> bool { + name.find_byteset(b"\n\0").is_none() +} + fn validated_subsection(name: Cow<'_, BStr>) -> Result, Error> { - name.find_byteset(b"\n\0") - .is_none() + is_valid_subsection(name.as_ref()) .then(|| name) .ok_or(Error::InvalidSubSection) } @@ -55,6 +59,24 @@ fn validated_name(name: Cow<'_, BStr>) -> Result, Error> { .ok_or(Error::InvalidName) } +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn empty_header_names_are_legal() { + assert!(Header::new("", None).is_ok(), "yes, git allows this, so do we"); + } + + #[test] + fn empty_header_sub_names_are_legal() { + assert!( + Header::new("remote", Some("".into())).is_ok(), + "yes, git allows this, so do we" + ); + } +} + impl Header<'_> { ///Return true if this is a header like `[legacy.subsection]`, or false otherwise. pub fn is_legacy(&self) -> bool { diff --git a/git-config/src/parse/section/mod.rs b/git-config/src/parse/section/mod.rs index 459ad5906ef..040efa944b0 100644 --- a/git-config/src/parse/section/mod.rs +++ b/git-config/src/parse/section/mod.rs @@ -160,10 +160,10 @@ mod types { generate_case_insensitive!( Name, name, - "Valid names consist alphanumeric characters or dashes.", + "Valid names consist of alphanumeric characters or dashes.", is_valid_name, bstr::BStr, - "Wrapper struct for section header names, like `includeIf`, since these are case-insensitive." + "Wrapper struct for section header names, like `remote`, since these are case-insensitive." ); generate_case_insensitive!( diff --git a/git-config/tests/file/access/mutate.rs b/git-config/tests/file/access/mutate.rs index de28bf9c641..6fb90ee693d 100644 --- a/git-config/tests/file/access/mutate.rs +++ b/git-config/tests/file/access/mutate.rs @@ -1,3 +1,51 @@ +mod remove_section { + use std::convert::TryFrom; + + #[test] + fn removal_of_all_sections_programmatically_with_sections_and_ids_by_name() { + let mut file = git_config::File::try_from("[core] \na = b\nb=c\n\n[core \"name\"]\nd = 1\ne = 2").unwrap(); + for id in file + .sections_and_ids_by_name("core") + .expect("2 sections present") + .map(|(_, id)| id) + .collect::>() + { + file.remove_section_by_id(id); + } + assert!(file.is_void()); + assert_eq!(file.sections().count(), 0); + } + + #[test] + fn removal_of_all_sections_programmatically_with_sections_and_ids() { + let mut file = git_config::File::try_from("[core] \na = b\nb=c\n\n[core \"name\"]\nd = 1\ne = 2").unwrap(); + for id in file.sections_and_ids().map(|(_, id)| id).collect::>() { + file.remove_section_by_id(id); + } + assert!(file.is_void()); + assert_eq!(file.sections().count(), 0); + } + + #[test] + fn removal_is_complete_and_sections_can_be_readded() { + let mut file = git_config::File::try_from("[core] \na = b\nb=c\n\n[core \"name\"]\nd = 1\ne = 2").unwrap(); + assert_eq!(file.sections().count(), 2); + + let removed = file.remove_section("core", None).expect("removed correct section"); + assert_eq!(removed.header().name(), "core"); + assert_eq!(removed.header().subsection_name(), None); + assert_eq!(file.sections().count(), 1); + + let removed = file.remove_section("core", Some("name")).expect("found"); + assert_eq!(removed.header().name(), "core"); + assert_eq!(removed.header().subsection_name().expect("present"), "name"); + assert_eq!(file.sections().count(), 0); + + file.section_mut_or_create_new("core", None).expect("creation succeeds"); + file.section_mut_or_create_new("core", Some("name")) + .expect("creation succeeds"); + } +} mod rename_section { use std::{borrow::Cow, convert::TryFrom}; diff --git a/git-config/tests/file/mutable/section.rs b/git-config/tests/file/mutable/section.rs index 0f878bb8fde..b8e890480d5 100644 --- a/git-config/tests/file/mutable/section.rs +++ b/git-config/tests/file/mutable/section.rs @@ -28,6 +28,15 @@ fn section_mut_or_create_new_filter_may_reject_existing_sections() -> crate::Res Ok(()) } +#[test] +fn section_mut_by_id() { + let mut config = multi_value_section(); + let id = config.sections_and_ids().next().expect("at least one").1; + let section = config.section_mut_by_id(id).expect("present"); + assert_eq!(section.header().name(), "a"); + assert_eq!(section.header().subsection_name(), None); +} + mod remove { use super::multi_value_section; @@ -49,10 +58,7 @@ mod remove { } assert!(!section.is_void(), "everything is still there"); - assert_eq!( - config.to_string(), - "\n [a]\n \n \n \n \n " - ); + assert_eq!(config.to_string(), "\n [a]\n"); Ok(()) } } diff --git a/git-refspec/src/match_group/validate.rs b/git-refspec/src/match_group/validate.rs index c1eab28e5e1..783ef4c5d15 100644 --- a/git-refspec/src/match_group/validate.rs +++ b/git-refspec/src/match_group/validate.rs @@ -4,7 +4,7 @@ use bstr::BString; use crate::{ match_group::{Outcome, Source}, - RefSpecRef, + RefSpec, }; /// All possible issues found while validating matched mappings. @@ -49,13 +49,13 @@ impl std::fmt::Display for Issue { /// All possible fixes corrected while validating matched mappings. #[derive(Debug, PartialEq, Eq, Clone)] -pub enum Fix<'a> { +pub enum Fix { /// Removed a mapping that contained a partial destination entirely. MappingWithPartialDestinationRemoved { /// The destination ref name that was ignored. name: BString, /// The spec that defined the mapping - spec: RefSpecRef<'a>, + spec: RefSpec, }, } @@ -91,7 +91,7 @@ impl<'spec, 'item> Outcome<'spec, 'item> { /// Return `(modified self, issues)` providing a fixed-up set of mappings in `self` with the fixed `issues` /// provided as part of it. /// Terminal issues are communicated using the [`Error`] type accordingly. - pub fn validated(mut self) -> Result<(Self, Vec>), Error> { + pub fn validated(mut self) -> Result<(Self, Vec), Error> { let mut sources_by_destinations = BTreeMap::new(); for (dst, (spec_index, src)) in self .mappings @@ -126,7 +126,7 @@ impl<'spec, 'item> Outcome<'spec, 'item> { } else { fixed.push(Fix::MappingWithPartialDestinationRemoved { name: dst.as_ref().to_owned(), - spec: group.specs[m.spec_index], + spec: group.specs[m.spec_index].to_owned(), }); false } diff --git a/git-refspec/tests/match_group/mod.rs b/git-refspec/tests/match_group/mod.rs index 0b11a5c61df..565f533c5e7 100644 --- a/git-refspec/tests/match_group/mod.rs +++ b/git-refspec/tests/match_group/mod.rs @@ -169,15 +169,15 @@ mod multiple { [ Fix::MappingWithPartialDestinationRemoved { name: "foo/f1".into(), - spec: glob_spec_ref, + spec: glob_spec_ref.to_owned(), }, Fix::MappingWithPartialDestinationRemoved { name: "foo/f2".into(), - spec: glob_spec_ref, + spec: glob_spec_ref.to_owned(), }, Fix::MappingWithPartialDestinationRemoved { name: "foo/f3".into(), - spec: glob_spec_ref, + spec: glob_spec_ref.to_owned(), }, ], ["refs/heads/f1:refs/heads/f1"], diff --git a/git-refspec/tests/matching/mod.rs b/git-refspec/tests/matching/mod.rs index 12a086b76be..26cd5808820 100644 --- a/git-refspec/tests/matching/mod.rs +++ b/git-refspec/tests/matching/mod.rs @@ -56,9 +56,9 @@ pub mod baseline { agrees_and_applies_fixes(specs, Vec::new(), expected) } - pub fn agrees_and_applies_fixes<'a, 'b, 'c>( + pub fn agrees_and_applies_fixes<'a, 'b>( specs: impl IntoIterator + Clone, - fixes: impl IntoIterator>, + fixes: impl IntoIterator, expected: impl IntoIterator, ) { check_fetch_remote( @@ -127,14 +127,9 @@ pub mod baseline { of_objects_with_destinations_are_written_into_given_local_branches(specs, expected) } - enum Mode<'a> { - Normal { - validate_err: Option, - }, - Custom { - expected: Vec, - fixes: Vec>, - }, + enum Mode { + Normal { validate_err: Option }, + Custom { expected: Vec, fixes: Vec }, } fn check_fetch_remote<'a>(specs: impl IntoIterator + Clone, mode: Mode) { diff --git a/git-refspec/tests/parse/invalid.rs b/git-refspec/tests/parse/invalid.rs index 436d4a21f1c..e3cb5b1a46a 100644 --- a/git-refspec/tests/parse/invalid.rs +++ b/git-refspec/tests/parse/invalid.rs @@ -7,6 +7,14 @@ fn empty() { assert!(matches!(try_parse("", Operation::Push).unwrap_err(), Error::Empty)); } +#[test] +fn empty_component() { + assert!(matches!( + try_parse("refs/heads/test:refs/remotes//test", Operation::Fetch).unwrap_err(), + Error::ReferenceName(git_validate::refname::Error::RepeatedSlash) + )); +} + #[test] fn complex_patterns_with_more_than_one_asterisk() { for op in [Operation::Fetch, Operation::Push] { diff --git a/git-repository/src/clone.rs b/git-repository/src/clone.rs new file mode 100644 index 00000000000..256f5b2e916 --- /dev/null +++ b/git-repository/src/clone.rs @@ -0,0 +1,207 @@ +type ConfigureRemoteFn = Box) -> Result, crate::remote::init::Error>>; + +/// A utility to collect configuration on how to fetch from a remote and possibly create a working tree locally. +pub struct Prepare { + /// A freshly initialized repository which is owned by us, or `None` if it was handed to the user + repo: Option, + /// The name of the remote, which defaults to `origin` if not overridden. + remote_name: Option, + /// A function to configure a remote prior to fetching a pack. + configure_remote: Option, + /// Options for preparing a fetch operation. + #[cfg(any(feature = "async-network-client", feature = "blocking-network-client"))] + fetch_options: crate::remote::ref_map::Options, + /// The url to clone from + #[allow(dead_code)] + url: git_url::Url, +} + +/// +pub mod prepare { + use crate::clone::Prepare; + use crate::Repository; + use std::convert::TryInto; + + /// The error returned by [`Prepare::new()`]. + #[derive(Debug, thiserror::Error)] + #[allow(missing_docs)] + pub enum Error { + #[error(transparent)] + Init(#[from] crate::init::Error), + #[error(transparent)] + UrlParse(#[from] git_url::parse::Error), + } + + /// + #[cfg(feature = "blocking-network-client")] + pub mod fetch { + /// The error returned by [`Prepare::fetch_only()`][super::Prepare::fetch_only()]. + #[derive(Debug, thiserror::Error)] + #[allow(missing_docs)] + pub enum Error { + #[error(transparent)] + Connect(#[from] crate::remote::connect::Error), + #[error(transparent)] + PrepareFetch(#[from] crate::remote::fetch::prepare::Error), + #[error(transparent)] + Fetch(#[from] crate::remote::fetch::Error), + #[error(transparent)] + RemoteConfiguration(#[from] crate::remote::init::Error), + #[error("Default remote configured at `clone.defaultRemoteName` is invalid")] + RemoteName(#[from] crate::remote::name::Error), + #[error("Failed to load repo-local git configuration before writing")] + LoadConfig(#[from] git_config::file::init::from_paths::Error), + #[error("Failed to store configured remote in memory")] + SaveConfig(#[from] crate::remote::save::AsError), + #[error("Failed to write repository configuration to disk")] + SaveConfigIo(#[from] std::io::Error), + } + } + + /// Instantiation + impl Prepare { + /// Create a new repository at `path` with `crate_opts` which is ready to clone from `url`, possibly after making additional adjustments to + /// configuration and settings. + /// + /// Note that this is merely a handle to perform the actual connection to the remote, and if any of it fails the freshly initialized repository + /// will be removed automatically as soon as this instance drops. + pub fn new( + url: Url, + path: impl AsRef, + create_opts: crate::create::Options, + open_opts: crate::open::Options, + ) -> Result + where + Url: TryInto, + git_url::parse::Error: From, + { + let url = url.try_into().map_err(git_url::parse::Error::from)?; + let repo = crate::ThreadSafeRepository::init_opts(path, create_opts, open_opts)?.to_thread_local(); + Ok(Prepare { + url, + #[cfg(any(feature = "async-network-client", feature = "blocking-network-client"))] + fetch_options: Default::default(), + repo: Some(repo), + remote_name: None, + configure_remote: None, + }) + } + } + + /// Modification + impl Prepare { + /// Fetch a pack and update local branches according to refspecs, providing `progress` and checking `should_interrupt` to stop + /// the operation. + /// On success, the persisted repository is returned, and this method must not be called again to avoid a **panic**. + /// On error, the method may be called again to retry as often as needed. + /// + /// Note that all data we created will be removed once this instance drops if the operation wasn't successful. + #[cfg(feature = "blocking-network-client")] + pub fn fetch_only( + &mut self, + progress: impl crate::Progress, + should_interrupt: &std::sync::atomic::AtomicBool, + ) -> Result<(Repository, crate::remote::fetch::Outcome), fetch::Error> { + let repo = self + .repo + .as_mut() + .expect("user error: multiple calls are allowed only until it succeeds"); + + let remote_name = match self.remote_name.as_deref() { + Some(name) => name.to_owned(), + None => repo + .config + .resolved + .string("clone", None, "defaultRemoteName") + .map(|n| crate::remote::name::validated(n.to_string())) + .unwrap_or_else(|| Ok("origin".into()))?, + }; + + let mut remote = repo + .remote_at(self.url.clone())? + .with_refspec("+refs/heads/*:refs/remotes/origin/*", crate::remote::Direction::Fetch) + .expect("valid static spec"); + if let Some(f) = self.configure_remote.as_mut() { + remote = f(remote)?; + } + + let mut metadata = git_config::file::Metadata::from(git_config::Source::Local); + let config_path = repo.git_dir().join("config"); + metadata.path = Some(config_path.clone()); + let mut config = + git_config::File::from_paths_metadata(Some(metadata), Default::default())?.expect("one file to load"); + remote.save_as_to(remote_name, &mut config)?; + std::fs::write(config_path, config.to_bstring())?; + + let outcome = remote + .connect(crate::remote::Direction::Fetch, progress)? + .prepare_fetch(self.fetch_options.clone())? + .receive(should_interrupt)?; + + let repo_config = git_features::threading::OwnShared::make_mut(&mut repo.config.resolved); + let ids_to_remove: Vec<_> = repo_config + .sections_and_ids() + .filter_map(|(s, id)| (s.meta().source == git_config::Source::Local).then(|| id)) + .collect(); + for id in ids_to_remove { + repo_config.remove_section_by_id(id); + } + repo_config.append(config); + + Ok((self.repo.take().expect("still present"), outcome)) + } + } + + /// Builder + impl Prepare { + /// Set additional options to adjust parts of the fetch operation that are not affected by the git configuration. + #[cfg(any(feature = "async-network-client", feature = "blocking-network-client"))] + pub fn with_fetch_options(mut self, opts: crate::remote::ref_map::Options) -> Self { + self.fetch_options = opts; + self + } + /// Use `f` to apply arbitrary changes to the remote that is about to be used to fetch a pack. + /// + /// The passed in `remote` will be un-named and pre-configured to be a default remote as we know it from git-clone. + /// It is not yet present in the configuration of the repository, + /// but each change it will eventually be written to the configuration prior to performing a the fetch operation. + pub fn configure_remote( + mut self, + f: impl FnMut(crate::Remote<'_>) -> Result, crate::remote::init::Error> + 'static, + ) -> Self { + self.configure_remote = Some(Box::new(f)); + self + } + + /// Set the remote's name to the given value after it was configured using the function provided via + /// [`configure_remote()`][Self::configure_remote()]. + /// + /// If not set here, it defaults to `origin` or the value of `clone.defaultRemoteName`. + pub fn with_remote_name(mut self, name: impl Into) -> Result { + self.remote_name = Some(crate::remote::name::validated(name)?); + Ok(self) + } + } + + /// Consumption + impl Prepare { + /// Persist the contained repository as is even if an error may have occurred when interacting with the remote or checking out the main working tree. + pub fn persist(mut self) -> Repository { + self.repo.take().expect("present and consumed once") + } + } + + impl Drop for Prepare { + fn drop(&mut self) { + if let Some(repo) = self.repo.take() { + std::fs::remove_dir_all(repo.work_dir().unwrap_or_else(|| repo.path())).ok(); + } + } + } + + impl From for Repository { + fn from(prep: Prepare) -> Self { + prep.persist() + } + } +} diff --git a/git-repository/src/lib.rs b/git-repository/src/lib.rs index 8c957e1fdfc..1ac52181064 100644 --- a/git-repository/src/lib.rs +++ b/git-repository/src/lib.rs @@ -183,6 +183,8 @@ pub use types::{ Worktree, }; +/// +pub mod clone; pub mod commit; pub mod head; pub mod id; @@ -220,6 +222,59 @@ pub fn init_bare(directory: impl AsRef) -> Result( + url: Url, + path: impl AsRef, +) -> Result +where + Url: std::convert::TryInto, + git_url::parse::Error: From, +{ + clone::Prepare::new( + url, + path, + create::Options { + bare: true, + fs_capabilities: None, + }, + open_opts_with_git_binary_config(), + ) +} + +/// Create a platform for configuring a clone with main working tree from `url` to the local `path`, using default options for opening it +/// (but amended with using configuration from the git installation to ensure all authentication options are honored). +/// +/// See [`clone::Prepare::new()] for a function to take full control over all options. +pub fn prepare_clone( + url: Url, + path: impl AsRef, +) -> Result +where + Url: std::convert::TryInto, + git_url::parse::Error: From, +{ + clone::Prepare::new( + url, + path, + create::Options { + bare: false, + fs_capabilities: None, + }, + open_opts_with_git_binary_config(), + ) +} + +fn open_opts_with_git_binary_config() -> open::Options { + use git_sec::trust::DefaultForLevel; + let mut opts = open::Options::default_for_level(git_sec::Trust::Full); + opts.permissions.config.git_binary = true; + opts +} + /// See [ThreadSafeRepository::open()], but returns a [`Repository`] instead. pub fn open(directory: impl Into) -> Result { ThreadSafeRepository::open(directory).map(Into::into) diff --git a/git-repository/src/remote/connection/fetch/mod.rs b/git-repository/src/remote/connection/fetch/mod.rs index 8f8d3e104a9..e56309abf31 100644 --- a/git-repository/src/remote/connection/fetch/mod.rs +++ b/git-repository/src/remote/connection/fetch/mod.rs @@ -60,9 +60,9 @@ pub enum Status { /// The outcome of receiving a pack via [`Prepare::receive()`]. #[derive(Debug, Clone)] -pub struct Outcome<'spec> { +pub struct Outcome { /// The result of the initial mapping of references, the prerequisite for any fetch. - pub ref_map: RefMap<'spec>, + pub ref_map: RefMap, /// The status of the operation to indicate what happened. pub status: Status, } @@ -125,7 +125,18 @@ where /// /// "fetch.negotiationAlgorithm" describes algorithms `git` uses currently, with the default being `consecutive` and `skipping` being /// experimented with. We currently implement something we could call 'naive' which works for now. - pub fn receive(mut self, should_interrupt: &AtomicBool) -> Result, Error> { + /// + /// + /// ### Deviation + /// + /// When **updating refs**, the `git-fetch` docs state that the following: + /// + /// > Unlike when pushing with git-push, any updates outside of refs/{tags,heads}/* will be accepted without + in the refspec (or --force), whether that’s swapping e.g. a tree object for a blob, or a commit for another commit that’s doesn’t have the previous commit as an ancestor etc. + /// + /// We explicitly don't special case those refs and expect the user to take control. Note that by its nature, + /// force only applies to refs pointing to commits and if they don't, they will be updated either way in our + /// implementation as well. + pub fn receive(mut self, should_interrupt: &AtomicBool) -> Result { let mut con = self.con.take().expect("receive() can only be called once"); let handshake = &self.ref_map.handshake; @@ -258,7 +269,7 @@ where T: Transport, { con: Option>, - ref_map: RefMap<'remote>, + ref_map: RefMap, dry_run: DryRun, } diff --git a/git-repository/src/remote/connection/fetch/negotiate.rs b/git-repository/src/remote/connection/fetch/negotiate.rs index f020732b74d..51bb25ff294 100644 --- a/git-repository/src/remote/connection/fetch/negotiate.rs +++ b/git-repository/src/remote/connection/fetch/negotiate.rs @@ -20,7 +20,7 @@ pub(crate) fn one_round( algo: Algorithm, round: usize, repo: &crate::Repository, - ref_map: &crate::remote::fetch::RefMap<'_>, + ref_map: &crate::remote::fetch::RefMap, arguments: &mut git_protocol::fetch::Arguments, _previous_response: Option<&git_protocol::fetch::Response>, ) -> Result { diff --git a/git-repository/src/remote/connection/ref_map.rs b/git-repository/src/remote/connection/ref_map.rs index e231f783218..fc43a332096 100644 --- a/git-repository/src/remote/connection/ref_map.rs +++ b/git-repository/src/remote/connection/ref_map.rs @@ -63,7 +63,7 @@ where /// Due to management of the transport, it's cleanest to only use it for a single interaction. Thus it's consumed along with /// the connection. #[git_protocol::maybe_async::maybe_async] - pub async fn ref_map(mut self, options: Options) -> Result, Error> { + pub async fn ref_map(mut self, options: Options) -> Result { let res = self.ref_map_inner(options).await; git_protocol::fetch::indicate_end_of_interaction(&mut self.transport) .await @@ -78,7 +78,7 @@ where prefix_from_spec_as_filter_on_remote, handshake_parameters, }: Options, - ) -> Result, Error> { + ) -> Result { let remote = self .fetch_refs(prefix_from_spec_as_filter_on_remote, handshake_parameters) .await?; diff --git a/git-repository/src/remote/fetch.rs b/git-repository/src/remote/fetch.rs index bc528969726..b23cad887fe 100644 --- a/git-repository/src/remote/fetch.rs +++ b/git-repository/src/remote/fetch.rs @@ -12,11 +12,11 @@ pub(crate) enum DryRun { /// Information about the relationship between our refspecs, and remote references with their local counterparts. #[derive(Default, Debug, Clone)] -pub struct RefMap<'spec> { +pub struct RefMap { /// A mapping between a remote reference and a local tracking branch. pub mappings: Vec, /// Information about the fixes applied to the `mapping` due to validation and sanitization. - pub fixes: Vec>, + pub fixes: Vec, /// All refs advertised by the remote. pub remote_refs: Vec, /// Additional information provided by the server as part of the handshake. diff --git a/git-repository/src/remote/init.rs b/git-repository/src/remote/init.rs index 85dbf43d89d..bb29280c7a6 100644 --- a/git-repository/src/remote/init.rs +++ b/git-repository/src/remote/init.rs @@ -11,6 +11,8 @@ mod error { #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { + #[error(transparent)] + Name(#[from] crate::remote::name::Error), #[error(transparent)] Url(#[from] git_url::parse::Error), #[error("The rewritten {kind} url {rewritten_url:?} failed to parse")] @@ -42,7 +44,7 @@ impl<'repo> Remote<'repo> { .then(|| rewrite_urls(&repo.config, url.as_ref(), push_url.as_ref())) .unwrap_or(Ok((None, None)))?; Ok(Remote { - name, + name: name.map(remote::name::validated).transpose()?, url, url_alias, push_url, diff --git a/git-repository/src/remote/mod.rs b/git-repository/src/remote/mod.rs index 661ed9e5910..809b38c6a4a 100644 --- a/git-repository/src/remote/mod.rs +++ b/git-repository/src/remote/mod.rs @@ -22,6 +22,30 @@ mod build; mod errors; pub use errors::find; +/// +pub mod name { + /// The error returned by [validated()]. + #[derive(Debug, thiserror::Error)] + #[error("remote names must be valid within refspecs for fetching: {name:?}")] + #[allow(missing_docs)] + pub struct Error { + source: git_refspec::parse::Error, + name: String, + } + + /// Return `name` if it is valid or convert it into an `Error`. + pub fn validated(name: impl Into) -> Result { + let name = name.into(); + match git_refspec::parse( + format!("refs/heads/test:refs/remotes/{name}/test").as_str().into(), + git_refspec::parse::Operation::Fetch, + ) { + Ok(_) => Ok(name), + Err(err) => Err(Error { source: err, name }), + } + } +} + /// pub mod init; @@ -38,5 +62,8 @@ mod connection; #[cfg(any(feature = "async-network-client", feature = "blocking-network-client"))] pub use connection::{ref_map, Connection}; +/// +pub mod save; + mod access; pub(crate) mod url; diff --git a/git-repository/src/remote/save.rs b/git-repository/src/remote/save.rs new file mode 100644 index 00000000000..530992bcc3a --- /dev/null +++ b/git-repository/src/remote/save.rs @@ -0,0 +1,104 @@ +use crate::Remote; +use std::convert::TryInto; + +/// The error returned by [`Remote::save_to()`]. +#[derive(Debug, thiserror::Error)] +#[allow(missing_docs)] +pub enum Error { + #[error("The remote pointing to {} is anonymous and can't be saved.", url.to_bstring())] + NameMissing { url: git_url::Url }, +} + +/// The error returned by [`Remote::save_as_to()`]. +/// +/// Note that this type should rather be in the `as` module, but cannot be as it's part of the Rust syntax. +#[derive(Debug, thiserror::Error)] +#[allow(missing_docs)] +pub enum AsError { + #[error(transparent)] + Save(#[from] Error), + #[error(transparent)] + Name(#[from] crate::remote::name::Error), +} + +/// Serialize into git-config. +impl Remote<'_> { + /// Save ourselves to the given `config` if we are a named remote or fail otherwise. + /// + /// Note that all sections named `remote ""` will be cleared of all values we are about to write, + /// and the last `remote ""` section will be containing all relevant values so that reloading the remote + /// from `config` would yield the same in-memory state. + pub fn save_to(&self, config: &mut git_config::File<'static>) -> Result<(), Error> { + let name = self.name().ok_or_else(|| Error::NameMissing { + url: self + .url + .as_ref() + .or(self.push_url.as_ref()) + .expect("one url is always set") + .to_owned(), + })?; + if let Some(section_ids) = config.sections_and_ids_by_name("remote").map(|it| { + it.filter_map(|(s, id)| (s.header().subsection_name() == Some(name.into())).then(|| id)) + .collect::>() + }) { + let mut sections_to_remove = Vec::new(); + const KEYS_TO_REMOVE: &[&str] = &["url", "pushurl", "fetch", "push"]; + for id in section_ids { + let mut section = config.section_mut_by_id(id).expect("just queried"); + let was_empty = section.num_values() == 0; + + for key in KEYS_TO_REMOVE { + while section.remove(key).is_some() {} + } + + let is_empty_after_deletions_of_values_to_be_written = section.num_values() == 0; + if !was_empty && is_empty_after_deletions_of_values_to_be_written { + sections_to_remove.push(id); + } + } + for id in sections_to_remove { + config.remove_section_by_id(id); + } + } + let mut section = config + .section_mut_or_create_new("remote", Some(name)) + .expect("section name is validated and 'remote' is acceptable"); + if let Some(url) = self.url.as_ref() { + section.push("url".try_into().expect("valid"), Some(url.to_bstring().as_ref())) + } + if let Some(url) = self.push_url.as_ref() { + section.push("pushurl".try_into().expect("valid"), Some(url.to_bstring().as_ref())) + } + for (key, spec) in self + .fetch_specs + .iter() + .map(|spec| ("fetch", spec)) + .chain(self.push_specs.iter().map(|spec| ("push", spec))) + { + section.push( + key.try_into().expect("valid"), + Some(spec.to_ref().to_bstring().as_ref()), + ) + } + Ok(()) + } + + /// Forcefully set our name to `name` and write our state to `config` similar to [`save_to()`][Self::save_to()]. + /// + /// Note that this sets a name for anonymous remotes, but overwrites the name for those who were named before. + /// If this name is different from the current one, the git configuration will still contain the previous name, + /// and the caller should account for that. + pub fn save_as_to( + &mut self, + name: impl Into, + config: &mut git_config::File<'static>, + ) -> Result<(), AsError> { + let name = crate::remote::name::validated(name)?; + let prev_name = self.name.take(); + self.name = name.into(); + self.save_to(config).map_err(|err| { + self.name = prev_name; + err.into() + }) + } +} diff --git a/git-repository/tests/clone/mod.rs b/git-repository/tests/clone/mod.rs new file mode 100644 index 00000000000..ad0b3ccce79 --- /dev/null +++ b/git-repository/tests/clone/mod.rs @@ -0,0 +1,94 @@ +use crate::remote; +use git_repository as git; + +#[test] +#[cfg(feature = "blocking-network-client")] +fn fetch_only_with_configuration() -> crate::Result { + let tmp = git_testtools::tempfile::TempDir::new()?; + let called_configure_remote = std::sync::Arc::new(std::sync::atomic::AtomicBool::default()); + let remote_name = "special"; + let mut prepare = git::prepare_clone_bare(remote::repo("base").path(), tmp.path())? + .with_remote_name(remote_name)? + .configure_remote({ + let called_configure_remote = called_configure_remote.clone(); + move |r| { + called_configure_remote.store(true, std::sync::atomic::Ordering::Relaxed); + Ok( + r.with_refspec("+refs/tags/*:refs/tags/*", git::remote::Direction::Fetch) + .expect("valid static spec"), + ) + } + }); + let (repo, out) = prepare.fetch_only(git::progress::Discard, &std::sync::atomic::AtomicBool::default())?; + drop(prepare); + + assert!( + called_configure_remote.load(std::sync::atomic::Ordering::Relaxed), + "custom remote configuration is called" + ); + assert_eq!(repo.remote_names().len(), 1, "only ever one remote"); + let remote = repo.find_remote(remote_name)?; + assert_eq!( + remote.refspecs(git::remote::Direction::Fetch).len(), + 2, + "our added spec was stored as well" + ); + + assert_eq!(out.ref_map.mappings.len(), 13); + match out.status { + git_repository::remote::fetch::Status::Change { update_refs, .. } => { + for edit in &update_refs.edits { + use git_odb::Find; + assert!( + repo.objects.contains(edit.change.new_value().expect("always set").id()), + "part of the fetched pack" + ); + } + } + _ => unreachable!("clones are always causing changes and dry-runs aren't possible"), + } + Ok(()) +} + +#[test] +#[cfg(feature = "blocking-network-client")] +fn fetch_only_without_configuration() -> crate::Result { + let tmp = git_testtools::tempfile::TempDir::new()?; + let (repo, _out) = git::prepare_clone_bare(remote::repo("base").path(), tmp.path())? + .fetch_only(git::progress::Discard, &std::sync::atomic::AtomicBool::default())?; + assert!(repo.find_remote("origin").is_ok(), "default remote name is 'origin'"); + Ok(()) +} + +#[test] +fn clone_and_early_persist_without_receive() -> crate::Result { + let tmp = git_testtools::tempfile::TempDir::new()?; + let repo = git::prepare_clone_bare(remote::repo("base").path(), tmp.path())?.persist(); + assert!(repo.is_bare(), "repo is now ours and remains"); + Ok(()) +} + +#[test] +fn clone_bare_into_empty_directory_and_early_drop() -> crate::Result { + let tmp = git_testtools::tempfile::TempDir::new()?; + // this breaks isolation, but shouldn't be affecting the test. If so, use isolation options for opening the repo. + let prep = git::prepare_clone_bare(remote::repo("base").path(), tmp.path())?; + let head = tmp.path().join("HEAD"); + assert!(head.is_file(), "now a bare basic repo is present"); + drop(prep); + + assert!(!head.is_file(), "we cleanup if the clone isn't followed through"); + Ok(()) +} + +#[test] +fn clone_into_empty_directory_and_early_drop() -> crate::Result { + let tmp = git_testtools::tempfile::TempDir::new()?; + let prep = git::prepare_clone(remote::repo("base").path(), tmp.path())?; + let head = tmp.path().join(".git").join("HEAD"); + assert!(head.is_file(), "now a basic repo is present"); + drop(prep); + + assert!(!head.is_file(), "we cleanup if the clone isn't followed through"); + Ok(()) +} diff --git a/git-repository/tests/git-with-regex.rs b/git-repository/tests/git-with-regex.rs index 0be6ff1efe4..dbaf2fb20e3 100644 --- a/git-repository/tests/git-with-regex.rs +++ b/git-repository/tests/git-with-regex.rs @@ -1,6 +1,7 @@ mod util; use util::*; +mod clone; mod commit; mod head; mod id; diff --git a/git-repository/tests/git.rs b/git-repository/tests/git.rs index 305de9db9dd..661db12d9ef 100644 --- a/git-repository/tests/git.rs +++ b/git-repository/tests/git.rs @@ -3,6 +3,8 @@ mod util; #[cfg(not(feature = "regex"))] use util::*; +#[cfg(not(feature = "regex"))] +mod clone; #[cfg(not(feature = "regex"))] mod commit; #[cfg(not(feature = "regex"))] diff --git a/git-repository/tests/remote/fetch.rs b/git-repository/tests/remote/fetch.rs index 9a0cfabfad7..32689098962 100644 --- a/git-repository/tests/remote/fetch.rs +++ b/git-repository/tests/remote/fetch.rs @@ -28,7 +28,7 @@ mod blocking_io { let mut remote = repo.head()?.into_remote(Fetch).expect("present")?; remote.replace_refspecs(Some("HEAD:refs/remotes/origin/does-not-exist"), Fetch)?; - let res: git::remote::fetch::Outcome<'_> = remote + let res: git::remote::fetch::Outcome = remote .connect(Fetch, git::progress::Discard)? .prepare_fetch(Default::default())? .receive(&AtomicBool::default())?; diff --git a/git-repository/tests/remote/mod.rs b/git-repository/tests/remote/mod.rs index ea4b5c03e75..4ce5f8638a5 100644 --- a/git-repository/tests/remote/mod.rs +++ b/git-repository/tests/remote/mod.rs @@ -19,3 +19,11 @@ pub(crate) fn cow_str(s: &str) -> Cow { mod connect; mod fetch; mod ref_map; +mod save; +mod name { + use git_repository as git; + #[test] + fn empty_is_invalid() { + assert!(git::remote::name::validated("").is_err()); + } +} diff --git a/git-repository/tests/remote/save.rs b/git-repository/tests/remote/save.rs new file mode 100644 index 00000000000..1025647ffbc --- /dev/null +++ b/git-repository/tests/remote/save.rs @@ -0,0 +1,118 @@ +mod save_to { + use crate::remote; + use crate::remote::save::uniformize; + use git_repository as git; + + #[test] + fn named_remotes_save_as_is() -> crate::Result { + let repo = remote::repo("clone"); + let remote = repo.find_remote("origin")?; + + let mut config = git::config::File::default(); + remote.save_to(&mut config)?; + let actual = uniformize(config.to_string()); + assert!( + actual.starts_with("[remote \"origin\"]\n\turl = "), + "workaround absolute paths in test fixture…" + ); + assert!( + actual.ends_with("/base\n\tfetch = +refs/heads/*:refs/remotes/origin/*\n"), + "…by checking only the parts that are similar" + ); + + let previous_remote_state = repo + .config_snapshot() + .plumbing() + .section("remote", Some("origin")) + .expect("present") + .to_bstring(); + let mut config = repo.config_snapshot().plumbing().clone(); + remote.save_to(&mut config)?; + assert_eq!( + config.sections_by_name("remote").expect("more than one").count(), + 2, + "amount of remotes are unaltered" + ); + assert_eq!( + config.section("remote", Some("origin")).expect("present").to_bstring(), + previous_remote_state, + "the serialization doesn't modify anything" + ); + Ok(()) + } +} + +mod save_as_to { + use crate::basic_repo; + use crate::remote::save::uniformize; + use git_repository as git; + use std::convert::TryInto; + + #[test] + fn anonymous_remotes_cannot_be_save_lacking_a_name() -> crate::Result { + let repo = basic_repo()?; + let remote = repo.remote_at("https://example.com/path")?; + assert!(matches!( + remote.save_to(&mut git::config::File::default()).unwrap_err(), + git::remote::save::Error::NameMissing { .. } + )); + Ok(()) + } + + #[test] + fn new_anonymous_remote_with_name() -> crate::Result { + let repo = basic_repo()?; + let mut remote = repo + .remote_at("https://example.com/path")? + .push_url("https://ein.hub/path")? + .with_refspec("+refs/heads/*:refs/remotes/any/*", git::remote::Direction::Fetch)? + .with_refspec( + "refs/heads/special:refs/heads/special-upstream", + git::remote::Direction::Fetch, + )? + .with_refspec("refs/heads/main:refs/heads/main", git::remote::Direction::Push)? // similar to 'simple' for `push.default` + .with_refspec(":", git::remote::Direction::Push)?; // similar to 'matching' + let remote_name = "origin"; + assert!( + repo.find_remote(remote_name).is_err(), + "there is no remote of that name" + ); + assert_eq!(remote.name(), None); + let mut config = git::config::File::default(); + remote.save_as_to(remote_name, &mut config)?; + let expected = "[remote \"origin\"]\n\turl = https://example.com/path\n\tpushurl = https://ein.hub/path\n\tfetch = +refs/heads/*:refs/remotes/any/*\n\tfetch = refs/heads/special:refs/heads/special-upstream\n\tpush = refs/heads/main:refs/heads/main\n\tpush = :\n"; + assert_eq!(uniformize(config.to_string()), expected); + + remote.save_as_to(remote_name, &mut config)?; + assert_eq!( + uniformize(config.to_string()), + expected, + "it appears to be idempotent in this case" + ); + + { + let mut new_section = config.section_mut_or_create_new("unrelated", None).expect("works"); + new_section.push("a".try_into().unwrap(), Some("value".into())); + + config + .section_mut_or_create_new("initially-empty-not-removed", Some("name")) + .expect("works"); + + let mut existing_section = config + .section_mut_or_create_new("remote", Some("origin")) + .expect("works"); + existing_section.push("free".try_into().unwrap(), Some("should not be removed".into())) + } + remote.save_as_to(remote_name, &mut config)?; + assert_eq!( + uniformize(config.to_string()), + "[remote \"origin\"]\n\tfree = should not be removed\n\turl = https://example.com/path\n\tpushurl = https://ein.hub/path\n\tfetch = +refs/heads/*:refs/remotes/any/*\n\tfetch = refs/heads/special:refs/heads/special-upstream\n\tpush = refs/heads/main:refs/heads/main\n\tpush = :\n[unrelated]\n\ta = value\n[initially-empty-not-removed \"name\"]\n", + "unrelated keys are kept, and so are keys in the sections we edit" + ); + Ok(()) + } +} + +fn uniformize(input: String) -> String { + input.replace("\r\n", "\n") +} diff --git a/git-repository/tests/repository/remote.rs b/git-repository/tests/repository/remote.rs index 1da830e4b85..4028a0b8e9b 100644 --- a/git-repository/tests/repository/remote.rs +++ b/git-repository/tests/repository/remote.rs @@ -16,7 +16,7 @@ mod remote_at { let mut remote = remote.push_url("user@host.xz:./relative")?; assert_eq!( remote.url(Direction::Push).unwrap().to_bstring(), - "ssh://user@host.xz/relative" + "user@host.xz:relative" ); assert_eq!(remote.url(Direction::Fetch).unwrap().to_bstring(), fetch_url); diff --git a/git-url/src/impls.rs b/git-url/src/impls.rs index 15881520515..e191f4c79a9 100644 --- a/git-url/src/impls.rs +++ b/git-url/src/impls.rs @@ -1,4 +1,7 @@ -use std::{convert::TryFrom, path::PathBuf}; +use std::{ + convert::TryFrom, + path::{Path, PathBuf}, +}; use bstr::BStr; @@ -7,6 +10,7 @@ use crate::{parse, Scheme, Url}; impl Default for Url { fn default() -> Self { Url { + serialize_alternative_form: false, scheme: Scheme::Ssh, user: None, host: None, @@ -41,6 +45,15 @@ impl TryFrom for Url { } } +impl TryFrom<&Path> for Url { + type Error = parse::Error; + + fn try_from(value: &Path) -> Result { + use std::convert::TryInto; + git_path::into_bstr(value).try_into() + } +} + impl TryFrom<&std::ffi::OsStr> for Url { type Error = parse::Error; diff --git a/git-url/src/lib.rs b/git-url/src/lib.rs index d47aa85942c..ed3deedd2c3 100644 --- a/git-url/src/lib.rs +++ b/git-url/src/lib.rs @@ -38,6 +38,8 @@ pub struct Url { user: Option, /// The host to which to connect. Localhost is implied if `None`. host: Option, + /// When serializing, use the alternative forms as it was parsed as such. + serialize_alternative_form: bool, /// The port to use when connecting to a host. If `None`, standard ports depending on `scheme` will be used. pub port: Option, /// The path portion of the URL, usually the location of the git repository. @@ -61,6 +63,7 @@ impl Url { host, port, path, + serialize_alternative_form: false, } .to_bstring() .as_ref(), @@ -78,6 +81,18 @@ impl Url { } } +/// Builder +impl Url { + /// Enable alternate serialization for this url, e.g. `file:///path` becomes `/path`. + /// + /// This is automatically set correctly for parsed URLs, but can be set here for urls + /// created by constructor. + pub fn serialize_alternate_form(mut self, use_alternate_form: bool) -> Self { + self.serialize_alternative_form = use_alternate_form; + self + } +} + /// Access impl Url { /// Returns the user mentioned in the url, if present. @@ -112,8 +127,10 @@ impl Url { impl Url { /// Write this URL losslessly to `out`, ready to be parsed again. pub fn write_to(&self, mut out: impl std::io::Write) -> std::io::Result<()> { - out.write_all(self.scheme.as_str().as_bytes())?; - out.write_all(b"://")?; + if !(self.serialize_alternative_form && (self.scheme == Scheme::File || self.scheme == Scheme::Ssh)) { + out.write_all(self.scheme.as_str().as_bytes())?; + out.write_all(b"://")?; + } match (&self.user, &self.host) { (Some(user), Some(host)) => { out.write_all(user.as_bytes())?; @@ -129,7 +146,12 @@ impl Url { if let Some(port) = &self.port { write!(&mut out, ":{}", port)?; } - out.write_all(&self.path)?; + if self.serialize_alternative_form && self.scheme == Scheme::Ssh { + out.write_all(b":")?; + out.write_all(&self.path[1..])?; + } else { + out.write_all(&self.path)?; + } Ok(()) } @@ -148,6 +170,7 @@ impl Url { } } +/// Deserialization impl Url { /// Parse a URL from `bytes` pub fn from_bytes(bytes: &BStr) -> Result { diff --git a/git-url/src/parse.rs b/git-url/src/parse.rs index 2a2f3951cb6..c6f33f6c4e0 100644 --- a/git-url/src/parse.rs +++ b/git-url/src/parse.rs @@ -60,16 +60,13 @@ fn has_no_explicit_protocol(url: &[u8]) -> bool { url.find(b"://").is_none() } -fn possibly_strip_file_protocol(url: &[u8]) -> &[u8] { - if url.starts_with(b"file://") { - &url[b"file://".len()..] - } else { - url - } +fn try_strip_file_protocol(url: &[u8]) -> Option<&[u8]> { + url.strip_prefix(b"file://") } fn to_owned_url(url: url::Url) -> Result { Ok(crate::Url { + serialize_alternative_form: false, scheme: str_to_protocol(url.scheme())?, user: if url.username().is_empty() { None @@ -90,25 +87,30 @@ fn to_owned_url(url: url::Url) -> Result { /// For file-paths, we don't expect UTF8 encoding either. pub fn parse(input: &BStr) -> Result { let guessed_protocol = guess_protocol(input); - if possibly_strip_file_protocol(input) != input || (has_no_explicit_protocol(input) && guessed_protocol == "file") { + let path_without_protocol = try_strip_file_protocol(input); + if path_without_protocol.is_some() || (has_no_explicit_protocol(input) && guessed_protocol == "file") { return Ok(crate::Url { scheme: Scheme::File, - path: possibly_strip_file_protocol(input).into(), + path: path_without_protocol.unwrap_or(input).into(), + serialize_alternative_form: !input.starts_with(b"file://"), ..Default::default() }); } let url_str = std::str::from_utf8(input)?; - let mut url = match url::Url::parse(url_str) { - Ok(url) => url, - Err(::url::ParseError::RelativeUrlWithoutBase) => { + let (mut url, mut sanitized_scp) = match url::Url::parse(url_str) { + Ok(url) => (url, false), + Err(url::ParseError::RelativeUrlWithoutBase) => { // happens with bare paths as well as scp like paths. The latter contain a ':' past the host portion, // which we are trying to detect. - url::Url::parse(&format!( - "{}://{}", - guessed_protocol, - sanitize_for_protocol(guessed_protocol, url_str) - ))? + ( + url::Url::parse(&format!( + "{}://{}", + guessed_protocol, + sanitize_for_protocol(guessed_protocol, url_str) + ))?, + true, + ) } Err(err) => return Err(err.into()), }; @@ -116,6 +118,7 @@ pub fn parse(input: &BStr) -> Result { if url.scheme().find('.').is_some() { // try again with prefixed protocol url = url::Url::parse(&format!("ssh://{}", sanitize_for_protocol("ssh", url_str)))?; + sanitized_scp = true; } if url.scheme() != "rad" && url.path().is_empty() { return Err(Error::EmptyPath); @@ -124,5 +127,5 @@ pub fn parse(input: &BStr) -> Result { return Err(Error::RelativeUrl { url: url.into() }); } - to_owned_url(url) + to_owned_url(url).map(|url| url.serialize_alternate_form(sanitized_scp)) } diff --git a/git-url/tests/parse/file.rs b/git-url/tests/parse/file.rs index f697e4a0712..00a8e9293ab 100644 --- a/git-url/tests/parse/file.rs +++ b/git-url/tests/parse/file.rs @@ -1,7 +1,7 @@ use bstr::ByteSlice; use git_url::Scheme; -use crate::parse::{assert_url_and, assert_url_roundtrip, url}; +use crate::parse::{assert_url_and, assert_url_roundtrip, url, url_alternate}; #[test] fn file_path_with_protocol() -> crate::Result { @@ -13,15 +13,23 @@ fn file_path_with_protocol() -> crate::Result { #[test] fn file_path_without_protocol() -> crate::Result { - let url = assert_url_and("/path/to/git", url(Scheme::File, None, None, None, b"/path/to/git"))?.to_bstring(); - assert_eq!(url, "file:///path/to/git"); + let url = assert_url_and( + "/path/to/git", + url_alternate(Scheme::File, None, None, None, b"/path/to/git"), + )? + .to_bstring(); + assert_eq!(url, "/path/to/git"); Ok(()) } #[test] fn no_username_expansion_for_file_paths_without_protocol() -> crate::Result { - let url = assert_url_and("~/path/to/git", url(Scheme::File, None, None, None, b"~/path/to/git"))?.to_bstring(); - assert_eq!(url, "file://~/path/to/git"); + let url = assert_url_and( + "~/path/to/git", + url_alternate(Scheme::File, None, None, None, b"~/path/to/git"), + )? + .to_bstring(); + assert_eq!(url, "~/path/to/git"); Ok(()) } #[test] @@ -35,14 +43,17 @@ fn no_username_expansion_for_file_paths_with_protocol() -> crate::Result { #[test] fn non_utf8_file_path_without_protocol() -> crate::Result { let parsed = git_url::parse(b"/path/to\xff/git".as_bstr())?; - assert_eq!(parsed, url(Scheme::File, None, None, None, b"/path/to\xff/git",)); + assert_eq!( + parsed, + url_alternate(Scheme::File, None, None, None, b"/path/to\xff/git",) + ); let url_lossless = parsed.to_bstring(); assert_eq!( url_lossless.to_string(), - "file:///path/to�/git", + "/path/to�/git", "non-unicode is made unicode safe after conversion" ); - assert_eq!(url_lossless, &b"file:///path/to\xff/git"[..], "otherwise it's lossless"); + assert_eq!(url_lossless, &b"/path/to\xff/git"[..], "otherwise it's lossless"); Ok(()) } @@ -50,12 +61,16 @@ fn non_utf8_file_path_without_protocol() -> crate::Result { fn relative_file_path_without_protocol() -> crate::Result { let parsed = assert_url_and( "../../path/to/git", - url(Scheme::File, None, None, None, b"../../path/to/git"), + url_alternate(Scheme::File, None, None, None, b"../../path/to/git"), + )? + .to_bstring(); + assert_eq!(parsed, "../../path/to/git"); + let url = assert_url_and( + "path/to/git", + url_alternate(Scheme::File, None, None, None, b"path/to/git"), )? .to_bstring(); - assert_eq!(parsed, "file://../../path/to/git"); - let url = assert_url_and("path/to/git", url(Scheme::File, None, None, None, b"path/to/git"))?.to_bstring(); - assert_eq!(url, "file://path/to/git"); + assert_eq!(url, "path/to/git"); Ok(()) } @@ -63,23 +78,26 @@ fn relative_file_path_without_protocol() -> crate::Result { fn interior_relative_file_path_without_protocol() -> crate::Result { let url = assert_url_and( "/abs/path/../../path/to/git", - url(Scheme::File, None, None, None, b"/abs/path/../../path/to/git"), + url_alternate(Scheme::File, None, None, None, b"/abs/path/../../path/to/git"), )? .to_bstring(); - assert_eq!(url, "file:///abs/path/../../path/to/git"); + assert_eq!(url, "/abs/path/../../path/to/git"); Ok(()) } mod windows { use git_url::Scheme; - use crate::parse::{assert_url_and, assert_url_roundtrip, url}; + use crate::parse::{assert_url_and, assert_url_roundtrip, url, url_alternate}; #[test] fn file_path_without_protocol() -> crate::Result { - let url = - assert_url_and("x:/path/to/git", url(Scheme::File, None, None, None, b"x:/path/to/git"))?.to_bstring(); - assert_eq!(url, "file://x:/path/to/git"); + let url = assert_url_and( + "x:/path/to/git", + url_alternate(Scheme::File, None, None, None, b"x:/path/to/git"), + )? + .to_bstring(); + assert_eq!(url, "x:/path/to/git"); Ok(()) } @@ -87,10 +105,10 @@ mod windows { fn file_path_with_backslashes_without_protocol() -> crate::Result { let url = assert_url_and( "x:\\path\\to\\git", - url(Scheme::File, None, None, None, b"x:\\path\\to\\git"), + url_alternate(Scheme::File, None, None, None, b"x:\\path\\to\\git"), )? .to_bstring(); - assert_eq!(url, "file://x:\\path\\to\\git"); + assert_eq!(url, "x:\\path\\to\\git"); Ok(()) } diff --git a/git-url/tests/parse/mod.rs b/git-url/tests/parse/mod.rs index 47d8a6e1798..64c05254bbf 100644 --- a/git-url/tests/parse/mod.rs +++ b/git-url/tests/parse/mod.rs @@ -43,6 +43,16 @@ fn url( .expect("valid") } +fn url_alternate( + protocol: Scheme, + user: impl Into>, + host: impl Into>, + port: impl Into>, + path: &'static [u8], +) -> git_url::Url { + url(protocol, user, host, port, path).serialize_alternate_form(true) +} + mod file; mod invalid; mod ssh; diff --git a/git-url/tests/parse/ssh.rs b/git-url/tests/parse/ssh.rs index 349069f398a..6654949aea9 100644 --- a/git-url/tests/parse/ssh.rs +++ b/git-url/tests/parse/ssh.rs @@ -1,6 +1,6 @@ use git_url::Scheme; -use crate::parse::{assert_url_and, assert_url_roundtrip, url}; +use crate::parse::{assert_url_and, assert_url_roundtrip, url, url_alternate}; #[test] fn without_user_and_without_port() -> crate::Result { @@ -51,10 +51,10 @@ fn with_user_and_without_port() -> crate::Result { fn scp_like_without_user() -> crate::Result { let url = assert_url_and( "host.xz:path/to/git", - url(Scheme::Ssh, None, "host.xz", None, b"/path/to/git"), + url_alternate(Scheme::Ssh, None, "host.xz", None, b"/path/to/git"), )? .to_bstring(); - assert_eq!(url, "ssh://host.xz/path/to/git"); + assert_eq!(url, "host.xz:path/to/git"); Ok(()) } @@ -62,10 +62,10 @@ fn scp_like_without_user() -> crate::Result { fn scp_like_without_user_and_username_expansion_without_username() -> crate::Result { let url = assert_url_and( "host.xz:~/to/git", - url(Scheme::Ssh, None, "host.xz", None, b"/~/to/git"), + url_alternate(Scheme::Ssh, None, "host.xz", None, b"/~/to/git"), )? .to_bstring(); - assert_eq!(url, "ssh://host.xz/~/to/git"); + assert_eq!(url, "host.xz:~/to/git"); Ok(()) } @@ -73,10 +73,10 @@ fn scp_like_without_user_and_username_expansion_without_username() -> crate::Res fn scp_like_without_user_and_username_expansion_with_username() -> crate::Result { let url = assert_url_and( "host.xz:~byron/to/git", - url(Scheme::Ssh, None, "host.xz", None, b"/~byron/to/git"), + url_alternate(Scheme::Ssh, None, "host.xz", None, b"/~byron/to/git"), )? .to_bstring(); - assert_eq!(url, "ssh://host.xz/~byron/to/git"); + assert_eq!(url, "host.xz:~byron/to/git"); Ok(()) } @@ -84,9 +84,16 @@ fn scp_like_without_user_and_username_expansion_with_username() -> crate::Result fn scp_like_with_user_and_relative_path_turns_into_absolute_path() -> crate::Result { let url = assert_url_and( "user@host.xz:./relative", - url(Scheme::Ssh, "user", "host.xz", None, b"/relative"), + url_alternate(Scheme::Ssh, "user", "host.xz", None, b"/relative"), )? .to_bstring(); - assert_eq!(url, "ssh://user@host.xz/relative"); + assert_eq!(url, "user@host.xz:relative"); + + let url = assert_url_and( + "user@host.xz:../relative", + url_alternate(Scheme::Ssh, "user", "host.xz", None, b"/../relative"), + )? + .to_bstring(); + assert_eq!(url, "user@host.xz:relative"); Ok(()) } diff --git a/gitoxide-core/src/repository/fetch.rs b/gitoxide-core/src/repository/fetch.rs index bfe6096c26f..0fe450845f8 100644 --- a/gitoxide-core/src/repository/fetch.rs +++ b/gitoxide-core/src/repository/fetch.rs @@ -43,7 +43,7 @@ pub(crate) mod function { if !ref_specs.is_empty() { remote.replace_refspecs(ref_specs.iter(), git::remote::Direction::Fetch)?; } - let res: git::remote::fetch::Outcome<'_> = remote + let res: git::remote::fetch::Outcome = remote .connect(git::remote::Direction::Fetch, progress)? .prepare_fetch(Default::default())? .with_dry_run(dry_run) @@ -84,7 +84,7 @@ pub(crate) mod function { repo: &git::Repository, update_refs: git::remote::fetch::refs::update::Outcome, refspecs: &[git::refspec::RefSpec], - mut map: git::remote::fetch::RefMap<'_>, + mut map: git::remote::fetch::RefMap, mut out: impl std::io::Write, mut err: impl std::io::Write, ) -> anyhow::Result<()> { @@ -121,8 +121,11 @@ pub(crate) mod function { err, "The following destination refs were removed as they didn't start with 'ref/'" )?; - map.fixes.sort_by_key(|f| match f { - Fix::MappingWithPartialDestinationRemoved { spec, .. } => *spec, + map.fixes.sort_by(|l, r| match (l, r) { + ( + Fix::MappingWithPartialDestinationRemoved { spec: l, .. }, + Fix::MappingWithPartialDestinationRemoved { spec: r, .. }, + ) => l.cmp(&r), }); let mut prev_spec = None; for fix in &map.fixes { @@ -130,7 +133,7 @@ pub(crate) mod function { Fix::MappingWithPartialDestinationRemoved { name, spec } => { if prev_spec.map_or(true, |prev_spec| prev_spec != spec) { prev_spec = spec.into(); - spec.write_to(&mut err)?; + spec.to_ref().write_to(&mut err)?; writeln!(err)?; } writeln!(err, "\t{name}")?; diff --git a/gitoxide-core/src/repository/remote.rs b/gitoxide-core/src/repository/remote.rs index 6bbbd53de3b..2543b6103e7 100644 --- a/gitoxide-core/src/repository/remote.rs +++ b/gitoxide-core/src/repository/remote.rs @@ -95,7 +95,7 @@ mod refs_impl { pub(crate) fn print_refmap( repo: &git::Repository, refspecs: &[RefSpec], - mut map: git::remote::fetch::RefMap<'_>, + mut map: git::remote::fetch::RefMap, mut out: impl std::io::Write, mut err: impl std::io::Write, ) -> anyhow::Result<()> { @@ -141,8 +141,11 @@ mod refs_impl { err, "The following destination refs were removed as they didn't start with 'ref/'" )?; - map.fixes.sort_by_key(|f| match f { - Fix::MappingWithPartialDestinationRemoved { spec, .. } => *spec, + map.fixes.sort_by(|l, r| match (l, r) { + ( + Fix::MappingWithPartialDestinationRemoved { spec: l, .. }, + Fix::MappingWithPartialDestinationRemoved { spec: r, .. }, + ) => l.cmp(&r), }); let mut prev_spec = None; for fix in &map.fixes { @@ -150,7 +153,7 @@ mod refs_impl { Fix::MappingWithPartialDestinationRemoved { name, spec } => { if prev_spec.map_or(true, |prev_spec| prev_spec != spec) { prev_spec = spec.into(); - spec.write_to(&mut err)?; + spec.to_ref().write_to(&mut err)?; writeln!(err)?; } writeln!(err, "\t{name}")?; diff --git a/src/plumbing/progress.rs b/src/plumbing/progress.rs index eddd738a07c..a6ae7be2afc 100644 --- a/src/plumbing/progress.rs +++ b/src/plumbing/progress.rs @@ -10,6 +10,9 @@ enum Usage { Planned { note: Option<&'static str>, }, + NotPlanned { + reason: &'static str, + }, InModule { name: &'static str, deviation: Option<&'static str>, @@ -24,6 +27,10 @@ impl Display for Usage { match self { Puzzled => f.write_str("❓")?, NotApplicable => f.write_str("not applicable")?, + NotPlanned { reason } => { + write!(f, "{}", "not planned".blink())?; + write!(f, " ℹ {} ℹ", reason.bright_white())?; + } Planned { note } => { write!(f, "{}", "planned".blink())?; if let Some(note) = note { @@ -47,6 +54,7 @@ impl Usage { Puzzled => "?", NotApplicable => "❌", Planned { .. } => "🕒", + NotPlanned { .. } => "🤔", InModule { deviation, .. } => deviation.is_some().then(|| "👌️").unwrap_or("✅"), } } @@ -190,6 +198,25 @@ static GIT_CONFIG: &[Record] = &[ deviation: Some("defaults to 'gitoxide@localhost'"), }, }, + Record { + config: "clone.filterSubmodules,", + usage: Planned { + note: Some("currently object filtering isn't support, a prerequisite for this, see --filter=blob:none for more"), + }, + }, + Record { + config: "clone.defaultRemoteName", + usage: InModule { + name: "clone::prepare", + deviation: None + }, + }, + Record { + config: "clone.rejectShallow", + usage: NotPlanned { + reason: "it's not a use-case we consider important now, but once that changes it can be implemented", + }, + }, Record { config: "fetch.recurseSubmodules", usage: Planned {