feat: Remote knows about its tagOpt configuration.

That way it's clear if it should or shouldn't fetch included/reachable
tags automatically.

The default setting for this is to include tags, similar to `git`.

The `fetch_tags()` accessor allows to query this information, and the
`with_fetch_tags()` builder method allows to set the value comfortably
right after creating the `Remote` instance.

The `tagOpt` key will also be written as part of the remote's git
configuration.

Clone operations can set the `Tags` setting when configuring the
remote in a callback.

Author: Byron

git-repository/src/clone/fetch/mod.rs

@@ -12,7 +12,9 @@ pub enum Error {
     #[error(transparent)]
     Fetch(#[from] crate::remote::fetch::Error),
     #[error(transparent)]
-    RemoteConfiguration(#[from] crate::remote::init::Error),
+    RemoteInit(#[from] crate::remote::init::Error),
+    #[error("Custom configuration of remote to clone from failed")]
+    RemoteConfiguration(#[source] Box<dyn std::error::Error + Send + Sync>),
     #[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")]
@@ -24,7 +26,7 @@ pub enum Error {
     #[error("The remote HEAD points to a reference named {head_ref_name:?} which is invalid.")]
     InvalidHeadRef {
         source: git_validate::refname::Error,
-        head_ref_name: crate::bstr::BString,
+        head_ref_name: BString,
     },
     #[error("Failed to update HEAD with values from remote")]
     HeadUpdate(#[from] crate::reference::edit::Error),
@@ -51,6 +53,7 @@ impl PrepareFetch {
         P: crate::Progress,
         P::SubProgress: 'static,
     {
+        use crate::remote;
         use crate::{bstr::ByteVec, remote::fetch::RefLogMessage};
 
         let repo = self
@@ -64,36 +67,39 @@ impl PrepareFetch {
                 .config
                 .resolved
                 .string_by_key("clone.defaultRemoteName")
-                .map(|n| crate::remote::name::validated(n.into_owned()))
+                .map(|n| remote::name::validated(n.into_owned()))
                 .unwrap_or_else(|| Ok("origin".into()))?,
         };
 
         let mut remote = repo
             .remote_at(self.url.clone())?
             .with_refspecs(
                 Some(format!("+refs/heads/*:refs/remotes/{remote_name}/*").as_str()),
-                crate::remote::Direction::Fetch,
+                remote::Direction::Fetch,
             )
             .expect("valid static spec");
         if let Some(f) = self.configure_remote.as_mut() {
-            remote = f(remote)?;
+            remote = f(remote).map_err(|err| Error::RemoteConfiguration(err))?;
         }
 
         let config = util::write_remote_to_local_config_file(&mut remote, remote_name.clone())?;
 
         // Add HEAD after the remote was written to config, we need it to know what to checkout later, and assure
         // the ref that HEAD points to is present no matter what.
-        remote.fetch_specs.push(
-            git_refspec::parse(
-                format!("HEAD:refs/remotes/{remote_name}/HEAD").as_str().into(),
-                git_refspec::parse::Operation::Fetch,
-            )
-            .expect("valid")
-            .to_owned(),
-        );
-        let pending_pack: crate::remote::fetch::Prepare<'_, '_, _, _> = remote
-            .connect(crate::remote::Direction::Fetch, progress)?
-            .prepare_fetch(self.fetch_options.clone())?;
+        let head_refspec = git_refspec::parse(
+            format!("HEAD:refs/remotes/{remote_name}/HEAD").as_str().into(),
+            git_refspec::parse::Operation::Fetch,
+        )
+        .expect("valid")
+        .to_owned();
+        let pending_pack: remote::fetch::Prepare<'_, '_, _, _> =
+            remote.connect(remote::Direction::Fetch, progress)?.prepare_fetch({
+                let mut opts = self.fetch_options.clone();
+                if !opts.extra_refspecs.contains(&head_refspec) {
+                    opts.extra_refspecs.push(head_refspec)
+                }
+                opts
+            })?;
         if pending_pack.ref_map().object_hash != repo.object_hash() {
             unimplemented!("configure repository to expect a different object hash as advertised by the server")
         }
@@ -148,10 +154,13 @@ impl PrepareFetch {
     ///
     /// 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.
+    /// but each change it will eventually be written to the configuration prior to performing a the fetch operation,
+    /// _all changes done in `f()` will be persisted_.
+    ///
+    /// It can also be used to configure additional options, like those for fetching tags.
     pub fn configure_remote(
         mut self,
-        f: impl FnMut(crate::Remote<'_>) -> Result<crate::Remote<'_>, crate::remote::init::Error> + 'static,
+        f: impl FnMut(crate::Remote<'_>) -> Result<crate::Remote<'_>, Box<dyn std::error::Error + Send + Sync>> + 'static,
     ) -> Self {
         self.configure_remote = Some(Box::new(f));
         self

git-repository/src/clone/mod.rs

@@ -2,7 +2,8 @@ use std::convert::TryInto;
 
 use crate::bstr::BString;
 
-type ConfigureRemoteFn = Box<dyn FnMut(crate::Remote<'_>) -> Result<crate::Remote<'_>, crate::remote::init::Error>>;
+type ConfigureRemoteFn =
+    Box<dyn FnMut(crate::Remote<'_>) -> Result<crate::Remote<'_>, Box<dyn std::error::Error + Send + Sync>>>;
 
 /// A utility to collect configuration on how to fetch from a remote and initiate a fetch operation. It will delete the newly
 /// created repository on when dropped without successfully finishing a fetch.

git-repository/src/remote/access.rs

@@ -22,6 +22,11 @@ impl<'repo> Remote<'repo> {
         }
     }
 
+    /// Return how we handle tags when fetching the remote.
+    pub fn fetch_tags(&self) -> remote::fetch::Tags {
+        self.fetch_tags
+    }
+
     /// Return the url used for the given `direction` with rewrites from `url.<base>.insteadOf|pushInsteadOf`, unless the instance
     /// was created with one of the `_without_url_rewrite()` methods.
     /// For pushing, this is the `remote.<name>.pushUrl` or the `remote.<name>.url` used for fetching, and for fetching it's

git-repository/src/remote/build.rs

@@ -23,6 +23,12 @@ impl Remote<'_> {
         self.push_url_inner(url, false)
     }
 
+    /// Configure how tags should be handled when fetching from the remote.
+    pub fn with_fetch_tags(mut self, tags: remote::fetch::Tags) -> Self {
+        self.fetch_tags = tags;
+        self
+    }
+
     fn push_url_inner<Url, E>(mut self, push_url: Url, should_rewrite_urls: bool) -> Result<Self, remote::init::Error>
     where
         Url: TryInto<git_url::Url, Error = E>,

git-repository/src/remote/connection/fetch/receive_pack.rs

@@ -175,6 +175,7 @@ where
                 .unwrap_or_else(|| RefLogMessage::Prefixed { action: "fetch".into() }),
             &self.ref_map.mappings,
             con.remote.refspecs(remote::Direction::Fetch),
+            &self.ref_map.extra_refspecs,
             self.dry_run,
             self.write_packed_refs,
         )?;

git-repository/src/remote/connection/fetch/update_refs/mod.rs

@@ -33,7 +33,7 @@ impl From<update::Mode> for Update {
     }
 }
 
-/// Update all refs as derived from `mappings` and produce an `Outcome` informing about all applied changes in detail, with each
+/// Update all refs as derived from `refmap.mappings` and produce an `Outcome` informing about all applied changes in detail, with each
 /// [`update`][Update] corresponding to the [`fetch::Mapping`] of at the same index.
 /// If `dry_run` is true, ref transactions won't actually be applied, but are assumed to work without error so the underlying
 /// `repo` is not actually changed. Also it won't perform an 'object exists' check as these are likely not to exist as the pack
@@ -46,18 +46,24 @@ pub(crate) fn update(
     message: RefLogMessage,
     mappings: &[fetch::Mapping],
     refspecs: &[git_refspec::RefSpec],
+    extra_refspecs: &[git_refspec::RefSpec],
     dry_run: fetch::DryRun,
     write_packed_refs: fetch::WritePackedRefs,
 ) -> Result<update::Outcome, update::Error> {
     let mut edits = Vec::new();
     let mut updates = Vec::new();
 
+    // TODO: handle impclit tags according to the remote's tagopt - we can't write them as their destinations don't exist.
     for (remote, local, spec) in mappings.iter().filter_map(
         |fetch::Mapping {
              remote,
              local,
              spec_index,
-         }| refspecs.get(*spec_index).map(|spec| (remote, local, spec)),
+         }| {
+            spec_index
+                .get(refspecs, &extra_refspecs)
+                .map(|spec| (remote, local, spec))
+        },
     ) {
         let remote_id = match remote.as_id() {
             Some(id) => id,

git-repository/src/remote/connection/fetch/update_refs/tests.rs

@@ -33,6 +33,7 @@ mod update {
     }
     use git_ref::{transaction::Change, TargetRef};
 
+    use crate::remote::fetch::SpecIndex;
     use crate::{
         bstr::BString,
         remote::{
@@ -128,6 +129,7 @@ mod update {
                 prefixed("action"),
                 &mapping,
                 &specs,
+                &[],
                 reflog_message.map(|_| fetch::DryRun::Yes).unwrap_or(fetch::DryRun::No),
                 fetch::WritePackedRefs::Never,
             )
@@ -190,6 +192,7 @@ mod update {
                 prefixed("action"),
                 &mappings,
                 &specs,
+                &[],
                 fetch::DryRun::Yes,
                 fetch::WritePackedRefs::Never,
             )?;
@@ -219,6 +222,7 @@ mod update {
                 prefixed("action"),
                 &mappings,
                 &specs,
+                &[],
                 fetch::DryRun::Yes,
                 fetch::WritePackedRefs::Never,
             )
@@ -246,13 +250,14 @@ mod update {
                 object: hex_to_id("f99771fe6a1b535783af3163eba95a927aae21d5"),
             }),
             local: Some("refs/heads/symbolic".into()),
-            spec_index: 0,
+            spec_index: SpecIndex::ExplicitInRemote(0),
         });
         let out = fetch::refs::update(
             &repo,
             prefixed("action"),
             &mappings,
             &specs,
+            &[],
             fetch::DryRun::Yes,
             fetch::WritePackedRefs::Never,
         )
@@ -294,6 +299,7 @@ mod update {
             prefixed("action"),
             &mappings,
             &specs,
+            &[],
             fetch::DryRun::Yes,
             fetch::WritePackedRefs::Never,
         )
@@ -318,6 +324,7 @@ mod update {
             prefixed("action"),
             &mappings,
             &specs,
+            &[],
             fetch::DryRun::Yes,
             fetch::WritePackedRefs::Never,
         )
@@ -359,13 +366,14 @@ mod update {
                 object: hex_to_id("f99771fe6a1b535783af3163eba95a927aae21d5"),
             }),
             local: Some("refs/remotes/origin/main".into()),
-            spec_index: 0,
+            spec_index: SpecIndex::ExplicitInRemote(0),
         });
         let out = fetch::refs::update(
             &repo,
             prefixed("action"),
             &mappings,
             &specs,
+            &[],
             fetch::DryRun::Yes,
             fetch::WritePackedRefs::Never,
         )
@@ -412,6 +420,7 @@ mod update {
             },
             &mappings,
             &specs,
+            &[],
             fetch::DryRun::Yes,
             fetch::WritePackedRefs::Never,
         )
@@ -444,6 +453,7 @@ mod update {
             prefixed("action"),
             &mappings,
             &specs,
+            &[],
             fetch::DryRun::No,
             fetch::WritePackedRefs::Never,
         )
@@ -464,6 +474,7 @@ mod update {
             prefixed("prefix"),
             &mappings,
             &specs,
+            &[],
             fetch::DryRun::No,
             fetch::WritePackedRefs::Never,
         )
@@ -495,6 +506,7 @@ mod update {
             prefixed("prefix"),
             &mappings,
             &specs,
+            &[],
             fetch::DryRun::No,
             fetch::WritePackedRefs::Never,
         )
@@ -536,7 +548,7 @@ mod update {
                         _ => unreachable!("not a ref, must be id: {:?}", m),
                     }),
                 local: m.rhs.map(|r| r.into_owned()),
-                spec_index: m.spec_index,
+                spec_index: SpecIndex::ExplicitInRemote(m.spec_index),
             })
             .collect();
         (mappings, vec![spec.to_owned()])

git-repository/src/remote/connection/fetch/update_refs/update.rs

@@ -101,6 +101,7 @@ impl Outcome {
         &self,
         mappings: &'a [fetch::Mapping],
         refspecs: &'b [git_refspec::RefSpec],
+        extra_refspecs: &'b [git_refspec::RefSpec],
     ) -> impl Iterator<
         Item = (
             &super::Update,
@@ -113,7 +114,7 @@ impl Outcome {
             (
                 update,
                 mapping,
-                refspecs.get(mapping.spec_index),
+                mapping.spec_index.get(refspecs, extra_refspecs),
                 update.edit_index.and_then(|idx| self.edits.get(idx)),
             )
         })