Re: [PATCH] checkout: indicate when a detached head is checked out for a branch
Junio C Hamano venit, vidit, dixit 19.07.2014 00:18: > Dennis Kaarsemaker writes: > >> My use case for this is checking out the same branch (or commit, so >> already on a detached HEAD) in multiple different places to run >> independent actions (e.g. make test with different compiler options, or >> creating several different packages) and I would really appreciate it if >> that would keep working. > > I do not have any problem if multiple working trees have the same > commit checked out on their own detached HEADs at all. The "should > error out" was solely for the case where the user asked not to detach > but to obtain a state where a named branch is checked out. In such > a case, the command should not turn it into a detached HEAD, with or > without a warning. Exactly, all of that, plus: * "git checkout --detach --to foo bar" could/should be a way to spell out "git checkout --to foo bar^0". * In the case of erroring out, "git checkout" could suggest one of the above two commands. Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] checkout: indicate when a detached head is checked out for a branch
Dennis Kaarsemaker writes: > My use case for this is checking out the same branch (or commit, so > already on a detached HEAD) in multiple different places to run > independent actions (e.g. make test with different compiler options, or > creating several different packages) and I would really appreciate it if > that would keep working. I do not have any problem if multiple working trees have the same commit checked out on their own detached HEADs at all. The "should error out" was solely for the case where the user asked not to detach but to obtain a state where a named branch is checked out. In such a case, the command should not turn it into a detached HEAD, with or without a warning. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] checkout: indicate when a detached head is checked out for a branch
On vr, 2014-07-18 at 10:36 -0700, Junio C Hamano wrote: > Michael J Gruber writes: > > > I really like the new --to feature and will convert all my "new workdir" > > checkouts to that. But that detached checkout is so easy to miss - in fact > > I noticed it only when I compared "new-workdir" to "checkout --to" for a > > test repo with one branch, to see what a converter would need to do. > > > > I'm even wondering whether we should do this DWIMmery at all, given how > > "dangerous" a detached head is for those who are not aware of it > > before gc kicks in. > > As long as the amount of warning about 'detached HEAD' is about the > same between this case and a "git checkout v1.2.3" in a normal > repository, I do not think there is no additional "danger" you need > to be worried about. > > But I do agree that there should not be any DWIM here. > > The reason to introduce this new set of rather intrusive changes is > so that working trees can be aware of branches other working trees > have checked out. And the whole point of wanting to have that > mutual awareness is to enable us to forbid users from mucking with > the same branch from two different places. > > But Git is not in the position to dictate which alternative action > the user would want to take, when her "git checkout foo" is > prevented by this mechanism. In one scenario, she may say "I only > wanted to take a peek" and run "git checkout foo^0" instead. In > another, she may say "Ah, I forgot I already was doing this change > in the other one" and run "cd ../foo". There may be other classes > of alternative actions. > > Don't make it easier for the first class of scenario and make it > less useful and more dangerous for the second class, especially the > second class involve forgetful users who are likely to forget seeing > the "we've warned you that we detached without being asked" message. > > Please fix it to always just error out. I really would appreciate it if it wouldn't always error out. Erroring out by default is fine, but please keep it overridable. My use case for this is checking out the same branch (or commit, so already on a detached HEAD) in multiple different places to run independent actions (e.g. make test with different compiler options, or creating several different packages) and I would really appreciate it if that would keep working. -- Dennis Kaarsemaker www.kaarsemaker.net -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] checkout: indicate when a detached head is checked out for a branch
Michael J Gruber writes: > I really like the new --to feature and will convert all my "new workdir" > checkouts to that. But that detached checkout is so easy to miss - in fact > I noticed it only when I compared "new-workdir" to "checkout --to" for a > test repo with one branch, to see what a converter would need to do. > > I'm even wondering whether we should do this DWIMmery at all, given how > "dangerous" a detached head is for those who are not aware of it > before gc kicks in. As long as the amount of warning about 'detached HEAD' is about the same between this case and a "git checkout v1.2.3" in a normal repository, I do not think there is no additional "danger" you need to be worried about. But I do agree that there should not be any DWIM here. The reason to introduce this new set of rather intrusive changes is so that working trees can be aware of branches other working trees have checked out. And the whole point of wanting to have that mutual awareness is to enable us to forbid users from mucking with the same branch from two different places. But Git is not in the position to dictate which alternative action the user would want to take, when her "git checkout foo" is prevented by this mechanism. In one scenario, she may say "I only wanted to take a peek" and run "git checkout foo^0" instead. In another, she may say "Ah, I forgot I already was doing this change in the other one" and run "cd ../foo". There may be other classes of alternative actions. Don't make it easier for the first class of scenario and make it less useful and more dangerous for the second class, especially the second class involve forgetful users who are likely to forget seeing the "we've warned you that we detached without being asked" message. Please fix it to always just error out. > (Sorry if that dupes something on the list, can't keep up these days; > so this is coming from a "mere user" ;-) > > builtin/checkout.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/builtin/checkout.c b/builtin/checkout.c > index cfc6db7..38a5670 100644 > --- a/builtin/checkout.c > +++ b/builtin/checkout.c > @@ -645,9 +645,9 @@ static void update_refs_for_switch(const struct > checkout_opts *opts, > detach_advice(new->name); > describe_detached_head(_("HEAD is now at"), > new->commit); > if (new->checkout && !*new->checkout) > - fprintf(stderr, _("hint: the main checkout is > holding this branch\n")); > + fprintf(stderr, _("hint: the main checkout is > holding this branch; detaching branch head instead.\n")); > else if (new->checkout) > - fprintf(stderr, _("hint: the linked checkout %s > is holding this branch\n"), > + fprintf(stderr, _("hint: the linked checkout %s > is holding this branch; detaching branch head instead.\n"), > new->checkout); > } > } else if (new->path) { /* Switch branches. */ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] checkout: indicate when a detached head is checked out for a branch
Hi. On Fri, Jul 18, 2014 at 4:27 PM, Michael J Gruber wrote: > Duy Nguyen venit, vidit, dixit 18.07.2014 12:58: >> This is what this series needs, user's opinions (bad or good). Actually, if options "-b branch" works with the "--to" (does it?), then user probably shouldn't need to create detached checkouts (I need them only for scripts), so this action is probably a mistake. And when user does want to create detached checkout he can use the "--detach" option. So I would say checkout of already checked out branch should fail, suggesting using "-b" or "--detach" options. -- Max -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] checkout: indicate when a detached head is checked out for a branch
Duy Nguyen venit, vidit, dixit 18.07.2014 12:58: > On Fri, Jul 18, 2014 at 4:50 PM, Michael J Gruber > wrote: >> I really like the new --to feature and will convert all my "new workdir" >> checkouts to that. But that detached checkout is so easy to miss - in fact >> I noticed it only when I compared "new-workdir" to "checkout --to" for a >> test repo with one branch, to see what a converter would need to do. >> >> I'm even wondering whether we should do this DWIMmery at all, > > This is what this series needs, user's opinions (bad or good). The > other option is abort the checkout immediately. I think I made detach > behavior default is because it's more work (and needs to be proven > feasible). How about a config key that lets user decide what to do > here, abort or detach. We may change the default behavior too if > people think the current one is not good. Uh, config bloat :) I think DWIMmery is OK if it's made clear to the user what happened, and it is somewhat expected/probably intended behavior. Do we have a precedent where a detached head is produced when a branch checkout is requested, or something similar? I think checking out remote tracking branches is somehow in that same boat. >> given how "dangerous" a detached head is for those who are not aware of it >> before gc kicks in. > > Wait, what danger are we talking about? I thought gc pays attention to > detached heads as well.. As long as HEAD points to it, of course. I think detached head is one of the killer features of git, in both senses of the meaning... Don't we DWIM (or suggest) "git checkout origin/master" to "git checkout --track origin/master" which creates master with upstream origin/master? Maybe I'm mixing things up, but I think we try to produce detached heads only on special requests. New users get confused by them, some don't understand the (well crafted) message you get when you switch away from them, and while you can recover them from HEAD's reflog, they are gone with the next gc unless they remain checked out (or get referenced). I think I've just convinced myself that we shouldn't DWIm to a detached head, and rather tell the user how to produce one if she really intended to: "git checkout --detach..." That one seems to be broken by multiple workdir setup (in the sense of producing an unnecessary hint). Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] checkout: indicate when a detached head is checked out for a branch
On Fri, Jul 18, 2014 at 4:50 PM, Michael J Gruber wrote: > I really like the new --to feature and will convert all my "new workdir" > checkouts to that. But that detached checkout is so easy to miss - in fact > I noticed it only when I compared "new-workdir" to "checkout --to" for a > test repo with one branch, to see what a converter would need to do. > > I'm even wondering whether we should do this DWIMmery at all, This is what this series needs, user's opinions (bad or good). The other option is abort the checkout immediately. I think I made detach behavior default is because it's more work (and needs to be proven feasible). How about a config key that lets user decide what to do here, abort or detach. We may change the default behavior too if people think the current one is not good. > given how "dangerous" a detached head is for those who are not aware of it > before gc kicks in. Wait, what danger are we talking about? I thought gc pays attention to detached heads as well.. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html