git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: Tao Klerks via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org,  Tao Klerks <tao@klerks.biz>
Subject: Re: [PATCH] replace-refs: fix support of qualified replace ref paths
Date: Tue, 29 Apr 2025 11:46:39 -0700	[thread overview]
Message-ID: <xmqqh6266c1c.fsf@gitster.g> (raw)
In-Reply-To: <aBCt8YrqJ7IM0ld6@pks.im> (Patrick Steinhardt's message of "Tue, 29 Apr 2025 12:46:09 +0200")

Patrick Steinhardt <ps@pks.im> writes:

> I wonder whether this really was an intentional choice or whether it is
> simply a bug that led to useful behaviour. In any case, git-replace(1)
> itself does not mention your behaviour at all -- it simply talks about
> the "name of the replace reference".

Yup, I am reasonbly sure that this was not a designed behaviour.  If
this were discovered during the original review, I would probably
have suggested tightening the rule to ignore anything with extra
levels in the middle.  It can be argued that it is too late, but
with this ...

>> The only way this didn't "work" is in the commit decoration process,

... 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.

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.  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.

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.

> If we can agree that this is something that we want we should definitely
> amend git-replace(1) to document this new format.
>
> 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?

"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.

  reply	other threads:[~2025-04-29 18:46 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-26  7:10 [PATCH] replace-refs: fix support of qualified replace ref paths Tao Klerks via GitGitGadget
2025-04-29 10:46 ` Patrick Steinhardt
2025-04-29 18:46   ` Junio C Hamano [this message]
2025-04-29 19:35     ` Tao Klerks
2025-04-30  0:11       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqh6266c1c.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=ps@pks.im \
    --cc=tao@klerks.biz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).