Re: [PATCH 6/8] reflog_expire(): ignore --updateref for symbolic references

[Stefan Beller <sbeller@google.com>]

On Thu, Feb 12, 2015 at 8:08 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> On 02/11/2015 01:44 AM, Stefan Beller wrote:
>> On Mon, Feb 9, 2015 at 1:12 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>>
>>> If we are expiring reflog entries for a symbolic reference, then how
>>> should --updateref be handled if the newest reflog entry is expired?
>>>
>>> Option 1: Update the referred-to reference. (This is what the current
>>> code does.) This doesn't make sense, because the referred-to reference
>>> has its own reflog, which hasn't been rewritten.
>>>
>>> Option 2: Update the symbolic reference itself (as in, REF_NODEREF).
>>> This would convert the symbolic reference into a non-symbolic
>>> reference (e.g., detaching HEAD), which is surely not what a user
>>> would expect.
>>>
>>> Option 3: Error out. This is plausible, but it would make the
>>> following usage impossible:
>>>
>>>     git reflog expire ... --updateref --all
>>>
>>> Option 4: Ignore --updateref for symbolic references.
>>>
>>
>> Ok let me ask a question first about the symbolic refs.
>>
>> We used to use symbolic links for that, but because of
>> portability issues we decided to not make it a link, but rather
>> a standard file containing the pointing link (The content of
>> .git/HEAD is "ref: refs/heads/master\n" except when detached)
>>
>> So this is the only distinction? Or is there also a concept of
>> symbolic links/pointers for the reflog handling?
>
> A symbolic reference can have a reflog just like a normal reference can.

Yes I can understand that, maybe I was thinking one step
further in the wrong direction. The question was rather:
Do we have symbolic reflogs, i.e.
$ cat .git/logs/<some_ref>:
symbolic log: find log in other reflog

>
> When a reference is updated through a symbolic reference, then
> write_ref_sha1() writes a reflog entry for both the reference and the
> symbolic reference. Also (as an extra kludge), if *any* reference is
> updated directly and it happens to be the current HEAD reference, then
> an entry is added to HEAD's reflog.

Yes because we cannot do inverse symref resolution, we have this kludge
(with the long comment, "This should do 99% of the time in theory
and 100% in practise") of checking if it is also HEAD whenever we
touch another ref.

>
> "HEAD" is the only symbolic reference that is ever transferred across
> repositories.
>
> Symbolic references are always stored loose (i.e., not in packed-refs).
>
> Does that answer your questions?
>
>>> We choose to implement option 4.
>>
>> You're only saying why the other options are insane, not why this
>> is sane.
>>
>> Also I'd rather tend for option 3 than 4, as it is a safety measure
>> (assuming we give a hint to the user what the problem is, and
>> how it is circumventable)
>
> This is a pretty exotic usage. I can't think of any real-life use case
> for using "--updateref" together with a symbolic reference. In our
> entire code base, "--updateref" is only used a single time, in
> "git-stash.sh", and that is always for "refs/stash", which is never a
> symbolic reference. "git-stash" itself is implemented in a very stylized
> way ("stylized" being a polite way of saying "bizarre"), and I doubt
> that there are many more users of this option in the wild, let alone
> "--updateref" together with a symbolic reference.
>
> So, honestly, I don't think it is worth the effort of deciding between 3
> vs. 4. Since 4 is easier to implement (and already implemented), I'd
> rather leave it as is. If you want to submit a patch implementing 3, I
> won't argue against it.

I am not going to bring a patch for option 3. I just learned to be extra
suspicious if we want to drop something silently, so I wanted to
understand in what circumstances we'd run into trouble with that.
That said:

Reviewed-by: Stefan Beller <sbeller@google.com>

>
> Michael
>
> --
> Michael Haggerty
> mhagger@alum.mit.edu
>