On Wed, Nov 13, 2024 at 7:25 PM Patrick Steinhardt <ps@xxxxxx> wrote:
>
> When executing git-push(1) with the "--porcelain" flag, then we will
> print updated references in a machine-readable format that looks like
> this:
>
> To destination
> = refs/heads/noop:refs/heads/noop [up to date]
> ! refs/heads/rejected:refs/heads/rejected [rejected] (atomic push failed)
> ! refs/heads/noff:refs/heads/(off (non-fast-forward)
> Done
>
> The final "Done" stanza was introduced via 77555854be (git-push: make
> git push --porcelain print "Done", 2010-02-26), where it was printed
> "unless any errors have occurred". This behaviour was later changed via
> 7dcbeaa0df (send-pack: fix inconsistent porcelain output, 2020-04-17)
> such that the stanza will also be printed when there was an error with
> atomic pushes, which was previously inconsistent with non-atomic pushes.
> The fixup commit has introduced a new issue though. During atomic pushes
> it is expected that git-receive-pack(1) may exit early, and that causes
> us to receive an error on the client-side. We (seemingly) still want to
> print the "Done" stanza, but given that we only do so when the push has
> been successful we started to ignore the error code by the child process
> completely when doing an atomic push.
I introduced the commit 7dcbeaa0df (send-pack: fix inconsistent porcelain
output, 2020-04-17), because the porcelain output for git push over local
file protocol and HTTP protocol are different and was hard to write the
some test cases to work for both protocols. I acknowledge the patch was
not perfect.
I read the commit 77555854be (git-push: make git push --porcelain
print "Done", 2010-02-26) and the code path of git push over two
protocols a second time, and find something:
The code snippet from 77555854be:
> - ret = transport->push_refs(transport, remote_refs, flags);
> + push_ret = transport->push_refs(transport, remote_refs, flags);
> err = push_had_errors(remote_refs);
> -
> - ret |= err;
> + ret = push_ret | err;
>
The return code "push_ret" of push_refs() from different transport
vtable is not consist. For HTTP protocol, "push_ret" is zero if no
connection error or no protocol errors. So we should consider
“push_ret" as a protocol error rather than a reference update error.
If we want to print "Done" in porcelain mode when there are no
errors. (In dry run mode, ref-update-errors should not be
considered as errors, but the opposite should be regarded as errors).
Instead of using the following code,
> + if (porcelain && !push_ret)
> + puts("Done");
We can use like this ("pretend" is the flag for dry-run mode):
+ ret = pretend ? push_ret : (push_ret | err);
+ if (porcelain && !ret)
+ puts("Done");
I will send patches follow this patch series.
--
Jiang Xin
