add precompose_unicode related tests

* reading precomposes unicode
* writing precomposes unicode of input names for good measure

Author: Byron

Cargo.lock

@@ -25,6 +25,7 @@ gix-path = { version = "^0.10.3", path = "../gix-path" }
 gix-hash = { version = "^0.14.1", path = "../gix-hash" }
 gix-date = { version = "^0.8.3", path = "../gix-date" }
 gix-object = { version = "^0.40.1", path = "../gix-object" }
+gix-utils = { version = "^0.1.8", path = "../gix-utils" }
 gix-validate = { version = "^0.8.3", path = "../gix-validate" }
 gix-actor = { version = "^0.29.1", path = "../gix-actor" }
 gix-lock = { version = "^12.0.0", path = "../gix-lock" }

gix-ref/Cargo.toml

@@ -10,9 +10,19 @@ pub use error::Error;
 use crate::{
     file,
     store_impl::{file::loose, packed},
-    BStr, BString, FullNameRef, PartialNameRef, Reference,
+    BStr, BString, FullNameRef, PartialName, PartialNameRef, Reference,
 };
 
+/// ### Finding References - notes about precomposed unicode.
+///
+/// Generally, ref names and the target of symbolic refs are stored as-is if [`Self::precompose_unicode`] is `false`.
+/// If `true`, refs are stored as precomposed unicode in `packed-refs`, but stored as is on disk as it is then assumed
+/// to be indifferent, i.e. `"a\u{308}"` is the same as `"ä"`.
+///
+/// This also means that when refs are packed for transmission to another machine, both their names and the target of
+/// symbolic references need to be precomposed.
+///
+/// Namespaces are left as is as they never get past the particular repository that uses them.
 impl file::Store {
     /// Find a single reference by the given `path` which is required to be a valid reference name.
     ///
@@ -66,27 +76,62 @@ impl file::Store {
         partial_name: &PartialNameRef,
         packed: Option<&packed::Buffer>,
     ) -> Result<Option<Reference>, Error> {
+        fn decompose_if(mut r: Reference, input_changed_to_precomposed: bool) -> Reference {
+            if input_changed_to_precomposed {
+                use gix_object::bstr::ByteSlice;
+                let decomposed = r
+                    .name
+                    .0
+                    .to_str()
+                    .ok()
+                    .map(|name| gix_utils::str::decompose(name.into()));
+                if let Some(Cow::Owned(decomposed)) = decomposed {
+                    r.name.0 = decomposed.into();
+                }
+            }
+            r
+        }
         let mut buf = BString::default();
+        let mut precomposed_partial_name_storage = packed.filter(|_| self.precompose_unicode).and_then(|_| {
+            use gix_object::bstr::ByteSlice;
+            let precomposed = partial_name.0.to_str().ok()?;
+            let precomposed = gix_utils::str::precompose(precomposed.into());
+            match precomposed {
+                Cow::Owned(precomposed) => Some(PartialName(precomposed.into())),
+                Cow::Borrowed(_) => None,
+            }
+        });
+        let precomposed_partial_name = precomposed_partial_name_storage
+            .as_ref()
+            .map(std::convert::AsRef::as_ref);
         for inbetween in &["", "tags", "heads", "remotes"] {
-            match self.find_inner(inbetween, partial_name, packed, &mut buf) {
-                Ok(Some(r)) => return Ok(Some(r)),
+            match self.find_inner(inbetween, partial_name, precomposed_partial_name, packed, &mut buf) {
+                Ok(Some(r)) => return Ok(Some(decompose_if(r, precomposed_partial_name.is_some()))),
                 Ok(None) => {
                     continue;
                 }
                 Err(err) => return Err(err),
             }
         }
         if partial_name.as_bstr() != "HEAD" {
+            if let Some(mut precomposed) = precomposed_partial_name_storage {
+                precomposed = precomposed.join("HEAD".into()).expect("HEAD is valid name");
+                precomposed_partial_name_storage = Some(precomposed);
+            }
             self.find_inner(
                 "remotes",
                 partial_name
                     .to_owned()
                     .join("HEAD".into())
                     .expect("HEAD is valid name")
                     .as_ref(),
+                precomposed_partial_name_storage
+                    .as_ref()
+                    .map(std::convert::AsRef::as_ref),
                 None,
                 &mut buf,
             )
+            .map(|res| res.map(|r| decompose_if(r, precomposed_partial_name_storage.is_some())))
         } else {
             Ok(None)
         }
@@ -96,10 +141,13 @@ impl file::Store {
         &self,
         inbetween: &str,
         partial_name: &PartialNameRef,
+        precomposed_partial_name: Option<&PartialNameRef>,
         packed: Option<&packed::Buffer>,
         path_buf: &mut BString,
     ) -> Result<Option<Reference>, Error> {
-        let full_name = partial_name.construct_full_name_ref(inbetween, path_buf);
+        let full_name = precomposed_partial_name
+            .unwrap_or(partial_name)
+            .construct_full_name_ref(inbetween, path_buf);
         let content_buf = self.ref_contents(full_name).map_err(|err| Error::ReadFileContents {
             source: err,
             path: self.reference_path(full_name),

gix-ref/src/store/file/find.rs

@@ -33,7 +33,9 @@ mod init {
         /// Create a new instance at the given `git_dir`, which commonly is a standard git repository with a
         /// `refs/` subdirectory.
         /// The `object_hash` defines which kind of hash we should recognize.
-        /// `precompose_unicode` is used to set to the value of [`Self::precompose_unicode].
+        ///
+        /// Note that if `precompose_unicode` is set, the `git_dir` is also expected to use precomposed unicode,
+        /// or else some operations that strip prefixes will fail.
         pub fn at(
             git_dir: PathBuf,
             write_reflog: file::WriteReflog,
@@ -54,6 +56,9 @@ mod init {
 
         /// Like [`at()`][file::Store::at()], but for _linked_ work-trees which use `git_dir` as private ref store and `common_dir` for
         /// shared references.
+        ///
+        /// Note that if `precompose_unicode` is set, the `git_dir` and `common_dir` are also expected to use precomposed unicode,
+        /// or else some operations that strip prefixes will fail.
         pub fn for_linked_worktree(
             git_dir: PathBuf,
             common_dir: PathBuf,

gix-ref/src/store/file/loose/mod.rs

@@ -29,6 +29,10 @@ pub struct Store {
     pub namespace: Option<Namespace>,
     /// If set, we will convert decomposed unicode like `a\u308` into precomposed unicode like `ä` when reading
     /// ref names from disk.
+    /// Note that this is an internal operation that isn't observable on the outside, but it's needed for lookups
+    /// to packed-refs or symlinks to work correctly.
+    /// Iterated references will be returned verbatim, thus when sending them over the wire they have to be precomposed
+    /// as needed.
     pub precompose_unicode: bool,
     /// A packed buffer which can be mapped in one version and shared as such.
     /// It's updated only in one spot, which is prior to reading it based on file stamps.

gix-ref/src/store/file/mod.rs

@@ -208,6 +208,9 @@ impl file::Store {
     /// Return a platform to obtain iterator over all references, or prefixed ones, loose or packed, sorted by their name.
     ///
     /// Errors are returned similarly to what would happen when loose and packed refs where iterated by themselves.
+    ///
+    /// Note that since packed-refs are storing refs as precomposed unicode if [`Self::precompose_unicode`] is true, for consistency
+    /// we also return loose references as precomposed unicode.
     pub fn iter(&self) -> Result<Platform<'_>, packed::buffer::open::Error> {
         Ok(Platform {
             store: self,

gix-ref/src/store/file/overlay_iter.rs

@@ -16,6 +16,8 @@ impl file::Store {
         Ok(packed::Transaction::new_from_pack_and_lock(
             self.assure_packed_refs_uptodate()?,
             lock,
+            self.precompose_unicode,
+            self.namespace.clone(),
         ))
     }
 

gix-ref/src/store/file/packed.rs

@@ -11,14 +11,21 @@ use crate::{
 /// How to handle packed refs during a transaction
 #[derive(Default)]
 pub enum PackedRefs<'a> {
-    /// Only propagate deletions of references. This is the default
+    /// Only propagate deletions of references. This is the default.
+    /// This means deleted references are removed from disk if they are loose and from the packed-refs file if they are present.
     #[default]
     DeletionsOnly,
-    /// Propagate deletions as well as updates to references which are peeled, that is contain an object id
+    /// Propagate deletions as well as updates to references which are peeled and contain an object id.
+    ///
+    /// This means deleted references are removed from disk if they are loose and from the packed-refs file if they are present,
+    /// while updates are also written into the loose file as well as into packed-refs, potentially creating an entry.
     DeletionsAndNonSymbolicUpdates(Box<dyn gix_object::Find + 'a>),
-    /// Propagate deletions as well as updates to references which are peeled, that is contain an object id. Furthermore delete the
+    /// Propagate deletions as well as updates to references which are peeled and contain an object id. Furthermore delete the
     /// reference which is originally updated if it exists. If it doesn't, the new value will be written into the packed ref right away.
     /// Note that this doesn't affect symbolic references at all, which can't be placed into packed refs.
+    ///
+    /// Thus, this is similar to `DeletionsAndNonSymbolicUpdates`, but removes the loose reference after the update, leaving only their copy
+    /// in `packed-refs`.
     DeletionsAndNonSymbolicUpdatesRemoveLooseSourceReference(Box<dyn gix_object::Find + 'a>),
 }
 

gix-ref/src/store/file/transaction/mod.rs

@@ -329,8 +329,13 @@ impl<'s, 'p> Transaction<'s, 'p> {
                         self.store
                             .assure_packed_refs_uptodate()?
                             .map(|p| {
-                                buffer_into_transaction(p, packed_refs_lock_fail_mode)
-                                    .map_err(Error::PackedTransactionAcquire)
+                                buffer_into_transaction(
+                                    p,
+                                    packed_refs_lock_fail_mode,
+                                    self.store.precompose_unicode,
+                                    self.store.namespace.clone(),
+                                )
+                                .map_err(Error::PackedTransactionAcquire)
                             })
                             .transpose()?
                     };

gix-ref/src/store/file/transaction/prepare.rs

@@ -22,6 +22,9 @@ impl crate::Store {
     ///
     /// `object_hash` defines the kind of hash to assume when dealing with refs.
     /// `precompose_unicode` is used to set to the value of [`crate::file::Store::precompose_unicode].
+    ///
+    /// Note that if `precompose_unicode` is set, the `git_dir` is also expected to use precomposed unicode,
+    /// or else some operations that strip prefixes will fail.
     pub fn at(
         git_dir: PathBuf,
         reflog_mode: WriteReflog,

gix-ref/src/store/general/init.rs

@@ -4,7 +4,7 @@ use gix_hash::ObjectId;
 use gix_object::bstr::{BStr, BString};
 use memmap2::Mmap;
 
-use crate::{file, transaction::RefEdit, FullNameRef};
+use crate::{file, transaction::RefEdit, FullNameRef, Namespace};
 
 #[derive(Debug)]
 enum Backing {
@@ -38,6 +38,9 @@ pub(crate) struct Transaction {
     lock: Option<gix_lock::File>,
     #[allow(dead_code)] // It just has to be kept alive, hence no reads
     closed_lock: Option<gix_lock::Marker>,
+    precompose_unicode: bool,
+    /// The namespace to use when preparing or writing refs
+    namespace: Option<Namespace>,
 }
 
 /// A reference as parsed from the `packed-refs` file

gix-ref/src/store/packed/mod.rs

@@ -1,10 +1,11 @@
+use std::borrow::Cow;
 use std::{fmt::Formatter, io::Write};
 
 use crate::{
     file,
     store_impl::{packed, packed::Edit},
     transaction::{Change, RefEdit},
-    Target,
+    Namespace, Target,
 };
 
 pub(crate) const HEADER_LINE: &[u8] = b"# pack-refs with: peeled fully-peeled sorted \n";
@@ -14,12 +15,16 @@ impl packed::Transaction {
     pub(crate) fn new_from_pack_and_lock(
         buffer: Option<file::packed::SharedBufferSnapshot>,
         lock: gix_lock::File,
+        precompose_unicode: bool,
+        namespace: Option<Namespace>,
     ) -> Self {
         packed::Transaction {
             buffer,
             edits: None,
             lock: Some(lock),
             closed_lock: None,
+            precompose_unicode,
+            namespace,
         }
     }
 }
@@ -55,6 +60,32 @@ impl packed::Transaction {
         // Remove all edits which are deletions that aren't here in the first place
         let mut edits: Vec<Edit> = edits
             .into_iter()
+            .map(|mut edit| {
+                use gix_object::bstr::ByteSlice;
+                if self.precompose_unicode {
+                    let precomposed = edit
+                        .name
+                        .0
+                        .to_str()
+                        .ok()
+                        .map(|name| gix_utils::str::precompose(name.into()));
+                    match precomposed {
+                        None | Some(Cow::Borrowed(_)) => edit,
+                        Some(Cow::Owned(precomposed)) => {
+                            edit.name.0 = precomposed.into();
+                            edit
+                        }
+                    }
+                } else {
+                    edit
+                }
+            })
+            .map(|mut edit| {
+                if let Some(namespace) = &self.namespace {
+                    edit.name = namespace.clone().into_namespaced_name(edit.name.as_ref());
+                }
+                edit
+            })
             .filter(|edit| {
                 if let Change::Delete { .. } = edit.change {
                     buffer.as_ref().map_or(true, |b| b.find(edit.name.as_ref()).is_ok())
@@ -227,13 +258,17 @@ fn write_edit(out: &mut dyn std::io::Write, edit: &Edit, lines_written: &mut i32
 pub(crate) fn buffer_into_transaction(
     buffer: file::packed::SharedBufferSnapshot,
     lock_mode: gix_lock::acquire::Fail,
+    precompose_unicode: bool,
+    namespace: Option<Namespace>,
 ) -> Result<packed::Transaction, gix_lock::acquire::Error> {
     let lock = gix_lock::File::acquire_to_update_resource(&buffer.path, lock_mode, None)?;
     Ok(packed::Transaction {
         buffer: Some(buffer),
         lock: Some(lock),
         closed_lock: None,
         edits: None,
+        precompose_unicode,
+        namespace,
     })
 }