{"thread":{"id":"29246","subject":"[PATCH] add post-fetch hook","startedAt":"2011-12-24T23:42:12Z","lastAt":"2011-12-28T19:30:08Z","messageCount":16,"participants":["Joey Hess","Junio C Hamano","Jakub Narebski","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"181669","messageId":"20111224234212.GA21533@gnu.kitenet.net","threadId":"29246","inReplyTo":null,"subject":"[PATCH] add post-fetch hook","fromName":"Joey Hess","fromEmail":"joey@kitenet.net","sentAt":"2011-12-24T23:42:12Z","receivedAt":"2011-12-24T23:42:12Z","isPatch":true,"sender":{"key":"joey@kitenet.net","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"The post-fetch hook is fed on its stdin all refs that were newly fetched.\nIt is not allowed to abort the fetch (or pull), but can modify what\nwas fetched or take other actions.\n\nOne example use of this hook is to automatically merge certain remote\nbranches into a local branch. Another is to update a local cache\n(such as a search index) with the fetched refs. No other hook is run\nnear fetch time, except for post-merge, which doesn't always run after a\nfetch, which is why this additional hook is useful.\n\nSigned-off-by: Joey Hess \n\n---\n\nThe #1 point of confusion for git-annex users is the need to run\n\"git annex merge\" after fetching. That does a union merge of newly\nfetched remote git-annex branches into the local git-annex branch.\nIf a user does a \"git pull; ... ; git push\" and forgets to git annex merge\nin between, their push often fails as the git-annex branches have diverged.\nWith this hook, that confusing step can be eliminated.\n\nSince git annex merge could be run at any point between fetch and push,\nI considered several different hooks, including this one, a pre-push hook,\nand a variant of this hook that does not feed the hook any information\non stdin. I chose this one, with the information on stdin because it seems\nthe most generally useful, and will let me make git annex merge slightly\nmore optimal than it would be without the stdin.\n\n Documentation/githooks.txt | 12 ++++++++++\n builtin/fetch.c | 50 ++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 62 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 28edefa..96a588c 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -162,6 +162,18 @@ This hook can be used to perform repository validity checks, auto-display\n differences from the previous HEAD if different, or set working dir metadata\n properties.\n \n+post-fetch\n+~~~~~~~~~~\n+\n+This hook is invoked by 'git fetch' (commonly called by 'git pull'), after\n+refs have been fetched from the remote repository. It takes no arguments,\n+but is fed a list of new or updated refs on its standard input. This hook\n+cannot affect the outcome of 'git fetch' and is not executed, if nothing\n+was fetched.\n+\n+This hook can make modifications to the fetched refs, or take other\n+actions.\n+\n post-merge\n ~~~~~~~~~~\n \ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 33ad3aa..d813b8e 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -89,6 +89,52 @@ static struct option builtin_fetch_options[] = {\n \tOPT_END()\n };\n \n+static const char post_fetch_hook[] = \"post-fetch\";\n+struct ref *fetched_refs = NULL;\n+void run_post_fetch_hook (void) {\n+\tstruct ref *ref;\n+\tstruct child_process proc;\n+\tconst char *argv[2];\n+\tFILE *f;\n+\n+\tif (! fetched_refs)\n+\t\treturn;\n+\n+\targv[0] = git_path(\"hooks/%s\", post_fetch_hook);\n+\tif (access(argv[0], X_OK) < 0)\n+\t\treturn;\n+\targv[1] = NULL;\n+\n+\tmemset(&proc, 0, sizeof(proc));\n+\tproc.argv = argv;\n+\tproc.in = -1;\n+\tproc.stdout_to_stderr = 1;\n+\tif (start_command(&proc) != 0)\n+\t\treturn;\n+\n+\tf = fdopen(proc.in, \"w\");\n+\tif (f == NULL) {\n+\t\tclose(proc.in);\n+\t\tgoto cleanup;\n+\t}\n+\tfor (ref = fetched_refs; ref; ref = ref->next)\n+\t\tfprintf(f, \"%s\\n\", ref->name);\n+\tfclose(f);\n+\n+cleanup:\n+\tfree_refs(fetched_refs);\n+\tfetched_refs = NULL;\n+\n+\tfinish_command(&proc);\n+\tclose(proc.in);\n+}\n+\n+void post_fetch_hook_observe (const struct ref *fetched_ref) {\n+\tstruct ref *ref = copy_ref(fetched_ref);\n+\tref->next = fetched_refs;\n+\tfetched_refs = ref;\n+}\n+\n static void unlock_pack(void)\n {\n \tif (transport)\n@@ -233,6 +279,7 @@ static int s_update_ref(const char *action,\n \tif (write_ref_sha1(lock, ref->new_sha1, msg) < 0)\n \t\treturn errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :\n \t\t\t\t\t STORE_REF_ERROR_OTHER;\n+\tpost_fetch_hook_observe(ref);\n \treturn 0;\n }\n \n@@ -755,6 +802,9 @@ static int do_fetch(struct transport *transport,\n \t\tfree_refs(ref_map);\n \t}\n \n+\t/* Run hook only after fetching all refs. */\n+\trun_post_fetch_hook();\n+\n \treturn 0;\n }\n \n-- \n1.7.7.3\n"},{"id":"181682","messageId":"7v4nwpbaxq.fsf@alter.siamese.dyndns.org","threadId":"29246","inReplyTo":"20111224234212.GA21533@gnu.kitenet.net","subject":"Re: [PATCH] add post-fetch hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-25T03:13:37Z","receivedAt":"2011-12-25T03:13:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joey Hess writes:\n\n> The post-fetch hook is fed on its stdin all refs that were newly fetched.\n> It is not allowed to abort the fetch (or pull), but can modify what\n> was fetched or take other actions.\n>\n> One example use of this hook is to automatically merge certain remote\n> branches into a local branch. Another is to update a local cache\n> (such as a search index) with the fetched refs. No other hook is run\n> near fetch time, except for post-merge, which doesn't always run after a\n> fetch, which is why this additional hook is useful.\n>\n> Signed-off-by: Joey Hess \n\nA typical \"'git pull' invokes 'git fetch' and then lets 'git merge' (or\n'git rebase') integrate the work on the current branch with what was\nfetched\" sequence goes like this:\n\n - 'git fetch':\n . grabs the necessary objects from the remote;\n . decides what remote tracking branches are updated to point\n at what objects, and what updates are to be denied;\n . updates remote tracking branches accordingly; and\n . writes $GIT_DIR/FETCH_HEAD to communicate what have been fetched\n and what are to be merged.\n\n - 'git merge':\n . reads $GIT_DIR/FETCH_HEAD to learn what commits to be merged; and\n . merges the commits to the current branch.\n\nEven though we do not add arbitrary hooks on the client side that could\neasily be implemented by wrapping the client side commands (i.e. you could\nimplement \"git myfetch\" that runs \"git fetch\" followed by whatever script\nthat mucks with the result of the fetch) in general, I can see that it\nwould be useful to have a hook that can tweak the result of the fetch run\ninside of the \"git pull\", because you cannot tell \"git pull\" to run \"git\nmyfetch\" instead of \"git fetch\".\n\nBecause the sequence of \"git fetch\" followed by \"git merge\", both commands\nissued by the end user, should be equivalent to \"git pull\" from an end\nuser's point of view, the hook must be called from near the end of \"git\nfetch\" if we were to have such a hook that tweaks the result of the fetch\ninside \"pull\". IOW, the implementation, even though logically it belongs\nto \"pull\", has to be inside \"fetch\", not \"pull\".\n\nIn that sense, I am not fundamentally opposed to the idea of adding a post\nfetch hook that allows tweaking of the result.\n\n*HOWEVER*\n\nIf we _were_ to sanction the use of the hook to tweak the result, I do not\nwant to see it implemented as an ad-hoc hack that tells the hook writers\nthat it is _entirely_ their responsiblity to update the remote tracking\nbranches from what it fetched, and also update $GIT_DIR/FETCH_HEAD to\nmaintain consistency between these two places.\n\nA very cursory look at the patch tells me that there are a few problems\nwith it. It does not seem to affect what will go to $GIT_DIR/FETCH_HEAD\nat all, and hence it does not have any way to affect the result of the\nfetch that does not store it to any of our remote tracking branches.\n\n> The #1 point of confusion for git-annex users is the need to run\n> \"git annex merge\" after fetching. That does a union merge of newly\n> fetched remote git-annex branches into the local git-annex branch.\n\nThat use case sounds like that \"git fetch\" is called as a first class UI,\nwhich is covered by \"git myfetch\" (you can call it \"git annex fetch\")\nwrapper approach, the canonical example of a hook that we explicitly do\nnot want to add. It also does not seem to call for mucking with the result\nof the fetch at all.\n\nPerhaps the two concepts should be separated into different hooks?\n"},{"id":"181684","messageId":"20111225035059.GA29852@gnu.kitenet.net","threadId":"29246","inReplyTo":"7v4nwpbaxq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] add post-fetch hook","fromName":"Joey Hess","fromEmail":"joey@kitenet.net","sentAt":"2011-12-25T03:50:59Z","receivedAt":"2011-12-25T03:50:59Z","isPatch":true,"sender":{"key":"joey@kitenet.net","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Junio C Hamano wrote:\n> If we _were_ to sanction the use of the hook to tweak the result, I do not\n> want to see it implemented as an ad-hoc hack that tells the hook writers\n> that it is _entirely_ their responsiblity to update the remote tracking\n> branches from what it fetched, and also update $GIT_DIR/FETCH_HEAD to\n> maintain consistency between these two places.\n> \n> A very cursory look at the patch tells me that there are a few problems\n> with it. It does not seem to affect what will go to $GIT_DIR/FETCH_HEAD\n> at all, and hence it does not have any way to affect the result of the\n> fetch that does not store it to any of our remote tracking branches.\n\nTrue, it does not update FETCH_HEAD. I had not considered using the hook\nthat way.\n\nI suppose that after running the hook, fetch could check each remote\ntracking branch for a new value, and only then write to FETCH_HEAD.\n\n> > The #1 point of confusion for git-annex users is the need to run\n> > \"git annex merge\" after fetching. That does a union merge of newly\n> > fetched remote git-annex branches into the local git-annex branch.\n> \n> That use case sounds like that \"git fetch\" is called as a first class UI,\n> which is covered by \"git myfetch\" (you can call it \"git annex fetch\")\n> wrapper approach, the canonical example of a hook that we explicitly do\n> not want to add. It also does not seem to call for mucking with the result\n> of the fetch at all.\n\nMost users are fetching by calling git pull as part of their normal\nworkflow. I would like to avoid git-annex needing its own special pull\ncommand. For one thing, there can be many programs that use git branches\nin similar ways (another one is pristine-tar), and a user shouldn't have\nto run multiple wrapped versions of git fetch or pull when using\nmultiple such programs.\n\n-- \nsee shy jo\n"},{"id":"181686","messageId":"7vsjk99exw.fsf@alter.siamese.dyndns.org","threadId":"29246","inReplyTo":"20111225035059.GA29852@gnu.kitenet.net","subject":"Re: [PATCH] add post-fetch hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-25T09:30:03Z","receivedAt":"2011-12-25T09:30:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joey Hess writes:\n\n> Junio C Hamano wrote:\n>> If we _were_ to sanction the use of the hook to tweak the result, I do not\n>> want to see it implemented as an ad-hoc hack that tells the hook writers\n>> that it is _entirely_ their responsiblity to update the remote tracking\n>> branches from what it fetched, and also update $GIT_DIR/FETCH_HEAD to\n>> maintain consistency between these two places.\n>> \n>> A very cursory look at the patch tells me that there are a few problems\n>> with it. It does not seem to affect what will go to $GIT_DIR/FETCH_HEAD\n>> at all, and hence it does not have any way to affect the result of the\n>> fetch that does not store it to any of our remote tracking branches.\n>\n> True, it does not update FETCH_HEAD. I had not considered using the hook\n> that way.\n\nPerhaps I misread what you wrote in the commit log message then. I\nsomehow got an impression that one of the advertised way for the hook to\nbe used was to lie which commits the remote tracking refs point at by\nletting it run \"update-ref refs/*/*\" and the lie is later picked up by\nre-reading them, but I wasn't reading the patch very carefully.\n\nIf your use case does not involve updating the remote tracking refs nor\nFETCH_HEAD (updating only one and not the other is a no-starter), then we\nshould explicitly forbid it in the documentation, as allowing updates will\ninvite inconsistencies.\n\nAlthough I do not deeply care between such a \"trigger to only notify, no\ntouching\" hook and a full-blown \"allow hook writers to easily lie about\nwhat happened in the fetch\" hook, I was hoping that we would get this\nright and useful if we were spending our brain bandwidth on it. I am not\nvery fond of an easier \"trigger to only notify\" hook because people are\nbound to misuse the interface and try updating the refs anyway, making it\neasy to introduce inconsistencies between refs and FETCH_HEAD that will\nconfuse the later \"merge\" step.\n\nAs hook writers are more prone to write such an incorrect code than people\nwho implement the mechanism to call hooks on the git-core side, the more\nthe hook interface helps hook writers to avoid such mistakes, the better.\n\nSo if we were to allow the hook to lie what commits were fetched and store\nsomething different from what we fetched in the remote tracking refs, I\nthink the correct place to do so would be in store_updated_refs(),\nimmediately before we call check_everything_connected().\n\n - Feed the contents of the ref_map to the hook. For each entry, the hook\n would get (at least):\n . the object name;\n . the refname at the remote;\n . the refname at the local (which could be empty when we are not\n copying it to any of our local ref); and\n . if the entry is to be used for merge.\n\n - The hook must read _everything_ from its standard input, and then\n return the\n re-written result in the same format as its input. The hook could\n . update the object name (i.e. re-point the remote tracking ref);\n . update the local refname (i.e. use different remote tracking ref);\n . change \"merge\" flag between true/false; and/or\n . add or remove entries\n\n - You read from the hook and replace the ref_map list that is fed to\n check_everything_connected(). This ref_map list is what is used in the\n next for() loop that calls update_local_ref() to update the remote\n tracking ref, records the entry in FETCH_HEAD, and produces the report.\n\nThis way, the hook cannot screw up, as what it tells us will consistently\nbe written by us to where it should go.\n"},{"id":"181688","messageId":"m3liq0fwkz.fsf@localhost.localdomain","threadId":"29246","inReplyTo":"7vsjk99exw.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] add post-fetch hook","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-12-25T16:24:52Z","receivedAt":"2011-12-25T16:24:52Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano writes:\n\n> [...] if we were to allow the hook to lie what commits were fetched and store\n> something different from what we fetched in the remote tracking refs, I\n> think the correct place to do so would be in store_updated_refs(),\n> immediately before we call check_everything_connected().\n> \n> - Feed the contents of the ref_map to the hook. For each entry, the hook\n> would get (at least):\n> . the object name;\n> . the refname at the remote;\n> . the refname at the local (which could be empty when we are not\n> copying it to any of our local ref); and\n> . if the entry is to be used for merge.\n> \n> - The hook must read _everything_ from its standard input, and then\n> return the\n> re-written result in the same format as its input. The hook could\n> . update the object name (i.e. re-point the remote tracking ref);\n> . update the local refname (i.e. use different remote tracking ref);\n> . change \"merge\" flag between true/false; and/or\n> . add or remove entries\n\nThis is a very nice idea, IMHO, both because it makes it simple to\nimplement no-op (example) hook by just using \"cat\", and beause it\nmakes possible to stack such hooks (e.g. one from git-annex with the\none from pristine-tar etc.).\n\nOne thing that needs to be specified is what should happen if the hook\nchanges \"the refname at the remote\" part...\n \n> - You read from the hook and replace the ref_map list that is fed to\n> check_everything_connected(). This ref_map list is what is used in the\n> next for() loop that calls update_local_ref() to update the remote\n> tracking ref, records the entry in FETCH_HEAD, and produces the report.\n> \n> This way, the hook cannot screw up, as what it tells us will consistently\n> be written by us to where it should go.\n\n-- \nJakub Narebski\n"},{"id":"181692","messageId":"20111225175424.GA32626@gnu.kitenet.net","threadId":"29246","inReplyTo":"7vsjk99exw.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] add post-fetch hook","fromName":"Joey Hess","fromEmail":"joey@kitenet.net","sentAt":"2011-12-25T17:54:24Z","receivedAt":"2011-12-25T17:54:24Z","isPatch":true,"sender":{"key":"joey@kitenet.net","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Junio C Hamano wrote:\n> So if we were to allow the hook to lie what commits were fetched and store\n> something different from what we fetched in the remote tracking refs, I\n> think the correct place to do so would be in store_updated_refs(),\n> immediately before we call check_everything_connected().\n> \n> - Feed the contents of the ref_map to the hook. For each entry, the hook\n> would get (at least):\n> . the object name;\n> . the refname at the remote;\n> . the refname at the local (which could be empty when we are not\n> copying it to any of our local ref); and\n> . if the entry is to be used for merge.\n> \n> - The hook must read _everything_ from its standard input, and then\n> return the\n> re-written result in the same format as its input. The hook could\n> . update the object name (i.e. re-point the remote tracking ref);\n> . update the local refname (i.e. use different remote tracking ref);\n> . change \"merge\" flag between true/false; and/or\n> . add or remove entries\n> \n> - You read from the hook and replace the ref_map list that is fed to\n> check_everything_connected(). This ref_map list is what is used in the\n> next for() loop that calls update_local_ref() to update the remote\n> tracking ref, records the entry in FETCH_HEAD, and produces the report.\n> \n> This way, the hook cannot screw up, as what it tells us will consistently\n> be written by us to where it should go.\n\nThis is a good plan, the only problem I see with it is that\nstore_updated_refs is potentially called twice in a fetch, when the\nautomated tag following is done. I don't see that as a large problem,\nperhaps it could even be optimised away.\n\nThe format of .git/FETCH_HEAD does not seem appropriate for this hook\nto use (it's not documented, and it doesn't quite have all the necessary\nfields, particularly missing the local refname). Instead, how about this,\nfor the hook's input/output format?\n\n SP SP SP LF\n\nExample:\n\n5d6dfc7cb140a6eb90138334fab2245b69bc8bc4 merge refs/heads/master refs/remotes/origin/master\nf95247ea15bc62a2dab0f6ae3cd247267a0639b8 not-for-merge refs/heads/pu refs/remotes/origin/pu\n2ce0edcd786b790fed580e7df56291619834d276 not-for-merge refs/tags/v1.7.8.1 refs/tags/v1.7.8.1\n\nAllowing the hook to change the merge flag does open up some other\ninteresting uses of the hook. I can now think of three use cases for it:\n\n1. Only accepting tags that meet some criteria, such as being signed\n by a trusted signature.\n2. Causing branches that would not normally be merged to get merged.\n For example, a hook could set the merge flag on a branch when it was\n pulled from a remote other than branch.master.remote. This could be useful\n when using git without a single central origin and with a number of\n repositories that are always wanted to be kept merged.\n3. My git annex merge case.\n\n-- \nsee shy jo\n"},{"id":"181693","messageId":"20111225180629.GB32626@gnu.kitenet.net","threadId":"29246","inReplyTo":"m3liq0fwkz.fsf@localhost.localdomain","subject":"Re: [PATCH] add post-fetch hook","fromName":"Joey Hess","fromEmail":"joey@kitenet.net","sentAt":"2011-12-25T18:06:29Z","receivedAt":"2011-12-25T18:06:29Z","isPatch":true,"sender":{"key":"joey@kitenet.net","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Jakub Narebski wrote:\n> This is a very nice idea, IMHO, both because it makes it simple to\n> implement no-op (example) hook by just using \"cat\", and beause it\n> makes possible to stack such hooks (e.g. one from git-annex with the\n> one from pristine-tar etc.).\n\nIndeed.\n\n> One thing that needs to be specified is what should happen if the hook\n> changes \"the refname at the remote\" part...\n\nI'm not sure what the use case is for including the remote's refname is yet.\nChanging it would allow changing what's written into FETCH_HEAD though.\nFor example:\n\n-2ce0edcd786b790fed580e7df56291619834d276 not-for-merge branch 'maint' of git://git.kernel.org/pub/scm/git/git\n+2ce0edcd786b790fed580e7df56291619834d276 not-for-merge branch 'jc-maint' of git://git.kernel.org/pub/scm/git/git\n\nAnd that would in turn be used by a few things that consume that\ninformation. Whether that's useful, I don't know.\n\n-- \nsee shy jo\n"},{"id":"181697","messageId":"20111226023154.GA3243@gnu.kitenet.net","threadId":"29246","inReplyTo":"7vsjk99exw.fsf@alter.siamese.dyndns.org","subject":"[PATCH] add post-fetch hook","fromName":"Joey Hess","fromEmail":"joey@kitenet.net","sentAt":"2011-12-26T02:31:54Z","receivedAt":"2011-12-26T02:31:54Z","isPatch":true,"sender":{"key":"joey@kitenet.net","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"The post-fetch hook is fed lines on stdin for all refs that were fetched, and\noutputs on stdout possibly modified lines. Its output is parsed and used\nwhen git fetch updates the remote tracking refs, records the entries in\nFETCH_HEAD, and produces its report.\n\n---\n\nNot quite ready to sign off on this yet, but it does work.\nComments and code review appreciated.\n\nDemo:\n\njoey@gnu:~/tmp/demo>cat .git/hooks/post-fetch\n#!/bin/sh\n# Rename branches, and block all tags.\nsed 's!foo!bar!g' | grep -v refs/tags/\njoey@gnu:~/tmp/demo>chmod +x .git/hooks/post-fetch\njoey@gnu:~/tmp/demo>git pull\nFrom /home/joey/tmp/a\n * [new branch] bar -> origin/bar\nAlready up-to-date.\njoey@gnu:~/tmp/demo>chmod -x .git/hooks/post-fetch\njoey@gnu:~/tmp/demo>git pull\nFrom /home/joey/tmp/a\n * [new branch] foo -> origin/foo\n * [new tag] v1.0 -> v1.0\nAlready up-to-date.\njoey@gnu:~/tmp/demo>chmod +x .git/hooks/post-fetch\njoey@gnu:~/tmp/demo>git remote update --prune\nFetching origin\n x [deleted] (none) -> origin/foo\n\n\n Documentation/githooks.txt | 29 ++++++\n builtin/fetch.c | 228 ++++++++++++++++++++++++++++++++++++++++----\n 2 files changed, 238 insertions(+), 19 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 28edefa..9c2b6bf 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -162,6 +162,35 @@ This hook can be used to perform repository validity checks, auto-display\n differences from the previous HEAD if different, or set working dir metadata\n properties.\n \n+post-fetch\n+~~~~~~~~~~\n+\n+This hook is invoked by 'git fetch' (commonly called by 'git pull'), after\n+refs have been fetched from the remote repository. It is not executed, if\n+nothing was fetched.\n+\n+It takes no arguments, but is fed a line of the following format on\n+its standard input for each ref that was fetched.\n+\n+ SP not-for-merge|merge SP SP LF\n+\n+Where the \"not-for-merge\" flag indicates the ref is not to be merged into the\n+current branch, and the \"merge\" flag indicates that 'git merge' should\n+later merge it. The `` is the remote's name for the ref\n+that was pulled, and `` is a name of a remote-tracking branch,\n+like \"refs/remotes/origin/master\", or can be empty if the fetched ref is not\n+being stored in a local refname.\n+\n+The hook must consume all of its standard input, and output back lines\n+of the same format. It can modify its input as desired, including\n+adding or removing lines, updating the sha1 (i.e. re-point the\n+remote-tracking branch), changing the merge flag, and changing the\n+`` (i.e. use different remote-tracking branch).\n+\n+The output of the hook is used to update the remote-tracking branches, and\n+`.git/FETCH_HEAD`, in preparation for for a later merge operation done by\n+'git merge'.\n+\n post-merge\n ~~~~~~~~~~\n \ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 33ad3aa..aa401b2 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -89,6 +89,188 @@ static struct option builtin_fetch_options[] = {\n \tOPT_END()\n };\n \n+static int add_existing(const char *refname, const unsigned char *sha1,\n+\t\t\tint flag, void *cbdata)\n+{\n+\tstruct string_list *list = (struct string_list *)cbdata;\n+\tstruct string_list_item *item = string_list_insert(list, refname);\n+\titem->util = (void *)sha1;\n+\treturn 0;\n+}\n+\t\n+static const char post_fetch_hook[] = \"post-fetch\";\n+struct ref *post_fetch_hook_refs;\n+\n+int feed_post_fetch_hook (int in, int out, void *data)\n+{\n+\tstruct ref *ref;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tint ret;\n+\n+\tfor (ref = post_fetch_hook_refs; ref; ref = ref->next) {\n+\t\tstrbuf_addstr(&buf, sha1_to_hex(ref->old_sha1));\n+\t\tstrbuf_addch(&buf, ' ');\n+\t\tstrbuf_addstr(&buf, ref->merge ? \"merge\" : \"not-for-merge\");\n+\t\tstrbuf_addch(&buf, ' ');\n+\t\tif (ref->name)\n+\t\t\tstrbuf_addstr(&buf, ref->name);\n+\t\tstrbuf_addch(&buf, ' ');\n+\t\tif (ref->peer_ref && ref->peer_ref->name)\n+\t\t\tstrbuf_addstr(&buf, ref->peer_ref->name);\n+\t\tstrbuf_addch(&buf, '\\n');\n+\t}\n+\n+\tret = write_in_full(out, buf.buf, buf.len) != buf.len;\n+\tif (ret)\n+\t\twarning(\"%s hook failed to consume all its input\",\n+\t\t\t\tpost_fetch_hook);\n+\tclose(out);\n+\tstrbuf_release(&buf);\n+\treturn ret;\n+}\n+\t\n+struct ref *parse_post_fetch_hook_line (char *l, \n+\t\tstruct string_list *existing_refs)\n+{\n+\tstruct ref *ref = NULL, *peer_ref = NULL;\n+\tstruct string_list_item *peer_item = NULL;\n+\tchar *words[4];\n+\tint i, word=0;\n+\tchar *problem;\n+\n+\tfor (i=0; l[i]; i++) {\n+\t\tif (isspace(l[i])) {\n+\t\t\tl[i]='\\0';\n+\t\t\twords[word]=l;\n+\t\t\tl+=i+1;\n+\t\t\ti=0;\n+\t\t\tword++;\n+\t\t\tif (word > 3) {\n+\t\t\t\tproblem=\"too many words\";\n+\t\t\t\tgoto unparsable;\n+\t\t\t}\n+\t\t}\n+\t}\n+\tif (word < 3) {\n+\t\tproblem=\"not enough words\";\n+\t\tgoto unparsable;\n+\t}\n+\t\n+\tref = alloc_ref(words[2]);\n+\tpeer_ref = ref->peer_ref = alloc_ref(l);\n+\tref->peer_ref->force=1;\n+\n+\tif (get_sha1_hex(words[0], ref->old_sha1)) {\n+\t\tproblem=\"bad sha1\";\n+\t\tgoto unparsable;\n+\t}\n+\n+\tif (strcmp(words[1], \"merge\") == 0) {\n+\t\tref->merge=1;\n+\t}\n+\telse if (strcmp(words[1], \"not-for-merge\") != 0) {\n+\t\tproblem=\"bad merge flag\";\n+\t\tgoto unparsable;\n+\t}\n+\n+\tpeer_item = string_list_lookup(existing_refs, peer_ref->name);\n+\tif (peer_item)\n+\t\thashcpy(peer_ref->old_sha1, peer_item->util);\n+\n+\treturn ref;\n+\n+ unparsable:\n+\twarning(\"%s hook output a wrongly formed line: %s\",\n+\t\t\tpost_fetch_hook, problem);\n+\tfree(ref);\n+\tfree(peer_ref);\n+\treturn NULL;\n+}\n+\n+/* The hook is fed lines of the form:\n+ * SP SP SP LF\n+ * And should output rewritten lines of the same form.\n+ */\n+struct ref *run_post_fetch_hook (struct ref *fetched_refs)\n+{\n+\tstruct ref *new_refs = NULL;\n+\tstruct string_list existing_refs = STRING_LIST_INIT_NODUP;\n+\tstruct child_process hook;\n+\tstruct async async;\n+\tconst char *argv[2];\n+\tFILE *f;\n+\tstruct strbuf buf;\n+\tstruct ref *ref, *prevref=NULL;\n+\tint ok = 1;\n+\n+\tif (! fetched_refs)\n+\t\treturn fetched_refs;\n+\n+\targv[0] = git_path(\"hooks/%s\", post_fetch_hook);\n+\tif (access(argv[0], X_OK) < 0)\n+\t\treturn fetched_refs;\n+\targv[1] = NULL;\n+\n+\tmemset(&hook, 0, sizeof(hook));\n+\thook.argv = argv;\n+\thook.in = -1;\n+\thook.out = -1;\n+\tif (start_command(&hook) != 0)\n+\t\treturn fetched_refs;\n+\n+\t/* Use an async writer to feed the hook process.\n+\t * This allows the hook to read and write a line at\n+\t * a time without blocking. */\n+\tmemset(&async, 0, sizeof(async));\n+\tasync.proc = feed_post_fetch_hook;\n+\tpost_fetch_hook_refs = fetched_refs;\n+\tasync.out = hook.in;\n+\tif (start_async(&async))\n+\t\tgoto failed_hook;\n+\n+\tfor_each_ref(add_existing, &existing_refs);\n+\n+\tf = fdopen(hook.out, \"r\");\n+\tif (f == NULL)\n+\t\tgoto failed_hook;\n+\tstrbuf_init(&buf, 128);\n+\twhile (strbuf_getline(&buf, f, '\\n') != EOF) {\n+\t\tchar *l = strbuf_detach(&buf, NULL);\n+\t\tref = parse_post_fetch_hook_line(l, &existing_refs);\n+\t\tif (ref) {\n+\t\t\tif (prevref) {\n+\t\t\t\tprevref->next=ref;\n+\t\t\t\tprevref=ref;\n+\t\t\t}\n+\t\t\telse {\n+\t\t\t\tnew_refs = prevref = ref;\n+\t\t\t}\n+\t\t}\n+\t\telse {\n+\t\t\tok = 0; /* ignore the other output */\n+\t\t}\n+\t\tfree(l);\n+\t}\n+\tstrbuf_release(&buf);\n+\tfclose(f);\n+\n+\tif (finish_async(&async))\n+\t\tgoto failed_hook;\n+\tfinish_command(&hook);\n+\n+\tif (! ok)\n+\t\treturn fetched_refs;\n+\t/* The new_refs are returned, to be used in place of fetched_refs,\n+\t * so it is not needed anymore and can be freed here. */\n+\tfree_refs(fetched_refs);\n+\treturn new_refs;\n+\n+failed_hook:\n+\tclose(hook.out);\n+\tfinish_command(&hook);\n+\treturn fetched_refs;\n+}\n+\n static void unlock_pack(void)\n {\n \tif (transport)\n@@ -507,17 +689,30 @@ static int quickfetch(struct ref *ref_map)\n \treturn check_everything_connected(iterate_ref_map, 1, &rm);\n }\n \n-static int fetch_refs(struct transport *transport, struct ref *ref_map)\n+struct fetch_refs_result {\n+\tstruct ref *new_refs;\n+\tint status;\n+};\n+\n+static struct fetch_refs_result fetch_refs(struct transport *transport,\n+\t\tstruct ref *ref_map)\n {\n-\tint ret = quickfetch(ref_map);\n-\tif (ret)\n-\t\tret = transport_fetch_refs(transport, ref_map);\n-\tif (!ret)\n-\t\tret |= store_updated_refs(transport->url,\n+\tstruct fetch_refs_result res;\n+\tres.status = quickfetch(ref_map);\n+\tif (res.status)\n+\t\tres.status = transport_fetch_refs(transport, ref_map);\n+\tif (!res.status) {\n+\t\tres.new_refs = run_post_fetch_hook(ref_map);\n+\n+\t\tres.status |= store_updated_refs(transport->url,\n \t\t\t\ttransport->remote->name,\n-\t\t\t\tref_map);\n+\t\t\t\tres.new_refs);\n+\t}\n+\telse {\n+\t\tres.new_refs = ref_map;\n+\t}\n \ttransport_unlock_pack(transport);\n-\treturn ret;\n+\treturn res;\n }\n \n static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)\n@@ -542,15 +737,6 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)\n \treturn result;\n }\n \n-static int add_existing(const char *refname, const unsigned char *sha1,\n-\t\t\tint flag, void *cbdata)\n-{\n-\tstruct string_list *list = (struct string_list *)cbdata;\n-\tstruct string_list_item *item = string_list_insert(list, refname);\n-\titem->util = (void *)sha1;\n-\treturn 0;\n-}\n-\n static int will_fetch(struct ref **head, const unsigned char *sha1)\n {\n \tstruct ref *rm = *head;\n@@ -673,6 +859,7 @@ static int do_fetch(struct transport *transport,\n \tstruct string_list_item *peer_item = NULL;\n \tstruct ref *ref_map;\n \tstruct ref *rm;\n+\tstruct fetch_refs_result res;\n \tint autotags = (transport->remote->fetch_tags == 1);\n \n \tfor_each_ref(add_existing, &existing_refs);\n@@ -710,7 +897,9 @@ static int do_fetch(struct transport *transport,\n \n \tif (tags == TAGS_DEFAULT && autotags)\n \t\ttransport_set_option(transport, TRANS_OPT_FOLLOWTAGS, \"1\");\n-\tif (fetch_refs(transport, ref_map)) {\n+\tres = fetch_refs(transport, ref_map);\n+\tref_map = res.new_refs;\n+\tif (res.status) {\n \t\tfree_refs(ref_map);\n \t\treturn 1;\n \t}\n@@ -750,7 +939,8 @@ static int do_fetch(struct transport *transport,\n \t\tif (ref_map) {\n \t\t\ttransport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);\n \t\t\ttransport_set_option(transport, TRANS_OPT_DEPTH, \"0\");\n-\t\t\tfetch_refs(transport, ref_map);\n+\t\t\tres = fetch_refs(transport, ref_map);\n+\t\t\tref_map = res.new_refs;\n \t\t}\n \t\tfree_refs(ref_map);\n \t}\n\n-- \n1.7.7.3\n"},{"id":"181698","messageId":"7vlipz930t.fsf@alter.siamese.dyndns.org","threadId":"29246","inReplyTo":"20111226023154.GA3243@gnu.kitenet.net","subject":"Re: [PATCH] add post-fetch hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-26T07:59:46Z","receivedAt":"2011-12-26T07:59:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joey Hess writes:\n\n> The post-fetch hook is fed lines on stdin for all refs that were fetched, and\n> outputs on stdout possibly modified lines. Its output is parsed and used\n> when git fetch updates the remote tracking refs, records the entries in\n> FETCH_HEAD, and produces its report.\n>\n> ---\n>\n> Not quite ready to sign off on this yet, but it does work.\n> Comments and code review appreciated.\n\nThanks. I do have a few comments.\n\nThis hook is no longer a \"post\" fetch hook. The mechanism lets the object\ntransfer phase does its work and then rewrites/tweaks the result before\nfetch completes. To an outside observer, what the hook does is an integral\npart of what \"fetch\" does, and not something that happens _after_ fetch\ncompletes. I am bad at naming things, but something along the lines of\n\"tweak-fetch\" that makes it clear that what happens in the hook is still\npart of the fetching may be a more appropriate name, methinks.\n\nI very much on purpose said that the hook \"must read everything from its\nstandard input, *and* *then* return...\" in my response. Your \"Demo\" hook\nemits output as it reads its input with sed, but your main process invokes\nthe hook, drains everything with write_in_full() before starting to read a\nsingle line, so I suspect that your hook will deadlock when its output\npipe buffer fills up without being read by the main process. Of course,\nfor this deadlock to actually happen, you need to be fetching quite a lot\nof refs.\n\nTo make the life of hook writers easier, however, it would be good to\nsupport a hook written in the style of your \"Demo\" hook, instead of\nrequiring it to read everything before emiting any output. I think you\ncould solve this by having a select(2) loop to avoid the deadlock on our\nend (lets call the code that spawns a hook with run_command() and\ninteracts with it a \"hook driver\" in the rest of this message), but before\ngoing in that direction, I would like to see us stepping back and bit and\nthink about the way hooks are called in the current code.\n\nWe seem to already have too many hook drivers, each of which hand-roll\nsimilar logic using run-command API. At some point, we would want to have\na single \"run_hook\" helper function that takes:\n\n - the name of a hook;\n - the command line arguments to be fed;\n - a callback \"generator\" function to feed the standard input stream of\n the hook process;\n - a callback \"consumer\" function to receive the standard output stream of\n the hook process; and\n - set of environment tweaks while running the hook (e.g. run the hook\n while setting GIT_INDEX_FILE to a temporary file).\n\nand hides away the complexity from hook drivers. The command line\narguments, input and output callback functions are all optional depending\non the API between the hook driver and the hook (e.g. the \"post-update\"\nhook takes arguments from its command line but does not interact with the\nstandard I/O stream, while the \"post-receive\" hook takes its input from\nthe standard input stream). Tweaking of the environment is also optional;\nnot many hooks need it.\n\nBy formalizing the hook driver API that way, any hook driver that drives a\ntricky hook that may need a select(2) loop to avoid a deadlock in a way\nsimilar to your patch do would not have to worry about the issue, as the\nrun_hook() helper would take care of it by reading from the hook's output\npipe and drain the pipe by calling the \"consumer\" callback before calling\nthe \"generator\" callback and feed more input to the hook to cause a\ndeadlock. We also could in the future do many other things if and when we\nwanted to that the current code structure makes difficult. A few examples\nthat readily come to my mind are:\n\n - relocate where the hook scripts live, by limiting the hook driver API\n to take just the \"name\" of the hook. The current hook callsites know\n that the hooks live in git_path(\"hooks/$name\") and call run_command()\n on their own, but the run_hook() helper could redirect it away to\n somewhere else, e.g. \"/etc/git-core/hooks/$name\".\n\n - run a set of hooks on the same triggering condition. You may want to\n have two \"post-receive\" hooks, one to feed an e-mail based notification\n system and another to drive an autobuilder, for example. For this to\n work, the \"generator\" and \"consumer\" callbacks need to have a way for\n us to tell \"beginning of a new session\" and \"end of a new session\", so\n that they can produce/consume the same set of values more than once.\n\n - perhaps ignore SIGPIPE if the hook chooses not to read any information\n the hook driver provides with it.\n\nI have been wondering when would be the good time to refactor the hook\ndriver API. We can add your patch, after polishing it enough to make it\nready for inclusion, independent of the hook API refactoring. But that\nwould mean that it would require more work when refactoring the hook API,\nas we would have one more hand-rolled hook caller that is based on\nrun_command().\n"},{"id":"181699","messageId":"7vhb0n92jx.fsf@alter.siamese.dyndns.org","threadId":"29246","inReplyTo":"m3liq0fwkz.fsf@localhost.localdomain","subject":"Re: [PATCH] add post-fetch hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-26T08:09:54Z","receivedAt":"2011-12-26T08:09:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> One thing that needs to be specified is what should happen if the hook\n> changes \"the refname at the remote\" part...\n\nHmm, good eyes.\n\nThe mechanism is to allow the hook to rewrite what happened during the\nfetch. If we decided refs/heads/master on the remote that points at the\ncommit $X should update refs/remotes/origin/master, we tell that to the\nhook, and the hook reads it. The hook may tell us to pretend that we\nfetched refs/heads/next on the remote that points at the commit $Y should\nupdate refs/remotes/origin/pu. In FETCH_HEAD we leave where the commit was\nfetched from and hook will affect that information (which is used in the\nresulting merge commit log message), but otherwise I do not think anything\nunexpected would happen, as tracking refs do not record where the stuff\ncame from (perhaps they are in reflog? I didn't check).\n"},{"id":"181704","messageId":"20111226155152.GA29582@gnu.kitenet.net","threadId":"29246","inReplyTo":"7vlipz930t.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] add post-fetch hook","fromName":"Joey Hess","fromEmail":"joey@kitenet.net","sentAt":"2011-12-26T15:51:52Z","receivedAt":"2011-12-26T15:51:52Z","isPatch":true,"sender":{"key":"joey@kitenet.net","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Junio C Hamano wrote:\n> Thanks. I do have a few comments.\n> \n> This hook is no longer a \"post\" fetch hook. The mechanism lets the object\n> transfer phase does its work and then rewrites/tweaks the result before\n> fetch completes. To an outside observer, what the hook does is an integral\n> part of what \"fetch\" does, and not something that happens _after_ fetch\n> completes. I am bad at naming things, but something along the lines of\n> \"tweak-fetch\" that makes it clear that what happens in the hook is still\n> part of the fetching may be a more appropriate name, methinks.\n\nI'd be happy to call it tweak-fetch.\n\n> I very much on purpose said that the hook \"must read everything from its\n> standard input, *and* *then* return...\" in my response. Your \"Demo\" hook\n> emits output as it reads its input with sed, but your main process invokes\n> the hook, drains everything with write_in_full() before starting to read a\n> single line, so I suspect that your hook will deadlock when its output\n> pipe buffer fills up without being read by the main process. Of course,\n> for this deadlock to actually happen, you need to be fetching quite a lot\n> of refs.\n\nDoesn't having the hook be fed by the async process/thread avoid this\nproblem? That's exactly why I did that, being familiar with the problem\nfrom stupid perl scripts like this one:\n\n#!/usr/bin/perl\nuse FileHandle;\nuse IPC::Open2;\n$pid = open2(*Reader, *Writer, \"cat\");\nprint Writer \"stuff $_\\n\" for 1..100000; # blocks forever\nclose Writer;\nprint \"got: $_\" while ;\n\nI've tested feeding large quantities of data through my demo hook\n(just feed it all the lines repeated 100 thousand times) and have\nnot seen it block. And other code in git uses an async feeder similarly,\nsee for example convert.c's apply_filter(). So I think this is ok..?\n\n> We seem to already have too many hook drivers, each of which hand-roll\n> similar logic using run-command API. At some point, we would want to have\n> a single \"run_hook\" helper function that takes:\n\nThis was also my feeling as I looked around the code.\n\n> By formalizing the hook driver API that way, any hook driver that drives a\n> tricky hook that may need a select(2) loop to avoid a deadlock in a way\n> similar to your patch do would not have to worry about the issue, as the\n> run_hook() helper would take care of it by reading from the hook's output\n> pipe and drain the pipe by calling the \"consumer\" callback before calling\n> the \"generator\" callback and feed more input to the hook to cause a\n> deadlock.\n\nActually, run_hook would need to either have a select loop, or an async\nfeeder like I've used. The method you describe is itself prone to deadlock!\nConsider a hook that *does* consume all its input before outputting anything.\nThe first call to the \"consumer\" callback would block.\n\n> - run a set of hooks on the same triggering condition. You may want to\n> have two \"post-receive\" hooks, one to feed an e-mail based notification\n> system and another to drive an autobuilder\n\nSomething I've always wanted, in fact..\n\n> I have been wondering when would be the good time to refactor the hook\n> driver API. We can add your patch, after polishing it enough to make it\n> ready for inclusion, independent of the hook API refactoring. But that\n> would mean that it would require more work when refactoring the hook API,\n> as we would have one more hand-rolled hook caller that is based on\n> run_command().\n\nOn the other hand, I have uses for this hook that are blocked on it\ngetting into git, so would rather not see it hung up in a general\nrefactoring. If it helps, I'd be happy to help with a hook refactoring\nlater.\n\nThe latest version of my patch (attached) already lifts reading from the\nhook out into a helper function, so should need not many changes in such\na refactoring -- most of my run_tweak_fetch_hook would simply go away\nthen. I've also cleaned it up a lot and and pretty happy with it now.\n\n-- \nsee shy jo\n\n\nFrom a06b5ef9908f56692a07fb37f5794c4122f10491 Mon Sep 17 00:00:00 2001\nFrom: Joey Hess \nDate: Mon, 26 Dec 2011 10:44:55 -0400\nSubject: [PATCH 1/2] preparations for tweak-fetch hook\n\nNo behavior changes yet, only some groundwork for the next\nchange.\n\nThe refs_result structure combines a status code with a ref map,\nwhich can be NULL even on success. This will be needed when\nthere's a tweak-fetch hook, because it can filter out all refs,\nwhile still succeeding.\n\nfetch_refs returns a refs_result, so that it can modify the ref_map.\n---\n builtin/fetch.c | 54 +++++++++++++++++++++++++++++++++++-------------------\n 1 files changed, 35 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 33ad3aa..70b9f89 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -29,6 +29,11 @@ enum {\n \tTAGS_SET = 2\n };\n \n+struct refs_result {\n+\tstruct ref *new_refs;\n+\tint status;\n+};\n+\n static int all, append, dry_run, force, keep, multiple, prune, update_head_ok, verbosity;\n static int progress, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n static int tags = TAGS_DEFAULT;\n@@ -89,6 +94,15 @@ static struct option builtin_fetch_options[] = {\n \tOPT_END()\n };\n \n+static int add_existing(const char *refname, const unsigned char *sha1,\n+\t\t\tint flag, void *cbdata)\n+{\n+\tstruct string_list *list = (struct string_list *)cbdata;\n+\tstruct string_list_item *item = string_list_insert(list, refname);\n+\titem->util = (void *)sha1;\n+\treturn 0;\n+}\n+\n static void unlock_pack(void)\n {\n \tif (transport)\n@@ -507,17 +521,24 @@ static int quickfetch(struct ref *ref_map)\n \treturn check_everything_connected(iterate_ref_map, 1, &rm);\n }\n \n-static int fetch_refs(struct transport *transport, struct ref *ref_map)\n+static struct refs_result fetch_refs(struct transport *transport,\n+\t\tstruct ref *ref_map)\n {\n-\tint ret = quickfetch(ref_map);\n-\tif (ret)\n-\t\tret = transport_fetch_refs(transport, ref_map);\n-\tif (!ret)\n-\t\tret |= store_updated_refs(transport->url,\n+\tstruct refs_result res;\n+\tres.status = quickfetch(ref_map);\n+\tif (res.status)\n+\t\tres.status = transport_fetch_refs(transport, ref_map);\n+\tif (!res.status) {\n+\t\tres.new_refs = ref_map\n+\t\tres.status |= store_updated_refs(transport->url,\n \t\t\t\ttransport->remote->name,\n-\t\t\t\tref_map);\n+\t\t\t\tres.new_refs);\n+\t}\n+\telse {\n+\t\tres.new_refs = ref_map;\n+\t}\n \ttransport_unlock_pack(transport);\n-\treturn ret;\n+\treturn res;\n }\n \n static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)\n@@ -542,15 +563,6 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)\n \treturn result;\n }\n \n-static int add_existing(const char *refname, const unsigned char *sha1,\n-\t\t\tint flag, void *cbdata)\n-{\n-\tstruct string_list *list = (struct string_list *)cbdata;\n-\tstruct string_list_item *item = string_list_insert(list, refname);\n-\titem->util = (void *)sha1;\n-\treturn 0;\n-}\n-\n static int will_fetch(struct ref **head, const unsigned char *sha1)\n {\n \tstruct ref *rm = *head;\n@@ -673,6 +685,7 @@ static int do_fetch(struct transport *transport,\n \tstruct string_list_item *peer_item = NULL;\n \tstruct ref *ref_map;\n \tstruct ref *rm;\n+\tstruct refs_result res;\n \tint autotags = (transport->remote->fetch_tags == 1);\n \n \tfor_each_ref(add_existing, &existing_refs);\n@@ -710,7 +723,9 @@ static int do_fetch(struct transport *transport,\n \n \tif (tags == TAGS_DEFAULT && autotags)\n \t\ttransport_set_option(transport, TRANS_OPT_FOLLOWTAGS, \"1\");\n-\tif (fetch_refs(transport, ref_map)) {\n+\tres = fetch_refs(transport, ref_map);\n+\tref_map = res.new_refs;\n+\tif (res.status) {\n \t\tfree_refs(ref_map);\n \t\treturn 1;\n \t}\n@@ -750,7 +765,8 @@ static int do_fetch(struct transport *transport,\n \t\tif (ref_map) {\n \t\t\ttransport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);\n \t\t\ttransport_set_option(transport, TRANS_OPT_DEPTH, \"0\");\n-\t\t\tfetch_refs(transport, ref_map);\n+\t\t\tres = fetch_refs(transport, ref_map);\n+\t\t\tref_map = res.new_refs;\n \t\t}\n \t\tfree_refs(ref_map);\n \t}\n-- \n1.7.7.3\n\n\n\nFrom 073b0921bb5988628e7af423924c410f522f403a Mon Sep 17 00:00:00 2001\nFrom: Joey Hess \nDate: Mon, 26 Dec 2011 10:53:27 -0400\nSubject: [PATCH 2/2] add tweak-fetch hook\n\nThe tweak-fetch hook is fed lines on stdin for all refs that were fetched,\nand outputs on stdout possibly modified lines. Its output is parsed and\nused when git fetch updates the remote tracking refs, records the entries\nin FETCH_HEAD, and produces its report.\n---\n Documentation/githooks.txt | 29 +++++++\n builtin/fetch.c | 191 +++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 219 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 28edefa..be2624c 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -162,6 +162,35 @@ This hook can be used to perform repository validity checks, auto-display\n differences from the previous HEAD if different, or set working dir metadata\n properties.\n \n+tweak-fetch\n+~~~~~~~~~~\n+\n+This hook is invoked by 'git fetch' (commonly called by 'git pull'), after\n+refs have been fetched from the remote repository. It is not executed, if\n+nothing was fetched.\n+\n+The output of the hook is used to update the remote-tracking branches, and\n+`.git/FETCH_HEAD`, in preparation for for a later merge operation done by\n+'git merge'.\n+\n+It takes no arguments, but is fed a line of the following format on\n+its standard input for each ref that was fetched.\n+\n+ SP not-for-merge|merge SP SP LF\n+\n+Where the \"not-for-merge\" flag indicates the ref is not to be merged into the\n+current branch, and the \"merge\" flag indicates that 'git merge' should\n+later merge it. The `` is the remote's name for the ref\n+that was pulled, and `` is a name of a remote-tracking branch,\n+like \"refs/remotes/origin/master\", or can be empty if the fetched ref is not\n+being stored in a local refname.\n+\n+The hook must consume all of its standard input, and output back lines\n+of the same format. It can modify its input as desired, including\n+adding or removing lines, updating the sha1 (i.e. re-point the\n+remote-tracking branch), changing the merge flag, and changing the\n+`` (i.e. use different remote-tracking branch).\n+\n post-merge\n ~~~~~~~~~~\n \ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 70b9f89..5434b6f 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -103,6 +103,194 @@ static int add_existing(const char *refname, const unsigned char *sha1,\n \treturn 0;\n }\n \n+static const char tweak_fetch_hook[] = \"tweak-fetch\";\n+\n+int feed_tweak_fetch_hook (int in, int out, void *data)\n+{\n+\tstruct ref *ref;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tint ret;\n+\n+\tfor (ref = data; ref; ref = ref->next) {\n+\t\tstrbuf_addstr(&buf, sha1_to_hex(ref->old_sha1));\n+\t\tstrbuf_addch(&buf, ' ');\n+\t\tstrbuf_addstr(&buf, ref->merge ? \"merge\" : \"not-for-merge\");\n+\t\tstrbuf_addch(&buf, ' ');\n+\t\tif (ref->name)\n+\t\t\tstrbuf_addstr(&buf, ref->name);\n+\t\tstrbuf_addch(&buf, ' ');\n+\t\tif (ref->peer_ref && ref->peer_ref->name)\n+\t\t\tstrbuf_addstr(&buf, ref->peer_ref->name);\n+\t\tstrbuf_addch(&buf, '\\n');\n+\t}\n+\n+\tret = write_in_full(out, buf.buf, buf.len) != buf.len;\n+\tif (ret)\n+\t\twarning(\"%s hook failed to consume all its input\",\n+\t\t\t\ttweak_fetch_hook);\n+\tclose(out);\n+\tstrbuf_release(&buf);\n+\treturn ret;\n+}\n+\t\n+struct ref *parse_tweak_fetch_hook_line (char *l, \n+\t\tstruct string_list *existing_refs)\n+{\n+\tstruct ref *ref = NULL, *peer_ref = NULL;\n+\tstruct string_list_item *peer_item = NULL;\n+\tchar *words[4];\n+\tint i, word=0;\n+\tchar *problem;\n+\n+\tfor (i=0; l[i]; i++) {\n+\t\tif (isspace(l[i])) {\n+\t\t\tl[i]='\\0';\n+\t\t\twords[word]=l;\n+\t\t\tl+=i+1;\n+\t\t\ti=0;\n+\t\t\tword++;\n+\t\t\tif (word > 3) {\n+\t\t\t\tproblem=\"too many words\";\n+\t\t\t\tgoto unparsable;\n+\t\t\t}\n+\t\t}\n+\t}\n+\tif (word < 3) {\n+\t\tproblem=\"not enough words\";\n+\t\tgoto unparsable;\n+\t}\n+\t\n+\tref = alloc_ref(words[2]);\n+\tpeer_ref = ref->peer_ref = alloc_ref(l);\n+\tref->peer_ref->force=1;\n+\n+\tif (get_sha1_hex(words[0], ref->old_sha1)) {\n+\t\tproblem=\"bad sha1\";\n+\t\tgoto unparsable;\n+\t}\n+\n+\tif (strcmp(words[1], \"merge\") == 0) {\n+\t\tref->merge=1;\n+\t}\n+\telse if (strcmp(words[1], \"not-for-merge\") != 0) {\n+\t\tproblem=\"bad merge flag\";\n+\t\tgoto unparsable;\n+\t}\n+\n+\tpeer_item = string_list_lookup(existing_refs, peer_ref->name);\n+\tif (peer_item)\n+\t\thashcpy(peer_ref->old_sha1, peer_item->util);\n+\n+\treturn ref;\n+\n+ unparsable:\n+\twarning(\"%s hook output a wrongly formed line: %s\",\n+\t\t\ttweak_fetch_hook, problem);\n+\tfree(ref);\n+\tfree(peer_ref);\n+\treturn NULL;\n+}\n+\n+struct refs_result read_tweak_fetch_hook (int in) {\n+\tstruct refs_result res;\n+\tFILE *f;\n+\tstruct strbuf buf;\n+\tstruct string_list existing_refs = STRING_LIST_INIT_NODUP;\n+\tstruct ref *ref, *prevref=NULL;\n+\n+\tres.status = 0;\n+\tres.new_refs = NULL;\n+\n+\tf = fdopen(in, \"r\");\n+\tif (f == NULL) {\n+\t\tres.status = 1;\n+\t\treturn res;\n+\t}\n+\n+\tstrbuf_init(&buf, 128);\n+\tfor_each_ref(add_existing, &existing_refs);\n+\n+\twhile (strbuf_getline(&buf, f, '\\n') != EOF) {\n+\t\tchar *l = strbuf_detach(&buf, NULL);\n+\t\tref = parse_tweak_fetch_hook_line(l, &existing_refs);\n+\t\tif (ref) {\n+\t\t\tif (prevref) {\n+\t\t\t\tprevref->next=ref;\n+\t\t\t\tprevref=ref;\n+\t\t\t}\n+\t\t\telse {\n+\t\t\t\tres.new_refs = prevref = ref;\n+\t\t\t}\n+\t\t}\n+\t\telse {\n+\t\t\tres.status = 1;\n+\t\t}\n+\t\tfree(l);\n+\t}\n+\n+\tstring_list_clear(&existing_refs, 0);\n+\tstrbuf_release(&buf);\n+\tfclose(f);\n+\treturn res;\n+}\n+\n+/* The hook is fed lines of the form:\n+ * SP SP SP LF\n+ * And should output rewritten lines of the same form.\n+ */\n+struct ref *run_tweak_fetch_hook (struct ref *fetched_refs)\n+{\n+\tstruct child_process hook;\n+\tconst char *argv[2];\n+\tstruct async async;\n+\tstruct refs_result res;\n+\n+\tif (! fetched_refs)\n+\t\treturn fetched_refs;\n+\n+\targv[0] = git_path(\"hooks/%s\", tweak_fetch_hook);\n+\tif (access(argv[0], X_OK) < 0)\n+\t\treturn fetched_refs;\n+\targv[1] = NULL;\n+\n+\tmemset(&hook, 0, sizeof(hook));\n+\thook.argv = argv;\n+\thook.in = -1;\n+\thook.out = -1;\n+\tif (start_command(&hook))\n+\t\treturn fetched_refs;\n+\n+\t/* Use an async writer to feed the hook process.\n+\t * This allows the hook to read and write a line at\n+\t * a time without blocking. */\n+\tmemset(&async, 0, sizeof(async));\n+\tasync.proc = feed_tweak_fetch_hook;\n+\tasync.data = fetched_refs;\n+\tasync.out = hook.in;\n+\tif (start_async(&async)) {\n+\t\tclose(hook.in);\n+\t\tclose(hook.out);\n+\t\tfinish_command(&hook);\n+\t\treturn fetched_refs;\n+\t}\n+\tres = read_tweak_fetch_hook(hook.out);\n+\tres.status |= finish_async(&async);\n+\tres.status |= finish_command(&hook);\n+\n+\tif (res.status) {\n+\t\twarning(\"%s hook failed, ignoring its output\", tweak_fetch_hook);\n+\t\tfree(res.new_refs);\n+\t\treturn fetched_refs;\n+\t}\n+\telse {\n+\t\t/* The new_refs are returned, to be used in place of\n+\t\t * fetched_refs, so it is not needed anymore and can\n+\t\t * be freed here. */\n+\t\tfree_refs(fetched_refs);\n+\t\treturn res.new_refs;\n+\t}\n+}\n+\n static void unlock_pack(void)\n {\n \tif (transport)\n@@ -529,7 +717,8 @@ static struct refs_result fetch_refs(struct transport *transport,\n \tif (res.status)\n \t\tres.status = transport_fetch_refs(transport, ref_map);\n \tif (!res.status) {\n-\t\tres.new_refs = ref_map\n+\t\tres.new_refs = run_tweak_fetch_hook(ref_map);\n+\n \t\tres.status |= store_updated_refs(transport->url,\n \t\t\t\ttransport->remote->name,\n \t\t\t\tres.new_refs);\n-- \n1.7.7.3\n\n"},{"id":"181711","messageId":"7v8vly8qqx.fsf@alter.siamese.dyndns.org","threadId":"29246","inReplyTo":"20111226155152.GA29582@gnu.kitenet.net","subject":"Re: [PATCH] add post-fetch hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-27T06:37:10Z","receivedAt":"2011-12-27T06:37:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joey Hess writes:\n\n> .... And other code in git uses an async feeder similarly,\n> see for example convert.c's apply_filter(). So I think this is ok..?\n\nYeah, I didn't look at your patch (sorry) but if it uses async like the\nfiltering codepath does, it should be perfectly fine (please forget about\nthe select(2) based kludge I alluded to; the async interface is the right\nthing to use here).\n\nThanks.\n"},{"id":"181716","messageId":"20111227154907.GB15006@gnu.kitenet.net","threadId":"29246","inReplyTo":"7v8vly8qqx.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] add post-fetch hook","fromName":"Joey Hess","fromEmail":"joey@kitenet.net","sentAt":"2011-12-27T15:49:07Z","receivedAt":"2011-12-27T15:49:07Z","isPatch":true,"sender":{"key":"joey@kitenet.net","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Junio C Hamano wrote:\n> Joey Hess writes:\n> \n> > .... And other code in git uses an async feeder similarly,\n> > see for example convert.c's apply_filter(). So I think this is ok..?\n> \n> Yeah, I didn't look at your patch (sorry) but if it uses async like the\n> filtering codepath does, it should be perfectly fine (please forget about\n> the select(2) based kludge I alluded to; the async interface is the right\n> thing to use here).\n\nNo problem, I was surprised to be getting responses at all over the\nholidays. :)\n\nThen async also seems the right thing to use for the hook refactoring. A\ncaller can provide two function pointers; a feeder function that is\ncalled async, and a reader that is *not* called async (which would allow\nit to modify program state), and the refactored hook function handles\nrunning the hook(s) and connecting them to the feeder and/or reader.\n\n-- \nsee shy jo\n"},{"id":"181733","messageId":"4EFA3833.80409@kdbg.org","threadId":"29246","inReplyTo":"20111226023154.GA3243@gnu.kitenet.net","subject":"Re: [PATCH] add post-fetch hook","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2011-12-27T21:27:15Z","receivedAt":"2011-12-27T21:27:15Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 26.12.2011 03:31, schrieb Joey Hess:\n> +int feed_post_fetch_hook (int in, int out, void *data)\n> +{\n> +\tstruct ref *ref;\n> +\tstruct strbuf buf = STRBUF_INIT;\n\nIs there a particular reason that you accumulate everything in a buffer?\n\nIf I read the loop below correctly, you should be able to run it using\nonly the functions sha1_to_hex(), strlen() and write_in_full(). This\nwould avoid any problems with concurrent calls to xmalloc().\n\n> +\tint ret;\n> +\n> +\tfor (ref = post_fetch_hook_refs; ref; ref = ref->next) {\n> +\t\tstrbuf_addstr(&buf, sha1_to_hex(ref->old_sha1));\n\nsha1_to_hex() works with a static buffer. Are you certain that it is not\ncalled concurrently in the main thread?\n\n> +\t\tstrbuf_addch(&buf, ' ');\n> +\t\tstrbuf_addstr(&buf, ref->merge ? \"merge\" : \"not-for-merge\");\n> +\t\tstrbuf_addch(&buf, ' ');\n> +\t\tif (ref->name)\n> +\t\t\tstrbuf_addstr(&buf, ref->name);\n> +\t\tstrbuf_addch(&buf, ' ');\n> +\t\tif (ref->peer_ref && ref->peer_ref->name)\n> +\t\t\tstrbuf_addstr(&buf, ref->peer_ref->name);\n> +\t\tstrbuf_addch(&buf, '\\n');\n> +\t}\n\n-- Hannes\n"},{"id":"181738","messageId":"7vehvp4n0i.fsf@alter.siamese.dyndns.org","threadId":"29246","inReplyTo":"20111226155152.GA29582@gnu.kitenet.net","subject":"Re: [PATCH] add post-fetch hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-27T23:23:41Z","receivedAt":"2011-12-27T23:23:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joey Hess writes:\n\n> From 073b0921bb5988628e7af423924c410f522f403a Mon Sep 17 00:00:00 2001\n> From: Joey Hess \n> Date: Mon, 26 Dec 2011 10:53:27 -0400\n> Subject: [PATCH 2/2] add tweak-fetch hook\n>\n> The tweak-fetch hook is fed lines on stdin for all refs that were fetched,\n> and outputs on stdout possibly modified lines. Its output is parsed and\n> used when git fetch updates the remote tracking refs, records the entries\n> in FETCH_HEAD, and produces its report.\n> ---\n\nJust a few style things, as this is not a signed-off patch yet.\n\n> @@ -162,6 +162,35 @@ This hook can be used to perform repository validity checks, auto-display\n> differences from the previous HEAD if different, or set working dir metadata\n> properties.\n> \n> +tweak-fetch\n> +~~~~~~~~~~\n\nThe underline does not match what is being underlined. Does this format well?\n\n> +This hook is invoked by 'git fetch' (commonly called by 'git pull'), after\n> +refs have been fetched from the remote repository. It is not executed, if\n> +nothing was fetched.\n> +\n> +The output of the hook is used to update the remote-tracking branches, and\n> +`.git/FETCH_HEAD`, in preparation for for a later merge operation done by\n> +'git merge'.\n> +\n> +It takes no arguments, but is fed a line of the following format on\n> +its standard input for each ref that was fetched.\n> +\n> + SP not-for-merge|merge SP SP LF\n> +\n> +Where the \"not-for-merge\" flag indicates the ref is not to be merged into the\n> +current branch, and the \"merge\" flag indicates that 'git merge' should\n> +later merge it. The `` is the remote's name for the ref\n> +that was pulled, and `` is a name of a remote-tracking branch,\n\ns/pulled/fetched/; I think. The remainder of the new text seems to use the\nright terminology.\n\n> +int feed_tweak_fetch_hook (int in, int out, void *data)\n\nNo SP between function name and the opening parenthesis of its parameter\nlist. We have SP after control-flow keywords e.g. \"for (;;)\" though.\n\nDoes this name need to be external (same question to many other new\nfunctions in this patch)?\n\nThe \"in\" parameter seems unused. Does it have to be there for the \"feed\"\ncallback of the generic hook driver? As long as it is the \"feed\" callback,\nI think that it just needs to take \"out\" and no \"in\", no?\n\n> +\tfor (ref = data; ref; ref = ref->next) {\n> +\t\tstrbuf_addstr(&buf, sha1_to_hex(ref->old_sha1));\n> +\t\tstrbuf_addch(&buf, ' ');\n> +\t\tstrbuf_addstr(&buf, ref->merge ? \"merge\" : \"not-for-merge\");\n> +\t\tstrbuf_addch(&buf, ' ');\n\nstrbuf_addf()?\n\nBut this might be a moot point, as J6t seems to have valid worries on\nrunning functions that allocate memory in general...\n\n> +\tret = write_in_full(out, buf.buf, buf.len) != buf.len;\n> +\tif (ret)\n> +\t\twarning(\"%s hook failed to consume all its input\",\n> +\t\t\t\ttweak_fetch_hook);\n> +\tclose(out);\n\nI was hoping that this part would be part of more generic hook driver\ninfrastructure. Even if we were to take this series before we refactor\nexisting other hook drivers, in order to avoid duplicated work later, we\ncould at least start from a right implementation of a generic hook driver\nwith a single user (which is the \"tweak-fetch\" hook driver), no?\n\n> +struct ref *parse_tweak_fetch_hook_line (char *l, \n> +\t\tstruct string_list *existing_refs)\n> +{\n> +\tstruct ref *ref = NULL, *peer_ref = NULL;\n> +\tstruct string_list_item *peer_item = NULL;\n> +\tchar *words[4];\n> +\tint i, word=0;\n\nSP around assingment and initialization \"var = val\" (throughout this\npatch).\n\n> +\tchar *problem;\n> +\n> +\tfor (i=0; l[i]; i++) {\n\nLikewise.\n\n> +\t\tif (isspace(l[i])) {\n> +\t\t\tl[i]='\\0';\n> +\t\t\twords[word]=l;\n> +\t\t\tl+=i+1;\n> +\t\t\ti=0;\n> +\t\t\tword++;\n> +\t\t\tif (word > 3) {\n> +\t\t\t\tproblem=\"too many words\";\n> +\t\t\t\tgoto unparsable;\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\tif (word < 3) {\n> +\t\tproblem=\"not enough words\";\n> +\t\tgoto unparsable;\n> +\t}\n\nPerhaps loop for up-to ARRAY_SIZE(words) times and use strchr()?\n\n> +\tif (strcmp(words[1], \"merge\") == 0) {\n\nWe tend to say \"if (!strcmp(...))\" instead.\n\n> +\t\tref->merge=1;\n> +\t}\n> +\telse if (strcmp(words[1], \"not-for-merge\") != 0) {\n\nLikewise.\n\n> +struct refs_result read_tweak_fetch_hook (int in) {\n\nOpening brace at column 1 of the next line.\n\n> +\t\t\tif (prevref) {\n> +\t\t\t\tprevref->next=ref;\n> +\t\t\t\tprevref=ref;\n> +\t\t\t}\n> +\t\t\telse {\n\n\tif (...) {\n\t\t...\n\t} else {\n\t\t...\n\t}\n\n> +/* The hook is fed lines of the form:\n> + * SP SP SP LF\n> + * And should output rewritten lines of the same form.\n> + */\n\n\t/*\n * We write our multi-line comments\n * like this (applies to a few other comments\n * in this patch).\n */\n"},{"id":"181756","messageId":"20111228193008.GB17521@gnu.kitenet.net","threadId":"29246","inReplyTo":"4EFA3833.80409@kdbg.org","subject":"Re: [PATCH] add post-fetch hook","fromName":"Joey Hess","fromEmail":"joey@kitenet.net","sentAt":"2011-12-28T19:30:08Z","receivedAt":"2011-12-28T19:30:08Z","isPatch":true,"sender":{"key":"joey@kitenet.net","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Johannes Sixt wrote:\n> If I read the loop below correctly, you should be able to run it using\n> only the functions sha1_to_hex(), strlen() and write_in_full(). This\n> would avoid any problems with concurrent calls to xmalloc().\n> \n> > +\tint ret;\n> > +\n> > +\tfor (ref = post_fetch_hook_refs; ref; ref = ref->next) {\n> > +\t\tstrbuf_addstr(&buf, sha1_to_hex(ref->old_sha1));\n> \n> sha1_to_hex() works with a static buffer. Are you certain that it is not\n> called concurrently in the main thread?\n\nThanks very much for pointing that thread-unsafty out.\n\nBased on that, my current thinking for a generic hook interface is that,\nrather than the caller providing an arbitrary \"feeder\" function that gets\nrun async, the caller should provide a function that generates a strbuf\ncontaining the stdout for the hook, and then a very simple async writer\ncan handle the actual writing.\n\nstatic int feed_hook(int in, int out, void *data)\n{\n struct strbuf *buf = data;\n return write_in_full(out, buf->buf, buf->len) != buf->len;\n}\n\n(I assume that write_in_full is safe to be run async?)\n\nI am working on a patch that will involve adding a hook_complex()\nand changing hook() to be implemented in terms of it. The header\nfor that is included below, you should get a very good idea of how\nit will work from the data structure.\n\n/*\n * This data structure controls how a hook is run.\n */\nstruct hook {\n\t/* The name of the hook being run. */\n\tconst char *name;\n\t/* Parameters to pass to the hook program, not including the name\n\t * of the hook. May be NULL. */\n\tstruct argv_array *argv_array;\n\t/* Pathname to an index file to use, or NULL if the hook\n\t * uses the default index file or no index is needed. */\n\tconst char *index_file;\n\t/*\n\t * An arbitrary data structure, can be populated and modified to\n\t * communicate between the feeder, reader, and caller of the hook.\n\t */\n\tvoid *data;\n\t/* \n\t * A hook can optionally not consume all of its stdin.\n\t * If partial_stdin is 0, it is an error for some stdin not\n\t * to be consumed.\n\t */\n\tint partial_stdin;\n\t/* \n\t * feeder populates a strbuf with the content to send to the\n\t * hook on its standard input.\n\t *\n\t * May be NULL, if the hook does not consume standard input.\n\t *\n\t * Note that feeder might be run more than once, if multiple\n\t * programs are run as part of a single hook. It should avoid\n\t * taking any actions except for reading from data and generating\n\t * the strbuf. It will *not* be run async, and need not worry\n\t * about contending with other threads.\n\t */\n\tstruct strbuf *(*feeder)(struct hook *hook);\n\t/*\n\t * reader processes the hook's standard output from the handle,\n\t * returning 0 on success, non-zero on failure.\n\t *\n\t * May be NULL, if the hook's stdin is not processed. (It will\n\t * instead be redirected to stderr.)\n\t *\n\t * Note that reader might be run more than once, if multiple\n\t * programs are run as part of a single hook. It should avoid\n\t * taking any actions except for reading from the input handle,\n\t * changing the content of data, and printing any necessary\n\t * warnings. It will *not* be run async, and need not worry\n\t * about contending with other threads.\n\t */\n\tint (*reader)(struct hook *hook, int handle);\n};\n\nextern int run_hook(const char *index_file, const char *name, ...);\n\nextern int run_hook_complex(struct hook *hook);\n\n\nThis design allows for a future where multiple scripts get run for a\nsingle hook. In that case, the feeder and reader functions would get\ncalled repeatedly in a loop, with a data flow like this, where the\nreader modifies hook.data, providing the next call of the feeder with\nthe new data read from the hook script:\n feeder | hook_script_1 | reader | feeder | hook_script_2 | reader\n\n-- \nsee shy jo\n"}]}