Stop emitting an error message when deleting a packed reference if we
find another dangling packed reference that is overridden by a loose
reference.  See the previous commit for a longer explanation of the

We have to be careful to make sure that the invalid packed reference
really *is* overridden by a loose reference; otherwise what we have
found is repository corruption, which we *should* report.

Please note that this approach is vulnerable to a race condition
similar to the race conditions already known to affect packed
references [1]:

* Process 1 tries to peel packed reference X as part of deleting
  another packed reference.  It discovers that X does not refer to a
  valid object (because the object that it referred to has been
  garbage collected).

* Process 2 tries to delete reference X.  It starts by deleting the
  loose reference X.

* Process 1 checks whether there is a loose reference X.  There is not
  (it has just been deleted by process 2), so process 1 reports a
  spurious error "X does not point to a valid object!"

The worst case seems relatively harmless, and the fix is identical to
the fix that will be needed for the other race conditions (namely
holding a lock on the packed-refs file during *all* reference
deletions), so we leave the cleaning up of all of them as a future


Signed-off-by: Michael Haggerty <>
 refs.c               | 37 +++++++++++++++++++++++++++++++++++--
 t/ |  2 +-
 2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/refs.c b/refs.c
index ed54ed4..2957f6d 100644
--- a/refs.c
+++ b/refs.c
@@ -1901,8 +1901,41 @@ static int repack_without_ref_fn(struct ref_entry 
*entry, void *cb_data)
        if (!strcmp(data->refname, entry->name))
                return 0;
-       if (!ref_resolves_to_object(entry))
-               return 0; /* Skip broken refs */
+       if (entry->flag & REF_ISBROKEN) {
+               /* This shouldn't happen to packed refs. */
+               error("%s is broken!", entry->name);
+               return 0;
+       }
+       if (!has_sha1_file(entry->u.value.sha1)) {
+               unsigned char sha1[20];
+               int flags;
+               if (read_ref_full(entry->name, sha1, 0, &flags))
+                       /* We should at least have found the packed ref. */
+                       die("Internal error");
+               if ((flags & REF_ISSYMREF) || !(flags & REF_ISPACKED))
+                       /*
+                        * This packed reference is overridden by a
+                        * loose reference, so it is OK that its value
+                        * is no longer valid; for example, it might
+                        * refer to an object that has been garbage
+                        * collected.  For this purpose we don't even
+                        * care whether the loose reference itself is
+                        * invalid, broken, symbolic, etc.  Silently
+                        * omit the packed reference from the output.
+                        */
+                       return 0;
+               /*
+                * There is no overriding loose reference, so the fact
+                * that this reference doesn't refer to a valid object
+                * indicates some kind of repository corruption.
+                * Report the problem, then omit the reference from
+                * the output.
+                */
+               error("%s does not point to a valid object!", entry->name);
+               return 0;
+       }
        len = snprintf(line, sizeof(line), "%s %s\n",
                       sha1_to_hex(entry->u.value.sha1), entry->name);
        /* this should not happen but just being defensive */
diff --git a/t/ b/t/
index c032d88..559f602 100755
--- a/t/
+++ b/t/
@@ -142,7 +142,7 @@ test_expect_success 'delete ref with dangling packed 
version' '
        test_cmp /dev/null result
-test_expect_failure 'delete ref while another dangling packed ref' '
+test_expect_success 'delete ref while another dangling packed ref' '
        git branch lamb &&
        git commit --allow-empty -m "future garbage" &&
        git pack-refs --all &&

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to
More majordomo info at

Reply via email to