git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org, peff@peff.net, me@ttaylorr.com
Subject: Re: [PATCH RESEND] refs: Always pass old object name to reftx hook
Date: Tue, 19 Jan 2021 23:06:15 -0800	[thread overview]
Message-ID: <xmqqft2wgk9k.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <YAfNcyZ7mj4J69XT@ncase> (Patrick Steinhardt's message of "Wed, 20 Jan 2021 07:28:03 +0100")

Patrick Steinhardt <ps@pks.im> writes:

>> Even without this change, the current value the hook can learn by
>> looking the ref up itself if it really wanted to, no?
>
> I think the biggest problem is that right now, you cannot discern the
> actual intention of the user because the information provided to the
> hook is ambiguous in the branch creation case: "$ZERO_OID $NEW_OID $REF"
> could mean the user intends to create a new branch where it shouldn't
> have existed previously. BUT it could also mean that the user just
> doesn't care what the reference previously pointed to.

Yes, it can mean both, but when you pretend to be that hook,
wouldn't you check if the ref exists?  If not, the user is trying to
create it, and otherwise, the user does not know or care what the
original value is, no?

> The user could now try to derive the intention by manually looking up
> the current state of the reference. But that does feel kind of awkward
> to me.

So in short, with respect to the OLD slot, there are three kind of
end-user intention that could be conveyed to the hook:

 (1) the user does not care, so 0{40} appears in the OLD slot here,
 (2) the user is creating, so 0{40} apears in the OLD slot here, and
 (3) the user does care, and this is the OID in the OLD slot,

And (1) and (2) cannot be separated without looking at the ref (in
other words, if the hook really cares, it can find it out).

But if you replace 0{40} with the current OID, then you are making
it impossible to tell (1) and (3) apart.  The hook cannot tell the
distinction even if it is willing to go the extra mile.

So that sounds like a strict disimprovement to me.

If you can invent a way to help the hook to tell all three apart, I
am very much interested.  It would earn you a bonus point if you can
do so without breaking backward compatibility (but I doubt that it
is possible).

> To me, having clearly defined semantics ("The script always gets old and
> new value of the branch regardless of what the user did") is preferable
> to having ambiguous semantics.

But "The script always gets old that was given by the user and the
new value to be stored" is very clearly defined semantics already.

On the other hand, "The script gets a non-NULL object name in both
cases, either when the user says s/he does not care, or when the
user insists that the old value must be that, and it is not just
ambiguous but is impossible to tell apart" is worse than just being
ambiguous.

  reply	other threads:[~2021-01-20  7:07 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-04 14:58 [PATCH] refs: Always pass old object name to reftx hook Patrick Steinhardt
2020-12-04  8:37 ` Patrick Steinhardt
2021-01-12 21:07   ` Taylor Blau
2021-01-13 11:22     ` Patrick Steinhardt
2021-01-13 15:09       ` Taylor Blau
2021-01-13 20:11       ` Junio C Hamano
2021-01-18 12:44         ` Patrick Steinhardt
2021-01-18 12:49 ` [PATCH RESEND] " Patrick Steinhardt
2021-01-18 22:45   ` Junio C Hamano
2021-01-20  6:28     ` Patrick Steinhardt
2021-01-20  7:06       ` Junio C Hamano [this message]
2021-01-22  6:44         ` Patrick Steinhardt
2021-01-22 18:33           ` 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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqqft2wgk9k.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    --cc=ps@pks.im \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).