From: Junio C Hamano <gitster@pobox.com>
To: Christopher Lindee <christopher.lindee@webpros.com>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: [PATCH 2/2] Add transport message for up-to-date references
Date: Fri, 15 Mar 2024 09:42:58 -0700 [thread overview]
Message-ID: <xmqqil1nsay5.fsf@gitster.g> (raw)
In-Reply-To: <SA1PR14MB46913A0B1CB3F3D39ADA6A5F8D282@SA1PR14MB4691.namprd14.prod.outlook.com> (Christopher Lindee's message of "Fri, 15 Mar 2024 06:09:02 +0000")
Christopher Lindee <christopher.lindee@webpros.com> writes:
> The shoehorned `if` was to avoid duplicating the `strbuf_addstr` and
> `type` statements. It sounds like code duplication is not a concern,
> so I can make that change. However, with this new logic flow, maybe
> it would be better to have wholly unique values for the display,...
I wasn't avoiding duplication at all.
20% of the reason why I pointed it out was because the third member
"forced no-op update" that is added to the existing two, "forced
update" and "non-forced fast-forwarding update", looked as if it
were made a part of the "non-forced update", where the three at
least to me looked more like equals. And the rest 80% is exactly
because the "three equals" arrangement gives us a more freedom to
express how we show the update.
> deadbeef==deadbeef main -> main
> deadbeef..deadbeef main == main
>
> There's a fair amount of room for creativity here. Of course, using
> revisions is useful, but the existing output contains `+` for forced
> updates, which is not valid in a revision, so there is clearly space
> for novelty.
Sorry, but I do not understand the remark about '+' at all. In the
existing output, the flag char like '+' comes at the beginning and
followed by a whitespace before <from> and <to> ref correspondence
is reported, no?
I can buy the idea of using "summary" that is different from the
existing A..B or A...B form to make it stand out, but the "->"
between the two "main" in your example must stay as is. That part
of the output shows the correspondence between our ref and their
ref, and has nothing to do with what object their ref originally was
pointing at and what object their ref points at now. There is no
choice of the sign there, but even if there were a choice [*], it
should not be influenced by how the pair of <old,new> objects
involved in the update are related.
Side note: we could imagine using a different sign other
than "->" if you update a branch from a non-ref
(e.g. pushing the freshly made commit on detached HEAD state
to update a branch over there), or perhaps updating a ref
with a ref from different hierarchies (e.g. push the tip of
the current branch to update their refs/tags/v1.2.3).
Perhaps use "%" as the type and show just a single object name in
summary? I.e. something like the first one in this example:
% 4f9b731bde master -> main
1203cff8ae..6e790dbe36 next -> next
+ b7485789d7...3e580ca942 seen -> seen
> We may also want to consider what happens when both --force and this
> new option are used at the same time. When testing, the message was
> always "up-to-date", but I realize now that a server might report it
> as a forced update - it would be odd, but would it be impossible?
If I recall correctly what happens at the protocol level (without
looking at the code---look for comment "Finally.*tell the other end"
to find it out where it is), I do not think the receiving end can
even tell, because the only thing they see for each ref is the old
and the new object name and the refname. There are intricate
mechanism among the sender (push/send-pack), the receiver
(receive-pack), and the transport helpers, to carry necessary info
around to produce a report that says "this ref update went OK" and
"this ref update was NG, due to such and such reason", but I do not
think the receiving end does not compute the forcedness for
reporting. It only runs in_merge_bases() between old and new
objects to decide if it is a non-fast-forward, when configured to
reject such updates, and it may report the failure was due to
non-fast-forward condition, but I do not think there is more than
one kind of successful updates reported (they just say "ok").
Speaking of what happens on the receiving end, we may have to
consider what, if anything, we need to do to the hooks that run
there, which gets "the ref X was updated from object A to object B".
They never saw "object A to object A" update and may get confused.
next prev parent reply other threads:[~2024-03-15 16:43 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-12 21:55 [PATCH 2/2] Add transport message for up-to-date references Christopher Lindee
2024-03-14 0:07 ` Junio C Hamano
2024-03-15 6:09 ` Christopher Lindee
2024-03-15 6:47 ` Christopher Lindee
2024-03-15 16:49 ` Junio C Hamano
2024-03-15 16:42 ` Junio C Hamano [this message]
2024-03-14 21:44 ` Martin Ågren
2024-03-15 3:09 ` Christopher Lindee
2024-03-15 16:54 ` 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=xmqqil1nsay5.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=christopher.lindee@webpros.com \
--cc=git@vger.kernel.org \
/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).