upload-pack: send refs' objects despite "filter"

A filter line in a request to upload-pack filters out objects regardless of whether they are directly referenced by a "want" line or not. This means that cloning with "--filter=blob:none" (or another filter that excludes blobs) from a repository with at least one ref pointing to a blob (for example, the Git repository itself) results in output like the following: error: missing object referenced by 'refs/tags/junio-gpg-pub' and if that particular blob is not referenced by a fetched tree, the resulting clone fails fsck because there is no object from the remote to vouch that the missing object is a promisor object. Update both the protocol and the upload-pack implementation to include all explicitly specified "want" objects in the packfile regardless of the filter specification. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Tan committed Jul 6, 2018 at 12:34 UTC a0c9016abd566e9a8f988dcd387663cd0b2be078
7 files changed +42 -6
Documentation/technical/pack-protocol.txt
+3 -1
@@ -284,7 +284,9 @@ information is sent back to the client in the next step.
284 The client can optionally request that pack-objects omit various
285 objects from the packfile using one of several filtering techniques.
286 These are intended for use with partial clone and partial fetch
287 -operations. See `rev-list` for possible "filter-spec" values.
287 +operations. An object that does not meet a filter-spec value is
288 +omitted unless explicitly requested in a 'want' line. See `rev-list`
289 +for possible filter-spec values.
290
291 Once all the 'want's and 'shallow's (and optional 'deepen') are
292 transferred, clients MUST send a flush-pkt, to tell the server side
list-objects.c
+3 -3
@@ -47,7 +47,7 @@ static void process_blob(struct rev_info *revs,
47
48 pathlen = path->len;
49 strbuf_addstr(path, name);
50 - if (filter_fn)
50 + if (!(obj->flags & USER_GIVEN) && filter_fn)
51 r = filter_fn(LOFS_BLOB, obj,
52 path->buf, &path->buf[pathlen],
53 filter_data);
@@ -132,7 +132,7 @@ static void process_tree(struct rev_info *revs,
132 }
133
134 strbuf_addstr(base, name);
135 - if (filter_fn)
135 + if (!(obj->flags & USER_GIVEN) && filter_fn)
136 r = filter_fn(LOFS_BEGIN_TREE, obj,
137 base->buf, &base->buf[baselen],
138 filter_data);
@@ -171,7 +171,7 @@ static void process_tree(struct rev_info *revs,
171 cb_data, filter_fn, filter_data);
172 }
173
174 - if (filter_fn) {
174 + if (!(obj->flags & USER_GIVEN) && filter_fn) {
175 r = filter_fn(LOFS_END_TREE, obj,
176 base->buf, &base->buf[baselen],
177 filter_data);
object.h
+1 -1
@@ -27,7 +27,7 @@ struct object_array {
27
28 /*
29 * object flag allocation:
30 - * revision.h: 0---------10 26
30 + * revision.h: 0---------10 2526
31 * fetch-pack.c: 0----5
32 * walker.c: 0-2
33 * upload-pack.c: 4 11----------------19
revision.c
+1
@@ -172,6 +172,7 @@ static void add_pending_object_with_path(struct rev_info *revs,
172 strbuf_release(&buf);
173 return; /* do not add the commit itself */
174 }
175 + obj->flags |= USER_GIVEN;
176 add_object_array_with_path(obj, name, &revs->pending, mode, path);
177 }
178
revision.h
+2 -1
@@ -19,8 +19,9 @@
19 #define SYMMETRIC_LEFT (1u<<8)
20 #define PATCHSAME (1u<<9)
21 #define BOTTOM (1u<<10)
22 +#define USER_GIVEN (1u<<25) /* given directly by the user */
23 #define TRACK_LINEAR (1u<<26)
23 -#define ALL_REV_FLAGS (((1u<<11)-1) | TRACK_LINEAR)
24 +#define ALL_REV_FLAGS (((1u<<11)-1) | USER_GIVEN | TRACK_LINEAR)
25
26 #define DECORATE_SHORT_REFS 1
27 #define DECORATE_FULL_REFS 2
t/t5317-pack-objects-filter-objects.sh
+16
@@ -160,6 +160,22 @@ test_expect_success 'verify blob:limit=1k' '
160 test_cmp observed expected
161 '
162
163 +test_expect_success 'verify explicitly specifying oversized blob in input' '
164 + git -C r2 ls-files -s large.1000 large.10000 \
165 + | awk -f print_2.awk \
166 + | sort >expected &&
167 + git -C r2 pack-objects --rev --stdout --filter=blob:limit=1k >filter.pack <<-EOF &&
168 + HEAD
169 + $(git -C r2 rev-parse HEAD:large.10000)
170 + EOF
171 + git -C r2 index-pack ../filter.pack &&
172 + git -C r2 verify-pack -v ../filter.pack \
173 + | grep blob \
174 + | awk -f print_1.awk \
175 + | sort >observed &&
176 + test_cmp observed expected
177 +'
178 +
179 test_expect_success 'verify blob:limit=1m' '
180 git -C r2 ls-files -s large.1000 large.10000 \
181 | awk -f print_2.awk \
t/t5616-partial-clone.sh
+16
@@ -154,4 +154,20 @@ test_expect_success 'partial clone with transfer.fsckobjects=1 uses index-pack -
154 grep "git index-pack.*--fsck-objects" trace
155 '
156
157 +test_expect_success 'partial clone fetches blobs pointed to by refs even if normally filtered out' '
158 + rm -rf src dst &&
159 + git init src &&
160 + test_commit -C src x &&
161 + test_config -C src uploadpack.allowfilter 1 &&
162 + test_config -C src uploadpack.allowanysha1inwant 1 &&
163 +
164 + # Create a tag pointing to a blob.
165 + BLOB=$(echo blob-contents | git -C src hash-object --stdin -w) &&
166 + git -C src tag myblob "$BLOB" &&
167 +
168 + git clone --filter="blob:none" "file://$(pwd)/src" dst 2>err &&
169 + ! grep "does not point to a valid object" err &&
170 + git -C dst fsck
171 +'
172 +
173 test_done