{"thread":{"id":"33976","subject":"[PATCH v2 0/3] Early HEAD resolution in push.default = current","startedAt":"2013-05-29T19:21:48Z","lastAt":"2013-05-29T20:04:06Z","messageCount":8,"participants":["Ramkumar Ramachandra","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":3},"messages":[{"id":"218880","messageId":"1369855311-23367-1-git-send-email-artagnon@gmail.com","threadId":"33976","inReplyTo":null,"subject":"[PATCH v2 0/3] Early HEAD resolution in push.default = current","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-29T19:21:48Z","receivedAt":"2013-05-29T19:21:48Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"The commit message in [3/3] is rewritten although I've mentioned the\noriginal motivation at the end.\n\nNo other changes.\n\nRamkumar Ramachandra (3):\n push: factor out the detached HEAD error message\n push: fail early with detached HEAD and current\n push: make push.default = current resolve HEAD early\n\n builtin/push.c | 20 +++++++++++++-------\n 1 file changed, 13 insertions(+), 7 deletions(-)\n\n-- \n1.8.3.12.gbd56588\n"},{"id":"218883","messageId":"1369855311-23367-2-git-send-email-artagnon@gmail.com","threadId":"33976","inReplyTo":"1369855311-23367-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 1/3] push: factor out the detached HEAD error message","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-29T19:21:49Z","receivedAt":"2013-05-29T19:21:49Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"With push.default set to upstream or simple, and a detached HEAD, git\npush prints the following error:\n\n $ git push\n fatal: You are not currently on a branch.\n To push the history leading to the current (detached HEAD)\n state now, use\n\n git push ram HEAD:\n\nThis error is not unique to upstream or simple: current cannot push with\na detached HEAD either. So, factor out the error string in preparation\nfor using it in current.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n builtin/push.c | 14 ++++++++------\n 1 file changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 909c34d..ef3aa97 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -113,17 +113,19 @@ static NORETURN int die_push_simple(struct branch *branch, struct remote *remote\n \t remote->name, branch->name, advice_maybe);\n }\n \n+static const char message_detached_head_die[] =\n+\tN_(\"You are not currently on a branch.\\n\"\n+\t \"To push the history leading to the current (detached HEAD)\\n\"\n+\t \"state now, use\\n\"\n+\t \"\\n\"\n+\t \" git push %s HEAD:\\n\");\n+\n static void setup_push_upstream(struct remote *remote, int simple)\n {\n \tstruct strbuf refspec = STRBUF_INIT;\n \tstruct branch *branch = branch_get(NULL);\n \tif (!branch)\n-\t\tdie(_(\"You are not currently on a branch.\\n\"\n-\t\t \"To push the history leading to the current (detached HEAD)\\n\"\n-\t\t \"state now, use\\n\"\n-\t\t \"\\n\"\n-\t\t \" git push %s HEAD:\\n\"),\n-\t\t remote->name);\n+\t\tdie(_(message_detached_head_die), remote->name);\n \tif (!branch->merge_nr || !branch->merge || !branch->remote_name)\n \t\tdie(_(\"The current branch %s has no upstream branch.\\n\"\n \t\t \"To push the current branch and set the remote as upstream, use\\n\"\n-- \n1.8.3.12.gbd56588\n"},{"id":"218881","messageId":"1369855311-23367-3-git-send-email-artagnon@gmail.com","threadId":"33976","inReplyTo":"1369855311-23367-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 2/3] push: fail early with detached HEAD and current","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-29T19:21:50Z","receivedAt":"2013-05-29T19:21:50Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Setting push.default to current adds the refspec \"HEAD\" for the\ntransport layer to handle. If \"HEAD\" doesn't resolve to a branch (and\nsince no refspec rhs is specified), the push fails after some time with\na cryptic error message:\n\n $ git push\n error: unable to push to unqualified destination: HEAD\n The destination refspec neither matches an existing ref on the remote nor\n begins with refs/, and we are unable to guess a prefix based on the source ref.\n error: failed to push some refs to 'git@github.com:artagnon/git'\n\nFail early with a nicer error message:\n\n $ git push\n fatal: You are not currently on a branch.\n To push the history leading to the current (detached HEAD)\n state now, use\n\n git push ram HEAD:\n\nJust like in the upstream and simple cases.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n builtin/push.c | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex ef3aa97..a79038c 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -175,6 +175,8 @@ static void warn_unspecified_push_default_configuration(void)\n \n static void setup_default_push_refspecs(struct remote *remote)\n {\n+\tstruct branch *branch = branch_get(NULL);\n+\n \tswitch (push_default) {\n \tdefault:\n \tcase PUSH_DEFAULT_UNSPECIFIED:\n@@ -194,6 +196,8 @@ static void setup_default_push_refspecs(struct remote *remote)\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_CURRENT:\n+\t\tif (!branch)\n+\t\t\tdie(_(message_detached_head_die), remote->name);\n \t\tadd_refspec(\"HEAD\");\n \t\tbreak;\n \n-- \n1.8.3.12.gbd56588\n"},{"id":"218882","messageId":"1369855311-23367-4-git-send-email-artagnon@gmail.com","threadId":"33976","inReplyTo":"1369855311-23367-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 3/3] push: make push.default = current use resolved HEAD","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-29T19:21:51Z","receivedAt":"2013-05-29T19:21:51Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"With this change, the output of the push (with push.default set to\ncurrent) changes subtly from:\n\n $ git push\n ...\n * [new branch] HEAD -> push-current-head\n\nto:\n\n $ git push\n ...\n * [new branch] push-current-head -> push-current-head\n\nThis patch was written with a different motivation. There is a problem\nunique to push.default = current:\n\n # on branch push-current-head\n $ git push\n # on another terminal\n $ git checkout master\n # return to the first terminal\n # the push tried to push master!\n\nThis happens because the 'git checkout' on the second terminal races\nwith the 'git push' on the first terminal. Although this patch does not\nsolve the core problem (there is still no guarantee that 'git push' on\nthe first terminal will resolve HEAD before 'git checkout' changes HEAD\non the second), it works in practice.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n builtin/push.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex a79038c..d819487 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -198,7 +198,7 @@ static void setup_default_push_refspecs(struct remote *remote)\n \tcase PUSH_DEFAULT_CURRENT:\n \t\tif (!branch)\n \t\t\tdie(_(message_detached_head_die), remote->name);\n-\t\tadd_refspec(\"HEAD\");\n+\t\tadd_refspec(branch->name);\n \t\tbreak;\n \n \tcase PUSH_DEFAULT_NOTHING:\n-- \n1.8.3.12.gbd56588\n"},{"id":"218886","messageId":"7vppw9shq2.fsf@alter.siamese.dyndns.org","threadId":"33976","inReplyTo":"1369855311-23367-3-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 2/3] push: fail early with detached HEAD and current","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-29T19:34:29Z","receivedAt":"2013-05-29T19:34:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Setting push.default to current adds the refspec \"HEAD\" for the\n> transport layer to handle. If \"HEAD\" doesn't resolve to a branch (and\n> since no refspec rhs is specified), the push fails after some time with\n> a cryptic error message:\n>\n> $ git push\n> error: unable to push to unqualified destination: HEAD\n> The destination refspec neither matches an existing ref on the remote nor\n> begins with refs/, and we are unable to guess a prefix based on the source ref.\n> error: failed to push some refs to 'git@github.com:artagnon/git'\n>\n> Fail early with a nicer error message:\n>\n> $ git push\n> fatal: You are not currently on a branch.\n> To push the history leading to the current (detached HEAD)\n> state now, use\n>\n> git push ram HEAD:\n>\n> Just like in the upstream and simple cases.\n>\n> Signed-off-by: Ramkumar Ramachandra \n> ---\n> builtin/push.c | 4 ++++\n> 1 file changed, 4 insertions(+)\n>\n> diff --git a/builtin/push.c b/builtin/push.c\n> index ef3aa97..a79038c 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -175,6 +175,8 @@ static void warn_unspecified_push_default_configuration(void)\n> \n> static void setup_default_push_refspecs(struct remote *remote)\n> {\n> +\tstruct branch *branch = branch_get(NULL);\n> +\n> \tswitch (push_default) {\n> \tdefault:\n> \tcase PUSH_DEFAULT_UNSPECIFIED:\n> @@ -194,6 +196,8 @@ static void setup_default_push_refspecs(struct remote *remote)\n> \t\tbreak;\n> \n> \tcase PUSH_DEFAULT_CURRENT:\n> +\t\tif (!branch)\n> +\t\t\tdie(_(message_detached_head_die), remote->name);\n> \t\tadd_refspec(\"HEAD\");\n> \t\tbreak;\n\nThis means well, but I am not sure calling branch_get() for all\nother cases that do not care about the current branch is a right\nthing to do. It's not like you need branch variable to free some\nresource after done, or pass it around to callees from this\nfunction.\n\nWould it hurt to do\n\n\tif (!branch_get(NULL))\n\t\tdie(...);\n\nhere, without the first hunk? \n"},{"id":"218887","messageId":"CALkWK0mCY0F-EgO6hQv-LvV1Nb5H9jNXBWLDrY07rVNqLBPjkw@mail.gmail.com","threadId":"33976","inReplyTo":"7vppw9shq2.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] push: fail early with detached HEAD and current","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-29T19:37:50Z","receivedAt":"2013-05-29T19:37:50Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n>> case PUSH_DEFAULT_CURRENT:\n>> + if (!branch)\n>> + die(_(message_detached_head_die), remote->name);\n>> add_refspec(\"HEAD\");\n>> break;\n>\n> Would it hurt to do\n>\n> if (!branch_get(NULL))\n> die(...);\n>\n> here, without the first hunk?\n\nAnd how would I change the add_refspec() call to take branch->name in [3/3]?\n"},{"id":"218892","messageId":"7v7gihsgjr.fsf@alter.siamese.dyndns.org","threadId":"33976","inReplyTo":"CALkWK0mCY0F-EgO6hQv-LvV1Nb5H9jNXBWLDrY07rVNqLBPjkw@mail.gmail.com","subject":"Re: [PATCH 2/3] push: fail early with detached HEAD and current","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-29T19:59:52Z","receivedAt":"2013-05-29T19:59:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Junio C Hamano wrote:\n>>> case PUSH_DEFAULT_CURRENT:\n>>> + if (!branch)\n>>> + die(_(message_detached_head_die), remote->name);\n>>> add_refspec(\"HEAD\");\n>>> break;\n>>\n>> Would it hurt to do\n>>\n>> if (!branch_get(NULL))\n>> die(...);\n>>\n>> here, without the first hunk?\n>\n> And how would I change the add_refspec() call to take branch->name in [3/3]?\n\nBy doing something like this, perhaps?\n\n\tstruct ... branch;\n\n switch (...) {\n ...\n case it-only-matters-here:\n \tbranch = branch_get(NULL);\n if (!branch)\n\t\t\tdie(...);\n\t}\n"},{"id":"218894","messageId":"CALkWK0=W3B0wFSjwa5qrtyOGHyJFWnZQicqubpyvT6hS5EYXpQ@mail.gmail.com","threadId":"33976","inReplyTo":"7v7gihsgjr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] push: fail early with detached HEAD and current","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-29T20:04:06Z","receivedAt":"2013-05-29T20:04:06Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Here: put this at the end of the series and autosquash.\n\n-- 8< --\nFrom: Ramkumar Ramachandra \nDate: Thu, 30 May 2013 01:29:59 +0530\nSubject: [PATCH] fixup! HEAD~2\n\nSigned-off-by: Ramkumar Ramachandra \n---\n builtin/push.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex d819487..2d84d10 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -175,7 +175,7 @@ static void\nwarn_unspecified_push_default_configuration(void)\n\n static void setup_default_push_refspecs(struct remote *remote)\n {\n- struct branch *branch = branch_get(NULL);\n+ struct branch *branch;\n\n switch (push_default) {\n default:\n@@ -196,6 +196,7 @@ static void setup_default_push_refspecs(struct\nremote *remote)\n break;\n\n case PUSH_DEFAULT_CURRENT:\n+ branch = branch_get(NULL);\n if (!branch)\n die(_(message_detached_head_die), remote->name);\n add_refspec(branch->name);\n-- \n1.8.3.12.gbd56588\n"}]}