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>

Matthew DeVore committed Oct 5, 2018 at 14:31 UTC 7c0fe330d5f3d2fc7aac57a19c7580ea2543c799
6 files changed +110 -7
builtin/rev-list.c
+7 -4
@@ -6,6 +6,7 @@
6 #include "list-objects.h"
7 #include "list-objects-filter.h"
8 #include "list-objects-filter-options.h"
9 +#include "object.h"
10 #include "object-store.h"
11 #include "pack.h"
12 #include "pack-bitmap.h"
@@ -209,7 +210,8 @@ static inline void finish_object__ma(struct object *obj)
210 */
211 switch (arg_missing_action) {
212 case MA_ERROR:
212 - die("missing blob object '%s'", oid_to_hex(&obj->oid));
213 + die("missing %s object '%s'",
214 + type_name(obj->type), oid_to_hex(&obj->oid));
215 return;
216
217 case MA_ALLOW_ANY:
@@ -222,8 +224,8 @@ static inline void finish_object__ma(struct object *obj)
224 case MA_ALLOW_PROMISOR:
225 if (is_promisor_object(&obj->oid))
226 return;
225 - die("unexpected missing blob object '%s'",
226 - oid_to_hex(&obj->oid));
227 + die("unexpected missing %s object '%s'",
228 + type_name(obj->type), oid_to_hex(&obj->oid));
229 return;
230
231 default:
@@ -235,7 +237,7 @@ static inline void finish_object__ma(struct object *obj)
237 static int finish_object(struct object *obj, const char *name, void *cb_data)
238 {
239 struct rev_list_info *info = cb_data;
238 - if (obj->type == OBJ_BLOB && !has_object_file(&obj->oid)) {
240 + if (!has_object_file(&obj->oid)) {
241 finish_object__ma(obj);
242 return 1;
243 }
@@ -373,6 +375,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
375 init_revisions(&revs, prefix);
376 revs.abbrev = DEFAULT_ABBREV;
377 revs.commit_format = CMIT_FMT_UNSPECIFIED;
378 + revs.do_not_die_on_missing_tree = 1;
379
380 /*
381 * Scan the argument list before invoking setup_revisions(), so that we
list-objects.c
+8 -3
@@ -143,6 +143,7 @@ static void process_tree(struct traversal_context *ctx,
143 struct rev_info *revs = ctx->revs;
144 int baselen = base->len;
145 enum list_objects_filter_result r = LOFR_MARK_SEEN | LOFR_DO_SHOW;
146 + int failed_parse;
147
148 if (!revs->tree_objects)
149 return;
@@ -150,7 +151,9 @@ static void process_tree(struct traversal_context *ctx,
151 die("bad tree object");
152 if (obj->flags & (UNINTERESTING | SEEN))
153 return;
153 - if (parse_tree_gently(tree, 1) < 0) {
154 +
155 + failed_parse = parse_tree_gently(tree, 1);
156 + if (failed_parse) {
157 if (revs->ignore_missing_links)
158 return;
159
@@ -163,7 +166,8 @@ static void process_tree(struct traversal_context *ctx,
166 is_promisor_object(&obj->oid))
167 return;
168
166 - die("bad tree object %s", oid_to_hex(&obj->oid));
169 + if (!revs->do_not_die_on_missing_tree)
170 + die("bad tree object %s", oid_to_hex(&obj->oid));
171 }
172
173 strbuf_addstr(base, name);
@@ -178,7 +182,8 @@ static void process_tree(struct traversal_context *ctx,
182 if (base->len)
183 strbuf_addch(base, '/');
184
181 - process_tree_contents(ctx, tree, base);
185 + if (!failed_parse)
186 + process_tree_contents(ctx, tree, base);
187
188 if (!(obj->flags & USER_GIVEN) && ctx->filter_fn) {
189 r = ctx->filter_fn(LOFS_END_TREE, obj,
revision.h
+15
@@ -125,6 +125,21 @@ struct rev_info {
125 line_level_traverse:1,
126 tree_blobs_in_commit_order:1,
127
128 + /*
129 + * Blobs are shown without regard for their existence.
130 + * But not so for trees: unless exclude_promisor_objects
131 + * is set and the tree in question is a promisor object;
132 + * OR ignore_missing_links is set, the revision walker
133 + * dies with a "bad tree object HASH" message when
134 + * encountering a missing tree. For callers that can
135 + * handle missing trees and want them to be filterable
136 + * and showable, set this to true. The revision walker
137 + * will filter and show such a missing tree as usual,
138 + * but will not attempt to recurse into this tree
139 + * object.
140 + */
141 + do_not_die_on_missing_tree:1,
142 +
143 /* for internal use only */
144 exclude_promisor_objects:1;
145
t/t0410-partial-clone.sh
+45
@@ -186,6 +186,51 @@ test_expect_success 'rev-list stops traversal at missing and promised commit' '
186 ! grep $FOO out
187 '
188
189 +test_expect_success 'missing tree objects with --missing=allow-promisor and --exclude-promisor-objects' '
190 + rm -rf repo &&
191 + test_create_repo repo &&
192 + test_commit -C repo foo &&
193 + test_commit -C repo bar &&
194 + test_commit -C repo baz &&
195 +
196 + promise_and_delete $(git -C repo rev-parse bar^{tree}) &&
197 + promise_and_delete $(git -C repo rev-parse foo^{tree}) &&
198 +
199 + git -C repo config core.repositoryformatversion 1 &&
200 + git -C repo config extensions.partialclone "arbitrary string" &&
201 +
202 + git -C repo rev-list --missing=allow-promisor --objects HEAD >objs 2>rev_list_err &&
203 + test_must_be_empty rev_list_err &&
204 + # 3 commits, 3 blobs, and 1 tree
205 + test_line_count = 7 objs &&
206 +
207 + # Do the same for --exclude-promisor-objects, but with all trees gone.
208 + promise_and_delete $(git -C repo rev-parse baz^{tree}) &&
209 + git -C repo rev-list --exclude-promisor-objects --objects HEAD >objs 2>rev_list_err &&
210 + test_must_be_empty rev_list_err &&
211 + # 3 commits, no blobs or trees
212 + test_line_count = 3 objs
213 +'
214 +
215 +test_expect_success 'missing non-root tree object and rev-list' '
216 + rm -rf repo &&
217 + test_create_repo repo &&
218 + mkdir repo/dir &&
219 + echo foo >repo/dir/foo &&
220 + git -C repo add dir/foo &&
221 + git -C repo commit -m "commit dir/foo" &&
222 +
223 + promise_and_delete $(git -C repo rev-parse HEAD:dir) &&
224 +
225 + git -C repo config core.repositoryformatversion 1 &&
226 + git -C repo config extensions.partialclone "arbitrary string" &&
227 +
228 + git -C repo rev-list --missing=allow-any --objects HEAD >objs 2>rev_list_err &&
229 + test_must_be_empty rev_list_err &&
230 + # 1 commit and 1 tree
231 + test_line_count = 2 objs
232 +'
233 +
234 test_expect_success 'rev-list stops traversal at missing and promised tree' '
235 rm -rf repo &&
236 test_create_repo repo &&
t/t5317-pack-objects-filter-objects.sh
+13
@@ -59,6 +59,19 @@ test_expect_success 'verify normal and blob:none packfiles have same commits/tre
59 test_cmp observed expected
60 '
61
62 +test_expect_success 'get an error for missing tree object' '
63 + git init r5 &&
64 + echo foo >r5/foo &&
65 + git -C r5 add foo &&
66 + git -C r5 commit -m "foo" &&
67 + del=$(git -C r5 rev-parse HEAD^{tree} | sed "s|..|&/|") &&
68 + rm r5/.git/objects/$del &&
69 + test_must_fail git -C r5 pack-objects --rev --stdout 2>bad_tree <<-EOF &&
70 + HEAD
71 + EOF
72 + grep -q "bad tree object" bad_tree
73 +'
74 +
75 # Test blob:limit=<n>[kmg] filter.
76 # We boundary test around the size parameter. The filter is strictly less than
77 # the value, so size 500 and 1000 should have the same results, but 1001 should
t/t6112-rev-list-filters-objects.sh
+22
@@ -196,6 +196,28 @@ test_expect_success 'verify sparse:oid=oid-ish omits top-level files' '
196 test_cmp observed expected
197 '
198
199 +test_expect_success 'rev-list W/ --missing=print and --missing=allow-any for trees' '
200 + TREE=$(git -C r3 rev-parse HEAD:dir1) &&
201 +
202 + # Create a spare repo because we will be deleting objects from this one.
203 + git clone r3 r3.b &&
204 +
205 + rm r3.b/.git/objects/$(echo $TREE | sed "s|^..|&/|") &&
206 +
207 + git -C r3.b rev-list --quiet --missing=print --objects HEAD \
208 + >missing_objs 2>rev_list_err &&
209 + echo "?$TREE" >expected &&
210 + test_cmp expected missing_objs &&
211 +
212 + # do not complain when a missing tree cannot be parsed
213 + test_must_be_empty rev_list_err &&
214 +
215 + git -C r3.b rev-list --missing=allow-any --objects HEAD \
216 + >objs 2>rev_list_err &&
217 + ! grep $TREE objs &&
218 + test_must_be_empty rev_list_err
219 +'
220 +
221 # Delete some loose objects and use rev-list, but WITHOUT any filtering.
222 # This models previously omitted objects that we did not receive.
223