[PATCH] upload-pack: fix exit code when denying fetch of unreachable object ID

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2534 bytes --]

In 7ba7c52d76 (upload-pack: fix race condition in error messages,
2023-08-10), we have fixed a race in t5516-fetch-push.sh where sometimes
error messages got intermingled. This was done by splitting up the call
to `die()` such that we print the error message before writing to the
remote side, followed by a call to `exit(1)` afterwards.

This causes a subtle regression though as `die()` causes us to exit with
exit code 128, whereas we now call `exit(1)`. It's not really clear
whether we want to guarantee any specific error code in this case, and
neither do we document anything like that. But on the other hand, it
seems rather clear that this is an unintended side effect of the change
given that this change in behaviour was not mentioned at all.

Fix this regression by exiting with 128 again and tighten one of our
tests to catch such unintended side effects.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---

We have found this issue in our CI pipeline in Gitaly, which explicitly
checks for the error code. It is very much debatable whether we should
even be doing that in the first place given that error codes are not
even explicitly documented here. But I think this is worth fixing anyway
given that it seems like an unintended side effect to me and might be
biting others, as well.

If you folks agree with my reasoning, then I think we should pick this
up before Git v2.42.0 is released.

Patrick

 t/t5703-upload-pack-ref-in-want.sh | 2 +-
 upload-pack.c                      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t5703-upload-pack-ref-in-want.sh b/t/t5703-upload-pack-ref-in-want.sh
index df74f80061..fe6e056700 100755
--- a/t/t5703-upload-pack-ref-in-want.sh
+++ b/t/t5703-upload-pack-ref-in-want.sh
@@ -483,7 +483,7 @@ test_expect_success 'server is initially ahead - no ref in want' '
 	rm -rf local &&
 	cp -r "$LOCAL_PRISTINE" local &&
 	inconsistency main $(test_oid numeric) &&
-	test_must_fail git -C local fetch 2>err &&
+	test_expect_code 128 git -C local fetch 2>err &&
 	test_i18ngrep "fatal: remote error: upload-pack: not our ref" err
 '
 
diff --git a/upload-pack.c b/upload-pack.c
index ece111c629..15f3318d6d 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -782,7 +782,7 @@ static void check_non_tip(struct upload_pack_data *data)
 			packet_writer_error(&data->writer,
 					    "upload-pack: not our ref %s",
 					    oid_to_hex(&o->oid));
-			exit(1);
+			exit(128);
 		}
 	}
 }
-- 
2.41.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]