[PATCH] upload-pack: add a trigger for post-upload-pack hook

Subsystems: the rest

STALE3736d

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

[PATCH] upload-pack: add a trigger for post-upload-pack hook

From: Tom Preston-Werner <hidden>
Date: 2016-06-15 22:47:16

A post-upload-pack hook is desirable for Git hosts that need to
collect statistics on how many clones and/or fetches are made
on each repository.

The hook is called with either "clone" or "fetch" as the only
argument, depending on whether a full pack file was sent to the
client or not.

Signed-off-by: Tom Preston-Werner <redacted>
---
 upload-pack.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/upload-pack.c b/upload-pack.c
index f7d308a..96231dc 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -141,6 +141,13 @@ static int do_rev_list(int fd, void *create_full_pack)
 	return 0;
 }
 
+static void run_post_upload_pack_hook(int create_full_pack)
+{
+	const char *fetch_type;
+	fetch_type = (create_full_pack) ? "clone" : "fetch";
+	run_hook(get_index_file(), "post-upload-pack", fetch_type);
+}
+
 static void create_pack_file(void)
 {
 	struct async rev_list;
@@ -314,6 +321,8 @@ static void create_pack_file(void)
 	}
 	if (use_sideband)
 		packet_flush(1);
+
+	run_post_upload_pack_hook(create_full_pack);
 	return;
 
  fail:
-- 
1.6.3.1

Re: [PATCH] upload-pack: add a trigger for post-upload-pack hook

From: Tom Werner <hidden>
Date: 2016-06-15 22:47:18

On Tue, Aug 18, 2009 at 12:04 AM, Tom Preston-Werner[off-list ref] wrote:
A post-upload-pack hook is desirable for Git hosts that need to
collect statistics on how many clones and/or fetches are made
on each repository.

The hook is called with either "clone" or "fetch" as the only
argument, depending on whether a full pack file was sent to the
client or not.
I was hoping to get some feedback on this patch, either positive or
negative. Since we'll be applying this patch for our use of the Git
Daemon on GitHub, it would be great to see it in core, so we don't
have to maintain custom debian builds forever. I'd imagine that other
Git hosting sites would find this hook useful as well. Thanks!

Tom

--
Tom Preston-Werner
GitHub Cofounder
http://tom.preston-werner.com
github.com/mojombo

Re: [PATCH] upload-pack: add a trigger for post-upload-pack hook

From: Jeff King <hidden>
Date: 2016-06-15 22:47:18

On Tue, Aug 25, 2009 at 10:43:57AM -0700, Tom Werner wrote:
On Tue, Aug 18, 2009 at 12:04 AM, Tom Preston-Werner[off-list ref] wrote:
quoted
A post-upload-pack hook is desirable for Git hosts that need to
collect statistics on how many clones and/or fetches are made
on each repository.

The hook is called with either "clone" or "fetch" as the only
argument, depending on whether a full pack file was sent to the
client or not.
I was hoping to get some feedback on this patch, either positive or
negative. Since we'll be applying this patch for our use of the Git
Daemon on GitHub, it would be great to see it in core, so we don't
have to maintain custom debian builds forever. I'd imagine that other
Git hosting sites would find this hook useful as well. Thanks!
I expect it didn't get any response because nobody here cared one way or
the other. Not too surprising, since I think not many people are running
a GitHub-sized hosting site that cares about such statistics. ;) So I
think following up as you are doing is the right thing.

As for the hook itself, the concept certainly seems sane to me. It
passes the "hook" test defined here:

  http://thread.gmane.org/gmane.comp.version-control.git/70781/focus=71069

because it is a remote trigger.

But a few comments on the patch:
---
 upload-pack.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)
It needs at least a mention in Documentation/githooks.txt.
+static void run_post_upload_pack_hook(int create_full_pack)
+{
+	const char *fetch_type;
+	fetch_type = (create_full_pack) ? "clone" : "fetch";
+	run_hook(get_index_file(), "post-upload-pack", fetch_type);
+}
Does it really need an index file? This operation in question seems to
be totally disconnected from the index (and indeed, most bare
repositories won't even have one). Probably it should pass NULL as the
initial argument to run_hook.

Is there any other information that might be useful to other non-GitHub
users of the hook? The only thing I can think of is the list of refs
that were fetched. I don't want to over-engineer it, but nor do I want
to be left with the mess of retro-fitting more information onto an
existing hook later. Maybe others can comment on whether they would find
more information useful.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help