On Tue, Apr 29, 2025 at 8:46 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Patrick Steinhardt <ps@xxxxxx> writes:
>
> > I wonder whether this really was an intentional choice or whether it is
> > simply a bug that led to useful behaviour.
I don't know that for sure either - although the fact that we *do*
already have sanity checks against multiple replace refs targeting the
same object id leads me to believe it was intentionally allowed.
I tried to explain why this is useful, and why it should be *better*
supported, in the change description: *without* this behavior, replace
refs cannot be "namespaced" - they are always inherently an
undifferentiated pile, and that in turn makes *sharing* of replace
refs completely impractical.
>
> ... where the "funny" replace refs are honored in some but not all
> situations, it also can be argued that it is highly unlikely anybody
> sane is actually depending on this "feature", so such a tightening
> may not hurt too much.
Well, I guess my sanity can be reasonably questioned, but it would
certainly hurt me and my thousand plus users - we use replace refs to
keep nice tidy squash-based history by default, but allow for
different types of "deep blame" by setting up one or more fetch
refspecs that map into "refs/replace/SOMETHING/*"
>
> The reason why I would slightly prefer tightening the rule, rather
> than making the loophole even larger by adjusting the code for "the
> commit decoration process", is what it means to have two different
> replacement objects for the same object, which would naturally be
> prevented from happening if we did not allow extra levels in the
> middle.
This is already accounted for with a hard error: If you end up with
duplicate replace ref targets, many operations fail with a clear
error. Clean up your replace refs and you're off to the races again.
> Would one always consistently trump the other? When a
> filesystem rebalances, would we just pick one at randomly based
> purely on the first one readdir() happened to return? It smells
> to lead to nothing but a confused mess.
Reasonable concern, but already addressed.
>
> Maybe the answer is "don't do it, then". The same answer, however,
> can be given for creating any extra level between refs/replace and
> the object name itself, so... I dunno.
The same answer should probably not be given: One behavior is
*useful*, and one is not.
>
> > If we can agree that this is something that we want we should definitely
> > amend git-replace(1) to document this new format.
I will (propose to) do so.
> >
> > Which raises the question: is this something that we want? Are there any
> > arguments that would speak against loosening the format of replace refs
> > now?
My point here is that it's *not* a loosening - this "loose" behavior
already exists, it works, it is useful, and there are people using it.
At least my thousand users. All I'm proposing here is to show them
less spurious warnings when they run a log with --decorate.
>
> "What if refs/replace/{a,b,c}/$name point at different objects?"
> would be a solid reason why we shouldn't do this, but I personally
> do not feel too strongly about it, as "don't do it, then" may be a
> reasonable answer for such an insane scenario.
That is indeed the current outcome: don't do it or you get a hard (but
nondestructive) error. You can then of course fix it and you're set.
