Re: [PATCH] upload-pack: add a trigger for post-upload-pack hook
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 29, 2009, 06:17 UTC
- Message-ID
- <7vljl3p2iw.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <m3ws4pl6x7.fsf@localhost.localdomain>
Jakub Narebski <jnareb@gmail.com> writes:
Show 9 quoted lines
>> I'd like to suggest the following line from the original patch: >> >> full-pack integer:: >> 1 if the request was considered a full clone, 0 if it was a >> partial update (fetch) > > If it is all "want" and no "have", it is clone or fetch into empty > repository. If additionaly "want"s cover all refs, it is a clone. > No need to pass this information: it can be derived.
Well, not exactly.
Here is an iffy RFC patch. Iffy not in the sense that its implementation is questionable, but in the sense that I am not really convinced if the distinction between fetching some (or in the worst case, most) but not all refs, and fetching full set of refs, into an empty repository is something worth making.
Does anybody from GitHub have any input? Is there something that can still improved to suit GitHub's needs?
-- >8 -- Subject: [PATCH] upload-pack: feed "kind [clone|fetch]" to post-upload-pack hook
A request to clone the repository does not give any "have" but asks for all the refs we offer with "want". When a request does not ask to clone the repository fully, but asks to fetch some refs into an empty repository, it will not give any "have" but its "want" won't ask for all the refs we offer.
If we suppose (and I would say this is a rather big if) that it makes sense to distinguish these two cases, a hook cannot reliably do this alone. The hook can detect lack of "have" and bunch of "want", but there is no direct way to tell if the other end asked for all refs we offered, or merely most of them.
Between the time we talked with the other end and the time the hook got called, we may have acquired more refs or lost some refs in the repository by concurrent operations. Given that we plan to introduce selective advertisement of refs with a protocol extension, it would become even more difficult for hooks to guess between these two cases.
This adds "kind [clone|fetch]" to hook's input, as a stable interface to allow the hooks to tell these cases apart.
Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/githooks.txt | 4 ++++ t/t5501-post-upload-pack.sh | 24 ++++++++++++++++++++++-- upload-pack.c | 4 ++++ 3 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index 036f6c7..c308d29 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -332,6 +332,10 @@ time float:: size decimal:: Size of the resulting packfile in bytes. +kind string: + Either "clone" (when the client did not give us any "have", and asked + for all our refs with "want"), or "fetch" (otherwise). + pre-auto-gc ----------- diff --git a/t/t5501-post-upload-pack.sh b/t/t5501-post-upload-pack.sh index 47ee7b5..d89fb51 100755 --- a/t/t5501-post-upload-pack.sh +++ b/t/t5501-post-upload-pack.sh @@ -29,7 +29,9 @@ test_expect_success initial ' ) && want=$(sed -n "s/^want //p" "$LOGFILE") && test "$want" = "$(git rev-parse --verify B)" && - ! grep "^have " "$LOGFILE" + ! grep "^have " "$LOGFILE" && + kind=$(sed -n "s/^kind //p" "$LOGFILE") && + test "$kind" = fetch ' test_expect_success second ' @@ -43,7 +45,25 @@ test_expect_success second ' want=$(sed -n "s/^want //p" "$LOGFILE") && test "$want" = "$(git rev-parse --verify C)" && have=$(sed -n "s/^have //p" "$LOGFILE") && - test "$have" = "$(git rev-parse --verify B)" + test "$have" = "$(git rev-parse --verify B)" && + kind=$(sed -n "s/^kind //p" "$LOGFILE") && + test "$kind" = fetch +' + +test_expect_success all ' + rm -fr sub && + HERE=$(pwd) && + git init sub && + ( + cd sub && + git clone "file://$HERE/.git" new + ) && + sed -n "s/^want //p" "$LOGFILE" | sort >actual && + git rev-parse A B C | sort >expect && + test_cmp expect actual && + ! grep "^have " "$LOGFILE" && + kind=$(sed -n "s/^kind //p" "$LOGFILE") && + test "$kind" = clone ' test_done diff --git a/upload-pack.c b/upload-pack.c index 857440d..8e82179 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -187,6 +187,10 @@ static int run_post_upload_pack_hook(size_t total, struct timeval *tv) (long)tv->tv_sec, (long)tv->tv_usec); if (!err) err |= feed_msg_to_hook(proc.in, "size %ld\n", (long)total); + if (!err) + err |= feed_msg_to_hook(proc.in, "kind %s\n", + (nr_our_refs == want_obj.nr && !have_obj.nr) + ? "clone" : "fetch"); if (close(proc.in)) err = 1; if (finish_command(&proc))
-- 1.6.4.1.307.g70e9f