Skip to content

Commit b81f8c8

Browse files
jiangxingitster
authored andcommitted
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 commit 7dcbeaa (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 commit 7dcbeaa. Reported-by: Patrick Steinhardt <[email protected]> Signed-off-by: Jiang Xin <[email protected]> Signed-off-by: Patrick Steinhardt <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 60c208d commit b81f8c8

File tree

3 files changed

+4
-11
lines changed

3 files changed

+4
-11
lines changed

send-pack.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -633,6 +633,7 @@ int send_pack(struct repository *r,
633633
error("atomic push failed for ref %s. status: %d",
634634
ref->name, ref->status);
635635
ret = ERROR_SEND_PACK_BAD_REF_STATUS;
636+
packet_flush(out);
636637
goto out;
637638
}
638639
/* else fallthrough */

t/t5543-atomic-push.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ test_expect_success 'atomic push reports (reject by non-ff)' '
280280
test_cmp expect actual
281281
'
282282

283-
test_expect_failure 'atomic push reports exit code failure' '
283+
test_expect_success 'atomic push reports exit code failure' '
284284
write_script receive-pack-wrapper <<-\EOF &&
285285
git-receive-pack "$@"
286286
exit 1
@@ -296,7 +296,7 @@ test_expect_failure 'atomic push reports exit code failure' '
296296
test_cmp expect err
297297
'
298298

299-
test_expect_failure 'atomic push reports exit code failure with porcelain' '
299+
test_expect_success 'atomic push reports exit code failure with porcelain' '
300300
write_script receive-pack-wrapper <<-\EOF &&
301301
git-receive-pack "$@"
302302
exit 1

transport.c

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -948,15 +948,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
948948

949949
close(data->fd[1]);
950950
close(data->fd[0]);
951-
/*
952-
* Atomic push may abort the connection early and close the pipe,
953-
* which may cause an error for `finish_connect()`. Ignore this error
954-
* for atomic git-push.
955-
*/
956-
if (ret || args.atomic)
957-
finish_connect(data->conn);
958-
else
959-
ret = finish_connect(data->conn);
951+
ret |= finish_connect(data->conn);
960952
data->conn = NULL;
961953
data->finished_handshake = 0;
962954

0 commit comments

Comments
 (0)