{"thread":{"id":"11037","subject":"[PATCH] Allow update hooks to update refs on their own","startedAt":"2007-11-27T21:17:30Z","lastAt":"2007-12-06T07:50:53Z","messageCount":44,"participants":["Steven Grimm","Jakub Narebski","Junio C Hamano","Daniel Barkalow","Jeff King","Shawn O. Pearce","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"61123","messageId":"20071127211730.GA11861@midwinter.com","threadId":"11037","inReplyTo":null,"subject":"[PATCH] Allow update hooks to update refs on their own","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-11-27T21:17:30Z","receivedAt":"2007-11-27T21:17:30Z","isPatch":true,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"This is useful in cases where a hook needs to modify an incoming commit\nin some way, e.g., adding an annotation to the commit message, noting\nthe location of output from a profiling tool, or committing to an svn\nrepository using git-svn.\n\nSigned-off-by: Steven Grimm \n---\n\n\tI did this to support a bridge between svn and git. The idea is\n\tthat the people who use git can do \"git push\" to publish their\n\tchanges, and if they've pushed to certain branches, the changes\n\twill get transparently committed to svn using git-svn.\n\n\tThe git users therefore can operate in a totally git-centric\n\tenvironment. (This is a step toward transitioning my company\n\taway from svn entirely: get people used to a native git environment\n\twith no direct svn interactions.)\n\t\n\tOne problem is that git-svn wants to rewrite history to add its\n\tgit-svn-id: lines, which means that when the update hook returns\n\tafter doing the git-svn dcommit, HEAD is already pointed at the\n\tupdated commit. Without this change or something like it,\n\treceive-pack bombs out because HEAD has moved, and the push\n\tappears to fail. This patch addresses that particular problem.\n\n\tThere are other issues with the history rewriting, but this\n\tchange is hopefully useful on its own for cases like the ones\n\tlisted in the commit message.\n\n Documentation/git-receive-pack.txt | 8 +++-\n receive-pack.c | 68 +++++++++++++++++++++++-------------\n 2 files changed, 50 insertions(+), 26 deletions(-)\n\ndiff --git a/Documentation/git-receive-pack.txt b/Documentation/git-receive-pack.txt\nindex 2633d94..115ae97 100644\n--- a/Documentation/git-receive-pack.txt\n+++ b/Documentation/git-receive-pack.txt\n@@ -74,8 +74,12 @@ Note that the hook is called before the refname is updated,\n so either sha1-old is 0\\{40} (meaning there is no such ref yet),\n or it should match what is recorded in refname.\n \n-The hook should exit with non-zero status if it wants to disallow\n-updating the named ref. Otherwise it should exit with zero.\n+The hook may optionally choose to update the ref on its own, e.g.,\n+if it needs to modify incoming revisions in some way. If it updates\n+the ref, it should exit with a status of 100. The hook should exit\n+with a status between 1 and 99 if it wants to disallow updating the\n+named ref. Otherwise it should exit with zero, and the ref will be\n+updated automatically.\n \n Successful execution (a zero exit status) of this hook does not\n ensure the ref will actually be updated, it is only a prerequisite.\ndiff --git a/receive-pack.c b/receive-pack.c\nindex 38e35c0..19162ec 100644\n--- a/receive-pack.c\n+++ b/receive-pack.c\n@@ -18,6 +18,9 @@ static int report_status;\n static char capabilities[] = \" report-status delete-refs \";\n static int capabilities_sent;\n \n+/* Update hook exit code: hook has updated ref on its own */\n+#define EXIT_CODE_REF_UPDATED 100\n+\n static int receive_pack_config(const char *var, const char *value)\n {\n \tif (strcmp(var, \"receive.denynonfastforwards\") == 0) {\n@@ -70,8 +73,11 @@ static struct command *commands;\n static const char pre_receive_hook[] = \"hooks/pre-receive\";\n static const char post_receive_hook[] = \"hooks/post-receive\";\n \n-static int hook_status(int code, const char *hook_name)\n+static int hook_status(int code, const char *hook_name, int ok_start)\n {\n+\tif (ok_start && -code >= ok_start)\n+\t\treturn -code;\n+\n \tswitch (code) {\n \tcase 0:\n \t\treturn 0;\n@@ -121,7 +127,7 @@ static int run_hook(const char *hook_name)\n \n \tcode = start_command(&proc);\n \tif (code)\n-\t\treturn hook_status(code, hook_name);\n+\t\treturn hook_status(code, hook_name, 0);\n \tfor (cmd = commands; cmd; cmd = cmd->next) {\n \t\tif (!cmd->error_string) {\n \t\t\tsize_t n = snprintf(buf, sizeof(buf), \"%s %s %s\\n\",\n@@ -132,7 +138,7 @@ static int run_hook(const char *hook_name)\n \t\t\t\tbreak;\n \t\t}\n \t}\n-\treturn hook_status(finish_command(&proc), hook_name);\n+\treturn hook_status(finish_command(&proc), hook_name, 0);\n }\n \n static int run_update_hook(struct command *cmd)\n@@ -155,7 +161,8 @@ static int run_update_hook(struct command *cmd)\n \tproc.no_stdin = 1;\n \tproc.stdout_to_stderr = 1;\n \n-\treturn hook_status(run_command(&proc), update_hook);\n+\treturn hook_status(run_command(&proc), update_hook,\n+\t\t\t EXIT_CODE_REF_UPDATED);\n }\n \n static const char *update(struct command *cmd)\n@@ -194,32 +201,45 @@ static const char *update(struct command *cmd)\n \t\t\treturn \"non-fast forward\";\n \t\t}\n \t}\n-\tif (run_update_hook(cmd)) {\n-\t\terror(\"hook declined to update %s\", name);\n-\t\treturn \"hook declined\";\n-\t}\n-\n-\tif (is_null_sha1(new_sha1)) {\n-\t\tif (delete_ref(name, old_sha1)) {\n-\t\t\terror(\"failed to delete %s\", name);\n-\t\t\treturn \"failed to delete\";\n+\tswitch (run_update_hook(cmd)) {\n+\tcase 0:\n+\t\tif (is_null_sha1(new_sha1)) {\n+\t\t\tif (delete_ref(name, old_sha1)) {\n+\t\t\t\terror(\"failed to delete %s\", name);\n+\t\t\t\treturn \"failed to delete\";\n+\t\t\t}\n+\t\t\tfprintf(stderr, \"%s: %s -> deleted\\n\", name,\n+\t\t\t\tsha1_to_hex(old_sha1));\n \t\t}\n-\t\tfprintf(stderr, \"%s: %s -> deleted\\n\", name,\n-\t\t\tsha1_to_hex(old_sha1));\n-\t\treturn NULL; /* good */\n-\t}\n-\telse {\n-\t\tlock = lock_any_ref_for_update(name, old_sha1, 0);\n-\t\tif (!lock) {\n-\t\t\terror(\"failed to lock %s\", name);\n-\t\t\treturn \"failed to lock\";\n+\t\telse {\n+\t\t\tlock = lock_any_ref_for_update(name, old_sha1, 0);\n+\t\t\tif (!lock) {\n+\t\t\t\terror(\"failed to lock %s\", name);\n+\t\t\t\treturn \"failed to lock\";\n+\t\t\t}\n+\t\t\tif (write_ref_sha1(lock, new_sha1, \"push\")) {\n+\t\t\t\treturn \"failed to write\"; /* error() already called */\n+\t\t\t}\n+\t\t\tfprintf(stderr, \"%s: %s -> %s\\n\", name,\n+\t\t\t\tsha1_to_hex(old_sha1), sha1_to_hex(new_sha1));\n \t\t}\n-\t\tif (write_ref_sha1(lock, new_sha1, \"push\")) {\n-\t\t\treturn \"failed to write\"; /* error() already called */\n+\t\treturn NULL; /* good */\n+\n+\tcase EXIT_CODE_REF_UPDATED:\n+\t\t/* hook has taken care of updating ref, which means it\n+\t\t might be a different revision than we think. */\n+\t\tif (! resolve_ref(name, new_sha1, 1, NULL)) {\n+\t\t\terror(\"can't resolve ref %s after hook updated it\",\n+\t\t\t\tname);\n+\t\t\treturn \"ref not resolvable\";\n \t\t}\n \t\tfprintf(stderr, \"%s: %s -> %s\\n\", name,\n \t\t\tsha1_to_hex(old_sha1), sha1_to_hex(new_sha1));\n \t\treturn NULL; /* good */\n+\n+\tdefault:\n+\t\terror(\"hook declined to update %s\", name);\n+\t\treturn \"hook declined\";\n \t}\n }\n \n-- \n1.5.3.6.862.g7acd00-dirty\n"},{"id":"61124","messageId":"fii1od$d38$1@ger.gmane.org","threadId":"11037","inReplyTo":"20071127211730.GA11861@midwinter.com","subject":"Re: [PATCH] Allow update hooks to update refs on their own","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-11-27T21:21:20Z","receivedAt":"2007-11-27T21:21:20Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Steven Grimm wrote:\n\n> +The hook may optionally choose to update the ref on its own, e.g.,\n> +if it needs to modify incoming revisions in some way. If it updates\n> +the ref, it should exit with a status of 100.\n\nWhy 100, and not for example 127, 128 or -1?\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"61125","messageId":"C1C5EF48-689A-4C6D-9EBB-01AFD252C0DF@midwinter.com","threadId":"11037","inReplyTo":"fii1od$d38$1@ger.gmane.org","subject":"Re: [PATCH] Allow update hooks to update refs on their own","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-11-27T21:23:50Z","receivedAt":"2007-11-27T21:23:50Z","isPatch":true,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"On Nov 27, 2007, at 1:21 PM, Jakub Narebski wrote:\n>> +The hook may optionally choose to update the ref on its own, e.g.,\n>> +if it needs to modify incoming revisions in some way. If it updates\n>> +the ref, it should exit with a status of 100.\n>\n> Why 100, and not for example 127, 128 or -1?\n\nBecause those seemed much more likely to be used by existing hook \nscripts to denote an error condition, and I wanted to reduce the \nchance that this would break existing scripts.\n\n-Steve\n"},{"id":"61144","messageId":"7v4pf7b20b.fsf@gitster.siamese.dyndns.org","threadId":"11037","inReplyTo":"20071127211730.GA11861@midwinter.com","subject":"Re: [PATCH] Allow update hooks to update refs on their own","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-28T01:19:16Z","receivedAt":"2007-11-28T01:19:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"How does this interact with the \"pretend to have fetched back\nimmediately\" supported by modern git-push?\n"},{"id":"61167","messageId":"49EB8C6F-8100-48C1-BB2D-A8F6023BACAD@midwinter.com","threadId":"11037","inReplyTo":"7v4pf7b20b.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Allow update hooks to update refs on their own","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-11-28T02:40:38Z","receivedAt":"2007-11-28T02:40:38Z","isPatch":true,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"On Nov 27, 2007, at 5:19 PM, Junio C Hamano wrote:\n\n> How does this interact with the \"pretend to have fetched back\n> immediately\" supported by modern git-push?\n\n\nThat continues to fire, but it updates the local tracking ref to point \nto the SHA1 that was pushed, which isn't the actual remote ref. So you \nhave to do a real fetch to get the local tracking ref pointed to the \nright place. In other words, that feature doesn't do any good in this \ncontext, but it doesn't really hurt anything either.\n\nIt would of course be better if git-push could notice that it needs to \ndo an actual fetch. I think it'd be sufficient to transmit the final \nremote ref SHA1 back to git-push, and if it doesn't match what was \npushed, that's a sign that a fetch is needed. But that change wouldn't \nbe mutually exclusive with this patch, I believe.\n\n-Steve\n"},{"id":"61156","messageId":"Pine.LNX.4.64.0711272143470.5349@iabervon.org","threadId":"11037","inReplyTo":"49EB8C6F-8100-48C1-BB2D-A8F6023BACAD@midwinter.com","subject":"Re: [PATCH] Allow update hooks to update refs on their own","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2007-11-28T03:25:32Z","receivedAt":"2007-11-28T03:25:32Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 27 Nov 2007, Steven Grimm wrote:\n\n> On Nov 27, 2007, at 5:19 PM, Junio C Hamano wrote:\n> \n> >How does this interact with the \"pretend to have fetched back\n> >immediately\" supported by modern git-push?\n> \n> \n> That continues to fire, but it updates the local tracking ref to point to the\n> SHA1 that was pushed, which isn't the actual remote ref. So you have to do a\n> real fetch to get the local tracking ref pointed to the right place. In other\n> words, that feature doesn't do any good in this context, but it doesn't really\n> hurt anything either.\n> \n> It would of course be better if git-push could notice that it needs to do an\n> actual fetch. I think it'd be sufficient to transmit the final remote ref SHA1\n> back to git-push, and if it doesn't match what was pushed, that's a sign that\n> a fetch is needed. But that change wouldn't be mutually exclusive with this\n> patch, I believe.\n\nCouldn't you do this with a status message? (\"ok changed by \nhook\" or something.)\n\nI disagree that the feature doesn't do any good; it records that the state \nof the remote is at least as new as the local state, so you can tell \nwithout a network connection that you don't have any local changes you \nhaven't sent off.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"61158","messageId":"7vzlwz9ghg.fsf@gitster.siamese.dyndns.org","threadId":"11037","inReplyTo":"Pine.LNX.4.64.0711272143470.5349@iabervon.org","subject":"Re: [PATCH] Allow update hooks to update refs on their own","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-28T03:49:31Z","receivedAt":"2007-11-28T03:49:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow writes:\n\n> On Tue, 27 Nov 2007, Steven Grimm wrote:\n>\n>> On Nov 27, 2007, at 5:19 PM, Junio C Hamano wrote:\n>> \n>> >How does this interact with the \"pretend to have fetched back\n>> >immediately\" supported by modern git-push?\n>> \n>> \n>> That continues to fire, but it updates the local tracking ref to point to the\n>> SHA1 that was pushed, which isn't the actual remote ref. So you have to do a\n>> real fetch to get the local tracking ref pointed to the right place. In other\n>> words, that feature doesn't do any good in this context, but it doesn't really\n>> hurt anything either.\n>> \n>> It would of course be better if git-push could notice that it needs to do an\n>> actual fetch. I think it'd be sufficient to transmit the final remote ref SHA1\n>> back to git-push, and if it doesn't match what was pushed, that's a sign that\n>> a fetch is needed. But that change wouldn't be mutually exclusive with this\n>> patch, I believe.\n>\n> Couldn't you do this with a status message? (\"ok changed by \n> hook\" or something.)\n>\n> I disagree that the feature doesn't do any good; it records that the state \n> of the remote is at least as new as the local state, so you can tell \n> without a network connection that you don't have any local changes you \n> haven't sent off.\n\nYeah, and I am wondering why update hook needs to be changed for this.\nDidn't we introduce post-receive exactly for this sort of thing?\n"},{"id":"61172","messageId":"C24B0B38-BD37-4BDA-B138-275C900670AE@midwinter.com","threadId":"11037","inReplyTo":"7vzlwz9ghg.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Allow update hooks to update refs on their own","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-11-28T05:20:10Z","receivedAt":"2007-11-28T05:20:10Z","isPatch":true,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"\nOn Nov 27, 2007, at 7:49 PM, Junio C Hamano wrote:\n> Yeah, and I am wondering why update hook needs to be changed for this.\n> Didn't we introduce post-receive exactly for this sort of thing?\n\nI didn't think the post-receive hook could reject a revision. I need \nto be able to do that here, e.g., if the user's change fails to commit \nbecause it's rejected by an svn commit hook. (Hooks upon hooks upon \nhooks...) If I do the svn commit in post-receive it's not clear how \nthe user would repush the change after fixing it -- their fix would \nshow up as a delta on top of a revision that I can't commit on its own \nto svn, so I would somehow have to know to do a squash merge and there \nwould no longer be a one-to-one correspondence between git and svn \nrevisions. That's not a total showstopper (I'm rewriting history \nanyway) but it sure seems like it'll be confusing and error-prone.\n\nAlso, running in post-receive will subject me to race conditions that \naren't present in the update hook case; if I make my update hook \nscript do its own locking and update the refs on its own, I'm \nguaranteed no other push will come along and update my ref out from \nunder me. In post-receive there is no such guarantee and I may end up \nsending two pushes' worth of commits to svn when I think I'm only \nsending one.\n\nIf I'm misunderstanding the flow of control, please feel free to \ncorrect me. It just seemed like update was the only good place to do \nwhat I needed to do.\n\n-Steve\n"},{"id":"61227","messageId":"20071128161033.GA20308@coredump.intra.peff.net","threadId":"11037","inReplyTo":"Pine.LNX.4.64.0711272143470.5349@iabervon.org","subject":"Re: [PATCH] Allow update hooks to update refs on their own","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-28T16:10:34Z","receivedAt":"2007-11-28T16:10:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 27, 2007 at 10:25:32PM -0500, Daniel Barkalow wrote:\n\n> > It would of course be better if git-push could notice that it needs\n> > to do an actual fetch. I think it'd be sufficient to transmit the\n> > final remote ref SHA1 back to git-push, and if it doesn't match what\n> > was pushed, that's a sign that a fetch is needed. But that change\n> > wouldn't be mutually exclusive with this patch, I believe.\n> \n> Couldn't you do this with a status message? (\"ok changed by \n> hook\" or something.)\n\nHaving just touched this code, I believe the answer is yes. receive-pack\nhas always sent just \"ok \\n\", so we could start interpreting\nanything after the bit freely (I think \"ok \nchanged-to \" is even more informative, but perhaps not useful\ngiven that the sender probably doesn't have that commit object).\n\n-Peff\n"},{"id":"61272","messageId":"7vmysy5h5k.fsf@gitster.siamese.dyndns.org","threadId":"11037","inReplyTo":"20071128161033.GA20308@coredump.intra.peff.net","subject":"Re: [PATCH] Allow update hooks to update refs on their own","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-28T19:00:55Z","receivedAt":"2007-11-28T19:00:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> Couldn't you do this with a status message? (\"ok changed by \n>> hook\" or something.)\n>\n> Having just touched this code, I believe the answer is yes. receive-pack\n> has always sent just \"ok \\n\", so we could start interpreting\n> anything after the bit freely...\n\nMore importantly, no send-pack choked on \"ok \" followed by a SP\n(old ones just checked \"ok\" and nothing else, the current one NULs out\nthe SP and lets you use the rest as a message), so such a change to\nreceive-pack does not have to break older send-pack.\n"},{"id":"61277","messageId":"20071128194159.GA25977@midwinter.com","threadId":"11037","inReplyTo":"7vmysy5h5k.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] Allow update hooks to update refs on their own","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-11-28T19:41:59Z","receivedAt":"2007-11-28T19:41:59Z","isPatch":true,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"This is useful in cases where a hook needs to modify an incoming commit\nin some way, e.g., adding an annotation to the commit message, noting\nthe location of output from a profiling tool, or committing to an svn\nrepository using git-svn.\n\nrecieve-pack will now send the post-update SHA1 back to send-pack. If\nit's different than the one that was pushed, git-push won't do the\nfake update of the local tracking ref and will instead inform the user\nthat the tracking ref needs to be fetched.\n\nSigned-off-by: Steven Grimm \n---\n\n\tThe protocol was much easier to tweak than I expected. It\n\tseems like a reasonable extension to always send back the\n\tresulting SHA1; won't stop other people from adding more\n\tinformation later.\n\n\tIt would IMO be better still if git-push were to fetch\n\tautomatically here rather than just printing a message, but\n\tI'm not sure if that would smack too much of automagic behavior\n\tto people. Comments welcome.\n\n Documentation/git-receive-pack.txt | 8 +++-\n receive-pack.c | 72 +++++++++++++++++++++++-------------\n remote.c | 2 +-\n remote.h | 2 +\n send-pack.c | 64 +++++++++++++++++++++++++++-----\n 5 files changed, 109 insertions(+), 39 deletions(-)\n\ndiff --git a/Documentation/git-receive-pack.txt b/Documentation/git-receive-pack.txt\nindex 2633d94..115ae97 100644\n--- a/Documentation/git-receive-pack.txt\n+++ b/Documentation/git-receive-pack.txt\n@@ -74,8 +74,12 @@ Note that the hook is called before the refname is updated,\n so either sha1-old is 0\\{40} (meaning there is no such ref yet),\n or it should match what is recorded in refname.\n \n-The hook should exit with non-zero status if it wants to disallow\n-updating the named ref. Otherwise it should exit with zero.\n+The hook may optionally choose to update the ref on its own, e.g.,\n+if it needs to modify incoming revisions in some way. If it updates\n+the ref, it should exit with a status of 100. The hook should exit\n+with a status between 1 and 99 if it wants to disallow updating the\n+named ref. Otherwise it should exit with zero, and the ref will be\n+updated automatically.\n \n Successful execution (a zero exit status) of this hook does not\n ensure the ref will actually be updated, it is only a prerequisite.\ndiff --git a/receive-pack.c b/receive-pack.c\nindex 38e35c0..d07dd6d 100644\n--- a/receive-pack.c\n+++ b/receive-pack.c\n@@ -18,6 +18,9 @@ static int report_status;\n static char capabilities[] = \" report-status delete-refs \";\n static int capabilities_sent;\n \n+/* Update hook exit code: hook has updated ref on its own */\n+#define EXIT_CODE_REF_UPDATED 100\n+\n static int receive_pack_config(const char *var, const char *value)\n {\n \tif (strcmp(var, \"receive.denynonfastforwards\") == 0) {\n@@ -70,8 +73,11 @@ static struct command *commands;\n static const char pre_receive_hook[] = \"hooks/pre-receive\";\n static const char post_receive_hook[] = \"hooks/post-receive\";\n \n-static int hook_status(int code, const char *hook_name)\n+static int hook_status(int code, const char *hook_name, int ok_start)\n {\n+\tif (ok_start && -code >= ok_start)\n+\t\treturn -code;\n+\n \tswitch (code) {\n \tcase 0:\n \t\treturn 0;\n@@ -121,7 +127,7 @@ static int run_hook(const char *hook_name)\n \n \tcode = start_command(&proc);\n \tif (code)\n-\t\treturn hook_status(code, hook_name);\n+\t\treturn hook_status(code, hook_name, 0);\n \tfor (cmd = commands; cmd; cmd = cmd->next) {\n \t\tif (!cmd->error_string) {\n \t\t\tsize_t n = snprintf(buf, sizeof(buf), \"%s %s %s\\n\",\n@@ -132,7 +138,7 @@ static int run_hook(const char *hook_name)\n \t\t\t\tbreak;\n \t\t}\n \t}\n-\treturn hook_status(finish_command(&proc), hook_name);\n+\treturn hook_status(finish_command(&proc), hook_name, 0);\n }\n \n static int run_update_hook(struct command *cmd)\n@@ -155,7 +161,8 @@ static int run_update_hook(struct command *cmd)\n \tproc.no_stdin = 1;\n \tproc.stdout_to_stderr = 1;\n \n-\treturn hook_status(run_command(&proc), update_hook);\n+\treturn hook_status(run_command(&proc), update_hook,\n+\t\t\t EXIT_CODE_REF_UPDATED);\n }\n \n static const char *update(struct command *cmd)\n@@ -194,32 +201,45 @@ static const char *update(struct command *cmd)\n \t\t\treturn \"non-fast forward\";\n \t\t}\n \t}\n-\tif (run_update_hook(cmd)) {\n-\t\terror(\"hook declined to update %s\", name);\n-\t\treturn \"hook declined\";\n-\t}\n-\n-\tif (is_null_sha1(new_sha1)) {\n-\t\tif (delete_ref(name, old_sha1)) {\n-\t\t\terror(\"failed to delete %s\", name);\n-\t\t\treturn \"failed to delete\";\n+\tswitch (run_update_hook(cmd)) {\n+\tcase 0:\n+\t\tif (is_null_sha1(new_sha1)) {\n+\t\t\tif (delete_ref(name, old_sha1)) {\n+\t\t\t\terror(\"failed to delete %s\", name);\n+\t\t\t\treturn \"failed to delete\";\n+\t\t\t}\n+\t\t\tfprintf(stderr, \"%s: %s -> deleted\\n\", name,\n+\t\t\t\tsha1_to_hex(old_sha1));\n \t\t}\n-\t\tfprintf(stderr, \"%s: %s -> deleted\\n\", name,\n-\t\t\tsha1_to_hex(old_sha1));\n-\t\treturn NULL; /* good */\n-\t}\n-\telse {\n-\t\tlock = lock_any_ref_for_update(name, old_sha1, 0);\n-\t\tif (!lock) {\n-\t\t\terror(\"failed to lock %s\", name);\n-\t\t\treturn \"failed to lock\";\n+\t\telse {\n+\t\t\tlock = lock_any_ref_for_update(name, old_sha1, 0);\n+\t\t\tif (!lock) {\n+\t\t\t\terror(\"failed to lock %s\", name);\n+\t\t\t\treturn \"failed to lock\";\n+\t\t\t}\n+\t\t\tif (write_ref_sha1(lock, new_sha1, \"push\")) {\n+\t\t\t\treturn \"failed to write\"; /* error() already called */\n+\t\t\t}\n+\t\t\tfprintf(stderr, \"%s: %s -> %s\\n\", name,\n+\t\t\t\tsha1_to_hex(old_sha1), sha1_to_hex(new_sha1));\n \t\t}\n-\t\tif (write_ref_sha1(lock, new_sha1, \"push\")) {\n-\t\t\treturn \"failed to write\"; /* error() already called */\n+\t\treturn NULL; /* good */\n+\n+\tcase EXIT_CODE_REF_UPDATED:\n+\t\t/* hook has taken care of updating ref, which means it\n+\t\t might be a different revision than we think. */\n+\t\tif (! resolve_ref(name, new_sha1, 1, NULL)) {\n+\t\t\terror(\"can't resolve ref %s after hook updated it\",\n+\t\t\t\tname);\n+\t\t\treturn \"ref not resolvable\";\n \t\t}\n \t\tfprintf(stderr, \"%s: %s -> %s\\n\", name,\n \t\t\tsha1_to_hex(old_sha1), sha1_to_hex(new_sha1));\n \t\treturn NULL; /* good */\n+\n+\tdefault:\n+\t\terror(\"hook declined to update %s\", name);\n+\t\treturn \"hook declined\";\n \t}\n }\n \n@@ -419,8 +439,8 @@ static void report(const char *unpack_status)\n \t\t unpack_status ? unpack_status : \"ok\");\n \tfor (cmd = commands; cmd; cmd = cmd->next) {\n \t\tif (!cmd->error_string)\n-\t\t\tpacket_write(1, \"ok %s\\n\",\n-\t\t\t\t cmd->ref_name);\n+\t\t\tpacket_write(1, \"ok %s %s\\n\",\n+\t\t\t\t cmd->ref_name, sha1_to_hex(cmd->new_sha1));\n \t\telse\n \t\t\tpacket_write(1, \"ng %s %s\\n\",\n \t\t\t\t cmd->ref_name, cmd->error_string);\ndiff --git a/remote.c b/remote.c\nindex bec2ba1..07558c1 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -684,7 +684,7 @@ static int match_explicit_refs(struct ref *src, struct ref *dst,\n \treturn -errs;\n }\n \n-static struct ref *find_ref_by_name(struct ref *list, const char *name)\n+struct ref *find_ref_by_name(struct ref *list, const char *name)\n {\n \tfor ( ; list; list = list->next)\n \t\tif (!strcmp(list->name, name))\ndiff --git a/remote.h b/remote.h\nindex 878b4ec..e822f3c 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -56,6 +56,8 @@ void ref_remove_duplicates(struct ref *ref_map);\n \n struct refspec *parse_ref_spec(int nr_refspec, const char **refspec);\n \n+struct ref *find_ref_by_name(struct ref *list, const char *name);\n+\n int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,\n \t int nr_refspec, char **refspec, int all);\n \ndiff --git a/send-pack.c b/send-pack.c\nindex b74fd45..bf564ae 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -147,9 +147,31 @@ static void get_local_heads(void)\n \tfor_each_ref(one_local_ref, NULL);\n }\n \n-static int receive_status(int in)\n+/* Verifies that a remote ref matches the revision we pushed. */\n+static void verify_tracking_ref(const char *refname, const char *newsha1_hex, struct ref *remote_refs)\n+{\n+\tstruct ref *ref;\n+\tunsigned char newsha1[20];\n+\tif (get_sha1_hex(newsha1_hex, newsha1)) {\n+\t\tfprintf(stderr, \"protocol error: bad sha1 %s\\n\", newsha1_hex);\n+\t\treturn;\n+\t}\n+\tref = find_ref_by_name(remote_refs, refname);\n+\tif (ref != NULL) {\n+\t\tif (hashcmp(ref->new_sha1, newsha1)) {\n+\t\t\thashcpy(ref->new_sha1, newsha1);\n+\t\t\tref->force = 1;\n+\t\t}\n+\t\telse {\n+\t\t\tref->force = 0;\n+\t\t}\n+\t}\n+}\n+\n+static int receive_status(int in, struct ref *remote_refs)\n {\n \tchar line[1000];\n+\tchar *sha1;\n \tint ret = 0;\n \tint len = packet_read_line(in, line, sizeof(line));\n \tif (len < 10 || memcmp(line, \"unpack \", 7)) {\n@@ -170,8 +192,22 @@ static int receive_status(int in)\n \t\t\tret = -1;\n \t\t\tbreak;\n \t\t}\n-\t\tif (!memcmp(line, \"ok\", 2))\n+\t\tif (!memcmp(line, \"ok\", 2)) {\n+\t\t\t/* If the result looks like \"ok refname sha1\", we can\n+\t\t\t * compare the actual SHA1 for the remote ref with the\n+\t\t\t * one we pushed; if they differ then the remote\n+\t\t\t * may have rewritten our commits and we will need to\n+\t\t\t * do a real fetch rather than just updating the\n+\t\t\t * tracking refs.\n+\t\t\t */\n+\t\t\tif (line[2] == ' ' &&\n+\t\t\t (sha1 = strchr(line+3, ' '))) {\n+\t\t\t\t/* null-terminate refname */\n+\t\t\t\t*sha1++ = '\\0';\n+\t\t\t\tverify_tracking_ref(line+3, sha1, remote_refs);\n+\t\t\t}\n \t\t\tcontinue;\n+\t\t}\n \t\tfputs(line, stderr);\n \t\tret = -1;\n \t}\n@@ -196,13 +232,21 @@ static void update_tracking_ref(struct remote *remote, struct ref *ref)\n \t\treturn;\n \n \tif (!remote_find_tracking(remote, &rs)) {\n-\t\tfprintf(stderr, \"updating local tracking ref '%s'\\n\", rs.dst);\n-\t\tif (is_null_sha1(ref->peer_ref->new_sha1)) {\n-\t\t\tif (delete_ref(rs.dst, NULL))\n-\t\t\t\terror(\"Failed to delete\");\n-\t\t} else\n-\t\t\tupdate_ref(\"update by push\", rs.dst,\n-\t\t\t\t\tref->new_sha1, NULL, 0, 0);\n+\t\tif (ref->force) {\n+\t\t\tfprintf(stderr,\n+\t\t\t\t\"local tracking ref '%s' needs to be fetched\\n\",\n+\t\t\t\trs.dst);\n+\t\t}\n+\t\telse {\n+\t\t\tfprintf(stderr, \"updating local tracking ref '%s'\\n\",\n+\t\t\t\trs.dst);\n+\t\t\tif (is_null_sha1(ref->peer_ref->new_sha1)) {\n+\t\t\t\tif (delete_ref(rs.dst, NULL))\n+\t\t\t\t\terror(\"Failed to delete\");\n+\t\t\t} else\n+\t\t\t\tupdate_ref(\"update by push\", rs.dst,\n+\t\t\t\t\t\tref->new_sha1, NULL, 0, 0);\n+\t\t}\n \t\tfree(rs.dst);\n \t}\n }\n@@ -343,7 +387,7 @@ static int send_pack(int in, int out, struct remote *remote, int nr_refspec, cha\n \tclose(out);\n \n \tif (expect_status_report) {\n-\t\tif (receive_status(in))\n+\t\tif (receive_status(in, remote_refs))\n \t\t\tret = -4;\n \t}\n \n-- \n1.5.3.6.862.gb50ba-dirty\n"},{"id":"61279","messageId":"20071128194919.GC11396@coredump.intra.peff.net","threadId":"11037","inReplyTo":"20071128194159.GA25977@midwinter.com","subject":"Re: [PATCH] Allow update hooks to update refs on their own","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-28T19:49:19Z","receivedAt":"2007-11-28T19:49:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 28, 2007 at 11:41:59AM -0800, Steven Grimm wrote:\n\n> recieve-pack will now send the post-update SHA1 back to send-pack. If\n> it's different than the one that was pushed, git-push won't do the\n> fake update of the local tracking ref and will instead inform the user\n> that the tracking ref needs to be fetched.\n\nHrm, this is going to have nasty conflicts with 'next', which already\ndoes the remote ref matching. I think the best way to implement this\nwould probably be on top of the jk/send-pack topic in next, and add a\nnew REF_STATUS_REMOTE_CHANGED status type.\n\n-Peff\n"},{"id":"61282","messageId":"C1321BD5-8F6B-47F9-9BDB-C2BF819D6F17@midwinter.com","threadId":"11037","inReplyTo":"20071128194919.GC11396@coredump.intra.peff.net","subject":"Re: [PATCH] Allow update hooks to update refs on their own","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-11-28T20:16:27Z","receivedAt":"2007-11-28T20:16:27Z","isPatch":true,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"\nOn Nov 28, 2007, at 11:49 AM, Jeff King wrote:\n\n> Hrm, this is going to have nasty conflicts with 'next', which already\n> does the remote ref matching. I think the best way to implement this\n> would probably be on top of the jk/send-pack topic in next, and add a\n> new REF_STATUS_REMOTE_CHANGED status type.\n\nAh, yes, I see what you mean. Okay, please ignore my latest patch -- I \nwill redo it on top of \"next\" later today/tonight.\n\nWell, actually, I would still like opinions on one thing: What do \npeople think of having git-push do a fetch if the remote side changes \na ref to point to a revision that doesn't exist locally? Is there a \nsituation where you'd ever want to *not* do that?\n\n-Steve\n"},{"id":"61283","messageId":"20071128202250.GA12777@coredump.intra.peff.net","threadId":"11037","inReplyTo":"C1321BD5-8F6B-47F9-9BDB-C2BF819D6F17@midwinter.com","subject":"Re: [PATCH] Allow update hooks to update refs on their own","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-28T20:22:50Z","receivedAt":"2007-11-28T20:22:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 28, 2007 at 12:16:27PM -0800, Steven Grimm wrote:\n\n> Well, actually, I would still like opinions on one thing: What do people \n> think of having git-push do a fetch if the remote side changes a ref to \n> point to a revision that doesn't exist locally? Is there a situation where \n> you'd ever want to *not* do that?\n\nIt can be slow, since you have to make another connection to the server,\nso clearly it should only be done when you detect an update (which I\nthink is what you're proposing).\n\nA raw \"git-fetch\" might pull a lot of extra cruft that you didn't want\nto get right now. So if you did do it, I think it would make sense to\nconstruct a set of refspecs that match only the ones which need pulling\n(i.e., in update_tracking_ref, rather than doing the update, construct a\nrefspec of \"local:tracking\", and then hand all such refspecs to\ngit-fetch).\n\n-Peff\n"},{"id":"61293","messageId":"7vprxu3urt.fsf@gitster.siamese.dyndns.org","threadId":"11037","inReplyTo":"20071128194919.GC11396@coredump.intra.peff.net","subject":"Re: [PATCH] Allow update hooks to update refs on their own","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-28T21:49:42Z","receivedAt":"2007-11-28T21:49:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Hrm, this is going to have nasty conflicts with 'next', which already\n> does the remote ref matching. I think the best way to implement this\n> would probably be on top of the jk/send-pack topic in next, and add a\n> new REF_STATUS_REMOTE_CHANGED status type.\n\nI think Jeff is referring to sp/refspec-match (605b4978).\n\nI still have doubts about having this in the update hook, as the hook is\nabout accepting or refusing and has never been about rewriting.\n\nIf the implementation of the svn hook were to check if you can rebase\ncleanly in the update hook without actually rewriting the refs, and then\nto perform the real update of the refs in post-receive or post-update\nhook, that would feel much cleaner. But the end result would be the\nsame as you rewrote the refs inside the update hook like your patch\ndoes, so maybe I am worrying about conceptual cleanliness too much,\nneedlessly.\n"},{"id":"61298","messageId":"7vd4tu3u8b.fsf@gitster.siamese.dyndns.org","threadId":"11037","inReplyTo":"C1321BD5-8F6B-47F9-9BDB-C2BF819D6F17@midwinter.com","subject":"Re: [PATCH] Allow update hooks to update refs on their own","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-28T22:01:24Z","receivedAt":"2007-11-28T22:01:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steven Grimm writes:\n\n> Well, actually, I would still like opinions on one thing: What do\n> people think of having git-push do a fetch if the remote side changes\n> a ref to point to a revision that doesn't exist locally?\n\nI do not like it at all. git-push is about pushing what you did to the\nother end; you may or may not want to refetch what other people\nimmediately on top of what you did, and this includes what the hook\nscript did in the repository you have pushed into. If you want an\nimmediate refetch, you can script it so that you call push and then\nfetch; the latter needs to be a separate connection to a different\nservice anyway.\n"},{"id":"61301","messageId":"20071128221403.GA3256@midwinter.com","threadId":"11037","inReplyTo":"C1321BD5-8F6B-47F9-9BDB-C2BF819D6F17@midwinter.com","subject":"[PATCH v3] Allow update hooks to update refs on their own","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-11-28T22:14:03Z","receivedAt":"2007-11-28T22:14:03Z","isPatch":true,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"This is useful in cases where a hook needs to modify an incoming commit\nin some way, e.g., adding an annotation to the commit message, noting\nthe location of output from a profiling tool, or committing to an svn\nrepository using git-svn.\n\nrecieve-pack will now send the post-update SHA1 back to send-pack. If\nit's different than the one that was pushed, git-push won't do the\nfake update of the local tracking ref and will instead inform the user\nthat the tracking ref needs to be fetched.\n\nSigned-off-by: Steven Grimm \n---\n\n\tThis is on top of \"next\". It doesn't do any automatic fetching,\n\tjust prints a status message.\n\n Documentation/git-receive-pack.txt | 8 +++-\n builtin-send-pack.c | 65 +++++++++++++++++++++++++++-------\n cache.h | 1 +\n receive-pack.c | 68 +++++++++++++++++++++++-------------\n 4 files changed, 103 insertions(+), 39 deletions(-)\n\ndiff --git a/Documentation/git-receive-pack.txt b/Documentation/git-receive-pack.txt\nindex 2633d94..115ae97 100644\n--- a/Documentation/git-receive-pack.txt\n+++ b/Documentation/git-receive-pack.txt\n@@ -74,8 +74,12 @@ Note that the hook is called before the refname is updated,\n so either sha1-old is 0\\{40} (meaning there is no such ref yet),\n or it should match what is recorded in refname.\n \n-The hook should exit with non-zero status if it wants to disallow\n-updating the named ref. Otherwise it should exit with zero.\n+The hook may optionally choose to update the ref on its own, e.g.,\n+if it needs to modify incoming revisions in some way. If it updates\n+the ref, it should exit with a status of 100. The hook should exit\n+with a status between 1 and 99 if it wants to disallow updating the\n+named ref. Otherwise it should exit with zero, and the ref will be\n+updated automatically.\n \n Successful execution (a zero exit status) of this hook does not\n ensure the ref will actually be updated, it is only a prerequisite.\ndiff --git a/builtin-send-pack.c b/builtin-send-pack.c\nindex 25ae1fe..4ba716a 100644\n--- a/builtin-send-pack.c\n+++ b/builtin-send-pack.c\n@@ -164,7 +164,9 @@ static int receive_status(int in, struct ref *refs)\n \thint = NULL;\n \twhile (1) {\n \t\tchar *refname;\n+\t\tchar *newsha1_hex;\n \t\tchar *msg;\n+\t\tunsigned char newsha1[20];\n \t\tlen = packet_read_line(in, line, sizeof(line));\n \t\tif (!len)\n \t\t\tbreak;\n@@ -177,7 +179,16 @@ static int receive_status(int in, struct ref *refs)\n \n \t\tline[strlen(line)-1] = '\\0';\n \t\trefname = line + 3;\n-\t\tmsg = strchr(refname, ' ');\n+\t\tnewsha1_hex = strchr(refname, ' ');\n+\t\tif (newsha1_hex) {\n+\t\t\t*newsha1_hex++ = '\\0';\n+\t\t\tif (get_sha1_hex(newsha1_hex, newsha1)) {\n+\t\t\t\tfprintf(stderr, \"protocol error: bad sha1 %s\\n\",\n+\t\t\t\t\tnewsha1_hex);\n+\t\t\t\tnewsha1_hex = NULL;\n+\t\t\t}\n+\t\t}\n+\t\tmsg = strchr(newsha1_hex, ' ');\n \t\tif (msg)\n \t\t\t*msg++ = '\\0';\n \n@@ -197,8 +208,16 @@ static int receive_status(int in, struct ref *refs)\n \t\t\tcontinue;\n \t\t}\n \n-\t\tif (line[0] == 'o' && line[1] == 'k')\n-\t\t\thint->status = REF_STATUS_OK;\n+\t\tif (line[0] == 'o' && line[1] == 'k') {\n+\t\t\tif (newsha1_hex != NULL &&\n+\t\t\t hashcmp(hint->new_sha1, newsha1)) {\n+\t\t\t\thint->status = REF_STATUS_REMOTE_CHANGED;\n+\t\t\t\thashcpy(hint->new_sha1, newsha1);\n+\t\t\t}\n+\t\t\telse {\n+\t\t\t\thint->status = REF_STATUS_OK;\n+\t\t\t}\n+\t\t}\n \t\telse {\n \t\t\thint->status = REF_STATUS_REMOTE_REJECT;\n \t\t\tret = -1;\n@@ -215,21 +234,34 @@ static void update_tracking_ref(struct remote *remote, struct ref *ref)\n {\n \tstruct refspec rs;\n \n-\tif (ref->status != REF_STATUS_OK)\n+\tif (ref->status != REF_STATUS_OK &&\n+\t ref->status != REF_STATUS_REMOTE_CHANGED)\n \t\treturn;\n \n \trs.src = ref->name;\n \trs.dst = NULL;\n \n \tif (!remote_find_tracking(remote, &rs)) {\n-\t\tif (args.verbose)\n-\t\t\tfprintf(stderr, \"updating local tracking ref '%s'\\n\", rs.dst);\n-\t\tif (ref->deletion) {\n-\t\t\tif (delete_ref(rs.dst, NULL))\n-\t\t\t\terror(\"Failed to delete\");\n-\t\t} else\n-\t\t\tupdate_ref(\"update by push\", rs.dst,\n-\t\t\t\t\tref->new_sha1, NULL, 0, 0);\n+\t\tif (ref->status == REF_STATUS_REMOTE_CHANGED) {\n+\t\t\tif (args.verbose) {\n+\t\t\t\tfprintf(stderr, \"local tracking ref '%s' \"\n+\t\t\t\t\t\"needs to be fetched\\n\", rs.dst);\n+\t\t\t}\n+\t\t}\n+\t\telse {\n+\t\t\tif (args.verbose) {\n+\t\t\t\tfprintf(stderr, \"updating local tracking \"\n+\t\t\t\t\t\"ref '%s'\\n\", rs.dst);\n+\t\t\t}\n+\t\t\tif (ref->deletion) {\n+\t\t\t\tif (delete_ref(rs.dst, NULL))\n+\t\t\t\t\terror(\"Failed to delete\");\n+\t\t\t}\n+\t\t\telse {\n+\t\t\t\tupdate_ref(\"update by push\", rs.dst,\n+\t\t\t\t\t\tref->new_sha1, NULL, 0, 0);\n+\t\t\t}\n+\t\t}\n \t\tfree(rs.dst);\n \t}\n }\n@@ -329,6 +361,10 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count)\n \t\t\t\tref->deletion ? NULL : ref->peer_ref,\n \t\t\t\t\"remote failed to report status\");\n \t\tbreak;\n+\tcase REF_STATUS_REMOTE_CHANGED:\n+\t\tprint_ref_status('+', \"[changed]\", ref, ref->peer_ref,\n+\t\t\t\t\"remote made changes to revisions\");\n+\t\tbreak;\n \tcase REF_STATUS_OK:\n \t\tprint_ok_ref_status(ref);\n \t\tbreak;\n@@ -349,12 +385,14 @@ static void print_push_status(const char *dest, struct ref *refs)\n \t}\n \n \tfor (ref = refs; ref; ref = ref->next)\n-\t\tif (ref->status == REF_STATUS_OK)\n+\t\tif (ref->status == REF_STATUS_OK ||\n+\t\t ref->status == REF_STATUS_REMOTE_CHANGED)\n \t\t\tn += print_one_push_status(ref, dest, n);\n \n \tfor (ref = refs; ref; ref = ref->next) {\n \t\tif (ref->status != REF_STATUS_NONE &&\n \t\t ref->status != REF_STATUS_UPTODATE &&\n+\t\t ref->status != REF_STATUS_REMOTE_CHANGED &&\n \t\t ref->status != REF_STATUS_OK)\n \t\t\tn += print_one_push_status(ref, dest, n);\n \t}\n@@ -522,6 +560,7 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest\n \t\tswitch (ref->status) {\n \t\tcase REF_STATUS_NONE:\n \t\tcase REF_STATUS_UPTODATE:\n+\t\tcase REF_STATUS_REMOTE_CHANGED:\n \t\tcase REF_STATUS_OK:\n \t\t\tbreak;\n \t\tdefault:\ndiff --git a/cache.h b/cache.h\nindex 4e59646..ae2427d 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -516,6 +516,7 @@ struct ref {\n \t\tREF_STATUS_UPTODATE,\n \t\tREF_STATUS_REMOTE_REJECT,\n \t\tREF_STATUS_EXPECTING_REPORT,\n+\t\tREF_STATUS_REMOTE_CHANGED,\n \t} status;\n \tchar *remote_status;\n \tstruct ref *peer_ref; /* when renaming */\ndiff --git a/receive-pack.c b/receive-pack.c\nindex ed44b89..1101e18 100644\n--- a/receive-pack.c\n+++ b/receive-pack.c\n@@ -18,6 +18,9 @@ static int report_status;\n static char capabilities[] = \" report-status delete-refs \";\n static int capabilities_sent;\n \n+/* Update hook exit code: hook has updated ref on its own */\n+#define EXIT_CODE_REF_UPDATED 100\n+\n static int receive_pack_config(const char *var, const char *value)\n {\n \tif (strcmp(var, \"receive.denynonfastforwards\") == 0) {\n@@ -70,8 +73,11 @@ static struct command *commands;\n static const char pre_receive_hook[] = \"hooks/pre-receive\";\n static const char post_receive_hook[] = \"hooks/post-receive\";\n \n-static int hook_status(int code, const char *hook_name)\n+static int hook_status(int code, const char *hook_name, int ok_start)\n {\n+\tif (ok_start && -code >= ok_start)\n+\t\treturn -code;\n+\n \tswitch (code) {\n \tcase 0:\n \t\treturn 0;\n@@ -121,7 +127,7 @@ static int run_hook(const char *hook_name)\n \n \tcode = start_command(&proc);\n \tif (code)\n-\t\treturn hook_status(code, hook_name);\n+\t\treturn hook_status(code, hook_name, 0);\n \tfor (cmd = commands; cmd; cmd = cmd->next) {\n \t\tif (!cmd->error_string) {\n \t\t\tsize_t n = snprintf(buf, sizeof(buf), \"%s %s %s\\n\",\n@@ -132,7 +138,7 @@ static int run_hook(const char *hook_name)\n \t\t\t\tbreak;\n \t\t}\n \t}\n-\treturn hook_status(finish_command(&proc), hook_name);\n+\treturn hook_status(finish_command(&proc), hook_name, 0);\n }\n \n static int run_update_hook(struct command *cmd)\n@@ -155,7 +161,8 @@ static int run_update_hook(struct command *cmd)\n \tproc.no_stdin = 1;\n \tproc.stdout_to_stderr = 1;\n \n-\treturn hook_status(run_command(&proc), update_hook);\n+\treturn hook_status(run_command(&proc), update_hook,\n+\t\t\t EXIT_CODE_REF_UPDATED);\n }\n \n static const char *update(struct command *cmd)\n@@ -194,28 +201,41 @@ static const char *update(struct command *cmd)\n \t\t\treturn \"non-fast forward\";\n \t\t}\n \t}\n-\tif (run_update_hook(cmd)) {\n-\t\terror(\"hook declined to update %s\", name);\n-\t\treturn \"hook declined\";\n-\t}\n-\n-\tif (is_null_sha1(new_sha1)) {\n-\t\tif (delete_ref(name, old_sha1)) {\n-\t\t\terror(\"failed to delete %s\", name);\n-\t\t\treturn \"failed to delete\";\n+\tswitch (run_update_hook(cmd)) {\n+\tcase 0:\n+\t\tif (is_null_sha1(new_sha1)) {\n+\t\t\tif (delete_ref(name, old_sha1)) {\n+\t\t\t\terror(\"failed to delete %s\", name);\n+\t\t\t\treturn \"failed to delete\";\n+\t\t\t}\n+\t\t\tfprintf(stderr, \"%s: %s -> deleted\\n\", name,\n+\t\t\t\tsha1_to_hex(old_sha1));\n \t\t}\n-\t\treturn NULL; /* good */\n-\t}\n-\telse {\n-\t\tlock = lock_any_ref_for_update(name, old_sha1, 0);\n-\t\tif (!lock) {\n-\t\t\terror(\"failed to lock %s\", name);\n-\t\t\treturn \"failed to lock\";\n+\t\telse {\n+\t\t\tlock = lock_any_ref_for_update(name, old_sha1, 0);\n+\t\t\tif (!lock) {\n+\t\t\t\terror(\"failed to lock %s\", name);\n+\t\t\t\treturn \"failed to lock\";\n+\t\t\t}\n+\t\t\tif (write_ref_sha1(lock, new_sha1, \"push\")) {\n+\t\t\t\treturn \"failed to write\"; /* error() already called */\n+\t\t\t}\n \t\t}\n-\t\tif (write_ref_sha1(lock, new_sha1, \"push\")) {\n-\t\t\treturn \"failed to write\"; /* error() already called */\n+\t\treturn NULL; /* good */\n+\n+\tcase EXIT_CODE_REF_UPDATED:\n+\t\t/* hook has taken care of updating ref, which means it\n+\t\t might be a different revision than we think. */\n+\t\tif (! resolve_ref(name, new_sha1, 1, NULL)) {\n+\t\t\terror(\"can't resolve ref %s after hook updated it\",\n+\t\t\t\tname);\n+\t\t\treturn \"ref not resolvable\";\n \t\t}\n \t\treturn NULL; /* good */\n+\n+\tdefault:\n+\t\terror(\"hook declined to update %s\", name);\n+\t\treturn \"hook declined\";\n \t}\n }\n \n@@ -415,8 +435,8 @@ static void report(const char *unpack_status)\n \t\t unpack_status ? unpack_status : \"ok\");\n \tfor (cmd = commands; cmd; cmd = cmd->next) {\n \t\tif (!cmd->error_string)\n-\t\t\tpacket_write(1, \"ok %s\\n\",\n-\t\t\t\t cmd->ref_name);\n+\t\t\tpacket_write(1, \"ok %s %s\\n\",\n+\t\t\t\t cmd->ref_name, sha1_to_hex(cmd->new_sha1));\n \t\telse\n \t\t\tpacket_write(1, \"ng %s %s\\n\",\n \t\t\t\t cmd->ref_name, cmd->error_string);\n-- \n1.5.3.6.2040.g97735-dirty\n"},{"id":"61306","messageId":"20071128223720.GA13298@coredump.intra.peff.net","threadId":"11037","inReplyTo":"7vprxu3urt.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Allow update hooks to update refs on their own","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-28T22:37:20Z","receivedAt":"2007-11-28T22:37:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 28, 2007 at 01:49:42PM -0800, Junio C Hamano wrote:\n\n> > Hrm, this is going to have nasty conflicts with 'next', which already\n> > does the remote ref matching. I think the best way to implement this\n> > would probably be on top of the jk/send-pack topic in next, and add a\n> > new REF_STATUS_REMOTE_CHANGED status type.\n> \n> I think Jeff is referring to sp/refspec-match (605b4978).\n\nNo, I was actually referring to the jk/send-pack topic. I had thought it\ngraduated to master, but since Steven's work was based on a much older\nversion (e.g., with send-pack.c rather than builtin-send-pack.c), I\nassumed it had not graduated and didn't confirm.\n\nSo let me amend my comment to \"base this on a more recent master...\".\n\n-Peff\n"},{"id":"61317","messageId":"20071128230355.GB13964@coredump.intra.peff.net","threadId":"11037","inReplyTo":"20071128221403.GA3256@midwinter.com","subject":"Re: [PATCH v3] Allow update hooks to update refs on their own","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-28T23:03:55Z","receivedAt":"2007-11-28T23:03:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 28, 2007 at 02:14:03PM -0800, Steven Grimm wrote:\n\n> @@ -177,7 +179,16 @@ static int receive_status(int in, struct ref *refs)\n> \n> \t\tline[strlen(line)-1] = '\\0';\n> \t\trefname = line + 3;\n> -\t\tmsg = strchr(refname, ' ');\n> +\t\tnewsha1_hex = strchr(refname, ' ');\n> +\t\tif (newsha1_hex) {\n> +\t\t\t*newsha1_hex++ = '\\0';\n> +\t\t\tif (get_sha1_hex(newsha1_hex, newsha1)) {\n> +\t\t\t\tfprintf(stderr, \"protocol error: bad sha1 %s\\n\",\n> +\t\t\t\t\tnewsha1_hex);\n> +\t\t\t\tnewsha1_hex = NULL;\n> +\t\t\t}\n> +\t\t}\n> +\t\tmsg = strchr(newsha1_hex, ' ');\n> \t\tif (msg)\n> \t\t\t*msg++ = '\\0';\n\nDoesn't this always put the first \"word\" of a response into newsha1_hex?\nWe want to do this only for 'ok' responses; 'ng' responses are already\nusing that space as part of the error message.\n\n-Peff\n"},{"id":"61326","messageId":"7vve7m0wfo.fsf@gitster.siamese.dyndns.org","threadId":"11037","inReplyTo":"20071128230355.GB13964@coredump.intra.peff.net","subject":"Re: [PATCH v3] Allow update hooks to update refs on their own","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-28T23:42:03Z","receivedAt":"2007-11-28T23:42:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Nov 28, 2007 at 02:14:03PM -0800, Steven Grimm wrote:\n>\n>> @@ -177,7 +179,16 @@ static int receive_status(int in, struct ref *refs)\n>> \n>> \t\tline[strlen(line)-1] = '\\0';\n>> \t\trefname = line + 3;\n>> -\t\tmsg = strchr(refname, ' ');\n>> +\t\tnewsha1_hex = strchr(refname, ' ');\n>> +\t\tif (newsha1_hex) {\n>> +\t\t\t*newsha1_hex++ = '\\0';\n>> +\t\t\tif (get_sha1_hex(newsha1_hex, newsha1)) {\n>> +\t\t\t\tfprintf(stderr, \"protocol error: bad sha1 %s\\n\",\n>> +\t\t\t\t\tnewsha1_hex);\n>> +\t\t\t\tnewsha1_hex = NULL;\n>> +\t\t\t}\n>> +\t\t}\n>> +\t\tmsg = strchr(newsha1_hex, ' ');\n>> \t\tif (msg)\n>> \t\t\t*msg++ = '\\0';\n>\n> Doesn't this always put the first \"word\" of a response into newsha1_hex?\n> We want to do this only for 'ok' responses; 'ng' responses are already\n> using that space as part of the error message.\n\nI do not think reporting back the rewritten object name makes much sense\nnor adds any value; it won't be useful information until you fetch the\nobject.\n\nI do not think reporting back _anything_ other than \"ok\" adds much value\nat all. Sure, if the update hook did something funky you would get such\na report, but the situation is not any different if some warm body is\nsitting on the other end and building on top of what you pushed\nimmediately he sees any push into the repository, and in such a case\nyour git-push would not get any such reporting anyway.\n\nWe do not even have to worry about this reporting at all if we do not\nallow munging the refs in the update hook. In a sense, this patch is\ncreating a problem that does not need to be solved. Perhaps modifying\nupdate hook to allow so makes it possible to munge refs while holding a\nlock, but is it really worth this hassle? Isn't there a better way, I\nwonder?\n"},{"id":"61368","messageId":"35C5BEA0-0D6C-4D2E-85E7-1B78FB0BEADA@midwinter.com","threadId":"11037","inReplyTo":"7vve7m0wfo.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v3] Allow update hooks to update refs on their own","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-11-29T06:44:59Z","receivedAt":"2007-11-29T06:44:59Z","isPatch":true,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"On Nov 28, 2007, at 3:42 PM, Junio C Hamano wrote:\n> I do not think reporting back the rewritten object name makes much \n> sense\n> nor adds any value; it won't be useful information until you fetch the\n> object.\n\nRight, this was mostly in anticipation of doing an automatic fetch, so \nthat I would avoid fetching anything but the rewritten revisions; if I \njust fetched the remote ref as normal, then I'd potentially pick up \nunrelated changes that happened to hit just after my pack was \naccepted, which wouldn't maintain the \"update the tracking ref to \npoint to what I just pushed\" semantics.\n\nSince it sounds like that's a nonstarter, I agree this part of the \npatch isn't useful.\n\n> I do not think reporting back _anything_ other than \"ok\" adds much \n> value\n> at all. Sure, if the update hook did something funky you would get \n> such\n> a report, but the situation is not any different if some warm body is\n> sitting on the other end and building on top of what you pushed\n> immediately he sees any push into the repository, and in such a case\n> your git-push would not get any such reporting anyway.\n\nI disagree that it's the same. In this case the updated ref happens as \na component of the push operation (which of course includes running \nupdate hooks and at the very least looking at their exit codes to see \nif a change should be rejected), not as a result of some other process \nthat happens to occur at nearly the same time. Reporting back the new \nref, at the very least, tells you that it's not useful to update the \ntracking ref since it's 100% guaranteed to be wrong by the time the \npush finishes.\n\n> We do not even have to worry about this reporting at all if we do not\n> allow munging the refs in the update hook. In a sense, this patch is\n> creating a problem that does not need to be solved. Perhaps modifying\n> update hook to allow so makes it possible to munge refs while \n> holding a\n> lock, but is it really worth this hassle? Isn't there a better way, I\n> wonder?\n\nIf there is, I'm happy to hear it; for me this patch is a means, not \nan end. What I actually want is to be able to have a particular set of \nbranches in a particular git repository be as-transparent-as-possible \nconduits to corresponding branches in an svn repository.\n\nI arrived at this approach by following this train of thought:\n\n1. The update hook is the only hook that allows me to reject the push, \nwhich I need to do if svn refuses to accept the change.\n2. To tell whether svn accepts a change, I need to run git-svn \ndcommit; thanks to #1, I need to do that from inside the update hook.\n3. When I commit, git-svn needs to track that the git revision now \ncorresponds to an svn revision. It does that by modifying the commit \nmessage to add its git-svn-id: line.\n4. Modifying the commit comment causes the revision's SHA1 to change.\n5. Out-of-the-box push thinks a push has failed if the ref's SHA1 \nchanges in the update hook.\n6. Therefore push needs to be modified to not do that.\n\nIf any one of #1-5 wasn't true or was solvable in a different way, \nthen #6 wouldn't be needed. For example, if git-svn kept its mapping \nof git revisions to svn revisions somewhere else it could leave the \ncommit messages untouched, meaning the SHA1s wouldn't change.\n\n-Steve\n"},{"id":"61452","messageId":"7vr6i8sfsa.fsf@gitster.siamese.dyndns.org","threadId":"11037","inReplyTo":"35C5BEA0-0D6C-4D2E-85E7-1B78FB0BEADA@midwinter.com","subject":"Re: [PATCH v3] Allow update hooks to update refs on their own","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-30T01:06:29Z","receivedAt":"2007-11-30T01:06:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steven Grimm writes:\n\n> If any one of #1-5 wasn't true or was solvable in a different way,\n> then #6 wouldn't be needed. For example, if git-svn kept its mapping\n> of git revisions to svn revisions somewhere else it could leave the\n> commit messages untouched, meaning the SHA1s wouldn't change.\n\nThat does sound like an unfortunate design problem with how git-svn\nkeeps track of the correlation between two systems.\n\nBut after thinking about this issue a bit more, I think I agree that\nallowing to munge what was pushed inside update hook may make sense even\noutside of git-svn context (iow, if this were an ugly workaround for\ngit-svn deficiency then I would be unhappy but I think there are valid\nuse cases). \"You push A, I inspect it and may rewrite it to A' but only\nif A' is reasonable -- otherwise I reject your pushing of A\" could be a\nvalid thing to do in other contexts. A stupid example would be an update\nhook that tries to cleanse what you pushed for whitespace breakage and\ncommit a cleaned-up result only if the result passes the testsuite.\n"},{"id":"61680","messageId":"20071202212224.GA22117@midwinter.com","threadId":"11037","inReplyTo":"7vr6i8sfsa.fsf@gitster.siamese.dyndns.org","subject":"[PATCH v4] Allow update hooks to update refs on their own.","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-12-02T21:22:24Z","receivedAt":"2007-12-02T21:22:24Z","isPatch":true,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"This is useful in cases where a hook needs to modify an incoming commit\nin some way, e.g., fixing whitespace errors, adding an annotation to\nthe commit message, noting the location of output from a profiling tool,\nor committing to an svn repository using git-svn.\n\nSigned-off-by: Steven Grimm \n---\n\n\tSince Junio's main objection to this seemed to be the protocol\n\tchange to bypass the automatic update of the tracking ref in\n\tgit-send-pack, that code is gone (thus reverting this to the\n\tsame code change as the initial version!) and I added a section\n\tto the git-send-pack manual page describing the automatic\n\ttracking ref update behavior, which wasn't documented at all\n\tbefore. Someone please review my terminology there.\n\n Documentation/git-receive-pack.txt | 8 +++-\n Documentation/git-send-pack.txt | 16 ++++++++\n receive-pack.c | 70 +++++++++++++++++++++++-------------\n 3 files changed, 67 insertions(+), 27 deletions(-)\n\ndiff --git a/Documentation/git-receive-pack.txt b/Documentation/git-receive-pack.txt\nindex 2633d94..115ae97 100644\n--- a/Documentation/git-receive-pack.txt\n+++ b/Documentation/git-receive-pack.txt\n@@ -74,8 +74,12 @@ Note that the hook is called before the refname is updated,\n so either sha1-old is 0\\{40} (meaning there is no such ref yet),\n or it should match what is recorded in refname.\n \n-The hook should exit with non-zero status if it wants to disallow\n-updating the named ref. Otherwise it should exit with zero.\n+The hook may optionally choose to update the ref on its own, e.g.,\n+if it needs to modify incoming revisions in some way. If it updates\n+the ref, it should exit with a status of 100. The hook should exit\n+with a status between 1 and 99 if it wants to disallow updating the\n+named ref. Otherwise it should exit with zero, and the ref will be\n+updated automatically.\n \n Successful execution (a zero exit status) of this hook does not\n ensure the ref will actually be updated, it is only a prerequisite.\ndiff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt\nindex a2d9cb6..db64a1b 100644\n--- a/Documentation/git-send-pack.txt\n+++ b/Documentation/git-send-pack.txt\n@@ -115,6 +115,22 @@ Optionally, a parameter can be prefixed with a plus '+' sign\n to disable the fast-forward check only on that ref.\n \n \n+Remote Tracking Refs\n+--------------------\n+\n+After successfully sending a pack to the remote, 'git-send-pack'\n+updates the corresponding remote tracking ref in the local repository\n+to point to the same commit as was just sent to the remote side. In\n+most cases this eliminates the need to subsequently fetch from the\n+remote repository since there would be nothing new to fetch.\n+\n+If the remote side's update hook modifies the incoming commit\n+before applying it, the local repository's remote tracking ref will\n+point at a different commit than the corresponding remote ref (since\n+the local repository will not have a copy of the modified version).\n+In that case an explicit fetch will be required.\n+\n+\n Author\n ------\n Written by Linus Torvalds \ndiff --git a/receive-pack.c b/receive-pack.c\nindex fba4cf8..ca906bf 100644\n--- a/receive-pack.c\n+++ b/receive-pack.c\n@@ -18,6 +18,9 @@ static int report_status;\n static char capabilities[] = \" report-status delete-refs \";\n static int capabilities_sent;\n \n+/* Update hook exit code: hook has updated ref on its own */\n+#define EXIT_CODE_REF_UPDATED 100\n+\n static int receive_pack_config(const char *var, const char *value)\n {\n \tif (strcmp(var, \"receive.denynonfastforwards\") == 0) {\n@@ -70,8 +73,11 @@ static struct command *commands;\n static const char pre_receive_hook[] = \"hooks/pre-receive\";\n static const char post_receive_hook[] = \"hooks/post-receive\";\n \n-static int hook_status(int code, const char *hook_name)\n+static int hook_status(int code, const char *hook_name, int ok_start)\n {\n+\tif (ok_start && -code >= ok_start)\n+\t\treturn -code;\n+\n \tswitch (code) {\n \tcase 0:\n \t\treturn 0;\n@@ -121,7 +127,7 @@ static int run_hook(const char *hook_name)\n \n \tcode = start_command(&proc);\n \tif (code)\n-\t\treturn hook_status(code, hook_name);\n+\t\treturn hook_status(code, hook_name, 0);\n \tfor (cmd = commands; cmd; cmd = cmd->next) {\n \t\tif (!cmd->error_string) {\n \t\t\tsize_t n = snprintf(buf, sizeof(buf), \"%s %s %s\\n\",\n@@ -132,7 +138,7 @@ static int run_hook(const char *hook_name)\n \t\t\t\tbreak;\n \t\t}\n \t}\n-\treturn hook_status(finish_command(&proc), hook_name);\n+\treturn hook_status(finish_command(&proc), hook_name, 0);\n }\n \n static int run_update_hook(struct command *cmd)\n@@ -155,7 +161,8 @@ static int run_update_hook(struct command *cmd)\n \tproc.no_stdin = 1;\n \tproc.stdout_to_stderr = 1;\n \n-\treturn hook_status(run_command(&proc), update_hook);\n+\treturn hook_status(run_command(&proc), update_hook,\n+\t\t\t EXIT_CODE_REF_UPDATED);\n }\n \n static const char *update(struct command *cmd)\n@@ -194,32 +201,45 @@ static const char *update(struct command *cmd)\n \t\t\treturn \"non-fast forward\";\n \t\t}\n \t}\n-\tif (run_update_hook(cmd)) {\n-\t\terror(\"hook declined to update %s\", name);\n-\t\treturn \"hook declined\";\n-\t}\n-\n-\tif (is_null_sha1(new_sha1)) {\n-\t\tif (!parse_object(old_sha1)) {\n-\t\t\twarning (\"Allowing deletion of corrupt ref.\");\n-\t\t\told_sha1 = NULL;\n+\tswitch (run_update_hook(cmd)) {\n+\tcase 0:\n+\t\tif (is_null_sha1(new_sha1)) {\n+\t\t\tif (!parse_object(old_sha1)) {\n+\t\t\t\twarning (\"Allowing deletion of corrupt ref.\");\n+\t\t\t\told_sha1 = NULL;\n+\t\t\t}\n+\t\t\tif (delete_ref(name, old_sha1)) {\n+\t\t\t\terror(\"failed to delete %s\", name);\n+\t\t\t\treturn \"failed to delete\";\n+\t\t\t}\n+\t\t\tfprintf(stderr, \"%s: %s -> deleted\\n\", name,\n+\t\t\t\tsha1_to_hex(old_sha1));\n \t\t}\n-\t\tif (delete_ref(name, old_sha1)) {\n-\t\t\terror(\"failed to delete %s\", name);\n-\t\t\treturn \"failed to delete\";\n+\t\telse {\n+\t\t\tlock = lock_any_ref_for_update(name, old_sha1, 0);\n+\t\t\tif (!lock) {\n+\t\t\t\terror(\"failed to lock %s\", name);\n+\t\t\t\treturn \"failed to lock\";\n+\t\t\t}\n+\t\t\tif (write_ref_sha1(lock, new_sha1, \"push\")) {\n+\t\t\t\treturn \"failed to write\"; /* error() already called */\n+\t\t\t}\n \t\t}\n \t\treturn NULL; /* good */\n-\t}\n-\telse {\n-\t\tlock = lock_any_ref_for_update(name, old_sha1, 0);\n-\t\tif (!lock) {\n-\t\t\terror(\"failed to lock %s\", name);\n-\t\t\treturn \"failed to lock\";\n-\t\t}\n-\t\tif (write_ref_sha1(lock, new_sha1, \"push\")) {\n-\t\t\treturn \"failed to write\"; /* error() already called */\n+\n+\tcase EXIT_CODE_REF_UPDATED:\n+\t\t/* hook has taken care of updating ref, which means it\n+\t\t might be a different revision than we think. */\n+\t\tif (! resolve_ref(name, new_sha1, 1, NULL)) {\n+\t\t\terror(\"can't resolve ref %s after hook updated it\",\n+\t\t\t\tname);\n+\t\t\treturn \"ref not resolvable\";\n \t\t}\n \t\treturn NULL; /* good */\n+\n+\tdefault:\n+\t\terror(\"hook declined to update %s\", name);\n+\t\treturn \"hook declined\";\n \t}\n }\n \n-- \n1.5.3.6.2040.g97735-dirty\n"},{"id":"61686","messageId":"7vwsrwu5fs.fsf@gitster.siamese.dyndns.org","threadId":"11037","inReplyTo":"20071202212224.GA22117@midwinter.com","subject":"Re: [PATCH v4] Allow update hooks to update refs on their own.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-02T21:56:07Z","receivedAt":"2007-12-02T21:56:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steven Grimm writes:\n\n> +The hook may optionally choose to update the ref on its own, e.g.,\n> +if it needs to modify incoming revisions in some way. If it updates\n> +the ref, it should exit with a status of 100. The hook should exit\n> +with a status between 1 and 99 if it wants to disallow updating the\n> +named ref. Otherwise it should exit with zero, and the ref will be\n> +updated automatically.\n\nThis makes one wonder what happens if it returns 101, iow if there is\ndifference between returning 99 and 101, and if so why.\n\n> +Remote Tracking Refs\n> +--------------------\n> +\n> +After successfully sending a pack to the remote, 'git-send-pack'\n> +updates the corresponding remote tracking ref in the local repository\n> +to point to the same commit as was just sent to the remote side. In\n> +most cases this eliminates the need to subsequently fetch from the\n> +remote repository since there would be nothing new to fetch.\n\nMicronit. The above is all \"if exists\". Not everybody pushes to\nsomewhere he uses tracking with.\n\n> @@ -70,8 +73,11 @@ static struct command *commands;\n> static const char pre_receive_hook[] = \"hooks/pre-receive\";\n> static const char post_receive_hook[] = \"hooks/post-receive\";\n> \n> -static int hook_status(int code, const char *hook_name)\n> +static int hook_status(int code, const char *hook_name, int ok_start)\n> {\n> +\tif (ok_start && -code >= ok_start)\n> +\t\treturn -code;\n> +\n\nI've always been puzzled by this \"ok_start\" parameter from the very\nbeginning edition of your patch. It is not like \"if this is true, then\nit is ok to run the hook but otherwise do not run\". In layman's terms,\nwhat does the parameter mean?\n\nMaybe the variable is misnamed and not expressing the concept well\nenough. Maybe the concept itself is muddy and hard to understand. I\ncannot tell.\n"},{"id":"61705","messageId":"20071203021333.GC8322@coredump.intra.peff.net","threadId":"11037","inReplyTo":"20071202212224.GA22117@midwinter.com","subject":"Re: [PATCH v4] Allow update hooks to update refs on their own.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-12-03T02:13:33Z","receivedAt":"2007-12-03T02:13:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Dec 02, 2007 at 01:22:24PM -0800, Steven Grimm wrote:\n\n> \tSince Junio's main objection to this seemed to be the protocol\n> \tchange to bypass the automatic update of the tracking ref in\n> \tgit-send-pack, that code is gone (thus reverting this to the\n> \tsame code change as the initial version!) and I added a section\n> \tto the git-send-pack manual page describing the automatic\n> \ttracking ref update behavior, which wasn't documented at all\n> \tbefore. Someone please review my terminology there.\n\nI am dubious of the usefulness of passing back the new commit id, but an\n\"ok, but btw I changed your commit\" status from receive-pack seems like\nit would be useful, for two reasons:\n\n - it can be displayed differently, so the user is reminded to do a\n fetch afterwards\n - we can avoid updating the tracking ref, which makes it less likely\n to result in a non-fast forward fetch next time. For example,\n consider:\n\n 1. The remote master and my origin/master are at A.\n 2. I make a commit B on top of A.\n 3. I push B to remote, who rewrites it to B' on top of A. At the\n same time, I move my origin/master to B.\n 4. I fetch, and get non-ff going from B to B'.\n\n If I had never written anything to my origin/master, it would be a\n fast forward. And obviously git handles it just fine, but it is more\n useful to the user during the next fetch to see A..B rather than\n B'...B.\n\n-Peff\n"},{"id":"61706","messageId":"7vlk8csetl.fsf@gitster.siamese.dyndns.org","threadId":"11037","inReplyTo":"20071203021333.GC8322@coredump.intra.peff.net","subject":"Re: [PATCH v4] Allow update hooks to update refs on their own.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-03T02:16:22Z","receivedAt":"2007-12-03T02:16:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> ..., but an\n> \"ok, but btw I changed your commit\" status from receive-pack seems like\n> it would be useful, for two reasons:\n>\n> - it can be displayed differently, so the user is reminded to do a\n> fetch afterwards\n> - we can avoid updating the tracking ref, which makes it less likely\n> to result in a non-fast forward fetch next time. For example,\n> consider:\n>\n> 1. The remote master and my origin/master are at A.\n> 2. I make a commit B on top of A.\n> 3. I push B to remote, who rewrites it to B' on top of A. At the\n> same time, I move my origin/master to B.\n> 4. I fetch, and get non-ff going from B to B'.\n>\n> If I had never written anything to my origin/master, it would be a\n> fast forward. And obviously git handles it just fine, but it is more\n> useful to the user during the next fetch to see A..B rather than\n> B'...B.\n\nSensible argument. I stand corrected.\n"},{"id":"61716","messageId":"7vr6i4qw4s.fsf@gitster.siamese.dyndns.org","threadId":"11037","inReplyTo":"7vlk8csetl.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v4] Allow update hooks to update refs on their own.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-03T03:45:23Z","receivedAt":"2007-12-03T03:45:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jeff King writes:\n>\n>> ..., but an\n>> \"ok, but btw I changed your commit\" status from receive-pack seems like\n>> it would be useful, for two reasons:\n>>\n>> - it can be displayed differently, so the user is reminded to do a\n>> fetch afterwards\n>> - we can avoid updating the tracking ref, which makes it less likely\n>> to result in a non-fast forward fetch next time. For example,\n>> consider:\n>>\n>> 1. The remote master and my origin/master are at A.\n>> 2. I make a commit B on top of A.\n>> 3. I push B to remote, who rewrites it to B' on top of A. At the\n>> same time, I move my origin/master to B.\n>> 4. I fetch, and get non-ff going from B to B'.\n>>\n>> If I had never written anything to my origin/master, it would be a\n>> fast forward. And obviously git handles it just fine, but it is more\n>> useful to the user during the next fetch to see A..B rather than\n>> B'...B.\n>\n> Sensible argument. I stand corrected.\n\nHaving said that, I think the workflow that this \"letting update hook\nmunge\" patch supports has a bit more implications for the people who\ninteract with it. The pusher will need to force fetch the result, but\nafter that he needs to discard his own commit and replace it with\nwhatever the hook did. The question is how much to discard, and how to\nreconstruct the changes since the last push that was made on top of the\ncommit that was pushed.\n\nMaybe the push pushed out a string of five pearls and the hook may have\nrewritten only the tip, or all of them. You may have built a few\ncommits on top since then. If what you pushed out contained merges and\nthe hook rewrote it, you would need to potentially replay such a merge\nthat was rewritten by the hook.\n\nThis is all the same with a workflow that deals with a branch that is\nadvertised to constantly rewound and rebased, and the common approach is\nto use \"fetch + rebase\" (see recent discussion between Nico and Bruce).\nSo this patch is not creating a new problem (iow, I am not mentioning\nthis as the reason to reject this patch), but I thought I should bring\nit up so that people know what they are doing.\n"},{"id":"61717","messageId":"20071203040108.GS14735@spearce.org","threadId":"11037","inReplyTo":"20071202212224.GA22117@midwinter.com","subject":"Re: [PATCH v4] Allow update hooks to update refs on their own.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-12-03T04:01:09Z","receivedAt":"2007-12-03T04:01:09Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Steven Grimm wrote:\n> This is useful in cases where a hook needs to modify an incoming commit\n> in some way, e.g., fixing whitespace errors, adding an annotation to\n> the commit message, noting the location of output from a profiling tool,\n> or committing to an svn repository using git-svn.\n...\n> +/* Update hook exit code: hook has updated ref on its own */\n> +#define EXIT_CODE_REF_UPDATED 100\n\nHmm. I would actually rather move the ref locking to before we run\nthe update hook, so the ref is locked *while* the hook executes.\nIf we ran a hook and it exited 0 then re-read the ref to see if the\nvalue differs from old_sha1; if it does then we can assume the update\nhook took care of the update. If the value is still old_sha1 then we\nknow the hook didn't do the update and we need to do it for the hook.\n\nThis probably requires exporting the name of the ref we currently\nhave locked in an environment variable (and teach lockfile.c it)\nso we can effectively do recursive locking. That way the update\nhook can still use git-update-ref to change the ref safely.\n\nThe advantage of this approach is the hook programmer doesn't need\nto implement their own locking scheme and we also don't make a\nspecial case exit code where there wasn't one before.\n\n-- \nShawn.\n"},{"id":"61724","messageId":"7vmyssqrhk.fsf@gitster.siamese.dyndns.org","threadId":"11037","inReplyTo":"20071203040108.GS14735@spearce.org","subject":"Re: [PATCH v4] Allow update hooks to update refs on their own.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-03T05:25:43Z","receivedAt":"2007-12-03T05:25:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> This probably requires exporting the name of the ref we currently\n> have locked in an environment variable (and teach lockfile.c it)\n> so we can effectively do recursive locking. That way the update\n> hook can still use git-update-ref to change the ref safely.\n\nHeh, I like that, although I suspect getting this right would mean the\ntopic should be post 1.5.4 (which I do not mind). \n"},{"id":"61775","messageId":"Pine.LNX.4.64.0712031146520.27959@racer.site","threadId":"11037","inReplyTo":"20071203040108.GS14735@spearce.org","subject":"Re: [PATCH v4] Allow update hooks to update refs on their own.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-12-03T11:47:19Z","receivedAt":"2007-12-03T11:47:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 2 Dec 2007, Shawn O. Pearce wrote:\n\n> Steven Grimm wrote:\n> > This is useful in cases where a hook needs to modify an incoming commit\n> > in some way, e.g., fixing whitespace errors, adding an annotation to\n> > the commit message, noting the location of output from a profiling tool,\n> > or committing to an svn repository using git-svn.\n> ...\n> > +/* Update hook exit code: hook has updated ref on its own */\n> > +#define EXIT_CODE_REF_UPDATED 100\n> \n> Hmm. I would actually rather move the ref locking to before we run\n> the update hook, so the ref is locked *while* the hook executes.\n\nWould that not mean that you cannot use update-ref to update the ref, \nsince that wants to use the same lock?\n\nCiao,\nDscho\n"},{"id":"61851","messageId":"20071204015108.GV14735@spearce.org","threadId":"11037","inReplyTo":"Pine.LNX.4.64.0712031146520.27959@racer.site","subject":"Re: [PATCH v4] Allow update hooks to update refs on their own.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-12-04T01:51:08Z","receivedAt":"2007-12-04T01:51:08Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Schindelin wrote:\n> On Sun, 2 Dec 2007, Shawn O. Pearce wrote:\n> \n> > Steven Grimm wrote:\n> > > This is useful in cases where a hook needs to modify an incoming commit\n> > > in some way, e.g., fixing whitespace errors, adding an annotation to\n> > > the commit message, noting the location of output from a profiling tool,\n> > > or committing to an svn repository using git-svn.\n> > ...\n> > > +/* Update hook exit code: hook has updated ref on its own */\n> > > +#define EXIT_CODE_REF_UPDATED 100\n> > \n> > Hmm. I would actually rather move the ref locking to before we run\n> > the update hook, so the ref is locked *while* the hook executes.\n> \n> Would that not mean that you cannot use update-ref to update the ref, \n> since that wants to use the same lock?\n\nYou failed to quote the part of my email where I talked about how\nwe set an evironment variable to pass a hint to lockfile.c running\nwithin the git-update-ref subprocess to instruct it to perform a\ndifferent style of locking, one that would work as a \"recursive\"\nlock.\n\nSuch a recursive lock could be useful for a whole lot more than just\nthe update hook. But it would at least allow the update hook to\nuse git-update-ref to safely change the ref, without receive-pack\nlosing its own lock on the ref.\n\n-- \nShawn.\n"},{"id":"61852","messageId":"20071204015511.GX14735@spearce.org","threadId":"11037","inReplyTo":"7vmyssqrhk.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v4] Allow update hooks to update refs on their own.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-12-04T01:55:11Z","receivedAt":"2007-12-04T01:55:11Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> \"Shawn O. Pearce\" writes:\n> \n> > This probably requires exporting the name of the ref we currently\n> > have locked in an environment variable (and teach lockfile.c it)\n> > so we can effectively do recursive locking. That way the update\n> > hook can still use git-update-ref to change the ref safely.\n> \n> Heh, I like that, although I suspect getting this right would mean the\n> topic should be post 1.5.4 (which I do not mind). \n\nYea, most likely.\n\nAlso I won't have any time in the near future to work on this\nimplementation myself. I threw the idea out there in case someone\nelse can find the time. I'm willing to do the work myself as I think\nits the right approach to use here for this update hook change, but\nI just don't see myself getting to it anytime before say Christmas...\n\nIts slightly more involved then what Steven originally proposed,\nbut I think it solves the problem better and gives us more room\nfor future improvements where we may want/need something such as\na recursive ref locking.\n\n-- \nShawn.\n"},{"id":"61855","messageId":"Pine.LNX.4.64.0712040211270.27959@racer.site","threadId":"11037","inReplyTo":"20071204015108.GV14735@spearce.org","subject":"Re: [PATCH v4] Allow update hooks to update refs on their own.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-12-04T02:12:05Z","receivedAt":"2007-12-04T02:12:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 3 Dec 2007, Shawn O. Pearce wrote:\n\n> Johannes Schindelin wrote:\n> > On Sun, 2 Dec 2007, Shawn O. Pearce wrote:\n> > \n> > > Steven Grimm wrote:\n> > > > This is useful in cases where a hook needs to modify an incoming commit\n> > > > in some way, e.g., fixing whitespace errors, adding an annotation to\n> > > > the commit message, noting the location of output from a profiling tool,\n> > > > or committing to an svn repository using git-svn.\n> > > ...\n> > > > +/* Update hook exit code: hook has updated ref on its own */\n> > > > +#define EXIT_CODE_REF_UPDATED 100\n> > > \n> > > Hmm. I would actually rather move the ref locking to before we run\n> > > the update hook, so the ref is locked *while* the hook executes.\n> > \n> > Would that not mean that you cannot use update-ref to update the ref, \n> > since that wants to use the same lock?\n> \n> You failed to quote the part of my email where I talked about how\n> we set an evironment variable to pass a hint to lockfile.c running\n> within the git-update-ref subprocess to instruct it to perform a\n> different style of locking, one that would work as a \"recursive\"\n> lock.\n> \n> Such a recursive lock could be useful for a whole lot more than just\n> the update hook. But it would at least allow the update hook to\n> use git-update-ref to safely change the ref, without receive-pack\n> losing its own lock on the ref.\n\nIndeed, I even failed to read it fully ;-)\n\nWhat do you propose, though? .lock.?\n\nCiao,\nDscho\n"},{"id":"61857","messageId":"20071204022020.GA14735@spearce.org","threadId":"11037","inReplyTo":"Pine.LNX.4.64.0712040211270.27959@racer.site","subject":"Re: [PATCH v4] Allow update hooks to update refs on their own.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-12-04T02:20:20Z","receivedAt":"2007-12-04T02:20:20Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Schindelin wrote:\n> On Mon, 3 Dec 2007, Shawn O. Pearce wrote:\n> > You failed to quote the part of my email where I talked about how\n> > we set an evironment variable to pass a hint to lockfile.c running\n> > within the git-update-ref subprocess to instruct it to perform a\n> > different style of locking, one that would work as a \"recursive\"\n> > lock.\n> > \n> > Such a recursive lock could be useful for a whole lot more than just\n> > the update hook. But it would at least allow the update hook to\n> > use git-update-ref to safely change the ref, without receive-pack\n> > losing its own lock on the ref.\n> \n> Indeed, I even failed to read it fully ;-)\n> \n> What do you propose, though? .lock.?\n\nSure. :-)\n\nI was also hand-waving. Hoping someone else would fill in the\nmagic details.\n\nActually wouldn't be so bad. We could do something like:\n\n\tGIT_INHERITED_LOCKS=\" ...\"\n\nwhere is a ref name (which cannot contain spaces, even though\nsome people seem to forget that rule) and is the number\nof times it has been locked already. of 0 is the current\n\".lock\" file. So the first lock taken out by receive-pack would\nbe setting:\n\n\tGIT_INHERITED_LOCKS=\"refs/heads/master 0\"\n\nand another lock on the same ref by a subprocess would then update\nit to:\n\n\tGIT_INHERITED_LOCKS=\"refs/heads/master 1\"\n\netc...\n\n\n\n-- \nShawn.\n"},{"id":"61859","messageId":"Pine.LNX.4.64.0712040224080.27959@racer.site","threadId":"11037","inReplyTo":"20071204022020.GA14735@spearce.org","subject":"Re: [PATCH v4] Allow update hooks to update refs on their own.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-12-04T02:25:03Z","receivedAt":"2007-12-04T02:25:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 3 Dec 2007, Shawn O. Pearce wrote:\n\n> Johannes Schindelin wrote:\n> > On Mon, 3 Dec 2007, Shawn O. Pearce wrote:\n> > > You failed to quote the part of my email where I talked about how\n> > > we set an evironment variable to pass a hint to lockfile.c running\n> > > within the git-update-ref subprocess to instruct it to perform a\n> > > different style of locking, one that would work as a \"recursive\"\n> > > lock.\n> > > \n> > > Such a recursive lock could be useful for a whole lot more than just\n> > > the update hook. But it would at least allow the update hook to\n> > > use git-update-ref to safely change the ref, without receive-pack\n> > > losing its own lock on the ref.\n> > \n> > Indeed, I even failed to read it fully ;-)\n> > \n> > What do you propose, though? .lock.?\n> \n> Sure. :-)\n> \n> I was also hand-waving. Hoping someone else would fill in the\n> magic details.\n> \n> Actually wouldn't be so bad. We could do something like:\n> \n> \tGIT_INHERITED_LOCKS=\" ...\"\n\nI am somewhat wary of using environment variables in that context, since \nthe variables could leak to subprocesses, or (even worse), they could be \nset inadvertently by the user or other scripts.\n\nCiao,\nDscho\n"},{"id":"61860","messageId":"21AA9521-0FED-471F-AFBC-B5CEF4B4A5E9@midwinter.com","threadId":"11037","inReplyTo":"Pine.LNX.4.64.0712040224080.27959@racer.site","subject":"Re: [PATCH v4] Allow update hooks to update refs on their own.","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-12-04T02:33:12Z","receivedAt":"2007-12-04T02:33:12Z","isPatch":true,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"On Dec 3, 2007, at 6:25 PM, Johannes Schindelin wrote:\n> I am somewhat wary of using environment variables in that context, \n> since\n> the variables could leak to subprocesses, or (even worse), they \n> could be\n> set inadvertently by the user or other scripts.\n\nAgreed on the inadvertent setting, but isn't leaking to subprocesses \nthe whole point of the exercise here?\n\n-Steve\n"},{"id":"61862","messageId":"20071204023440.GD14735@spearce.org","threadId":"11037","inReplyTo":"Pine.LNX.4.64.0712040224080.27959@racer.site","subject":"Re: [PATCH v4] Allow update hooks to update refs on their own.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-12-04T02:34:40Z","receivedAt":"2007-12-04T02:34:40Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Schindelin wrote:\n> On Mon, 3 Dec 2007, Shawn O. Pearce wrote:\n> > Actually wouldn't be so bad. We could do something like:\n> > \n> > \tGIT_INHERITED_LOCKS=\" ...\"\n> \n> I am somewhat wary of using environment variables in that context, since \n> the variables could leak to subprocesses, or (even worse), they could be \n> set inadvertently by the user or other scripts.\n\nSure. But as bad as it is, its still more secure than the\n\"repository of record\" that my day-job uses for its source code\ntree (no, it doesn't use Git, and I wish it was as good as Visual\nSource Suck). \n\nI'd suggest also using something like getppid() to check the pid\nagainst a pid in the env, and *gasp* maybe do a SHA-1 hash in there\nor something to make it challening enough to fake that the average\nuser won't set it unless they really understand what they are doing.\n\n-- \nShawn.\n"},{"id":"62065","messageId":"5920F34B-A94B-4C24-A95B-D35F35A4F0C0@midwinter.com","threadId":"11037","inReplyTo":"7vlk8csetl.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v4] Allow update hooks to update refs on their own.","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-12-05T22:14:17Z","receivedAt":"2007-12-05T22:14:17Z","isPatch":true,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"On Dec 2, 2007, at 6:16 PM, Junio C Hamano wrote:\n>> ..., but an\n>> \"ok, but btw I changed your commit\" status from receive-pack seems \n>> like\n>> it would be useful, for two reasons:\n> Sensible argument. I stand corrected.\n\nIf we want that status in principle, I'd argue that sending down the \nupdated commit SHA1 is actually the right way to indicate it, because \nit gives the client all the information it needs to make an \nintelligent choice about what to do next. If you don't transmit the \nmodified SHA1, the client will have to do another fetch to find out \nwhat rewriting was done by the server, and if another push happened in \nthe meantime, the client will have to basically guess about which \ncommits correspond to the ones it pushed.\n\nI'm going to have to modify the \"ok\" line for this either way, and \nit's not like the extra 39 bytes (for sending a hex SHA1 instead of a \none-character status indicator) is going to hurt in any but the \nnarrowest corner cases.\n\nBut if people really object to that, I will add a simple flag to the \n\"ok\" line.\n\n-Steve\n"},{"id":"62066","messageId":"7vhciwn5rl.fsf@gitster.siamese.dyndns.org","threadId":"11037","inReplyTo":"5920F34B-A94B-4C24-A95B-D35F35A4F0C0@midwinter.com","subject":"Re: [PATCH v4] Allow update hooks to update refs on their own.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-05T22:19:58Z","receivedAt":"2007-12-05T22:19:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steven Grimm writes:\n\n> On Dec 2, 2007, at 6:16 PM, Junio C Hamano wrote:\n>>> ..., but an\n>>> \"ok, but btw I changed your commit\" status from receive-pack seems\n>>> like\n>>> it would be useful, for two reasons:\n>> Sensible argument. I stand corrected.\n>\n> If we want that status in principle, I'd argue that sending down the\n> updated commit SHA1 is actually the right way to indicate it, because\n> it gives the client all the information it needs to make an\n> intelligent choice about what to do next. If you don't transmit the\n> modified SHA1, the client will have to do another fetch to find out\n> what rewriting was done by the server, and if another push happened in\n> the meantime, the client will have to basically guess about which\n> commits correspond to the ones it pushed.\n\nOk, but the output from fetch is meant to be human readable and we do\nnot promise parsability, so if we go this route (which I think you made\na sensible argument for) we would need a hook on the pushing end to act\non this (perhaps record the correspondence of pushed and rewritten sha1\nsomewhere for the hook's own use).\n"},{"id":"62068","messageId":"7v8x48n5c0.fsf@gitster.siamese.dyndns.org","threadId":"11037","inReplyTo":"7vhciwn5rl.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v4] Allow update hooks to update refs on their own.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-05T22:29:19Z","receivedAt":"2007-12-05T22:29:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Ok, but the output from fetch is meant to be human readable and we do\n> not promise parsability, so if we go this route (which I think you made\n\ns/parsability/machine &/;\n\n> a sensible argument for) we would need a hook on the pushing end to act\n> on this (perhaps record the correspondence of pushed and rewritten sha1\n> somewhere for the hook's own use).\n\ns/on this/& information/;\ns/own use/& in machine readable way/;\n"},{"id":"62109","messageId":"20071206055723.GB23309@coredump.intra.peff.net","threadId":"11037","inReplyTo":"7vhciwn5rl.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v4] Allow update hooks to update refs on their own.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-12-06T05:57:23Z","receivedAt":"2007-12-06T05:57:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 05, 2007 at 02:19:58PM -0800, Junio C Hamano wrote:\n\n> > what rewriting was done by the server, and if another push happened in\n> > the meantime, the client will have to basically guess about which\n> > commits correspond to the ones it pushed.\n> \n> Ok, but the output from fetch is meant to be human readable and we do\n> not promise parsability, so if we go this route (which I think you made\n> a sensible argument for) we would need a hook on the pushing end to act\n> on this (perhaps record the correspondence of pushed and rewritten sha1\n> somewhere for the hook's own use).\n\nI am not clear on what you mean. Are you saying that the send-pack code\nshould _not_ recognize the \"ok, but I rewrote your commit\" status?\nBecause that is how we will avoid updating the tracking ref, which I\nthink is a good goal.\n\nOr are you saying \"it's ok to understand the 'ok, but...' response and\nnot update the tracking ref, but pulling the new hash from the message\nis up to a hook on the pushing side\"? Which I think it reasonable.\n\nOr alternatively, \"there should be a hook on the pushing side which is\nallowed to set the ref status to 'ok, but don't bother updating the\ntracking ref' or 'ok, but here is the actual thing to put in the\ntracking ref'\"? Which is also fine by me.\n\n-Peff\n"},{"id":"62115","messageId":"7veje0gwru.fsf@gitster.siamese.dyndns.org","threadId":"11037","inReplyTo":"20071206055723.GB23309@coredump.intra.peff.net","subject":"Re: [PATCH v4] Allow update hooks to update refs on their own.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-06T06:30:45Z","receivedAt":"2007-12-06T06:30:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Dec 05, 2007 at 02:19:58PM -0800, Junio C Hamano wrote:\n>\n>> > what rewriting was done by the server, and if another push happened in\n>> > the meantime, the client will have to basically guess about which\n>> > commits correspond to the ones it pushed.\n>> \n>> Ok, but the output from fetch is meant to be human readable and we do\n>> not promise parsability, so if we go this route (which I think you made\n>> a sensible argument for) we would need a hook on the pushing end to act\n>> on this (perhaps record the correspondence of pushed and rewritten sha1\n>> somewhere for the hook's own use).\n>\n> I am not clear on what you mean. Are you saying that the send-pack code\n> should _not_ recognize the \"ok, but I rewrote your commit\" status?\n> Because that is how we will avoid updating the tracking ref, which I\n> think is a good goal.\n>\n> Or are you saying \"it's ok to understand the 'ok, but...' response and\n> not update the tracking ref, but pulling the new hash from the message\n> is up to a hook on the pushing side\"? Which I think it reasonable.\n>\n> Or alternatively, \"there should be a hook on the pushing side which is\n> allowed to set the ref status to 'ok, but don't bother updating the\n> tracking ref' or 'ok, but here is the actual thing to put in the\n> tracking ref'\"? Which is also fine by me.\n\nWhat I meant in response to what I thought Steven was talking about was\nthis.\n\n * With Steven's patch, the sending side needs to expect what it pushes\n to be rewritten. If it starts with this history:\n\n ---o---o---o---Y---o---o---X\n\n where Y is what its remote tracking branch points at for the\n corresponding branch, three commits were built locally since the last\n fetch, and the sender pushes X.\n\n * Then the receiving end rewrites the history, making the history into\n this:\n\n\t\t o'--o'--X'\n /\n ---o---o---o---Y---o---o---X\n\n * Before the next fetch, the sending side can continue building on top\n of X, leading to this:\n\n\t\t o'--o'--X'\n /\n ---o---o---o---Y---o---o---X---o---Z\n\n * Similarly other people push into the same remote, get their commits\n rewritten and remote side's history becomes like this (but the\n original sender does not know about the upper history at all yet).\n\n\t\t o'--o'--X'--o'--o'--W'\n /\n ---o---o---o---Y---o---o---X---o---Z\n\n * Then the original sender fetches from the remote, now the tracking\n branch points at W' (it previously pointed at Y). You would want to\n rebase your work since the last push on top of that tracking branch.\n\nThe rebase would be \"rebase --onto W' X Z\", so it is not strictly\nnecessary to keep the fact that X corresponds to X', but somehow I\nthought it was necessary, and Steven's message was hinting about that:\n\n > If we want that status in principle, I'd argue that sending down the\n > updated commit SHA1 is actually the right way to indicate it, because\n > it gives the client all the information it needs to make an\n > intelligent choice about what to do next. If you don't transmit the\n > modified SHA1, the client will have to do another fetch to find out\n > what rewriting was done by the server, and if another push happened in\n > the meantime, the client will have to basically guess about which\n > commits correspond to the ones it pushed.\n\n(notice the last part).\n\nSo if we want to transmit minimum amount of information, we can just\nsend a bit (\"the ref was rewritten\") back to send-pack without telling\nit what X' is (but it would not hurt to send it back either). With that\none bit of information, send-pack can refrain from updating tracking ref\nfrom Y to X.\n\nIn the above scenario I illustrated, it turns out that getting\ncorrespondence between X and X' is not strictly necessary to perform a\nrebase later, but maybe there is some other scenario that keeping track\nof that information would be helpful. In such a case, a hook on the\nsend-pack end (which currently we do not have) can be called with X and\nX' as parameters (and perhaps the name of the ref and the corresponding\ntracking ref) to do whatever it wants to do with that information.\n\nEven if we do not send X' back but just one bit, having that hook would\nprobably be needed so that sender can record \"I've pushed up to X\" and\nperhaps \"now I cannot push out Z until I rebase\" after receiving \"push\nwas accepted but rewritten\" bit.\n\nThis is all handwaving --- I suspect for this to really work, send-pack\nmight need a pre-send-pack hook that pays attention to such \"now I\ncannot push out Z until I rebase\" information the previous round of push\nmay have left and declines to push. Of course, the receiving end would\nwould probably refuse such a push because it is not a fast-forward.\n"},{"id":"62117","messageId":"20071206063626.GA18698@coredump.intra.peff.net","threadId":"11037","inReplyTo":"7veje0gwru.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v4] Allow update hooks to update refs on their own.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-12-06T06:36:26Z","receivedAt":"2007-12-06T06:36:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 05, 2007 at 10:30:45PM -0800, Junio C Hamano wrote:\n\n> The rebase would be \"rebase --onto W' X Z\", so it is not strictly\n> necessary to keep the fact that X corresponds to X', but somehow I\n> thought it was necessary, and Steven's message was hinting about that:\n> \n> > If we want that status in principle, I'd argue that sending down the\n> > updated commit SHA1 is actually the right way to indicate it, because\n> > it gives the client all the information it needs to make an\n> > intelligent choice about what to do next. If you don't transmit the\n> > modified SHA1, the client will have to do another fetch to find out\n> > what rewriting was done by the server, and if another push happened in\n> > the meantime, the client will have to basically guess about which\n> > commits correspond to the ones it pushed.\n> \n> (notice the last part).\n> \n> So if we want to transmit minimum amount of information, we can just\n> send a bit (\"the ref was rewritten\") back to send-pack without telling\n> it what X' is (but it would not hurt to send it back either). With that\n> one bit of information, send-pack can refrain from updating tracking ref\n> from Y to X.\n\nAh, I thought his argument was \"we have to send back a bit, so why not\njust send the hash we made for informational purposes? It doesn't hurt,\nand maybe we can make use of it later.\"\n\nI was assuming that we were interested _only_ in fixing the send-pack\nissues at this time, and that the rebasing or merging part of the\nworkflow would be figured out later. But it is probably sensible to\nconsider the whole workflow to see what is necessary at each step.\n\n-Peff\n"},{"id":"62128","messageId":"602F3FCF-C761-4E0A-9854-723F9F8F96F7@midwinter.com","threadId":"11037","inReplyTo":"20071206063626.GA18698@coredump.intra.peff.net","subject":"Re: [PATCH v4] Allow update hooks to update refs on their own.","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-12-06T07:50:53Z","receivedAt":"2007-12-06T07:50:53Z","isPatch":true,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"On Dec 5, 2007, at 10:36 PM, Jeff King wrote:\n> Ah, I thought his argument was \"we have to send back a bit, so why not\n> just send the hash we made for informational purposes? It doesn't \n> hurt,\n> and maybe we can make use of it later.\"\n\nYeah, that was more or less my thinking. Keep it simple for now, but \nit seems like that information is bound to be useful at some point. In \nparticular, if you don't send it down, it's really difficult to \nunambiguously get back after the fact (given that a fetch might \ncontain subsequent revisions unrelated to yours.)\n\nMy v3 patch (which I will combine with a modified form of the \ndocumentation update now that it sounds like transmitting the SHA1 \nisn't objectionable) actually sent it down twice: once in the protocol \nmessage and once in the human-readable push status report.\n\n-Steve\n"}]}