send-pack: gracefully close the connection for atomic push
Patrick reported an issue that the exit code of git-receive-pack(1) is ignored during atomic push with "--porcelain" flag, and added new test cases in t5543. This issue originated from commitmaint7dcbeaa0df
(send-pack: fix inconsistent porcelain output, 2020-04-17). At that time, I chose to ignore the exit code of "finish_connect()" without investigating the root cause of the abnormal termination of git-receive-pack. That was an incorrect solution. The root cause is that an atomic push operation terminates early without sending a flush packet to git-receive-pack. As a result, git-receive-pack continues waiting for commands without exiting. By sending a flush packet at the appropriate location in "send_pack()", we ensure that the git-receive-pack process closes properly, avoiding an erroneous exit code for git-push. At the same time, revert the changes to the "transport.c" file made in commit7dcbeaa0df
. Reported-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
parent
60c208db58
commit
b81f8c8dd3
|
@ -633,6 +633,7 @@ int send_pack(struct repository *r,
|
|||
error("atomic push failed for ref %s. status: %d",
|
||||
ref->name, ref->status);
|
||||
ret = ERROR_SEND_PACK_BAD_REF_STATUS;
|
||||
packet_flush(out);
|
||||
goto out;
|
||||
}
|
||||
/* else fallthrough */
|
||||
|
|
|
@ -280,7 +280,7 @@ test_expect_success 'atomic push reports (reject by non-ff)' '
|
|||
test_cmp expect actual
|
||||
'
|
||||
|
||||
test_expect_failure 'atomic push reports exit code failure' '
|
||||
test_expect_success 'atomic push reports exit code failure' '
|
||||
write_script receive-pack-wrapper <<-\EOF &&
|
||||
git-receive-pack "$@"
|
||||
exit 1
|
||||
|
@ -296,7 +296,7 @@ test_expect_failure 'atomic push reports exit code failure' '
|
|||
test_cmp expect err
|
||||
'
|
||||
|
||||
test_expect_failure 'atomic push reports exit code failure with porcelain' '
|
||||
test_expect_success 'atomic push reports exit code failure with porcelain' '
|
||||
write_script receive-pack-wrapper <<-\EOF &&
|
||||
git-receive-pack "$@"
|
||||
exit 1
|
||||
|
|
10
transport.c
10
transport.c
|
@ -948,15 +948,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
|
|||
|
||||
close(data->fd[1]);
|
||||
close(data->fd[0]);
|
||||
/*
|
||||
* Atomic push may abort the connection early and close the pipe,
|
||||
* which may cause an error for `finish_connect()`. Ignore this error
|
||||
* for atomic git-push.
|
||||
*/
|
||||
if (ret || args.atomic)
|
||||
finish_connect(data->conn);
|
||||
else
|
||||
ret = finish_connect(data->conn);
|
||||
ret |= finish_connect(data->conn);
|
||||
data->conn = NULL;
|
||||
data->finished_handshake = 0;
|
||||
|
||||
|
|
Loading…
Reference in New Issue