Re: [PATCH 3/5] checkout --orphan: respect -l option always

[Erick Mattos <erick.mattos@gmail.com>]

Hi,

2010/5/26 Junio C Hamano <gitster@pobox.com>
>
> Erick Mattos <erick.mattos@gmail.com> writes:
> > @@ -684,8 +709,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
> >       if (opts.new_orphan_branch) {
> >               if (opts.new_branch)
> >                       die("--orphan and -b are mutually exclusive");
> > -             if (opts.track > 0 || opts.new_branch_log)
> > -                     die("--orphan cannot be used with -t or -l");
> > +             if (opts.track > 0)
> > +                     die("--orphan should not be used with -t");
>
> Why s/cannot/should not/?  Just being curious.

I have typed that text, not changed the original so this is not a fix
to your text.  Anyway for me "should not" is more polite, like "you
should not yell" meaning you really can not do it.  Or "you should not
disrespect the captain".

But that is not a fix.

> > +test_expect_success 'giving up --orphan not committed when -l and core.logAllRefUpdates = false deletes reflog' '
> > +     git checkout master &&
> > +     git checkout -l --orphan eta &&
> > +     test -f .git/logs/refs/heads/eta &&
> > +     test_must_fail PAGER= git reflog show eta &&
> > +     git checkout master &&
> > +     ! test -f .git/logs/refs/heads/eta &&
> > +     test_must_fail PAGER= git reflog show eta
> > +'
>
> I don't quite understand the title of this test, nor am I convinced that
> testing for .git/logs/refs/heads/eta is necessarily a good thing to do
> here.  "eta" branch is first prepared in an unborn state with the working
> tree and the index prepared to commit what is in 'master', and the first
> "git reflog" would fail because there is no eta branch at that point yet.
> Moving to 'master' from that state would still leave "eta" branch unborn
> and we will not see "git reflog" for that branch (we will fail "git log
> eta" too for that matter).  Perhaps two "test -f .git/logs/refs/heads/eta"
> shouldn't be there?  It feels that it is testing a bit too low level an
> implementation detail.

So I need to explain the solution:

When config core.logAllRefUpdates is set to false what really happens
is that the reflog is not created and any reflog change is saved only
when you have an existent reflog.

What I did was to make a "touch reflog".  Creating it, when the new
branch get eventually saved then the reflog would be written normally.
 But in case somebody give up this new branch before the first save,
moving back to a regular branch would leave a ghost reflog.

I have coded the cleaning commands for that and the test is just a
check of this behavior.

The first "test -f .git/logs/refs/heads/eta" tests if reflog was
created and the second if it was deleted.  No big deal.

Regards