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

[PATCH v2 4/4] bisect: consistently write BISECT_EXPECTED_REV via the refdb

From
Patrick Steinhardt <ps@pks.im>
Date
Dec 12, 2023, 07:19 UTC
Message-ID
<4a4447a3f5ba9ddef1c81703ef9231ee597c2946.1702365291.git.ps@pks.im>
In-Reply-To
<cover.1702365291.git.ps@pks.im>

We're inconsistently writing BISECT_EXPECTED_REV both via the filesystem and via the refdb, which violates the newly established rules for how special refs must be treated. This works alright in practice with the reffiles reference backend, but will cause bugs once we gain additional backends.

Fix this issue and consistently write BISECT_EXPECTED_REV via the refdb so that it is no longer a special ref.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 bisect.c                    | 25 ++++---------------------
 builtin/bisect.c            |  8 ++------
 refs.c                      |  3 ++-
 t/t6030-bisect-porcelain.sh |  2 +-
 4 files changed, 9 insertions(+), 29 deletions(-)
diff --git a/bisect.c b/bisect.c
index 1be8e0a271..985b96ed13 100644
--- a/bisect.c
+++ b/bisect.c
@@ -471,7 +471,6 @@ static int read_bisect_refs(void)
 }
 
 static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
-static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
 static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
 static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
 static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
@@ -707,26 +706,10 @@ static enum bisect_error error_if_skipped_commits(struct commit_list *tried,
 
 static int is_expected_rev(const struct object_id *oid)
 {
-	const char *filename = git_path_bisect_expected_rev();
-	struct stat st;
-	struct strbuf str = STRBUF_INIT;
-	FILE *fp;
-	int res = 0;
-
-	if (stat(filename, &st) || !S_ISREG(st.st_mode))
+	struct object_id expected_oid;
+	if (read_ref("BISECT_EXPECTED_REV", &expected_oid))
 		return 0;
-
-	fp = fopen_or_warn(filename, "r");
-	if (!fp)
-		return 0;
-
-	if (strbuf_getline_lf(&str, fp) != EOF)
-		res = !strcmp(str.buf, oid_to_hex(oid));
-
-	strbuf_release(&str);
-	fclose(fp);
-
-	return res;
+	return oideq(oid, &expected_oid);
 }
 
 enum bisect_error bisect_checkout(const struct object_id *bisect_rev,
@@ -1185,10 +1168,10 @@ int bisect_clean_state(void)
 	struct string_list refs_for_removal = STRING_LIST_INIT_NODUP;
 	for_each_ref_in("refs/bisect", mark_for_removal, (void *) &refs_for_removal);
 	string_list_append(&refs_for_removal, xstrdup("BISECT_HEAD"));
+	string_list_append(&refs_for_removal, xstrdup("BISECT_EXPECTED_REV"));
 	result = delete_refs("bisect: remove", &refs_for_removal, REF_NO_DEREF);
 	refs_for_removal.strdup_strings = 1;
 	string_list_clear(&refs_for_removal, 0);
-	unlink_or_warn(git_path_bisect_expected_rev());
 	unlink_or_warn(git_path_bisect_ancestors_ok());
 	unlink_or_warn(git_path_bisect_log());
 	unlink_or_warn(git_path_bisect_names());
diff --git a/builtin/bisect.c b/builtin/bisect.c
index 35938b05fd..4e2c43daf5 100644
--- a/builtin/bisect.c
+++ b/builtin/bisect.c
@@ -17,7 +17,6 @@
 #include "revision.h"
 
 static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
-static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
 static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
 static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
 static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
@@ -921,7 +920,6 @@ static enum bisect_error bisect_state(struct bisect_terms *terms, int argc,
 	const char *state;
 	int i, verify_expected = 1;
 	struct object_id oid, expected;
-	struct strbuf buf = STRBUF_INIT;
 	struct oid_array revs = OID_ARRAY_INIT;
 
 	if (!argc)
@@ -976,10 +974,8 @@ static enum bisect_error bisect_state(struct bisect_terms *terms, int argc,
 		oid_array_append(&revs, &commit->object.oid);
 	}
 
-	if (strbuf_read_file(&buf, git_path_bisect_expected_rev(), 0) < the_hash_algo->hexsz ||
-	    get_oid_hex(buf.buf, &expected) < 0)
+	if (read_ref("BISECT_EXPECTED_REV", &expected))
 		verify_expected = 0; /* Ignore invalid file contents */
-	strbuf_release(&buf);
 
 	for (i = 0; i < revs.nr; i++) {
 		if (bisect_write(state, oid_to_hex(&revs.oid[i]), terms, 0)) {
@@ -988,7 +984,7 @@ static enum bisect_error bisect_state(struct bisect_terms *terms, int argc,
 		}
 		if (verify_expected && !oideq(&revs.oid[i], &expected)) {
 			unlink_or_warn(git_path_bisect_ancestors_ok());
-			unlink_or_warn(git_path_bisect_expected_rev());
+			delete_ref(NULL, "BISECT_EXPECTED_REV", NULL, REF_NO_DEREF);
 			verify_expected = 0;
 		}
 	}
diff --git a/refs.c b/refs.c
index 8fe34d51e4..c76ce86bef 100644
--- a/refs.c
+++ b/refs.c
@@ -1840,6 +1840,8 @@ static int is_special_ref(const char *refname)
 	 * There are some exceptions that you might expect to see on this list
 	 * but which are handled exclusively via the reference backend:
 	 *
+	 * - BISECT_EXPECTED_REV
+	 *
 	 * - CHERRY_PICK_HEAD
 	 *
 	 * - HEAD
@@ -1857,7 +1859,6 @@ static int is_special_ref(const char *refname)
 	 */
 	static const char * const special_refs[] = {
 		"AUTO_MERGE",
-		"BISECT_EXPECTED_REV",
 		"FETCH_HEAD",
 		"MERGE_AUTOSTASH",
 		"MERGE_HEAD",
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 2a5b7d8379..792c1504bc 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -1176,7 +1176,7 @@ test_expect_success 'git bisect reset cleans bisection state properly' '
 	git bisect bad $HASH4 &&
 	git bisect reset &&
 	test -z "$(git for-each-ref "refs/bisect/*")" &&
-	test_path_is_missing ".git/BISECT_EXPECTED_REV" &&
+	test_ref_missing BISECT_EXPECTED_REV &&
 	test_path_is_missing ".git/BISECT_ANCESTORS_OK" &&
 	test_path_is_missing ".git/BISECT_LOG" &&
 	test_path_is_missing ".git/BISECT_RUN" &&

-- 
2.43.GIT
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 34 of 41 in “refs: improve handling of special refs”
  1. 0/4 refs: improve handling of special refsPatrick Steinhardt, Nov 29, 2023
  2. 1/4 wt-status: read HEAD and ORIG_HEAD via the refdbPatrick Steinhardt, Nov 29, 2023
  3. Taylor BlauNov 29, 2023
  4. Patrick SteinhardtNov 30, 2023
  5. Taylor BlauNov 30, 2023
  6. 2/4 refs: propagate errno when reading special refs failsPatrick Steinhardt, Nov 29, 2023
  7. Taylor BlauNov 29, 2023
  8. Patrick SteinhardtNov 30, 2023
  9. 3/4 refs: complete list of special refsPatrick Steinhardt, Nov 29, 2023
  10. Taylor BlauNov 29, 2023
  11. Patrick SteinhardtNov 30, 2023
  12. Phillip WoodNov 30, 2023
  13. Patrick SteinhardtDec 1, 2023
  14. Phillip WoodDec 4, 2023
  15. 4/4 bisect: consistently write BISECT_EXPECTED_REV via the refdbPatrick Steinhardt, Nov 29, 2023
  16. Taylor BlauNov 29, 2023
  17. Taylor BlauNov 29, 2023
  18. Patrick SteinhardtNov 30, 2023
  19. Taylor BlauNov 30, 2023
  20. 0/4 refs: improve handling of special refsPatrick Steinhardt, Dec 12, 2023
  21. 1/4 wt-status: read HEAD and ORIG_HEAD via the refdbPatrick Steinhardt, Dec 12, 2023
  22. Junio C HamanoDec 12, 2023
  23. Ramsay JonesDec 12, 2023
  24. Junio C HamanoDec 13, 2023
  25. Patrick SteinhardtDec 13, 2023
  26. Junio C HamanoDec 13, 2023
  27. Patrick SteinhardtDec 14, 2023
  28. Junio C HamanoDec 14, 2023
  29. Patrick SteinhardtDec 14, 2023
  30. 2/4 refs: propagate errno when reading special refs failsPatrick Steinhardt, Dec 12, 2023
  31. Junio C HamanoDec 12, 2023
  32. Patrick SteinhardtDec 13, 2023
  33. 3/4 refs: complete list of special refsPatrick Steinhardt, Dec 12, 2023
  34. 4/4 bisect: consistently write BISECT_EXPECTED_REV via the refdbPatrick Steinhardt, Dec 12, 2023
  35. 0/4 refs: improve handling of special refsPatrick Steinhardt, Dec 14, 2023
  36. 1/4 wt-status: read HEAD and ORIG_HEAD via the refdbPatrick Steinhardt, Dec 14, 2023
  37. 2/4 refs: propagate errno when reading special refs failsPatrick Steinhardt, Dec 14, 2023
  38. 3/4 refs: complete list of special refsPatrick Steinhardt, Dec 14, 2023
  39. 4/4 bisect: consistently write BISECT_EXPECTED_REV via the refdbPatrick Steinhardt, Dec 14, 2023
  40. Junio C HamanoDec 20, 2023
  41. Patrick SteinhardtDec 21, 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.