git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Packfile-uris support excluding commit objects
@ 2021-05-07  2:11 Teng Long
  2021-05-10 11:14 ` Ævar Arnfjörð Bjarmason
  2021-05-18  8:49 ` [PATCH v2 0/3] packfile-uris: commit objects exclusion Teng Long
  0 siblings, 2 replies; 73+ messages in thread
From: Teng Long @ 2021-05-07  2:11 UTC (permalink / raw)
  To: git; +Cc: jonathantanmy, Teng Long

On the server, more sophisticated means of excluding objects should be
supported, such as commit object. This commit introduces a new
configuration `uploadpack.commitpackfileuri` for this.

This patch only pack the commit object, not including the that commit
and all objects that it references. This work will be done in a further
patch recently.

Similarly, there are related documents that will be included in
subsequent patches.

Signed-off-by: Teng Long <dyroneteng@gmail.com>
---
 builtin/pack-objects.c |  8 ++---
 fetch-pack.c           |  8 +++++
 t/t5702-protocol-v2.sh | 71 +++++++++++++++++++++++++++++++++---------
 upload-pack.c          |  7 +++--
 4 files changed, 73 insertions(+), 21 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 6d13cd3e1a..2f1817fe28 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2985,7 +2985,7 @@ static int git_pack_config(const char *k, const char *v, void *cb)
 			pack_idx_opts.flags &= ~WRITE_REV;
 		return 0;
 	}
-	if (!strcmp(k, "uploadpack.blobpackfileuri")) {
+    if (!strcmp(k, "uploadpack.blobpackfileuri") || !strcmp(k, "uploadpack.commitpackfileuri")) {
 		struct configured_exclusion *ex = xmalloc(sizeof(*ex));
 		const char *oid_end, *pack_end;
 		/*
@@ -2998,11 +2998,11 @@ static int git_pack_config(const char *k, const char *v, void *cb)
 		    *oid_end != ' ' ||
 		    parse_oid_hex(oid_end + 1, &pack_hash, &pack_end) ||
 		    *pack_end != ' ')
-			die(_("value of uploadpack.blobpackfileuri must be "
-			      "of the form '<object-hash> <pack-hash> <uri>' (got '%s')"), v);
+            die(_("value of uploadpack.blobpackfileuri or upload.commitpackfileuri must be "
+                  "of the form '<object-hash> <pack-hash> <uri>' (got '%s')"), v);
 		if (oidmap_get(&configured_exclusions, &ex->e.oid))
 			die(_("object already configured in another "
-			      "uploadpack.blobpackfileuri (got '%s')"), v);
+			      "uploadpack.blobpackfileuri or uploadpack.commitpackfileuri (got '%s')"), v);
 		ex->pack_hash_hex = xcalloc(1, pack_end - oid_end);
 		memcpy(ex->pack_hash_hex, oid_end + 1, pack_end - oid_end - 1);
 		ex->uri = xstrdup(pack_end + 1);
diff --git a/fetch-pack.c b/fetch-pack.c
index 2318ebe680..24a947835b 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -23,6 +23,7 @@
 #include "fetch-negotiator.h"
 #include "fsck.h"
 #include "shallow.h"
+#include "strmap.h"
 
 static int transfer_unpack_limit = -1;
 static int fetch_unpack_limit = -1;
@@ -1677,6 +1678,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 		}
 	}
 
+	struct strset uris;
+	strset_init(&uris);
 	for (i = 0; i < packfile_uris.nr; i++) {
 		int j;
 		struct child_process cmd = CHILD_PROCESS_INIT;
@@ -1684,6 +1687,11 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 		const char *uri = packfile_uris.items[i].string +
 			the_hash_algo->hexsz + 1;
 
+        if (strset_contains(&uris, uri)) {
+            continue;
+        }
+
+        strset_add(&uris, uri);
 		strvec_push(&cmd.args, "http-fetch");
 		strvec_pushf(&cmd.args, "--packfile=%.*s",
 			     (int) the_hash_algo->hexsz,
diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index 2e1243ca40..d444177fb5 100755
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -824,12 +824,22 @@ test_expect_success 'when server does not send "ready", expect FLUSH' '
 '
 
 configure_exclusion () {
-	git -C "$1" hash-object "$2" >objh &&
-	git -C "$1" pack-objects "$HTTPD_DOCUMENT_ROOT_PATH/mypack" <objh >packh &&
-	git -C "$1" config --add \
-		"uploadpack.blobpackfileuri" \
-		"$(cat objh) $(cat packh) $HTTPD_URL/dumb/mypack-$(cat packh).pack" &&
-	cat objh
+    if test "$1" = "blob"
+        then
+            git -C "$2" hash-object "$3" >objh &&
+            git -C "$2" pack-objects "$HTTPD_DOCUMENT_ROOT_PATH/mypack" <objh >packh &&
+            git -C "$2" config --add \
+            		"uploadpack.blobpackfileuri" \
+            		"$(cat objh) $(cat packh) $HTTPD_URL/dumb/mypack-$(cat packh).pack" &&
+            cat objh
+        else
+            echo "$3" > objh &&
+            git -C "$2" pack-objects "$HTTPD_DOCUMENT_ROOT_PATH/mypack" <objh >packh &&
+            git -C "$2" config --add \
+            		"uploadpack.commitpackfileuri" \
+            		"$(cat objh) $(cat packh) $HTTPD_URL/dumb/mypack-$(cat packh).pack" &&
+            cat objh
+    fi
 }
 
 test_expect_success 'part of packfile response provided as URI' '
@@ -845,8 +855,8 @@ test_expect_success 'part of packfile response provided as URI' '
 	git -C "$P" add other-blob &&
 	git -C "$P" commit -m x &&
 
-	configure_exclusion "$P" my-blob >h &&
-	configure_exclusion "$P" other-blob >h2 &&
+	configure_exclusion blob "$P" my-blob >h &&
+	configure_exclusion blob "$P" other-blob >h2 &&
 
 	GIT_TRACE=1 GIT_TRACE_PACKET="$(pwd)/log" GIT_TEST_SIDEBAND_ALL=1 \
 	git -c protocol.version=2 \
@@ -881,7 +891,7 @@ test_expect_success 'part of packfile response provided as URI' '
 	test_line_count = 6 filelist
 '
 
-test_expect_success 'packfile URIs with fetch instead of clone' '
+test_expect_success 'blobs packfile URIs with fetch instead of clone' '
 	P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
 	rm -rf "$P" http_child log &&
 
@@ -892,7 +902,7 @@ test_expect_success 'packfile URIs with fetch instead of clone' '
 	git -C "$P" add my-blob &&
 	git -C "$P" commit -m x &&
 
-	configure_exclusion "$P" my-blob >h &&
+	configure_exclusion blob "$P" my-blob >h &&
 
 	git init http_child &&
 
@@ -902,6 +912,37 @@ test_expect_success 'packfile URIs with fetch instead of clone' '
 		fetch "$HTTPD_URL/smart/http_parent"
 '
 
+test_expect_success 'commits packfile URIs with fetch instead of clone' '
+	P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+	rm -rf "$P" http_child log &&
+
+	git init "$P" &&
+	git -C "$P" config "uploadpack.allowsidebandall" "true" &&
+
+	echo my-blob >"$P/my-blob" &&
+	git -C "$P" add my-blob &&
+	git -C "$P" commit -m x &&
+
+
+	mycommit=$(git -C "$P" rev-parse HEAD) &&
+    echo other-blob >"$P/other-blob" &&
+    git -C "$P" add other-blob &&
+    git -C "$P" commit -m x &&
+	othercommit=$(git -C "$P" rev-parse HEAD) &&
+	configure_exclusion commit "$P" "$mycommit" >h &&
+	configure_exclusion commit "$P" "$othercommit" >h2 &&
+
+	git init http_child &&
+
+	GIT_TRACE=1 GIT_TEST_SIDEBAND_ALL=1 \
+	git -C http_child -c protocol.version=2 \
+		-c fetch.uriprotocols=http,https \
+		fetch "$HTTPD_URL/smart/http_parent" &&
+	ls http_child/.git/objects/pack/*.pack \
+    	    http_child/.git/objects/pack/*.idx >filelist &&
+    	test_line_count = 6 filelist
+'
+
 test_expect_success 'fetching with valid packfile URI but invalid hash fails' '
 	P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
 	rm -rf "$P" http_child log &&
@@ -915,7 +956,7 @@ test_expect_success 'fetching with valid packfile URI but invalid hash fails' '
 	git -C "$P" add other-blob &&
 	git -C "$P" commit -m x &&
 
-	configure_exclusion "$P" my-blob >h &&
+	configure_exclusion blob "$P" my-blob >h &&
 	# Configure a URL for other-blob. Just reuse the hash of the object as
 	# the hash of the packfile, since the hash does not matter for this
 	# test as long as it is not the hash of the pack, and it is of the
@@ -944,7 +985,7 @@ test_expect_success 'packfile-uri with transfer.fsckobjects' '
 	git -C "$P" add my-blob &&
 	git -C "$P" commit -m x &&
 
-	configure_exclusion "$P" my-blob >h &&
+	configure_exclusion blob "$P" my-blob >h &&
 
 	sane_unset GIT_TEST_SIDEBAND_ALL &&
 	git -c protocol.version=2 -c transfer.fsckobjects=1 \
@@ -978,7 +1019,7 @@ test_expect_success 'packfile-uri with transfer.fsckobjects fails on bad object'
 	git -C "$P" add my-blob &&
 	git -C "$P" commit -m x &&
 
-	configure_exclusion "$P" my-blob >h &&
+	configure_exclusion blob "$P" my-blob >h &&
 
 	sane_unset GIT_TEST_SIDEBAND_ALL &&
 	test_must_fail git -c protocol.version=2 -c transfer.fsckobjects=1 \
@@ -1000,7 +1041,7 @@ test_expect_success 'packfile-uri with transfer.fsckobjects succeeds when .gitmo
 	git -C "$P" add .gitmodules &&
 	git -C "$P" commit -m x &&
 
-	configure_exclusion "$P" .gitmodules >h &&
+	configure_exclusion blob "$P" .gitmodules >h &&
 
 	sane_unset GIT_TEST_SIDEBAND_ALL &&
 	git -c protocol.version=2 -c transfer.fsckobjects=1 \
@@ -1026,7 +1067,7 @@ test_expect_success 'packfile-uri with transfer.fsckobjects fails when .gitmodul
 	git -C "$P" add .gitmodules &&
 	git -C "$P" commit -m x &&
 
-	configure_exclusion "$P" .gitmodules >h &&
+	configure_exclusion blob "$P" .gitmodules >h &&
 
 	sane_unset GIT_TEST_SIDEBAND_ALL &&
 	test_must_fail git -c protocol.version=2 -c transfer.fsckobjects=1 \
diff --git a/upload-pack.c b/upload-pack.c
index 5c1cd19612..34f8bb81a8 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -1744,9 +1744,12 @@ int upload_pack_advertise(struct repository *r,
 		     allow_sideband_all_value))
 			strbuf_addstr(value, " sideband-all");
 
-		if (!repo_config_get_string(the_repository,
+		if ((!repo_config_get_string(the_repository,
 					    "uploadpack.blobpackfileuri",
-					    &str) &&
+					    &str) ||
+            !repo_config_get_string(the_repository,
+                        "uploadpack.commitpackfileuri",
+                        &str)) &&
 		    str) {
 			strbuf_addstr(value, " packfile-uris");
 			free(str);
-- 
2.31.1.442.g7e39198978.dirty


^ permalink raw reply related	[flat|nested] 73+ messages in thread
* Re: [PATCH] Packfile-uris support excluding commit objects
@ 2021-05-18  8:08 Teng Long
  0 siblings, 0 replies; 73+ messages in thread
From: Teng Long @ 2021-05-18  8:08 UTC (permalink / raw)
  To: avarab; +Cc: dyroneteng, git, jonathantanmy

To Ævar Arnfjörð Bjarmaso wrote:

> It seems like this and your
> http://lore.kernel.org/git/20210506073354.27833-1-dyroneteng@gmail.com
> should be part of one series, not split up.

Obviously, the current series of patches will be longer, and the patches
of the separate documents you mentioned can be repaired and released in
advance. The latter, Junio said, will be added to the queue.

> Per my understanding in
> https://lore.kernel.org/git/87o8hk820f.fsf@evledraar.gmail.com/ this +
> Jonathan's earlier bfc2a36ff2a (Doc: clarify contents of packfile sent
> as URI, 2021-01-20) still makes this whole thing more confusing that it
> needs to be.

> I think we should just have a new uploadpack.excludeObject, and document
> that uploadpack.blobpackfileuri is an (unfortunately named) synonym for
> it. I.e. the actual implementation doesn't care about the objec type it
> just excludes any object listed via an oidmap. No?

Regarding the naming of "uploadpack.blobpackfileuri" and future
scalability issues, I have similar feelings. In next round, I will follow
your naming suggestions about "uploadpack.excludeObject". In addition,
the naming modification may cause a compatible question, although I know
packfile-uris is an experimental feature, I still hope to get some
compatibility-related suggestions.

> I realize you're probably not a native English speaker (neither am I),
> but I honestly can't understand that "This work will be done in a
> further patch recently.". Do you mean something like:
>        ......

Thanks for correcting, I may rewrite the commit message in the next
patch. I'm not a native English speaker, improving :)

> Please send the earlier doc cleanup + the spec change for this + any doc
> updates as one series.

The reason for splitting the two patches is mentioned above, and the
corresponding document modification to support the exclusion of commit
will be added in the next patch series.

> Nit: Split this across two lines.
> Indending with spaces.
> More indenting with spaces, also don't need the {} here.
> Don't indent the "then", also spaces...
> Use ">objh" not "> objh".

Thanks, will correct them in the next round.

> I think by having a uploadpack.excludeObject documented as the primary
>interface to this we could just say "object already listed by an earlier
>exclusion" or something like that.

Thanks, will refer to your suggestions.

> This whole if/else seems like it could be better split up by discovering
> the variable first, using that as a variable, and then avoiding the
> duplication. But if we just used uploadpack.excludeObject...

I will try to modify it, but I am not sure to fully understand what you
mean. If this problem persists in the next patch, please help to point
out the problem.

> Put stuff like this in "test_when_finished"
> You can just use test_commit here, no?

Thanks, now I know about "test_when_finished" and "test_commit".
In addition, there are some problems with using "rm" directly in t5702,
I will replace them in the next patch series.

> Personally I'd just skip this whole "rev-parse HEAD" etc. and just pass
> the tag name(s) created by earlier test_commit, then have
> configure_exclusion ust always do a rev-parse...

Thanks, will use tag name(s) instead of "HEAD".

^ permalink raw reply	[flat|nested] 73+ messages in thread

end of thread, other threads:[~2021-10-19 11:40 UTC | newest]

Thread overview: 73+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-07  2:11 [PATCH] Packfile-uris support excluding commit objects Teng Long
2021-05-10 11:14 ` Ævar Arnfjörð Bjarmason
2021-05-18  8:49 ` [PATCH v2 0/3] packfile-uris: commit objects exclusion Teng Long
2021-05-18  8:49   ` [PATCH v2 1/3] packfile-uris: support for excluding commit object Teng Long
2021-05-19  4:28     ` Junio C Hamano
2021-05-20  4:46     ` Junio C Hamano
2021-05-18  8:49   ` [PATCH v2 2/3] packfile-uris.txt: " Teng Long
2021-05-18  8:49   ` [PATCH v2 3/3] t5702: excluding commits with packfile-uris Teng Long
2021-07-26  9:46   ` [PATCH v3 0/3] packfile-uris: commit objects exclusio Teng Long
2021-07-26  9:46     ` [PATCH v3 1/3] packfile-uris: support for excluding commit objects Teng Long
2021-07-26 18:15       ` Junio C Hamano
2021-07-26 19:45         ` Felipe Contreras
2021-08-11  1:44         ` Teng Long
2021-07-26  9:46     ` [PATCH v3 2/3] t5702: " Teng Long
2021-07-26 15:03       ` Ævar Arnfjörð Bjarmason
2021-08-11  1:46         ` [PATCH v3 1/3] packfile-uris: " Teng Long
2021-07-26  9:46     ` [PATCH v3 3/3] packfile-uri.txt: " Teng Long
2021-07-26 20:52       ` Junio C Hamano
2021-08-11  1:47         ` Teng Long
2021-07-26 12:34     ` [PATCH v3 0/3] packfile-uris: commit objects exclusio Ævar Arnfjörð Bjarmason
2021-08-11  1:48       ` Teng Long
2021-08-11  7:45     ` [PATCH v4 0/7] packfile-uris: commits and trees exclusion Teng Long
2021-08-11  7:45       ` [PATCH v4 1/7] pack-objects.c: introduce new method `match_packfile_uri_exclusions` Teng Long
2021-08-11  7:45       ` [PATCH v4 2/7] Add new parameter "carry_data" for "show_object" function Teng Long
2021-08-11  7:45       ` [PATCH v4 3/7] packfile-uri: support for excluding commit objects Teng Long
2021-08-11  7:45       ` [PATCH v4 4/7] packfile-uri: support for excluding tree objects Teng Long
2021-08-11  7:45       ` [PATCH v4 5/7] packfile-uri.txt: support for excluding commits and trees Teng Long
2021-08-11  9:59         ` Bagas Sanjaya
2021-08-11  7:45       ` [PATCH v4 6/7] t5702: replace with "test_when_finished" for cleanup Teng Long
2021-08-11  7:45       ` [PATCH v4 7/7] t5702: support for excluding commit objects Teng Long
2021-08-25  2:21       ` [PATCH v5 00/14] packfile-uris: commits, trees and tags exclusion Teng Long
2021-08-25  2:21         ` [PATCH v5 01/14] pack-objects.c: introduce new method `match_packfile_uri_exclusions` Teng Long
2021-08-25  2:21         ` [PATCH v5 02/14] Add new parameter "carry_data" for "show_object" function Teng Long
2021-08-26 20:45           ` Junio C Hamano
2021-09-02 11:08             ` Teng Long
2021-08-25  2:21         ` [PATCH v5 03/14] packfile-uri: support for excluding commit objects Teng Long
2021-08-25 23:49           ` Ævar Arnfjörð Bjarmason
2021-09-02 12:26             ` Teng Long
2021-08-26 20:56           ` Junio C Hamano
2021-09-02 12:51             ` Teng Long
2021-08-25  2:21         ` [PATCH v5 04/14] packfile-uri: support for excluding tree objects Teng Long
2021-08-25  2:21         ` [PATCH v5 05/14] packfile-uri.txt: support for excluding commits and trees Teng Long
2021-08-25 23:52           ` Ævar Arnfjörð Bjarmason
2021-09-02 11:23             ` Teng Long
2021-08-25  2:21         ` [PATCH v5 06/14] t5702: replace with "test_when_finished" for cleanup Teng Long
2021-08-25 23:55           ` Ævar Arnfjörð Bjarmason
2021-09-02 11:37             ` Teng Long
2021-08-25  2:21         ` [PATCH v5 07/14] t5702: support for excluding commit objects Teng Long
2021-08-25  2:21         ` [PATCH v5 08/14] Add new parameter "carry_data" for "show_commit function Teng Long
2021-08-25  2:21         ` [PATCH v5 09/14] commit.h: add wrapped tags in commit struct Teng Long
2021-08-25 23:58           ` Ævar Arnfjörð Bjarmason
2021-09-02 12:17             ` Teng Long
2021-09-02 12:39           ` ZheNing Hu
2021-09-02 13:01             ` Teng Long
2021-08-25  2:21         ` [PATCH v5 10/14] object.h: add referred tags in `referred_objects` struct Teng Long
2021-08-25  2:21         ` [PATCH v5 11/14] packfile-uri: support for excluding tag objects Teng Long
2021-08-25  2:21         ` [PATCH v5 12/14] packfile-uri.txt: " Teng Long
2021-08-25  2:21         ` [PATCH v5 13/14] t5702: add tag exclusion test case Teng Long
2021-08-25  2:21         ` [PATCH v5 14/14] pack-objects.c: introduce `want_exclude_object` function Teng Long
2021-10-19 11:38         ` [PATCH v6 00/12] packfile-uri: support excluding multiple object types Teng Long
2021-10-19 11:38           ` [PATCH v6 01/12] objects.c: introduce `exclude_level` enum Teng Long
2021-10-19 11:38           ` [PATCH v6 02/12] Introduce function `match_packfile_uri_exclusions` Teng Long
2021-10-19 11:38           ` [PATCH v6 03/12] Replace `show_data` with structure `show_info` Teng Long
2021-10-19 11:38           ` [PATCH v6 04/12] Introduce `uploadpack.excludeobject` configuration Teng Long
2021-10-19 11:38           ` [PATCH v6 05/12] t5702: test cases for `uploadpack.excludeobject` Teng Long
2021-10-19 11:38           ` [PATCH v6 06/12] packfile-uri: support for excluding commits Teng Long
2021-10-19 11:38           ` [PATCH v6 07/12] t5702: test cases " Teng Long
2021-10-19 11:38           ` [PATCH v6 08/12] packfile-uri: support for excluding trees Teng Long
2021-10-19 11:38           ` [PATCH v6 09/12] t5702: test cases " Teng Long
2021-10-19 11:38           ` [PATCH v6 10/12] packfile-uri: support for excluding tags Teng Long
2021-10-19 11:38           ` [PATCH v6 11/12] t5702: test cases " Teng Long
2021-10-19 11:38           ` [PATCH v6 12/12] packfile-uri.txt: support multiple object types Teng Long
2021-05-18  8:08 [PATCH] Packfile-uris support excluding commit objects Teng Long

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).