rev-list: handle missing tree objects properly

Previously, we assumed only blob objects could be missing. This patch
makes rev-list handle missing trees like missing blobs. The --missing=*
and --exclude-promisor-objects flags now work for trees as they already
do for blobs. This is demonstrated in t6112.

Signed-off-by: Matthew DeVore <matvore@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 5b07f3f4a2cf6cd5f73f20a930934ca2277c8391..49d6deed7012cb2c529ad971c6e45abf654fe097 100644 (file)
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -6,6 +6,7 @@
 #include "list-objects.h"
 #include "list-objects-filter.h"
 #include "list-objects-filter-options.h"
+#include "object.h"
 #include "object-store.h"
 #include "pack.h"
 #include "pack-bitmap.h"
@@ -209,7 +210,8 @@ static inline void finish_object__ma(struct object *obj)
         */
        switch (arg_missing_action) {
        case MA_ERROR:
-               die("missing blob object '%s'", oid_to_hex(&obj->oid));
+               die("missing %s object '%s'",
+                   type_name(obj->type), oid_to_hex(&obj->oid));
                return;
 
        case MA_ALLOW_ANY:
@@ -222,8 +224,8 @@ static inline void finish_object__ma(struct object *obj)
        case MA_ALLOW_PROMISOR:
                if (is_promisor_object(&obj->oid))
                        return;
-               die("unexpected missing blob object '%s'",
-                   oid_to_hex(&obj->oid));
+               die("unexpected missing %s object '%s'",
+                   type_name(obj->type), oid_to_hex(&obj->oid));
                return;
 
        default:
@@ -235,7 +237,7 @@ static inline void finish_object__ma(struct object *obj)
 static int finish_object(struct object *obj, const char *name, void *cb_data)
 {
        struct rev_list_info *info = cb_data;
-       if (obj->type == OBJ_BLOB && !has_object_file(&obj->oid)) {
+       if (!has_object_file(&obj->oid)) {
                finish_object__ma(obj);
                return 1;
        }
@@ -373,6 +375,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
        init_revisions(&revs, prefix);
        revs.abbrev = DEFAULT_ABBREV;
        revs.commit_format = CMIT_FMT_UNSPECIFIED;
+       revs.do_not_die_on_missing_tree = 1;
 
        /*
         * Scan the argument list before invoking setup_revisions(), so that we

diff --git a/list-objects.c b/list-objects.c
index f9b51db7a72819b47e65510df956dd201adf0cb0..243192af5352752472908ab9a61e80fb2b138829 100644 (file)
--- a/list-objects.c
+++ b/list-objects.c
@@ -143,6 +143,7 @@ static void process_tree(struct traversal_context *ctx,
        struct rev_info *revs = ctx->revs;
        int baselen = base->len;
        enum list_objects_filter_result r = LOFR_MARK_SEEN | LOFR_DO_SHOW;
+       int failed_parse;
 
        if (!revs->tree_objects)
                return;
@@ -150,7 +151,9 @@ static void process_tree(struct traversal_context *ctx,
                die("bad tree object");
        if (obj->flags & (UNINTERESTING | SEEN))
                return;
-       if (parse_tree_gently(tree, 1) < 0) {
+
+       failed_parse = parse_tree_gently(tree, 1);
+       if (failed_parse) {
                if (revs->ignore_missing_links)
                        return;
 
@@ -163,7 +166,8 @@ static void process_tree(struct traversal_context *ctx,
                    is_promisor_object(&obj->oid))
                        return;
 
-               die("bad tree object %s", oid_to_hex(&obj->oid));
+               if (!revs->do_not_die_on_missing_tree)
+                       die("bad tree object %s", oid_to_hex(&obj->oid));
        }
 
        strbuf_addstr(base, name);
@@ -178,7 +182,8 @@ static void process_tree(struct traversal_context *ctx,
        if (base->len)
                strbuf_addch(base, '/');
 
-       process_tree_contents(ctx, tree, base);
+       if (!failed_parse)
+               process_tree_contents(ctx, tree, base);
 
        if (!(obj->flags & USER_GIVEN) && ctx->filter_fn) {
                r = ctx->filter_fn(LOFS_END_TREE, obj,

diff --git a/revision.h b/revision.h
index c599c34da91e57572b1c5b315c353d33399e8a09..5118aaaa9242c86080febfff9a3d0ab48da19b64 100644 (file)
--- a/revision.h
+++ b/revision.h
@@ -125,6 +125,21 @@ struct rev_info {
                        line_level_traverse:1,
                        tree_blobs_in_commit_order:1,
 
+                       /*
+                        * Blobs are shown without regard for their existence.
+                        * But not so for trees: unless exclude_promisor_objects
+                        * is set and the tree in question is a promisor object;
+                        * OR ignore_missing_links is set, the revision walker
+                        * dies with a "bad tree object HASH" message when
+                        * encountering a missing tree. For callers that can
+                        * handle missing trees and want them to be filterable
+                        * and showable, set this to true. The revision walker
+                        * will filter and show such a missing tree as usual,
+                        * but will not attempt to recurse into this tree
+                        * object.
+                        */
+                       do_not_die_on_missing_tree:1,
+
                        /* for internal use only */
                        exclude_promisor_objects:1;
 

diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index 4984ca583d03a7cc0a6fb9323799e776eee4565b..2f4ea487a4304d0282f655aca6f1013d075489ad 100755 (executable)
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh
@@ -186,6 +186,51 @@ test_expect_success 'rev-list stops traversal at missing and promised commit' '
        ! grep $FOO out
 '
 
+test_expect_success 'missing tree objects with --missing=allow-promisor and --exclude-promisor-objects' '
+       rm -rf repo &&
+       test_create_repo repo &&
+       test_commit -C repo foo &&
+       test_commit -C repo bar &&
+       test_commit -C repo baz &&
+
+       promise_and_delete $(git -C repo rev-parse bar^{tree}) &&
+       promise_and_delete $(git -C repo rev-parse foo^{tree}) &&
+
+       git -C repo config core.repositoryformatversion 1 &&
+       git -C repo config extensions.partialclone "arbitrary string" &&
+
+       git -C repo rev-list --missing=allow-promisor --objects HEAD >objs 2>rev_list_err &&
+       test_must_be_empty rev_list_err &&
+       # 3 commits, 3 blobs, and 1 tree
+       test_line_count = 7 objs &&
+
+       # Do the same for --exclude-promisor-objects, but with all trees gone.
+       promise_and_delete $(git -C repo rev-parse baz^{tree}) &&
+       git -C repo rev-list --exclude-promisor-objects --objects HEAD >objs 2>rev_list_err &&
+       test_must_be_empty rev_list_err &&
+       # 3 commits, no blobs or trees
+       test_line_count = 3 objs
+'
+
+test_expect_success 'missing non-root tree object and rev-list' '
+       rm -rf repo &&
+       test_create_repo repo &&
+       mkdir repo/dir &&
+       echo foo >repo/dir/foo &&
+       git -C repo add dir/foo &&
+       git -C repo commit -m "commit dir/foo" &&
+
+       promise_and_delete $(git -C repo rev-parse HEAD:dir) &&
+
+       git -C repo config core.repositoryformatversion 1 &&
+       git -C repo config extensions.partialclone "arbitrary string" &&
+
+       git -C repo rev-list --missing=allow-any --objects HEAD >objs 2>rev_list_err &&
+       test_must_be_empty rev_list_err &&
+       # 1 commit and 1 tree
+       test_line_count = 2 objs
+'
+
 test_expect_success 'rev-list stops traversal at missing and promised tree' '
        rm -rf repo &&
        test_create_repo repo &&

diff --git a/t/t5317-pack-objects-filter-objects.sh b/t/t5317-pack-objects-filter-objects.sh
index 6710c8bc8c4699035bec418258fcaec8a634855c..9839b48c1c537bfac14b31c7db913555553fcf67 100755 (executable)
--- a/t/t5317-pack-objects-filter-objects.sh
+++ b/t/t5317-pack-objects-filter-objects.sh
@@ -59,6 +59,19 @@ test_expect_success 'verify normal and blob:none packfiles have same commits/tre
        test_cmp observed expected
 '
 
+test_expect_success 'get an error for missing tree object' '
+       git init r5 &&
+       echo foo >r5/foo &&
+       git -C r5 add foo &&
+       git -C r5 commit -m "foo" &&
+       del=$(git -C r5 rev-parse HEAD^{tree} | sed "s|..|&/|") &&
+       rm r5/.git/objects/$del &&
+       test_must_fail git -C r5 pack-objects --rev --stdout 2>bad_tree <<-EOF &&
+       HEAD
+       EOF
+       grep -q "bad tree object" bad_tree
+'
+
 # Test blob:limit=<n>[kmg] filter.
 # We boundary test around the size parameter.  The filter is strictly less than
 # the value, so size 500 and 1000 should have the same results, but 1001 should

diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh
index 0a37dd5f97ee49fd65eff1c05d016abf2bcba38f..6d69e6a0aa5e9aad3a39d64778907265a5b99099 100755 (executable)
--- a/t/t6112-rev-list-filters-objects.sh
+++ b/t/t6112-rev-list-filters-objects.sh
@@ -196,6 +196,28 @@ test_expect_success 'verify sparse:oid=oid-ish omits top-level files' '
        test_cmp observed expected
 '
 
+test_expect_success 'rev-list W/ --missing=print and --missing=allow-any for trees' '
+       TREE=$(git -C r3 rev-parse HEAD:dir1) &&
+
+       # Create a spare repo because we will be deleting objects from this one.
+       git clone r3 r3.b &&
+
+       rm r3.b/.git/objects/$(echo $TREE | sed "s|^..|&/|") &&
+
+       git -C r3.b rev-list --quiet --missing=print --objects HEAD \
+               >missing_objs 2>rev_list_err &&
+       echo "?$TREE" >expected &&
+       test_cmp expected missing_objs &&
+
+       # do not complain when a missing tree cannot be parsed
+       test_must_be_empty rev_list_err &&
+
+       git -C r3.b rev-list --missing=allow-any --objects HEAD \
+               >objs 2>rev_list_err &&
+       ! grep $TREE objs &&
+       test_must_be_empty rev_list_err
+'
+
 # Delete some loose objects and use rev-list, but WITHOUT any filtering.
 # This models previously omitted objects that we did not receive.