Re: [PATCH 0/2] Optionally support push options on up-to-date branches

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 13, 2024, 22:58 UTC
Message-ID: <xmqqa5n168nh.fsf@gitster.g>
In-Reply-To: <SA1PR14MB4691B63C740F95D7D8A495628D2B2@SA1PR14MB4691.namprd14.prod.outlook.com>

Christopher Lindee <christopher.lindee@webpros.com> writes:

> Some Git servers can take actions on a branch using push options; to do this,
> the branch ref must be passed along with the push options.  However, if the
> branch is up-to-date, then Git submits push options without any refs, so the
> server is not given a branch to act upon and the push options become no-ops.

Yeah, the send-pack/receive-pack were written in the simpler days back when "pushing the same object again to the ref was an absolute no-op", and push options that breaks the expectation did not exist.

It makes sense to allow a (seemingly no-op) push to go through with an option.

And even without custom push option, recording this seemingly no-op push as a ref "update" does make sense--push certificate records what object the pusher wanted each target ref to have, and omitting a ref from the record only because the ref was already pointing at the desired object is losing information.

So I doubly agree with the reasoning beind this change.

> This changeset proposes to address this issue by adding an option to `push` and
> `send-pack` that, when specified, will send refs where the old-oid and new-oid

"where" -> "even if"

Show 6 quoted lines

> are identical - instead of silently skipping these refs.  The first commit
> introduces the `--send-up-to-date` option to toggle this behavior, while the
> second commit updates the commands to output an `(up-to-date)` notice for each
> branch with an identical old-oid and new-oid.
>
> Notably, the `--force` option will not send a ref when the remote is up-to-date.

And it makes sense *not* to update `--force` to do the no-op push, becaues you may not want to (accidentally) force push a ref that does not fast-forward. As I already said, tying this with use of the "-o" option is not sufficient. So I agree we may want a new option to trigger this behaviour.

A radical counter-proposal for the design is to update the client side to do this unconditionally, without needing any new option. For an already up-to-date ref, its only contribution to the cost of "git push" is from its "Finally, tell the other end!" instruction, which is in the order of 100 bytes per ref, and it should not add to the pack generation cost at all [*].

    Side note: But we have to be careful---if the receiving end is
    totally up-to-date and there is absolutely no ref, I think the
    current code will not even call pack_objects(), and you'd
    probably want a similar optimization to avoid the cost to spawn
    pack_objects().

I do not know if "send-up-to-date" is a great name for the option, though.

Show 18 quoted lines

>
> Chris Lindee (2):
>   Teach send-pack & push to --send-up-to-date refs
>   Add transport message for up-to-date references
>
>  Documentation/git-push.txt      | 8 +++++++-
>  Documentation/git-send-pack.txt | 7 +++++++
>  builtin/push.c                  | 1 +
>  builtin/send-pack.c             | 4 ++++
>  send-pack.c                     | 2 +-
>  send-pack.h                     | 3 ++-
>  transport-helper.c              | 7 ++++++-
>  transport.c                     | 3 +++
>  transport.h                     | 1 +
>  9 files changed, 32 insertions(+), 4 deletions(-)
>         
>
> base-commit: 3c2a3fdc388747b9eaf4a4a4f2035c1c9ddb26d0