Previously, we assumed only blob objects could be missing. This patch
makes rev-list handle missing trees like missing blobs. A missing tree
will cause an error if --missing indicates an error should be caused,
and the hash is printed even if the tree is missing.

In list-objects.c we no longer print a message to stderr if a tree
object is missing (quiet_on_missing is always true). I couldn't find
any place where this would matter, or where the caller of
traverse_commit_list would need to be fixed to show the error. However,
in the future it would be trivial to make the caller show the message if
we needed to.

This is not tested very thoroughly, since we cannot create promisor
objects in tests without using an actual partial clone. t0410 has a
promise_and_delete utility function, but the is_promisor_object function
does not return 1 for objects deleted in this way. More tests will will
come in a patch that implements a filter that can be used with git
clone.

Signed-off-by: Matthew DeVore <matv...@google.com>
---
 builtin/rev-list.c                     | 10 ++++++----
 list-objects.c                         | 17 +++++++++--------
 t/t5317-pack-objects-filter-objects.sh | 13 +++++++++++++
 3 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 5b07f3f4a..ea0daf0c4 100644
--- 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;
        }
diff --git a/list-objects.c b/list-objects.c
index ccc529e5e..aedcd0228 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -143,8 +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 gently = revs->ignore_missing_links ||
-                    revs->exclude_promisor_objects;
+       int parsed;
 
        if (!revs->tree_objects)
                return;
@@ -152,20 +151,21 @@ static void process_tree(struct traversal_context *ctx,
                die("bad tree object");
        if (obj->flags & (UNINTERESTING | SEEN))
                return;
-       if (parse_tree_gently(tree, gently) < 0) {
+       parsed = parse_tree_gently(tree, /*quiet_on_missing=*/1) >= 0;
+       if (!parsed) {
                if (revs->ignore_missing_links)
                        return;
 
+               if (!is_promisor_object(&obj->oid))
+                       die("bad tree object %s", oid_to_hex(&obj->oid));
+
                /*
                 * Pre-filter known-missing tree objects when explicitly
                 * requested.  This may cause the actual filter to report
                 * an incomplete list of missing objects.
                 */
-               if (revs->exclude_promisor_objects &&
-                   is_promisor_object(&obj->oid))
+               if (revs->exclude_promisor_objects)
                        return;
-
-               die("bad tree object %s", oid_to_hex(&obj->oid));
        }
 
        strbuf_addstr(base, name);
@@ -180,7 +180,8 @@ static void process_tree(struct traversal_context *ctx,
        if (base->len)
                strbuf_addch(base, '/');
 
-       process_tree_contents(ctx, tree, base);
+       if (parsed)
+               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/t/t5317-pack-objects-filter-objects.sh 
b/t/t5317-pack-objects-filter-objects.sh
index 6710c8bc8..5e35f33bf 100755
--- 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
-- 
2.18.0.597.ga71716f1ad-goog

Reply via email to