doc: improve documentation about the `Change` type.

Further, we update internal logic to decide when to write a reflog for symrefs. Now we will always do it when possible, i.e. there is object ids available.

Sebastian Thiel committed 3 years ago

Commit 5f6cf221 parent fadec773 files · +13 −11Browse files

Unified · Split

3 files

gix-ref/src/store/file/transaction/commit.rs +4 −1
gix-ref/src/transaction/mod.rs +1 −2
gix-ref/tests/file/transaction/prepare_and_commit/create_or_update/mod.rs +8 −8

gix-ref/src/store/file/transaction/commit.rs+4−1

                     if update_reflog {
                         let log_update = match new {
                             Target::Symbolic(_) => {
-                                // no reflog for symref changes, unless the ref is new and we can obtain a peeled id
+                                // Special HACK: no reflog for symref changes as there is no OID involved which the reflog needs.
+                                // Unless, the ref is new and we can obtain a peeled id
                                 // identified by the expectation of what could be there, as is the case when cloning.
                                 match expected {
                                     PreviousValue::ExistingMustMatch(Target::Peeled(oid)) => {
                             }
                             Target::Peeled(new_oid) => {
                                 let previous = match expected {
+                                    // Here, this means that the ref already existed, and that it will receive (even transitively)
+                                    // the given value
                                     PreviousValue::MustExistAndMatch(Target::Peeled(oid)) => Some(oid.to_owned()),
                                     _ => None,
                                 }

gix-ref/src/transaction/mod.rs+1−2

         /// The desired change to the reference log.
         log: LogChange,
         /// The expected value already present in the reference.
-        /// If a ref was existing previously it will be overwritten at `MustExistAndMatch(actual_value)` for use after
+        /// If a ref was existing previously this field will be overwritten with `MustExistAndMatch(actual_value)` for use after
         /// the transaction was committed successfully.
         expected: PreviousValue,
         /// The new state of the reference, either for updating an existing one or creating a new one.
     /// Return references to values that are in common between all variants and denote the previous observed value.
     pub fn previous_value(&self) -> Option<crate::TargetRef<'_>> {
         match self {
-            // TODO: use or-patterns once MRV is larger than 1.52 (and this is supported)
             Change::Update {
                 expected: PreviousValue::MustExistAndMatch(previous) | PreviousValue::ExistingMustMatch(previous),
                 ..

gix-ref/tests/file/transaction/prepare_and_commit/create_or_update/mod.rs+8−8

     );
     assert!(
         head.log_exists(&store),
-        "reflog is written for new symbolic ref with information about the peeled target id"
+        "reflog is written for new symbolic ref with information about the peeled target id, \
+         as special accommodation for the state during clone to allow us to get a peeled id into the log"
     );
     assert!(store.try_find_loose(referent)?.is_none(), "referent wasn't created");
             mode: RefLog::AndReference,
             force_create_reflog: false,
         };
-        let log_only = {
-            let mut l = log.clone();
-            l.mode = RefLog::Only;
-            l
-        };
         let edits = store
             .transaction()
             .prepare(
             vec![
                 RefEdit {
                     change: Change::Update {
-                        log: log_only.clone(),
+                        log: {
+                            let mut l = log.clone();
+                            l.mode = RefLog::Only;
+                            l
+                        },
                         new: new.clone(),
                         expected: PreviousValue::MustExistAndMatch(new_head_value.clone()),
                     },
                     change: Change::Update {
                         log,
                         new: new.clone(),
-                        expected: PreviousValue::Any,
+                        expected: PreviousValue::Any, // there is no previous value, so we can't put `MustExistAndMatch` here.
                     },
                     name: referent.try_into()?,
                     deref: false,