Merge pull request #2158 from GitoxideLabs/improvements

improvements

Author: Byron

gix-diff/src/blob/pipeline.rs

@@ -135,6 +135,8 @@ pub mod convert_to_diffable {
     pub enum Error {
         #[error("Entry at '{rela_path}' must be regular file or symlink, but was {actual:?}")]
         InvalidEntryKind { rela_path: BString, actual: EntryKind },
+        #[error("Entry at '{rela_path}' is declared as symlink but symlinks are disabled via core.symlinks")]
+        SymlinkDisabled { rela_path: BString },
         #[error("Entry at '{rela_path}' could not be read as symbolic link")]
         ReadLink { rela_path: BString, source: std::io::Error },
         #[error("Entry at '{rela_path}' could not be opened for reading or read from")]
@@ -240,7 +242,7 @@ impl Pipeline {
         out: &mut Vec<u8>,
     ) -> Result<Outcome, convert_to_diffable::Error> {
         let is_symlink = match mode {
-            EntryKind::Link if self.options.fs.symlink => true,
+            EntryKind::Link => true,
             EntryKind::Blob | EntryKind::BlobExecutable => false,
             _ => {
                 return Err(convert_to_diffable::Error::InvalidEntryKind {
@@ -272,6 +274,11 @@ impl Pipeline {
                 self.path.push(root);
                 self.path.push(gix_path::from_bstr(rela_path));
                 let data = if is_symlink {
+                    if !self.options.fs.symlink {
+                        return Err(convert_to_diffable::Error::SymlinkDisabled {
+                            rela_path: rela_path.to_owned(),
+                        });
+                    }
                     let target = none_if_missing(std::fs::read_link(&self.path)).map_err(|err| {
                         convert_to_diffable::Error::ReadLink {
                             rela_path: rela_path.to_owned(),

gix-ref/tests/refs/file/transaction/prepare_and_commit/delete.rs

@@ -1,18 +1,19 @@
-use gix_date::parse::TimeBuf;
-use gix_lock::acquire::Fail;
-use gix_ref::{
-    file::ReferenceExt,
-    transaction::{Change, PreviousValue, RefEdit, RefLog},
-    Reference, Target,
-};
-
 use crate::{
     file::{
         store_writable,
         transaction::prepare_and_commit::{committer, empty_store},
     },
     hex_to_id,
 };
+use gix_date::parse::TimeBuf;
+use gix_lock::acquire::Fail;
+use gix_ref::file::transaction::prepare::Error;
+use gix_ref::transaction::LogChange;
+use gix_ref::{
+    file::ReferenceExt,
+    transaction::{Change, PreviousValue, RefEdit, RefLog},
+    FullName, Reference, Target,
+};
 
 #[test]
 fn delete_a_ref_which_is_gone_succeeds() -> crate::Result {
@@ -208,6 +209,97 @@ fn delete_reflog_only_of_symbolic_with_deref() -> crate::Result {
     Ok(())
 }
 
+#[test]
+fn rename_a_to_a_slash_b_in_one_transaction() -> crate::Result {
+    let (_keep, store) = store_writable("make_repo_for_reflog.sh")?;
+    let old = store.find_loose("old")?;
+
+    let new_name: FullName = "refs/heads/old/new".try_into()?;
+    let err = store
+        .transaction()
+        .prepare(
+            [
+                RefEdit {
+                    change: Change::Delete {
+                        expected: PreviousValue::MustExist,
+                        log: RefLog::AndReference,
+                    },
+                    name: old.name.clone(),
+                    deref: true,
+                },
+                RefEdit {
+                    change: Change::Update {
+                        expected: PreviousValue::MustNotExist,
+                        log: LogChange::default(),
+                        new: old.target.clone(),
+                    },
+                    name: new_name.clone(),
+                    deref: true,
+                },
+            ],
+            Fail::Immediately,
+            Fail::Immediately,
+        )
+        .unwrap_err();
+
+    match err {
+        #[cfg(unix)]
+        Error::Io(err) => {
+            assert_eq!(
+                err.kind(),
+                std::io::ErrorKind::NotADirectory,
+                "For now this isn't supported in the same transaction."
+            );
+        }
+        #[cfg(windows)]
+        Error::LockAcquire { .. } => {
+            // It's bad that the error differs on Windows, but then again, doing this kind of pseudo-db on a filesystem
+            // probably is never going to be great, so let's wait for ref-tables.
+        }
+        err => unreachable!("unexpected error variant: {err:?}"),
+    }
+
+    let edits = store
+        .transaction()
+        .prepare(
+            [RefEdit {
+                change: Change::Delete {
+                    expected: PreviousValue::MustExist,
+                    log: RefLog::AndReference,
+                },
+                name: old.name,
+                deref: true,
+            }],
+            Fail::Immediately,
+            Fail::Immediately,
+        )?
+        .commit(committer().to_ref(&mut TimeBuf::default()))?;
+    assert_eq!(edits.len(), 1, "First delete to make space");
+
+    let edits = store
+        .transaction()
+        .prepare(
+            [RefEdit {
+                change: Change::Update {
+                    expected: PreviousValue::MustNotExist,
+                    log: LogChange::default(),
+                    new: old.target,
+                },
+                name: new_name.clone(),
+                deref: true,
+            }],
+            Fail::Immediately,
+            Fail::Immediately,
+        )?
+        .commit(committer().to_ref(&mut TimeBuf::default()))?;
+    assert_eq!(edits.len(), 1, "Then create the new ref in place of the old");
+    assert!(
+        store.try_find_loose(new_name.as_ref())?.is_some(),
+        "must have created the new reference"
+    );
+    Ok(())
+}
+
 #[test]
 /// Based on https://github.com/git/git/blob/master/refs/files-backend.c#L514:L515
 fn delete_broken_ref_that_must_exist_fails_as_it_is_no_valid_ref() -> crate::Result {