refactor

Author: Byron

gitoxide-core/src/pack/receive.rs

@@ -83,17 +83,21 @@ where
         .collect::<Result<_, _>>()?;
     let user_agent = ("agent", Some(agent.clone().into()));
 
-    let fetch_opts = gix::protocol::fetch::refmap::init::Options::fetch(fetch_refspecs.clone());
+    let context = gix::protocol::fetch::refmap::init::Context {
+        fetch_refspecs: fetch_refspecs.clone(),
+        extra_refspecs: vec![],
+    };
     let refmap = match handshake.refs.take() {
-        Some(refs) => gix::protocol::fetch::RefMap::from_refs(refs, &handshake.capabilities, fetch_opts)?,
+        Some(refs) => gix::protocol::fetch::RefMap::from_refs(refs, &handshake.capabilities, context)?,
         None => {
-            gix::protocol::fetch::RefMap::new(
+            gix::protocol::fetch::RefMap::fetch(
                 &mut progress,
                 &handshake.capabilities,
                 &mut transport.inner,
                 user_agent.clone(),
                 trace_packetlines,
-                fetch_opts,
+                true,
+                context,
             )
             .await?
         }

gix-protocol/src/fetch/mod.rs

@@ -5,7 +5,7 @@
 ///
 /// * [handshake](handshake())
 /// * **ls-refs**
-///     * [get available refs by refspecs](RefMap::new())
+///     * [get available refs by refspecs](RefMap::fetch())
 /// * **fetch pack**
 ///     * `negotiate` until a pack can be received (TBD)
 /// * [officially terminate the connection](crate::indicate_end_of_interaction())

gix-protocol/src/fetch/negotiate.rs

@@ -111,7 +111,7 @@ pub struct Round {
 /// * `graph`
 ///     - The commit-graph for use by the `negotiator` - we populate it with tips to initialize the graph traversal.
 /// * `ref_map`
-///     - The references known on the remote, as previously obtained with [`RefMap::new()`].
+///     - The references known on the remote, as previously obtained with [`RefMap::fetch()`].
 /// * `shallow`
 ///     - How to deal with shallow repositories. It does affect how negotiations are performed.
 /// * `mapping_is_ignored`

gix-protocol/src/fetch/refmap/init.rs

@@ -16,7 +16,7 @@ use crate::{
     handshake::Ref,
 };
 
-/// The error returned by [`RefMap::new()`].
+/// The error returned by [`RefMap::fetch()`].
 #[derive(Debug, thiserror::Error)]
 #[allow(missing_docs)]
 pub enum Error {
@@ -28,89 +28,60 @@ pub enum Error {
     ListRefs(#[from] crate::ls_refs::Error),
 }
 
-/// For use in [`RefMap::new()`].
+/// For use in [`RefMap::fetch()`].
 #[derive(Debug, Clone)]
-pub struct Options {
+pub struct Context {
     /// All explicit refspecs to identify references on the remote that you are interested in.
     /// Note that these are copied to [`RefMap::refspecs`] for convenience, as `RefMap::mappings` refer to them by index.
-    fetch_refspecs: Vec<gix_refspec::RefSpec>,
-    /// Use a two-component prefix derived from the ref-spec's source, like `refs/heads/`  to let the server pre-filter refs
-    /// with great potential for savings in traffic and local CPU time. Defaults to `true`.
-    prefix_from_spec_as_filter_on_remote: bool,
+    pub fetch_refspecs: Vec<gix_refspec::RefSpec>,
     /// A list of refspecs to use as implicit refspecs which won't be saved or otherwise be part of the remote in question.
     ///
     /// This is useful for handling `remote.<name>.tagOpt` for example.
-    extra_refspecs: Vec<gix_refspec::RefSpec>,
-    all_refspecs: Vec<gix_refspec::RefSpec>,
+    pub extra_refspecs: Vec<gix_refspec::RefSpec>,
 }
 
-impl Options {
-    /// Create options to fetch the given `fetch_refspecs`.
-    pub fn fetch(fetch_refspecs: Vec<gix_refspec::RefSpec>) -> Self {
-        Self::new(fetch_refspecs, true, Vec::new())
-    }
-
-    /// Specify all options for fetching.
-    pub fn new(
-        fetch_refspecs: Vec<gix_refspec::RefSpec>,
-        prefix_from_spec_as_filter_on_remote: bool,
-        extra_refspecs: Vec<gix_refspec::RefSpec>,
-    ) -> Self {
-        let mut all_refspecs = fetch_refspecs.clone();
-        all_refspecs.extend(extra_refspecs.iter().cloned());
-
-        Self {
-            all_refspecs,
-            fetch_refspecs: fetch_refspecs.clone(),
-            prefix_from_spec_as_filter_on_remote,
-            extra_refspecs,
-        }
-    }
-
-    fn push_prefix_arguments(&self, arguments: &mut Vec<BString>) {
-        if !self.prefix_from_spec_as_filter_on_remote {
-            return;
-        }
-
-        let mut seen = HashSet::new();
-        for spec in &self.all_refspecs {
-            let spec = spec.to_ref();
-            if seen.insert(spec.instruction()) {
-                let mut prefixes = Vec::with_capacity(1);
-                spec.expand_prefixes(&mut prefixes);
-                for mut prefix in prefixes {
-                    prefix.insert_str(0, "ref-prefix ");
-                    arguments.push(prefix);
-                }
-            }
-        }
+impl Context {
+    fn aggregate_refspecs(&self) -> Vec<gix_refspec::RefSpec> {
+        let mut all_refspecs = self.fetch_refspecs.clone();
+        all_refspecs.extend(self.extra_refspecs.iter().cloned());
+        all_refspecs
     }
 }
 
 impl RefMap {
-    /// Create a new instance by obtaining all references on the remote that have been filtered through our remote's
+    /// Create a new instance by obtaining all references on the remote that have been filtered through our remote's specs
     /// for _fetching_.
     ///
     /// * `progress` is used if `ls-refs` is invoked on the remote. Always the case when V2 is used.
+    /// * `capabilities` are the capabilities of the server, obtained by a [handshake](crate::handshake()).
+    /// * `transport` is a way to communicate with the server to obtain the reference listing.
+    /// * `user_agent` is passed to the server.
+    /// * `trace_packetlines` traces all packet lines if `true`, for debugging primarily.
+    /// * `prefix_from_spec_as_filter_on_remote`
+    ///     - Use a two-component prefix derived from the ref-spec's source, like `refs/heads/`  to let the server pre-filter refs
+    ///       with great potential for savings in traffic and local CPU time.
+    /// * `context` to provide more [configuration](Context).
     #[allow(clippy::result_large_err)]
     #[maybe_async::maybe_async]
-    pub async fn new<T>(
+    pub async fn fetch<T>(
         mut progress: impl Progress,
         capabilities: &Capabilities,
         transport: &mut T,
         user_agent: (&'static str, Option<Cow<'static, str>>),
         trace_packetlines: bool,
-        options: Options,
+        prefix_from_spec_as_filter_on_remote: bool,
+        context: Context,
     ) -> Result<Self, Error>
     where
         T: Transport,
     {
         let _span = gix_trace::coarse!("gix_protocol::fetch::RefMap::new()");
+        let all_refspecs = context.aggregate_refspecs();
         let remote_refs = crate::ls_refs(
             transport,
             capabilities,
             |_capabilities, arguments| {
-                options.push_prefix_arguments(arguments);
+                push_prefix_arguments(prefix_from_spec_as_filter_on_remote, arguments, &all_refspecs);
                 Ok(crate::ls_refs::Action::Continue)
             },
             &mut progress,
@@ -119,18 +90,16 @@ impl RefMap {
         )
         .await?;
 
-        Self::from_refs(remote_refs, capabilities, options)
+        Self::from_refs(remote_refs, capabilities, context)
     }
 
-    /// Create a ref-map from already obtained `remote_refs`.
-    pub fn from_refs(remote_refs: Vec<Ref>, capabilities: &Capabilities, options: Options) -> Result<RefMap, Error> {
-        let Options {
+    /// Create a ref-map from already obtained `remote_refs`. Use `context` to pass in refspecs.
+    pub fn from_refs(remote_refs: Vec<Ref>, capabilities: &Capabilities, context: Context) -> Result<RefMap, Error> {
+        let all_refspecs = context.aggregate_refspecs();
+        let Context {
             fetch_refspecs,
             extra_refspecs,
-            all_refspecs,
-            ..
-        } = options;
-
+        } = context;
         let num_explicit_specs = fetch_refspecs.len();
         let group = gix_refspec::MatchGroup::from_fetch_specs(all_refspecs.iter().map(gix_refspec::RefSpec::to_ref));
         let null = gix_hash::ObjectId::null(gix_hash::Kind::Sha1); // OK to hardcode Sha1, it's not supposed to match, ever.
@@ -191,3 +160,26 @@ impl RefMap {
         })
     }
 }
+
+fn push_prefix_arguments(
+    prefix_from_spec_as_filter_on_remote: bool,
+    arguments: &mut Vec<BString>,
+    all_refspecs: &[gix_refspec::RefSpec],
+) {
+    if !prefix_from_spec_as_filter_on_remote {
+        return;
+    }
+
+    let mut seen = HashSet::new();
+    for spec in all_refspecs {
+        let spec = spec.to_ref();
+        if seen.insert(spec.instruction()) {
+            let mut prefixes = Vec::with_capacity(1);
+            spec.expand_prefixes(&mut prefixes);
+            for mut prefix in prefixes {
+                prefix.insert_str(0, "ref-prefix ");
+                arguments.push(prefix);
+            }
+        }
+    }
+}

gix-protocol/src/fetch/types.rs

@@ -18,7 +18,7 @@ pub struct Options<'a> {
     pub reject_shallow_remote: bool,
 }
 
-/// For use in [`RefMap::new()`] and [`fetch`](crate::fetch()).
+/// For use in [`RefMap::fetch()`] and [`fetch`](crate::fetch()).
 #[cfg(feature = "handshake")]
 pub struct Context<'a, T> {
     /// The outcome of the handshake performed with the remote.
@@ -29,7 +29,7 @@ pub struct Context<'a, T> {
     ///
     /// This is always done if the underlying protocol is V2, which is implied by the absence of refs in the `handshake` outcome.
     pub transport: &'a mut T,
-    /// How to self-identify during the `ls-refs` call in [`RefMap::new()`] or the `fetch` call in [`fetch()`](crate::fetch()).
+    /// How to self-identify during the `ls-refs` call in [`RefMap::fetch()`] or the `fetch` call in [`fetch()`](crate::fetch()).
     ///
     /// This could be read from the `gitoxide.userAgent` configuration variable.
     pub user_agent: (&'static str, Option<std::borrow::Cow<'static, str>>),
@@ -123,7 +123,7 @@ mod with_fetch {
         /// [`refmap::SpecIndex::ExplicitInRemote`] in [`refmap::Mapping`].
         pub refspecs: Vec<gix_refspec::RefSpec>,
         /// Refspecs which have been added implicitly due to settings of the `remote`, usually pre-initialized from
-        /// [`extra_refspecs` in RefMap options](refmap::init::Options).
+        /// [`extra_refspecs` in RefMap options](refmap::init::Context).
         /// They are referred to by [`refmap::SpecIndex::Implicit`] in [`refmap::Mapping`].
         ///
         /// They are never persisted nor are they typically presented to the user.

gix-protocol/src/ls_refs.rs

@@ -60,9 +60,10 @@ pub(crate) mod function {
         indicate_end_of_interaction, Command,
     };
 
-    /// Invoke an ls-refs V2 command on `transport`, which requires a prior handshake that yielded
-    /// server `capabilities`. `prepare_ls_refs(capabilities, arguments, features)` can be used to alter the _ls-refs_. `progress` is used to provide feedback.
-    /// Note that `prepare_ls_refs()` is expected to add the `(agent, Some(name))` to the list of `features`.
+    /// Invoke a ls-refs V2 command on `transport`, which requires a prior handshake that yielded
+    /// server `capabilities`. `prepare_ls_refs(capabilities, arguments)` can be used to alter the _ls-refs_.
+    /// `progress` is used to provide feedback.
+    /// The `agent` information will be added to the features sent to the server.
     /// If `trace` is `true`, all packetlines received or sent will be passed to the facilities of the `gix-trace` crate.
     #[maybe_async]
     pub async fn ls_refs(

gix-transport/Cargo.toml

@@ -18,8 +18,6 @@ doctest = false
 default = []
 
 ## If set, blocking implementations of the typical git transports become available in `crate::client::blocking_io`
-##
-## If used in conjunction with an async implementation, this one takes precedence.
 blocking-client = ["gix-packetline/blocking-io"]
 ## Implies `blocking-client`, and adds support for the http and https transports.
 http-client = [

gix/src/remote/connection/ref_map.rs

@@ -151,31 +151,26 @@ where
         )
         .await?;
 
-        let fetch_opts = gix_protocol::fetch::refmap::init::Options::new(
-            self.remote.fetch_specs.clone(),
-            prefix_from_spec_as_filter_on_remote,
+        let context = gix_protocol::fetch::refmap::init::Context {
+            fetch_refspecs: self.remote.fetch_specs.clone(),
             extra_refspecs,
-        );
-
-        Ok(match handshake.refs.take() {
-            Some(refs) => {
-                let refmap = fetch::RefMap::from_refs(refs, &handshake.capabilities, fetch_opts)?;
-                self.handshake = Some(handshake);
-                refmap
-            }
+        };
+        let ref_map = match handshake.refs.take() {
+            Some(refs) => fetch::RefMap::from_refs(refs, &handshake.capabilities, context)?,
             None => {
-                let refmap = gix_protocol::fetch::RefMap::new(
+                gix_protocol::fetch::RefMap::fetch(
                     progress,
                     &handshake.capabilities,
                     &mut self.transport.inner,
                     self.remote.repo.config.user_agent_tuple(),
                     self.trace,
-                    fetch_opts,
+                    prefix_from_spec_as_filter_on_remote,
+                    context,
                 )
-                .await?;
-                self.handshake = Some(handshake);
-                refmap
+                .await?
             }
-        })
+        };
+        self.handshake = Some(handshake);
+        Ok(ref_map)
     }
 }