Merge branch 'fixes-and-improvements'

Author: Byron

cargo-smart-release/Cargo.lock

@@ -28,11 +28,11 @@ gix = { version = "^0.50.0", default-features = false, features = ["max-performa
 anyhow = "1.0.42"
 clap = { version = "4.1.0", features = ["derive", "cargo"] }
 env_logger = { version = "0.10.0", default-features = false, features = ["humantime", "auto-color"] }
-cargo_metadata = "0.15.0"
+cargo_metadata = "0.17.0"
 log = "0.4.14"
 toml_edit = "0.19.1"
 semver = "1.0.4"
-crates-index = { version = "2.0.0", default-features = false, features = ["git-performance", "git-https"] }
+crates-index = { version = "2.1.0", default-features = false, features = ["git-performance", "git-https"] }
 cargo_toml = "0.15.1"
 winnow = "0.5.1"
 git-conventional = "0.12.0"

cargo-smart-release/Cargo.toml

@@ -1,19 +1,12 @@
-use std::path::PathBuf;
-
 pub struct Index {
     inner: Option<crates_index::GitIndex>,
 }
 
 impl Index {
     /// Like the original one, but doesn't create it if it doesn't exist
     pub fn new_cargo_default() -> Result<Index, crates_index::Error> {
-        let path = default_path();
         Ok(Index {
-            inner: if path.is_dir() {
-                crates_index::GitIndex::new_cargo_default()?.into()
-            } else {
-                None
-            },
+            inner: crates_index::GitIndex::try_new_cargo_default()?,
         })
     }
 
@@ -29,9 +22,3 @@ impl Index {
         self.inner.as_ref().and_then(|idx| idx.crate_(name))
     }
 }
-
-fn default_path() -> PathBuf {
-    crates_index::local_path_and_canonical_url(crates_index::git::URL, None)
-        .expect("defaults are well-known")
-        .0
-}

cargo-smart-release/src/crates_index.rs

@@ -18,6 +18,7 @@ pub const PROGRESS_RANGE: std::ops::RangeInclusive<u8> = 1..=3;
 
 pub(crate) mod function {
     use anyhow::bail;
+    use gix::remote::fetch::refs::update::TypeChange;
     use gix::{prelude::ObjectIdExt, refspec::match_group::validate::Fix, remote::fetch::Status};
     use layout::{
         backends::svg::SVGWriter,
@@ -261,11 +262,28 @@ pub(crate) mod function {
                     crate::repository::remote::refs::print_ref(&mut out, r)?;
                 }
             };
+            let mode_and_type = update.type_change.map_or_else(
+                || format!("{}", update.mode),
+                |type_change| {
+                    format!(
+                        "{} ({})",
+                        update.mode,
+                        match type_change {
+                            TypeChange::DirectToSymbolic => {
+                                "direct ref overwrites symbolic"
+                            }
+                            TypeChange::SymbolicToDirect => {
+                                "symbolic ref overwrites direct"
+                            }
+                        }
+                    )
+                },
+            );
             match edit {
                 Some(edit) => {
-                    writeln!(out, " -> {} [{}]", edit.name, update.mode)
+                    writeln!(out, " -> {} [{mode_and_type}]", edit.name)
                 }
-                None => writeln!(out, " [{}]", update.mode),
+                None => writeln!(out, " [{mode_and_type}]"),
             }?;
         }
         consume_skipped_tags(&mut skipped_due_to_implicit_tag, &mut out)?;

gitoxide-core/src/repository/fetch.rs

@@ -244,6 +244,8 @@ mod refs_impl {
         },
         Symbolic {
             path: String,
+            #[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))]
+            tag: Option<String>,
             target: String,
             object: String,
         },
@@ -265,10 +267,12 @@ mod refs_impl {
                 },
                 handshake::Ref::Symbolic {
                     full_ref_name: path,
+                    tag,
                     target,
                     object,
                 } => JsonRef::Symbolic {
                     path: path.to_string(),
+                    tag: tag.map(|t| t.to_string()),
                     target: target.to_string(),
                     object: object.to_string(),
                 },
@@ -298,9 +302,15 @@ mod refs_impl {
             } => write!(&mut out, "{tag} {path} object:{object}").map(|_| tag.as_ref()),
             handshake::Ref::Symbolic {
                 full_ref_name: path,
+                tag,
                 target,
                 object,
-            } => write!(&mut out, "{object} {path} symref-target:{target}").map(|_| object.as_ref()),
+            } => match tag {
+                Some(tag) => {
+                    write!(&mut out, "{tag} {path} symref-target:{target} peeled:{object}").map(|_| tag.as_ref())
+                }
+                None => write!(&mut out, "{object} {path} symref-target:{target}").map(|_| object.as_ref()),
+            },
             handshake::Ref::Unborn { full_ref_name, target } => {
                 static NULL: gix::hash::ObjectId = gix::hash::ObjectId::null(gix::hash::Kind::Sha1);
                 write!(&mut out, "unborn {full_ref_name} symref-target:{target}").map(|_| NULL.as_ref())

gitoxide-core/src/repository/remote.rs

@@ -2,6 +2,7 @@ pub(crate) mod function {
     use bstr::ByteSlice;
     use btoi::btoi;
     use gix_date::{time::Sign, OffsetInSeconds, SecondsSinceUnixEpoch, Time};
+    use nom::multi::many1_count;
     use nom::{
         branch::alt,
         bytes::complete::{tag, take, take_until, take_while_m_n},
@@ -10,6 +11,7 @@ pub(crate) mod function {
         sequence::{terminated, tuple},
         IResult,
     };
+    use std::cell::RefCell;
 
     use crate::{IdentityRef, SignatureRef};
 
@@ -19,7 +21,9 @@ pub(crate) mod function {
     pub fn decode<'a, E: ParseError<&'a [u8]> + ContextError<&'a [u8]>>(
         i: &'a [u8],
     ) -> IResult<&'a [u8], SignatureRef<'a>, E> {
-        let (i, (identity, _, time, tzsign, hours, minutes)) = context(
+        use nom::Parser;
+        let tzsign = RefCell::new(b'-'); // TODO: there should be no need for this.
+        let (i, (identity, _, time, _tzsign_count, hours, minutes)) = context(
             "<name> <<email>> <timestamp> <+|-><HHMM>",
             tuple((
                 identity,
@@ -31,7 +35,13 @@ pub(crate) mod function {
                             .map_err(|_| nom::Err::Error(E::from_error_kind(i, nom::error::ErrorKind::MapRes)))
                     })
                 }),
-                context("+|-", alt((tag(b"-"), tag(b"+")))),
+                context(
+                    "+|-",
+                    alt((
+                        many1_count(tag(b"-")).map(|_| *tzsign.borrow_mut() = b'-'), // TODO: this should be a non-allocating consumer of consecutive tags
+                        many1_count(tag(b"+")).map(|_| *tzsign.borrow_mut() = b'+'),
+                    )),
+                ),
                 context("HH", |i| {
                     take_while_m_n(2usize, 2, is_digit)(i).and_then(|(i, v)| {
                         btoi::<OffsetInSeconds>(v)
@@ -40,7 +50,7 @@ pub(crate) mod function {
                     })
                 }),
                 context("MM", |i| {
-                    take_while_m_n(2usize, 2, is_digit)(i).and_then(|(i, v)| {
+                    take_while_m_n(1usize, 2, is_digit)(i).and_then(|(i, v)| {
                         btoi::<OffsetInSeconds>(v)
                             .map(|v| (i, v))
                             .map_err(|_| nom::Err::Error(E::from_error_kind(i, nom::error::ErrorKind::MapRes)))
@@ -49,8 +59,9 @@ pub(crate) mod function {
             )),
         )(i)?;
 
-        debug_assert!(tzsign[0] == b'-' || tzsign[0] == b'+', "parser assure it's +|- only");
-        let sign = if tzsign[0] == b'-' { Sign::Minus } else { Sign::Plus }; //
+        let tzsign = tzsign.into_inner();
+        debug_assert!(tzsign == b'-' || tzsign == b'+', "parser assure it's +|- only");
+        let sign = if tzsign == b'-' { Sign::Minus } else { Sign::Plus }; //
         let offset = (hours * 3600 + minutes * 60) * if sign == Sign::Minus { -1 } else { 1 };
 
         Ok((
@@ -148,6 +159,16 @@ mod tests {
             );
         }
 
+        #[test]
+        fn negative_offset_double_dash() {
+            assert_eq!(
+                decode(b"name <[email protected]> 1288373970 --700")
+                    .expect("parse to work")
+                    .1,
+                signature("name", "[email protected]", 1288373970, Sign::Minus, -252000)
+            );
+        }
+
         #[test]
         fn empty_name_and_email() {
             assert_eq!(

gix-actor/src/signature/decode.rs

@@ -160,6 +160,32 @@ fn pre_epoch() -> crate::Result {
     Ok(())
 }
 
+#[test]
+fn double_dash_special_time_offset() -> crate::Result {
+    let signature = || SignatureRef {
+        name: "name".into(),
+        email: "[email protected]".into(),
+        time: Time {
+            seconds: 1288373970,
+            offset: -252000,
+            sign: Sign::Minus,
+        },
+    };
+    assert_eq!(
+        CommitRef::from_bytes(&fixture_name("commit", "double-dash-date-offset.txt"))?,
+        CommitRef {
+            tree: b"0a851d7a2a66084ab10516c406a405d147e974ad".as_bstr(),
+            parents: SmallVec::from(vec![b"31350f4f0f459485eff2131517e3450cf251f6fa".as_bstr()]),
+            author: signature(),
+            committer: signature(),
+            encoding: None,
+            message: "msg\n".into(),
+            extra_headers: vec![]
+        }
+    );
+    Ok(())
+}
+
 #[test]
 fn with_trailer() -> crate::Result {
     let kim = SignatureRef {

gix-object/tests/commit/from_bytes.rs

@@ -0,0 +1,6 @@
+tree 0a851d7a2a66084ab10516c406a405d147e974ad
+parent 31350f4f0f459485eff2131517e3450cf251f6fa
+author name <[email protected]> 1288373970 --700
+committer name <[email protected]> 1288373970 --700
+
+msg

gix-object/tests/fixtures/commit/double-dash-date-offset.txt

@@ -493,6 +493,11 @@ impl super::Store {
         // Unlike libgit2, do not sort by modification date, but by size and put the biggest indices first. That way
         // the chance to hit an object should be higher. We leave it to the handle to sort by LRU.
         // Git itself doesn't change the order which may safe time, but we want it to be stable which also helps some tests.
+        // NOTE: this will work well for well-packed repos or those using geometric repacking, but force us to open a lot
+        //       of files when dealing with new objects, as there is no notion of recency here as would be with unmaintained
+        //       repositories. Different algorithms should be provided, like newest packs first, and possibly a mix of both
+        //       with big packs first, then sorting by recency for smaller packs.
+        //       We also want to implement `fetch.unpackLimit` to alleviate this issue a little.
         indices_by_modification_time.sort_by(|l, r| l.2.cmp(&r.2).reverse());
         Ok(indices_by_modification_time)
     }

gix-odb/src/store_impls/dynamic/load_index.rs

@@ -45,9 +45,7 @@ impl Arguments {
                 }
                 transport.invoke(
                     Command::Fetch.as_str(),
-                    self.features
-                        .iter()
-                        .filter_map(|(k, v)| v.as_ref().map(|v| (*k, Some(v.as_ref())))),
+                    self.features.iter().filter(|(_, v)| v.is_some()).cloned(),
                     Some(std::mem::replace(&mut self.args, retained_state).into_iter()),
                 )
             }

gix-protocol/src/fetch/arguments/blocking_io.rs

@@ -165,20 +165,30 @@ impl Arguments {
     pub fn use_include_tag(&mut self) {
         debug_assert!(self.supports_include_tag, "'include-tag' feature required");
         if self.supports_include_tag {
-            match self.version {
-                gix_transport::Protocol::V0 | gix_transport::Protocol::V1 => {
-                    let features = self
-                        .features_for_first_want
-                        .as_mut()
-                        .expect("call use_include_tag before want()");
-                    features.push("include-tag".into())
-                }
-                gix_transport::Protocol::V2 => {
-                    self.args.push("include-tag".into());
-                }
+            self.add_feature("include-tag");
+        }
+    }
+
+    /// Add the given `feature`, unconditionally.
+    ///
+    /// Note that sending an unknown or unsupported feature may cause the remote to terminate
+    /// the connection. Use this method if you know what you are doing *and* there is no specialized
+    /// method for this, e.g. [`Self::use_include_tag()`].
+    pub fn add_feature(&mut self, feature: &str) {
+        match self.version {
+            gix_transport::Protocol::V0 | gix_transport::Protocol::V1 => {
+                let features = self
+                    .features_for_first_want
+                    .as_mut()
+                    .expect("call add_feature before first want()");
+                features.push(feature.into())
+            }
+            gix_transport::Protocol::V2 => {
+                self.args.push(feature.into());
             }
         }
     }
+
     fn prefixed(&mut self, prefix: &str, value: impl fmt::Display) {
         self.args.push(format!("{prefix}{value}").into());
     }

gix-protocol/src/fetch/arguments/mod.rs

@@ -319,6 +319,7 @@ mod arguments {
             assert!(arguments.is_stateless(true), "V2 is stateless…");
             assert!(arguments.is_stateless(false), "…in all cases");
 
+            arguments.add_feature("no-progress");
             arguments.deepen(1);
             arguments.deepen_relative();
             arguments.want(id("7b333369de1221f9bfbbe03a3a13e9a09bc1c907"));
@@ -329,6 +330,7 @@ mod arguments {
                 b"0012command=fetch
 0001000ethin-pack
 000eofs-delta
+0010no-progress
 000ddeepen 1
 0014deepen-relative
 0032want 7b333369de1221f9bfbbe03a3a13e9a09bc1c907
@@ -347,6 +349,7 @@ mod arguments {
                 let mut t = transport(&mut out, *is_stateful);
                 let mut arguments = arguments_v2(Some("shallow"));
 
+                arguments.add_feature("no-progress");
                 arguments.deepen(1);
                 arguments.deepen_since(12345);
                 arguments.shallow(id("7b333369de1221f9bfbbe03a3a13e9a09bc1c9ff"));
@@ -362,6 +365,7 @@ mod arguments {
                     b"0012command=fetch
 0001000ethin-pack
 000eofs-delta
+0010no-progress
 000ddeepen 1
 0017deepen-since 12345
 0035shallow 7b333369de1221f9bfbbe03a3a13e9a09bc1c9ff
@@ -371,6 +375,7 @@ mod arguments {
 00000012command=fetch
 0001000ethin-pack
 000eofs-delta
+0010no-progress
 000ddeepen 1
 0017deepen-since 12345
 0035shallow 7b333369de1221f9bfbbe03a3a13e9a09bc1c9ff

gix-protocol/src/fetch/tests.rs

@@ -21,7 +21,7 @@ pub enum Ref {
         /// The hash of the object the ref points to.
         object: gix_hash::ObjectId,
     },
-    /// A symbolic ref pointing to `target` ref, which in turn points to an `object`
+    /// A symbolic ref pointing to `target` ref, which in turn, ultimately after possibly following `tag`, points to an `object`
     Symbolic {
         /// The name at which the symbolic ref is located, like `HEAD`.
         full_ref_name: BString,
@@ -31,7 +31,11 @@ pub enum Ref {
         ///
         /// [#205]: https://github.com/Byron/gitoxide/issues/205
         target: BString,
-        /// The hash of the object the `target` ref points to.
+        /// The hash of the annotated tag the ref points to, if present.
+        ///
+        /// Note that this field is also `None` if `full_ref_name` is a lightweight tag.
+        tag: Option<gix_hash::ObjectId>,
+        /// The hash of the object the `target` ref ultimately points to.
         object: gix_hash::ObjectId,
     },
     /// A ref is unborn on the remote and just points to the initial, unborn branch, as is the case in a newly initialized repository