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>

diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt
index 7fee6b780a869fcb9af979fb95a54587002b92f2..508a344cf19fa702219c62b61ef5e04960c246b9 100644 (file)
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -284,7 +284,9 @@ information is sent back to the client in the next step.
 The client can optionally request that pack-objects omit various
 objects from the packfile using one of several filtering techniques.
 These are intended for use with partial clone and partial fetch
 The client can optionally request that pack-objects omit various
 objects from the packfile using one of several filtering techniques.
 These are intended for use with partial clone and partial fetch

-operations.  See `rev-list` for possible "filter-spec" values.
+operations. An object that does not meet a filter-spec value is
+omitted unless explicitly requested in a 'want' line. See `rev-list`
+for possible filter-spec values.

 
 Once all the 'want's and 'shallow's (and optional 'deepen') are
 transferred, clients MUST send a flush-pkt, to tell the server side
 
 Once all the 'want's and 'shallow's (and optional 'deepen') are
 transferred, clients MUST send a flush-pkt, to tell the server side

diff --git a/list-objects.c b/list-objects.c
index 3eec510357337f5e33eea08a05faa22373d98c07..be4118889a086b4fa91578c0dbafc21c376d9654 100644 (file)
--- a/list-objects.c
+++ b/list-objects.c
@@ -47,7 +47,7 @@ static void process_blob(struct rev_info *revs,
 
        pathlen = path->len;
        strbuf_addstr(path, name);
 
        pathlen = path->len;
        strbuf_addstr(path, name);

-       if (filter_fn)
+       if (!(obj->flags & USER_GIVEN) && filter_fn)

                r = filter_fn(LOFS_BLOB, obj,
                              path->buf, &path->buf[pathlen],
                              filter_data);
                r = filter_fn(LOFS_BLOB, obj,
                              path->buf, &path->buf[pathlen],
                              filter_data);


@@ -132,7 +132,7 @@ static void process_tree(struct rev_info *revs,
        }
 
        strbuf_addstr(base, name);
        }
 
        strbuf_addstr(base, name);

-       if (filter_fn)
+       if (!(obj->flags & USER_GIVEN) && filter_fn)

                r = filter_fn(LOFS_BEGIN_TREE, obj,
                              base->buf, &base->buf[baselen],
                              filter_data);
                r = filter_fn(LOFS_BEGIN_TREE, obj,
                              base->buf, &base->buf[baselen],
                              filter_data);


@@ -171,7 +171,7 @@ static void process_tree(struct rev_info *revs,
                                     cb_data, filter_fn, filter_data);
        }
 
                                     cb_data, filter_fn, filter_data);
        }
 

-       if (filter_fn) {
+       if (!(obj->flags & USER_GIVEN) && filter_fn) {

                r = filter_fn(LOFS_END_TREE, obj,
                              base->buf, &base->buf[baselen],
                              filter_data);
                r = filter_fn(LOFS_END_TREE, obj,
                              base->buf, &base->buf[baselen],
                              filter_data);

diff --git a/object.h b/object.h
index 5c13955000cdaec252f29a3b93599560c35deb8a..cf8da6ba8301062903929263059b917fa812b1db 100644 (file)
--- a/object.h
+++ b/object.h
@@ -27,7 +27,7 @@ struct object_array {
 
 /*
  * object flag allocation:
 
 /*
  * object flag allocation:

- * revision.h:               0---------10                                26
+ * revision.h:               0---------10                              2526

  * fetch-pack.c:             0----5
  * walker.c:                 0-2
  * upload-pack.c:                4       11----------------19
  * fetch-pack.c:             0----5
  * walker.c:                 0-2
  * upload-pack.c:                4       11----------------19

diff --git a/revision.c b/revision.c
index 40fd91ff2b16bffc725f8602f45117279e27ba19..1b37da988d2dc8e8b357e4375583a072d5fed82a 100644 (file)
--- a/revision.c
+++ b/revision.c
@@ -172,6 +172,7 @@ static void add_pending_object_with_path(struct rev_info *revs,
                strbuf_release(&buf);
                return; /* do not add the commit itself */
        }
                strbuf_release(&buf);
                return; /* do not add the commit itself */
        }

+       obj->flags |= USER_GIVEN;

        add_object_array_with_path(obj, name, &revs->pending, mode, path);
 }
 
        add_object_array_with_path(obj, name, &revs->pending, mode, path);
 }
 

diff --git a/revision.h b/revision.h
index b8c47b98e22562ef320197b4ddbc8f0c3ee40f98..dc8f96366e3636d3de3e0b36288a425c1ca9d531 100644 (file)
--- a/revision.h
+++ b/revision.h
@@ -19,8 +19,9 @@
 #define SYMMETRIC_LEFT (1u<<8)
 #define PATCHSAME      (1u<<9)
 #define BOTTOM         (1u<<10)
 #define SYMMETRIC_LEFT (1u<<8)
 #define PATCHSAME      (1u<<9)
 #define BOTTOM         (1u<<10)

+#define USER_GIVEN     (1u<<25) /* given directly by the user */

 #define TRACK_LINEAR   (1u<<26)
 #define TRACK_LINEAR   (1u<<26)

-#define ALL_REV_FLAGS  (((1u<<11)-1) | TRACK_LINEAR)
+#define ALL_REV_FLAGS  (((1u<<11)-1) | USER_GIVEN | TRACK_LINEAR)

 
 #define DECORATE_SHORT_REFS    1
 #define DECORATE_FULL_REFS     2
 
 #define DECORATE_SHORT_REFS    1
 #define DECORATE_FULL_REFS     2

diff --git a/t/t5317-pack-objects-filter-objects.sh b/t/t5317-pack-objects-filter-objects.sh
index 1b0acc383bb216a3f420cd19ff066275a5b4706e..6710c8bc8c4699035bec418258fcaec8a634855c 100755 (executable)
--- a/t/t5317-pack-objects-filter-objects.sh
+++ b/t/t5317-pack-objects-filter-objects.sh
@@ -160,6 +160,22 @@ test_expect_success 'verify blob:limit=1k' '
        test_cmp observed expected
 '
 
        test_cmp observed expected
 '
 

+test_expect_success 'verify explicitly specifying oversized blob in input' '
+       git -C r2 ls-files -s large.1000 large.10000 \
+               | awk -f print_2.awk \
+               | sort >expected &&
+       git -C r2 pack-objects --rev --stdout --filter=blob:limit=1k >filter.pack <<-EOF &&
+       HEAD
+       $(git -C r2 rev-parse HEAD:large.10000)
+       EOF
+       git -C r2 index-pack ../filter.pack &&
+       git -C r2 verify-pack -v ../filter.pack \
+               | grep blob \
+               | awk -f print_1.awk \
+               | sort >observed &&
+       test_cmp observed expected
+'
+

 test_expect_success 'verify blob:limit=1m' '
        git -C r2 ls-files -s large.1000 large.10000 \
                | awk -f print_2.awk \
 test_expect_success 'verify blob:limit=1m' '
        git -C r2 ls-files -s large.1000 large.10000 \
                | awk -f print_2.awk \

diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
index cee556536757128861e166ea8e2cab7f975282c6..8a2bf86491f6b3cd23cad5b15f4e5df2c55589e4 100755 (executable)
--- a/t/t5616-partial-clone.sh
+++ b/t/t5616-partial-clone.sh
@@ -154,4 +154,20 @@ test_expect_success 'partial clone with transfer.fsckobjects=1 uses index-pack -
        grep "git index-pack.*--fsck-objects" trace
 '
 
        grep "git index-pack.*--fsck-objects" trace
 '
 

+test_expect_success 'partial clone fetches blobs pointed to by refs even if normally filtered out' '
+       rm -rf src dst &&
+       git init src &&
+       test_commit -C src x &&
+       test_config -C src uploadpack.allowfilter 1 &&
+       test_config -C src uploadpack.allowanysha1inwant 1 &&
+
+       # Create a tag pointing to a blob.
+       BLOB=$(echo blob-contents | git -C src hash-object --stdin -w) &&
+       git -C src tag myblob "$BLOB" &&
+
+       git clone --filter="blob:none" "file://$(pwd)/src" dst 2>err &&
+       ! grep "does not point to a valid object" err &&
+       git -C dst fsck
+'
+

 test_done
 test_done