{"thread":{"id":"37158","subject":"[PATCH] checkout: indicate when a detached head is checked out for a branch","startedAt":"2014-07-18T09:50:42Z","lastAt":"2014-07-21T13:03:28Z","messageCount":8,"participants":["Michael J Gruber","Duy Nguyen","Max Kirillov","Junio C Hamano","Dennis Kaarsemaker"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"246296","messageId":"35dbe7e3f3e4566d775bea19d816adc44db8ed5c.1405676303.git.git@drmicha.warpmail.net","threadId":"37158","inReplyTo":null,"subject":"[PATCH] checkout: indicate when a detached head is checked out for a branch","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2014-07-18T09:50:42Z","receivedAt":"2014-07-18T09:50:42Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"In a setup with more than 1 workdir, it can happen that a user requests\nto checkout a branch that is already checked out somewhere else. The\ncurrent \"hint\" about this gives the impression that it is merely warning\nabout this fact.\n\nActually, \"checkout\" changes its behavior in such a situation from\nchecking out the requested branch to checking out its head as a detached\nhead. The only indication is the usual checkout message which appears\nbefore the hint.\n\nChange the hint to make it clearer that checkout changed its behavior to\nsomething that is possible to do in that situation, but may or may not\nbe what the user intended.\n\nSigned-off-by: Michael J Gruber \n---\nI really like the new --to feature and will convert all my \"new workdir\"\ncheckouts to that. But that detached checkout is so easy to miss - in fact\nI noticed it only when I compared \"new-workdir\" to \"checkout --to\" for a\ntest repo with one branch, to see what a converter would need to do.\n\nI'm even wondering whether we should do this DWIMmery at all, given how\n\"dangerous\" a detached head is for those who are not aware of it\nbefore gc kicks in.\n\n(Sorry if that dupes something on the list, can't keep up these days;\nso this is coming from a \"mere user\" ;-)\n\n builtin/checkout.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex cfc6db7..38a5670 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -645,9 +645,9 @@ static void update_refs_for_switch(const struct checkout_opts *opts,\n \t\t\t\tdetach_advice(new->name);\n \t\t\tdescribe_detached_head(_(\"HEAD is now at\"), new->commit);\n \t\t\tif (new->checkout && !*new->checkout)\n-\t\t\t\tfprintf(stderr, _(\"hint: the main checkout is holding this branch\\n\"));\n+\t\t\t\tfprintf(stderr, _(\"hint: the main checkout is holding this branch; detaching branch head instead.\\n\"));\n \t\t\telse if (new->checkout)\n-\t\t\t\tfprintf(stderr, _(\"hint: the linked checkout %s is holding this branch\\n\"),\n+\t\t\t\tfprintf(stderr, _(\"hint: the linked checkout %s is holding this branch; detaching branch head instead.\\n\"),\n \t\t\t\t\tnew->checkout);\n \t\t}\n \t} else if (new->path) {\t/* Switch branches. */\n-- \n2.0.1.583.gbe96573\n"},{"id":"246302","messageId":"CACsJy8CG17tzWWO27Pv2c+CjDyYiYATzgBSFfMBaugYgQfZQ5g@mail.gmail.com","threadId":"37158","inReplyTo":"35dbe7e3f3e4566d775bea19d816adc44db8ed5c.1405676303.git.git@drmicha.warpmail.net","subject":"Re: [PATCH] checkout: indicate when a detached head is checked out for a branch","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-07-18T10:58:02Z","receivedAt":"2014-07-18T10:58:02Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Jul 18, 2014 at 4:50 PM, Michael J Gruber\n wrote:\n> I really like the new --to feature and will convert all my \"new workdir\"\n> checkouts to that. But that detached checkout is so easy to miss - in fact\n> I noticed it only when I compared \"new-workdir\" to \"checkout --to\" for a\n> test repo with one branch, to see what a converter would need to do.\n>\n> I'm even wondering whether we should do this DWIMmery at all,\n\nThis is what this series needs, user's opinions (bad or good). The\nother option is abort the checkout immediately. I think I made detach\nbehavior default is because it's more work (and needs to be proven\nfeasible). How about a config key that lets user decide what to do\nhere, abort or detach. We may change the default behavior too if\npeople think the current one is not good.\n\n> given how \"dangerous\" a detached head is for those who are not aware of it\n> before gc kicks in.\n\nWait, what danger are we talking about? I thought gc pays attention to\ndetached heads as well..\n-- \nDuy\n"},{"id":"246309","messageId":"53C920D6.80104@drmicha.warpmail.net","threadId":"37158","inReplyTo":"CACsJy8CG17tzWWO27Pv2c+CjDyYiYATzgBSFfMBaugYgQfZQ5g@mail.gmail.com","subject":"Re: [PATCH] checkout: indicate when a detached head is checked out for a branch","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2014-07-18T13:27:50Z","receivedAt":"2014-07-18T13:27:50Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Duy Nguyen venit, vidit, dixit 18.07.2014 12:58:\n> On Fri, Jul 18, 2014 at 4:50 PM, Michael J Gruber\n> wrote:\n>> I really like the new --to feature and will convert all my \"new workdir\"\n>> checkouts to that. But that detached checkout is so easy to miss - in fact\n>> I noticed it only when I compared \"new-workdir\" to \"checkout --to\" for a\n>> test repo with one branch, to see what a converter would need to do.\n>>\n>> I'm even wondering whether we should do this DWIMmery at all,\n> \n> This is what this series needs, user's opinions (bad or good). The\n> other option is abort the checkout immediately. I think I made detach\n> behavior default is because it's more work (and needs to be proven\n> feasible). How about a config key that lets user decide what to do\n> here, abort or detach. We may change the default behavior too if\n> people think the current one is not good.\n\nUh, config bloat :)\n\nI think DWIMmery is OK if it's made clear to the user what happened, and\nit is somewhat expected/probably intended behavior.\n\nDo we have a precedent where a detached head is produced when a branch\ncheckout is requested, or something similar? I think checking out remote\ntracking branches is somehow in that same boat.\n\n>> given how \"dangerous\" a detached head is for those who are not aware of it\n>> before gc kicks in.\n> \n> Wait, what danger are we talking about? I thought gc pays attention to\n> detached heads as well..\n\nAs long as HEAD points to it, of course.\n\nI think detached head is one of the killer features of git, in both\nsenses of the meaning...\n\nDon't we DWIM (or suggest) \"git checkout origin/master\" to \"git checkout\n--track origin/master\" which creates master with upstream origin/master?\n\nMaybe I'm mixing things up, but I think we try to produce detached heads\nonly on special requests. New users get confused by them, some don't\nunderstand the (well crafted) message you get when you switch away from\nthem, and while you can recover them from HEAD's reflog, they are gone\nwith the next gc unless they remain checked out (or get referenced).\n\nI think I've just convinced myself that we shouldn't DWIm to a detached\nhead, and rather tell the user how to produce one if she really intended\nto: \"git checkout --detach...\" That one seems to be broken by multiple\nworkdir setup (in the sense of producing an unnecessary hint).\n\nMichael\n"},{"id":"246310","messageId":"CAF7_NFRdDi_dgsooX8o9J12h39yGamoKUW4djZ0=arMrxBZbdg@mail.gmail.com","threadId":"37158","inReplyTo":"53C920D6.80104@drmicha.warpmail.net","subject":"Re: [PATCH] checkout: indicate when a detached head is checked out for a branch","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2014-07-18T14:13:29Z","receivedAt":"2014-07-18T14:13:29Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"Hi.\n\nOn Fri, Jul 18, 2014 at 4:27 PM, Michael J Gruber\n wrote:\n> Duy Nguyen venit, vidit, dixit 18.07.2014 12:58:\n>> This is what this series needs, user's opinions (bad or good).\n\nActually, if options \"-b branch\" works with the \"--to\" (does it?), then user\nprobably shouldn't need to create detached checkouts (I need them\nonly for scripts), so this action is probably a mistake. And when user\ndoes want to create detached checkout he can use the \"--detach\" option.\n\nSo I would say checkout of already checked out branch should fail, suggesting\nusing \"-b\" or \"--detach\" options.\n\n-- \nMax\n"},{"id":"246322","messageId":"xmqqr41imuwk.fsf@gitster.dls.corp.google.com","threadId":"37158","inReplyTo":"35dbe7e3f3e4566d775bea19d816adc44db8ed5c.1405676303.git.git@drmicha.warpmail.net","subject":"Re: [PATCH] checkout: indicate when a detached head is checked out for a branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-18T17:36:11Z","receivedAt":"2014-07-18T17:36:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> I really like the new --to feature and will convert all my \"new workdir\"\n> checkouts to that. But that detached checkout is so easy to miss - in fact\n> I noticed it only when I compared \"new-workdir\" to \"checkout --to\" for a\n> test repo with one branch, to see what a converter would need to do.\n>\n> I'm even wondering whether we should do this DWIMmery at all, given how\n> \"dangerous\" a detached head is for those who are not aware of it\n> before gc kicks in.\n\nAs long as the amount of warning about 'detached HEAD' is about the\nsame between this case and a \"git checkout v1.2.3\" in a normal\nrepository, I do not think there is no additional \"danger\" you need\nto be worried about.\n\nBut I do agree that there should not be any DWIM here.\n\nThe reason to introduce this new set of rather intrusive changes is\nso that working trees can be aware of branches other working trees\nhave checked out. And the whole point of wanting to have that\nmutual awareness is to enable us to forbid users from mucking with\nthe same branch from two different places.\n\nBut Git is not in the position to dictate which alternative action\nthe user would want to take, when her \"git checkout foo\" is\nprevented by this mechanism. In one scenario, she may say \"I only\nwanted to take a peek\" and run \"git checkout foo^0\" instead. In\nanother, she may say \"Ah, I forgot I already was doing this change\nin the other one\" and run \"cd ../foo\". There may be other classes\nof alternative actions.\n\nDon't make it easier for the first class of scenario and make it\nless useful and more dangerous for the second class, especially the\nsecond class involve forgetful users who are likely to forget seeing\nthe \"we've warned you that we detached without being asked\" message.\n\nPlease fix it to always just error out.\n\n> (Sorry if that dupes something on the list, can't keep up these days;\n> so this is coming from a \"mere user\" ;-)\n>\n> builtin/checkout.c | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index cfc6db7..38a5670 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -645,9 +645,9 @@ static void update_refs_for_switch(const struct checkout_opts *opts,\n> \t\t\t\tdetach_advice(new->name);\n> \t\t\tdescribe_detached_head(_(\"HEAD is now at\"), new->commit);\n> \t\t\tif (new->checkout && !*new->checkout)\n> -\t\t\t\tfprintf(stderr, _(\"hint: the main checkout is holding this branch\\n\"));\n> +\t\t\t\tfprintf(stderr, _(\"hint: the main checkout is holding this branch; detaching branch head instead.\\n\"));\n> \t\t\telse if (new->checkout)\n> -\t\t\t\tfprintf(stderr, _(\"hint: the linked checkout %s is holding this branch\\n\"),\n> +\t\t\t\tfprintf(stderr, _(\"hint: the linked checkout %s is holding this branch; detaching branch head instead.\\n\"),\n> \t\t\t\t\tnew->checkout);\n> \t\t}\n> \t} else if (new->path) {\t/* Switch branches. */\n"},{"id":"246342","messageId":"1405720477.13250.8.camel@spirit","threadId":"37158","inReplyTo":"xmqqr41imuwk.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] checkout: indicate when a detached head is checked out for a branch","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2014-07-18T21:54:37Z","receivedAt":"2014-07-18T21:54:37Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"On vr, 2014-07-18 at 10:36 -0700, Junio C Hamano wrote:\n> Michael J Gruber writes:\n> \n> > I really like the new --to feature and will convert all my \"new workdir\"\n> > checkouts to that. But that detached checkout is so easy to miss - in fact\n> > I noticed it only when I compared \"new-workdir\" to \"checkout --to\" for a\n> > test repo with one branch, to see what a converter would need to do.\n> >\n> > I'm even wondering whether we should do this DWIMmery at all, given how\n> > \"dangerous\" a detached head is for those who are not aware of it\n> > before gc kicks in.\n> \n> As long as the amount of warning about 'detached HEAD' is about the\n> same between this case and a \"git checkout v1.2.3\" in a normal\n> repository, I do not think there is no additional \"danger\" you need\n> to be worried about.\n> \n> But I do agree that there should not be any DWIM here.\n> \n> The reason to introduce this new set of rather intrusive changes is\n> so that working trees can be aware of branches other working trees\n> have checked out. And the whole point of wanting to have that\n> mutual awareness is to enable us to forbid users from mucking with\n> the same branch from two different places.\n> \n> But Git is not in the position to dictate which alternative action\n> the user would want to take, when her \"git checkout foo\" is\n> prevented by this mechanism. In one scenario, she may say \"I only\n> wanted to take a peek\" and run \"git checkout foo^0\" instead. In\n> another, she may say \"Ah, I forgot I already was doing this change\n> in the other one\" and run \"cd ../foo\". There may be other classes\n> of alternative actions.\n> \n> Don't make it easier for the first class of scenario and make it\n> less useful and more dangerous for the second class, especially the\n> second class involve forgetful users who are likely to forget seeing\n> the \"we've warned you that we detached without being asked\" message.\n> \n> Please fix it to always just error out.\n\nI really would appreciate it if it wouldn't always error out. Erroring\nout by default is fine, but please keep it overridable. \n\nMy use case for this is checking out the same branch (or commit, so\nalready on a detached HEAD) in multiple different places to run\nindependent actions (e.g. make test with different compiler options, or\ncreating several different packages) and I would really appreciate it if\nthat would keep working.\n\n-- \nDennis Kaarsemaker\nwww.kaarsemaker.net\n"},{"id":"246343","messageId":"xmqqpph2l39d.fsf@gitster.dls.corp.google.com","threadId":"37158","inReplyTo":"1405720477.13250.8.camel@spirit","subject":"Re: [PATCH] checkout: indicate when a detached head is checked out for a branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-18T22:18:38Z","receivedAt":"2014-07-18T22:18:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dennis Kaarsemaker writes:\n\n> My use case for this is checking out the same branch (or commit, so\n> already on a detached HEAD) in multiple different places to run\n> independent actions (e.g. make test with different compiler options, or\n> creating several different packages) and I would really appreciate it if\n> that would keep working.\n\nI do not have any problem if multiple working trees have the same\ncommit checked out on their own detached HEADs at all. The \"should\nerror out\" was solely for the case where the user asked not to detach\nbut to obtain a state where a named branch is checked out. In such\na case, the command should not turn it into a detached HEAD, with or\nwithout a warning.\n"},{"id":"246446","messageId":"53CD0FA0.1030600@drmicha.warpmail.net","threadId":"37158","inReplyTo":"xmqqpph2l39d.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] checkout: indicate when a detached head is checked out for a branch","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2014-07-21T13:03:28Z","receivedAt":"2014-07-21T13:03:28Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 19.07.2014 00:18:\n> Dennis Kaarsemaker writes:\n> \n>> My use case for this is checking out the same branch (or commit, so\n>> already on a detached HEAD) in multiple different places to run\n>> independent actions (e.g. make test with different compiler options, or\n>> creating several different packages) and I would really appreciate it if\n>> that would keep working.\n> \n> I do not have any problem if multiple working trees have the same\n> commit checked out on their own detached HEADs at all. The \"should\n> error out\" was solely for the case where the user asked not to detach\n> but to obtain a state where a named branch is checked out. In such\n> a case, the command should not turn it into a detached HEAD, with or\n> without a warning.\n\nExactly, all of that, plus:\n\n* \"git checkout --detach --to foo bar\" could/should be a way to spell out\n\"git checkout --to foo bar^0\".\n\n* In the case of erroring out, \"git checkout\" could suggest one of the\nabove two commands.\n\nMichael\n"}]}