Merge branch 'mh/reporting-broken-refs-from-for-each-ref'

"git for-each-ref" reported "missing object" for 0{40} when it
encounters a broken ref.  The lack of object whose name is 0{40} is
not the problem; the ref being broken is.

* mh/reporting-broken-refs-from-for-each-ref:
read_loose_refs(): treat NULL_SHA1 loose references as broken
read_loose_refs(): simplify function logic
for-each-ref: report broken references correctly
t6301: new tests of for-each-ref error handling

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index f7e51a7fadc40b1e4484a657fc55af8a3043967c..cb7db230d3ef76ee6a8b73971cbdabcc6d6590a9 100644 (file)
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -866,6 +866,11 @@ static int grab_single_ref(const char *refname, const struct object_id *oid,
                  return 0;
        }
 
+       if (flag & REF_ISBROKEN) {
+                 warning("ignoring broken ref %s", refname);
+                 return 0;
+       }
+
        if (*cb->grab_pattern) {
                const char **pattern;
                int namelen = strlen(refname);

diff --git a/refs.c b/refs.c
index 26d1ac1e32eb4fc4c6729abcf0354dbad6177d1b..7ac05cf21a25802f8e16d45ef75e37b7de2cda8a 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -1373,19 +1373,34 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
                                         create_dir_entry(refs, refname.buf,
                                                          refname.len, 1));
                } else {
+                       int read_ok;
+
                        if (*refs->name) {
                                hashclr(sha1);
                                flag = 0;
-                               if (resolve_gitlink_ref(refs->name, refname.buf, sha1) < 0) {
-                                       hashclr(sha1);
-                                       flag |= REF_ISBROKEN;
-                               }
-                       } else if (read_ref_full(refname.buf,
-                                                RESOLVE_REF_READING,
-                                                sha1, &flag)) {
+                               read_ok = !resolve_gitlink_ref(refs->name,
+                                                              refname.buf, sha1);
+                       } else {
+                               read_ok = !read_ref_full(refname.buf,
+                                                        RESOLVE_REF_READING,
+                                                        sha1, &flag);
+                       }
+
+                       if (!read_ok) {
                                hashclr(sha1);
                                flag |= REF_ISBROKEN;
+                       } else if (is_null_sha1(sha1)) {
+                               /*
+                                * It is so astronomically unlikely
+                                * that NULL_SHA1 is the SHA-1 of an
+                                * actual object that we consider its
+                                * appearance in a loose reference
+                                * file to be repo corruption
+                                * (probably due to a software bug).
+                                */
+                               flag |= REF_ISBROKEN;
                        }
+
                        if (check_refname_format(refname.buf,
                                                 REFNAME_ALLOW_ONELEVEL)) {
                                if (!refname_is_safe(refname.buf))

diff --git a/t/t6301-for-each-ref-errors.sh b/t/t6301-for-each-ref-errors.sh
new file mode 100755 (executable)
index 0000000..cdb67a0
--- /dev/null
+++ b/t/t6301-for-each-ref-errors.sh
@@ -0,0 +1,56 @@
+#!/bin/sh
+
+test_description='for-each-ref errors for broken refs'
+
+. ./test-lib.sh
+
+ZEROS=$_z40
+MISSING=abababababababababababababababababababab
+
+test_expect_success setup '
+       git commit --allow-empty -m "Initial" &&
+       git tag testtag &&
+       git for-each-ref >full-list &&
+       git for-each-ref --format="%(objectname) %(refname)" >brief-list
+'
+
+test_expect_success 'Broken refs are reported correctly' '
+       r=refs/heads/bogus &&
+       : >.git/$r &&
+       test_when_finished "rm -f .git/$r" &&
+       echo "warning: ignoring broken ref $r" >broken-err &&
+       git for-each-ref >out 2>err &&
+       test_cmp full-list out &&
+       test_cmp broken-err err
+'
+
+test_expect_success 'NULL_SHA1 refs are reported correctly' '
+       r=refs/heads/zeros &&
+       echo $ZEROS >.git/$r &&
+       test_when_finished "rm -f .git/$r" &&
+       echo "warning: ignoring broken ref $r" >zeros-err &&
+       git for-each-ref >out 2>err &&
+       test_cmp full-list out &&
+       test_cmp zeros-err err &&
+       git for-each-ref --format="%(objectname) %(refname)" >brief-out 2>brief-err &&
+       test_cmp brief-list brief-out &&
+       test_cmp zeros-err brief-err
+'
+
+test_expect_success 'Missing objects are reported correctly' '
+       r=refs/heads/missing &&
+       echo $MISSING >.git/$r &&
+       test_when_finished "rm -f .git/$r" &&
+       echo "fatal: missing object $MISSING for $r" >missing-err &&
+       test_must_fail git for-each-ref 2>err &&
+       test_cmp missing-err err &&
+       (
+               cat brief-list &&
+               echo "$MISSING $r"
+       ) | sort -k 2 >missing-brief-expected &&
+       git for-each-ref --format="%(objectname) %(refname)" >brief-out 2>brief-err &&
+       test_cmp missing-brief-expected brief-out &&
+       test_must_be_empty brief-err
+'
+
+test_done