From: Jiang Xin <zhiyou.jx@xxxxxxxxxxxxxxx>
# Changes since v2:
Changed the patches with help from Patrick.
# range-diff v2...v3
1: 0b9865f1df ! 1: 0a6c53de7c t5574: test porcelain output of atomic fetch
@@ Commit message
In a later commit, we will fix this issue.
+ Helped-by: Patrick Steinhardt <ps@xxxxxx>
Signed-off-by: Jiang Xin <zhiyou.jx@xxxxxxxxxxxxxxx>
## t/t5574-fetch-output.sh ##
@@ t/t5574-fetch-output.sh: test_expect_success 'fetch compact output' '
@@ t/t5574-fetch-output.sh: test_expect_success 'fetch porcelain output' '
+ FORCE_UPDATED_NEW=$(git rev-parse HEAD)
'
-+for opt in off on
++for opt in "" "--atomic"
+do
-+ case $opt in
-+ on)
-+ opt=--atomic
-+ ;;
-+ off)
-+ opt=
-+ ;;
-+ esac
+ test_expect_failure "fetch porcelain output ${opt:+(atomic)}" '
+ test_when_finished "rm -rf porcelain" &&
+
2: e10fa198dd ! 2: a8a7658fb2 fetch: no redundant error message for atomic fetch
@@ Commit message
will appear at the end of do_fetch(). It was introduced in b3a804663c
(fetch: make `--atomic` flag cover backfilling of tags, 2022-02-17).
- In function do_fetch(), a failure message is already shown before the
- retcode is set, so we should not call additional error() at the end of
- this function.
+ Because a failure message is displayed before setting retcode in the
+ function do_fetch(), calling error() on the err message at the end of
+ this function may result in redundant or empty error message to be
+ displayed.
We can remove the redundant error() function, because we know that
the function ref_transaction_abort() never fails. While we can find a
@@ Commit message
if (ref_transaction_abort(transaction, &error))
error("abort: %s", error.buf);
- We can fix this issue follow this pattern, and the test case "fetch
- porcelain output (atomic)" in t5574 will also be fixed. If in the future
- we decide that we don't need to check the return value of the function
- ref_transaction_abort(), this change can be fixed along with it.
+ Following this pattern, we can tolerate the return value of the function
+ ref_transaction_abort() being changed in the future. We also delay the
+ output of the err message to the end of do_fetch() to reduce redundant
+ code. With these changes, the test case "fetch porcelain output
+ (atomic)" in t5574 will also be fixed.
+ Helped-by: Patrick Steinhardt <ps@xxxxxx>
Signed-off-by: Jiang Xin <zhiyou.jx@xxxxxxxxxxxxxxx>
## builtin/fetch.c ##
+@@ builtin/fetch.c: static int do_fetch(struct transport *transport,
+ if (atomic_fetch) {
+ transaction = ref_transaction_begin(&err);
+ if (!transaction) {
+- retcode = error("%s", err.buf);
++ retcode = -1;
+ goto cleanup;
+ }
+ }
+@@ builtin/fetch.c: static int do_fetch(struct transport *transport,
+
+ retcode = ref_transaction_commit(transaction, &err);
+ if (retcode) {
+- error("%s", err.buf);
+ ref_transaction_free(transaction);
+ transaction = NULL;
+ goto cleanup;
@@ builtin/fetch.c: static int do_fetch(struct transport *transport,
}
cleanup:
- if (retcode && transaction) {
- ref_transaction_abort(transaction, &err);
-+ if (retcode && transaction && ref_transaction_abort(transaction, &err))
- error("%s", err.buf);
-- }
+- error("%s", err.buf);
++ if (retcode) {
++ if (err.len) {
++ error("%s", err.buf);
++ strbuf_reset(&err);
++ }
++ if (transaction && ref_transaction_abort(transaction, &err) &&
++ err.len)
++ error("%s", err.buf);
+ }
display_state_release(&display_state);
Jiang Xin (2):
t5574: test porcelain output of atomic fetch
fetch: no redundant error message for atomic fetch
builtin/fetch.c | 14 ++++---
t/t5574-fetch-output.sh | 89 +++++++++++++++++++++++------------------
2 files changed, 59 insertions(+), 44 deletions(-)
--
2.41.0.232.g2f6f0bca4f.agit.8.0.4.dev
