Re: [PATCH] t2017: redo physical reflog existance check

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

Hi,

2010/7/22 Junio C Hamano <gitster@pobox.com>
> I thought about it a bit when I sent out my patch, but I do not think that
> is necessary.
>
> The things you care about, after running "-l --orphan eta", are:
>
>  - If you make a commit, you get eta@{...} reflog that records it; and
>
>  - If you leave the still-to-be-born eta branch without making a commit,
>   you do not leave eta@{...} reflog behind.
>
> Your zeta@{...} test is about the former, and your eta@{...} test is about
> the latter.  I think they already check what they want to see happen.

You have separate my concerns in the scripts very well but the result
you pointed out is not quite true.

For zeta, not testing physical existence of reflog file is not really
important because at the end you will have the reflog created anyway,
which is well tested by latter "git rev-parse --verify".  But that is
not the case of eta therefore the check is necessary.

The solution to the problem of creating reflogs when using -l and
--orphan while configured core.logAllRefUpdates=false was a simple
trick, completely based on actual implementation of reflog saving:
reflogs are saved when there is already a reflog file.

To make the new orphan branch ready to have a reflog on that config
was as simple as creating a "touch file" for reflog.  This is the goal
achieved by code.  Testing this goal by checking the touch reflog file
is important while in zeta case redundant, because of the final result
of having a reflog saved and consequently the touch file filled with
real reflog data.

But this clean solution should not leave a bogus file in case it is
not being used by canceling the creation of the new orphan branch and
doing a checkout to another branch.

So in eta case it is important to check if the reflog physical file is
really deleted.  No reflog will be created anyway in eta case so "git
ref-parse --verify" is not even relevant.  I have added formal reflog
check just in case.

Testing more is always better than testing less so I prefered to be
redundant and test thoroughly in all levels of detail.

> I also am afraid that the "test -f" check would expose the implementation
> detail more than necessary.  We may want to come up with a different
> implementation of this behaviour later that may not create an empty file
> there.

No exposure is being done by using "test -f" inside a script which its
sole purpose is to check a controlled event for developers.  Folder t
has "test -f" being employed in 92 scripts.

The only reason for the t folder is to let the developers be aware of
what they are possibly breaking, isn't it?!

I think this implementation is quite good.  I don't see a reason for
changing it.

The point is that you give a command (-l --orphan on
core.logAllRefUpdates=false config) that have to save the preference
of creating the reflog for later.  Data need to be saved once git is a
simple call-run-quit software.  And the way it is being saved is, at
minimum, very efficient.  No outside file or special config is being
created and no memory persistent variable/objects is being left
behind.  The implementation is working as expected and the subject is
only the testing script.

We have to remember that all we are talking here is about a very
uncommon situation when core.logAllRefUpdates is set to false.  I
personally don't even foresee a possible reason for not having reflogs
saved automatically anyway. :-|

Regards