{"thread":{"id":"10350","subject":"bug: origin refs updated too soon locally","startedAt":"2007-10-18T01:35:53Z","lastAt":"2007-10-18T06:59:28Z","messageCount":8,"participants":["Perry Wagle","Shawn O. Pearce","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"56319","messageId":"8CEF6150-4BE7-4B4D-B58C-12CE4671007E@cs.indiana.edu","threadId":"10350","inReplyTo":null,"subject":"bug: origin refs updated too soon locally","fromName":"Perry Wagle","fromEmail":"wagle@cs.indiana.edu","sentAt":"2007-10-18T01:35:53Z","receivedAt":"2007-10-18T01:35:53Z","isPatch":false,"sender":{"key":"wagle@cs.indiana.edu","avatar":null},"body":"If I clone a remote repository, make a few commits, push them to the  \nremote repository, and the update hook on the remote repository  \nrejects them (exit 1), the local origin refs are still updated as if  \nthe push had gone through.  The workaround is to do a pull to set the  \norigin refs back.\n\n-- Perry\n"},{"id":"56320","messageId":"8E28A8CC-DC3B-4C97-8B14-742DAA8D3CE2@cs.indiana.edu","threadId":"10350","inReplyTo":"8CEF6150-4BE7-4B4D-B58C-12CE4671007E@cs.indiana.edu","subject":"Re: bug: origin refs updated too soon locally","fromName":"Perry Wagle","fromEmail":"wagle@cs.indiana.edu","sentAt":"2007-10-18T01:43:49Z","receivedAt":"2007-10-18T01:43:49Z","isPatch":false,"sender":{"key":"wagle@cs.indiana.edu","avatar":null},"body":"I take it back.  A git-pull is not a workaround if the ref moved on  \nthe remote end.\n\n-- Perry\n\n\nOn Oct 17, 2007, at 6:35 PM, Perry Wagle wrote:\n\n> If I clone a remote repository, make a few commits, push them to  \n> the remote repository, and the update hook on the remote repository  \n> rejects them (exit 1), the local origin refs are still updated as  \n> if the push had gone through.  The workaround is to do a pull to  \n> set the origin refs back.\n>\n> -- Perry\n> -\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"56338","messageId":"20071018045358.GB14735@spearce.org","threadId":"10350","inReplyTo":"8CEF6150-4BE7-4B4D-B58C-12CE4671007E@cs.indiana.edu","subject":"Re: bug: origin refs updated too soon locally","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-10-18T04:53:58Z","receivedAt":"2007-10-18T04:53:58Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Perry Wagle <wagle@cs.indiana.edu> wrote:\n> If I clone a remote repository, make a few commits, push them to the  \n> remote repository, and the update hook on the remote repository  \n> rejects them (exit 1), the local origin refs are still updated as if  \n> the push had gone through.  The workaround is to do a pull to set the  \n> origin refs back.\n\nHeh.  Yes, that's a known bug.  Someone should really fix it.\nThe problem is we are updating the local tracking ref before we\nactually get confirmation from the remote side that the remote side\nhas accepted (or rejected) that update request.\n\nThis is probably easier to do after the db/fetch-pack topic is\nmerged as the improvements there might make this easier.  But I\ncould be wrong.  Be nice if someone proved me wrong by writing up\na patch for git-send-pack.\n\nFor the time being the best way to recover from this is to use\ngit-fetch rather than git-pull.  Recall that git-pull is defined as\n\"fetch then merge\".  You really just need to refetch the tracking\nbranches again, so your tracking branches have the same value as\nthe remote side.\n\n-- \nShawn.\n"},{"id":"56348","messageId":"20071018052809.GA11938@coredump.intra.peff.net","threadId":"10350","inReplyTo":"20071018045358.GB14735@spearce.org","subject":"Re: bug: origin refs updated too soon locally","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-10-18T05:28:09Z","receivedAt":"2007-10-18T05:28:09Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 18, 2007 at 12:53:58AM -0400, Shawn O. Pearce wrote:\n\n> This is probably easier to do after the db/fetch-pack topic is\n> merged as the improvements there might make this easier.  But I\n> could be wrong.  Be nice if someone proved me wrong by writing up\n> a patch for git-send-pack.\n\nIt doesn't look too bad...patch series in a few minutes.\n\n-Peff\n"},{"id":"56359","messageId":"20071018061746.GA29531@coredump.intra.peff.net","threadId":"10350","inReplyTo":"20071018045358.GB14735@spearce.org","subject":"[PATCH] t5516: test update of local refs on push","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-10-18T06:17:46Z","receivedAt":"2007-10-18T06:17:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The first test (updating local refs) should succeed, but the\nsecond one (not updating on error) currently fails.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t5516-fetch-push.sh |   28 ++++++++++++++++++++++++++++\n 1 files changed, 28 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex ca46aaf..dd329d7 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -244,4 +244,32 @@ test_expect_success 'push with colon-less refspec (4)' '\n \n '\n \n+test_expect_success 'push updates local refs' '\n+\n+\trm -rf parent child &&\n+\tmkdir parent && cd parent && git init &&\n+\t\techo one >foo && git add foo && git commit -m one &&\n+\tcd .. &&\n+\tgit clone parent child && cd child &&\n+\t\techo two >foo && git commit -a -m two &&\n+\t\tgit push &&\n+\ttest $(git rev-parse master) = $(git rev-parse remotes/origin/master)\n+\n+'\n+\n+test_expect_success 'push does not update local refs on failure' '\n+\n+\trm -rf parent child &&\n+\tmkdir parent && cd parent && git init &&\n+\t\techo one >foo && git add foo && git commit -m one &&\n+\t\techo exit 1 >.git/hooks/pre-receive &&\n+\t\tchmod +x .git/hooks/pre-receive &&\n+\tcd .. &&\n+\tgit clone parent child && cd child &&\n+\t\techo two >foo && git commit -a -m two || exit 1\n+\t\tgit push && exit 1\n+\ttest $(git rev-parse master) != $(git rev-parse remotes/origin/master)\n+\n+'\n+\n test_done\n-- \n1.5.3.4.1162.gc3e8e-dirty\n"},{"id":"56360","messageId":"20071018061915.GB29531@coredump.intra.peff.net","threadId":"10350","inReplyTo":"20071018045358.GB14735@spearce.org","subject":"[PATCH 2/2] send-pack: don't update tracking refs on error","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-10-18T06:19:15Z","receivedAt":"2007-10-18T06:19:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Previously, we updated the tracking refs (which match refs we\nare pushing) while generating the list of refs to send.\nHowever, at that point we don't know whether the refs were\naccepted.\n\nInstead, we now wait until we get a response code from the\nserver. If an error was indicated, we don't update any local\ntracking refs. Technically some refs could have been updated\non the remote, but since the local ref update is just an\noptimization to avoid an extra fetch, we are better off\nerring on the side of correctness.\n\nThe user-visible message is now generated much later in the\nprogram, and has been tweaked to make more sense.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n send-pack.c |   50 ++++++++++++++++++++++++++++++++++----------------\n 1 files changed, 34 insertions(+), 16 deletions(-)\n\ndiff --git a/send-pack.c b/send-pack.c\nindex f74e66a..25d5c25 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -177,6 +177,35 @@ static int receive_status(int in)\n \treturn ret;\n }\n \n+static void update_tracking_ref(struct remote *remote, struct ref *ref)\n+{\n+\tstruct refspec rs;\n+\tint will_delete_ref;\n+\n+\trs.src = ref->name;\n+\trs.dst = NULL;\n+\n+\tif (!ref->peer_ref)\n+\t\treturn;\n+\n+\twill_delete_ref = is_null_sha1(ref->peer_ref->new_sha1);\n+\n+\tif (!will_delete_ref &&\n+\t\t\t!hashcmp(ref->old_sha1, ref->peer_ref->new_sha1))\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\tfree(rs.dst);\n+\t}\n+}\n+\n static int send_pack(int in, int out, struct remote *remote, int nr_refspec, char **refspec)\n {\n \tstruct ref *ref;\n@@ -302,22 +331,6 @@ static int send_pack(int in, int out, struct remote *remote, int nr_refspec, cha\n \t\t\tfprintf(stderr, \"\\n  from %s\\n  to   %s\\n\",\n \t\t\t\told_hex, new_hex);\n \t\t}\n-\t\tif (remote) {\n-\t\t\tstruct refspec rs;\n-\t\t\trs.src = ref->name;\n-\t\t\trs.dst = NULL;\n-\t\t\tif (!remote_find_tracking(remote, &rs)) {\n-\t\t\t\tfprintf(stderr, \" Also local %s\\n\", rs.dst);\n-\t\t\t\tif (will_delete_ref) {\n-\t\t\t\t\tif (delete_ref(rs.dst, NULL)) {\n-\t\t\t\t\t\terror(\"Failed to delete\");\n-\t\t\t\t\t}\n-\t\t\t\t} else\n-\t\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\tfree(rs.dst);\n-\t\t\t}\n-\t\t}\n \t}\n \n \tpacket_flush(out);\n@@ -330,6 +343,11 @@ static int send_pack(int in, int out, struct remote *remote, int nr_refspec, cha\n \t\t\tret = -4;\n \t}\n \n+\tif (remote && ret == 0) {\n+\t\tfor (ref = remote_refs; ref; ref = ref->next)\n+\t\t\tupdate_tracking_ref(remote, ref);\n+\t}\n+\n \tif (!new_refs && ret == 0)\n \t\tfprintf(stderr, \"Everything up-to-date\\n\");\n \treturn ret;\n-- \n1.5.3.4.1162.gc3e8e-dirty\n"},{"id":"56361","messageId":"20071018062136.GB11938@coredump.intra.peff.net","threadId":"10350","inReplyTo":"20071018061746.GA29531@coredump.intra.peff.net","subject":"Re: [PATCH] t5516: test update of local refs on push","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-10-18T06:21:36Z","receivedAt":"2007-10-18T06:21:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 18, 2007 at 02:17:46AM -0400, Jeff King wrote:\n\n> The first test (updating local refs) should succeed, but the\n> second one (not updating on error) currently fails.\n\nOops, this should of course be labeled as 1/2.\n\nFor the fix, I didn't need anything from 'next', after all, and 2/2 also\nworks fine there (it was almost literally a code move).\n\n-Peff\n"},{"id":"56365","messageId":"20071018065928.GJ14735@spearce.org","threadId":"10350","inReplyTo":"20071018062136.GB11938@coredump.intra.peff.net","subject":"Re: [PATCH] t5516: test update of local refs on push","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-10-18T06:59:28Z","receivedAt":"2007-10-18T06:59:28Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Jeff King <peff@peff.net> wrote:\n> On Thu, Oct 18, 2007 at 02:17:46AM -0400, Jeff King wrote:\n> > The first test (updating local refs) should succeed, but the\n> > second one (not updating on error) currently fails.\n> \n> Oops, this should of course be labeled as 1/2.\n> \n> For the fix, I didn't need anything from 'next', after all, and 2/2 also\n> works fine there (it was almost literally a code move).\n\nYay. I like it when I'm proven wrong.  Especially by a short patch.\n:)\n\nThis will be in next tonight.  Give it a few days, then probably\ngraduate up to master.\n\n-- \nShawn.\n"}]}