git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 2/2] fetch: no redundant error message for atomic fetch

From
Jiang Xin <worldhello.net@gmail.com>
Date
Dec 14, 2023, 12:33 UTC
Message-ID
<6fb83a00000563a79f3948f9087c634ae507b9f5.1702556642.git.zhiyou.jx@alibaba-inc.com>
In-Reply-To
<cover.1702556642.git.zhiyou.jx@alibaba-inc.com>
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

If an error occurs during an atomic fetch, a redundant error 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.

We can remove the redundant error() function, because we know that the function ref_transaction_abort() never fails. While we can find a common pattern for calling ref_transaction_abort() by running command "git grep -A1 ref_transaction_abort", e.g.:

    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.

Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
 builtin/fetch.c         | 4 +---
 t/t5574-fetch-output.sh | 2 +-
 2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index fd134ba74d..01a573cf8d 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1775,10 +1775,8 @@ 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);
-	}
 
 	display_state_release(&display_state);
 	close_fetch_head(&fetch_head);
diff --git a/t/t5574-fetch-output.sh b/t/t5574-fetch-output.sh
index bc747efefc..8d01e36b3d 100755
--- a/t/t5574-fetch-output.sh
+++ b/t/t5574-fetch-output.sh
@@ -98,7 +98,7 @@ do
 		opt=
 		;;
 	esac
-	test_expect_failure "fetch porcelain output ${opt:+(atomic)}" '
+	test_expect_success "fetch porcelain output ${opt:+(atomic)}" '
 		test_when_finished "rm -rf porcelain" &&
 
 		# Clone and pre-seed the repositories. We fetch references into two
-- 
2.41.0.232.g2f6f0bca4f.agit.8.0.4.dev
Previous: Junio C HamanoNext: Patrick Steinhardt
Message 14 of 19 in “t5574: test porcelain output of atomic fetch”
  1. 1/2 t5574: test porcelain output of atomic fetchJiang Xin, Oct 19, 2023
  2. 2/2 fetch: no redundant error message for atomic fetchJiang Xin, Oct 19, 2023
  3. Patrick SteinhardtOct 23, 2023
  4. Jiang XinOct 23, 2023
  5. Patrick SteinhardtOct 23, 2023
  6. Jiang XinOct 23, 2023
  7. Patrick SteinhardtOct 25, 2023
  8. Junio C HamanoOct 24, 2023
  9. 0/2 jx/fetch-atomic-error-message-fixJiang Xin, Dec 14, 2023
  10. 1/2 t5574: test porcelain output of atomic fetchJiang Xin, Dec 14, 2023
  11. Patrick SteinhardtDec 15, 2023
  12. Jiang XinDec 15, 2023
  13. Junio C HamanoDec 15, 2023
  14. 2/2 fetch: no redundant error message for atomic fetchJiang Xin, Dec 14, 2023
  15. Patrick SteinhardtDec 15, 2023
  16. 0/2 fix fetch atomic error messageJiang Xin, Dec 17, 2023
  17. 1/2 t5574: test porcelain output of atomic fetchJiang Xin, Dec 17, 2023
  18. 2/2 fetch: no redundant error message for atomic fetchJiang Xin, Dec 17, 2023
  19. Patrick SteinhardtDec 18, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.