bisect: consistently write BISECT_EXPECTED_REV via the refdb

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Dec 14, 2023 at 14:37 UTC 0a06892ddde5bc3a82a4fe2963e3ea294252ffdd
4 files changed +9 -29
bisect.c
+4 -21
@@ -471,7 +471,6 @@ static int read_bisect_refs(void)
471 }
472
473 static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
474 -static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
474 static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
475 static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
476 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,
706
707 static int is_expected_rev(const struct object_id *oid)
708 {
710 - const char *filename = git_path_bisect_expected_rev();
711 - struct stat st;
712 - struct strbuf str = STRBUF_INIT;
713 - FILE *fp;
714 - int res = 0;
715 -
716 - if (stat(filename, &st) || !S_ISREG(st.st_mode))
709 + struct object_id expected_oid;
710 + if (read_ref("BISECT_EXPECTED_REV", &expected_oid))
711 return 0;
718 -
719 - fp = fopen_or_warn(filename, "r");
720 - if (!fp)
721 - return 0;
722 -
723 - if (strbuf_getline_lf(&str, fp) != EOF)
724 - res = !strcmp(str.buf, oid_to_hex(oid));
725 -
726 - strbuf_release(&str);
727 - fclose(fp);
728 -
729 - return res;
712 + return oideq(oid, &expected_oid);
713 }
714
715 enum bisect_error bisect_checkout(const struct object_id *bisect_rev,
@@ -1185,10 +1168,10 @@ int bisect_clean_state(void)
1168 struct string_list refs_for_removal = STRING_LIST_INIT_NODUP;
1169 for_each_ref_in("refs/bisect", mark_for_removal, (void *) &refs_for_removal);
1170 string_list_append(&refs_for_removal, xstrdup("BISECT_HEAD"));
1171 + string_list_append(&refs_for_removal, xstrdup("BISECT_EXPECTED_REV"));
1172 result = delete_refs("bisect: remove", &refs_for_removal, REF_NO_DEREF);
1173 refs_for_removal.strdup_strings = 1;
1174 string_list_clear(&refs_for_removal, 0);
1191 - unlink_or_warn(git_path_bisect_expected_rev());
1175 unlink_or_warn(git_path_bisect_ancestors_ok());
1176 unlink_or_warn(git_path_bisect_log());
1177 unlink_or_warn(git_path_bisect_names());
builtin/bisect.c
+2 -6
@@ -17,7 +17,6 @@
17 #include "revision.h"
18
19 static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
20 -static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
20 static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
21 static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
22 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,
920 const char *state;
921 int i, verify_expected = 1;
922 struct object_id oid, expected;
924 - struct strbuf buf = STRBUF_INIT;
923 struct oid_array revs = OID_ARRAY_INIT;
924
925 if (!argc)
@@ -976,10 +974,8 @@ static enum bisect_error bisect_state(struct bisect_terms *terms, int argc,
974 oid_array_append(&revs, &commit->object.oid);
975 }
976
979 - if (strbuf_read_file(&buf, git_path_bisect_expected_rev(), 0) < the_hash_algo->hexsz ||
980 - get_oid_hex(buf.buf, &expected) < 0)
977 + if (read_ref("BISECT_EXPECTED_REV", &expected))
978 verify_expected = 0; /* Ignore invalid file contents */
982 - strbuf_release(&buf);
979
980 for (i = 0; i < revs.nr; i++) {
981 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,
984 }
985 if (verify_expected && !oideq(&revs.oid[i], &expected)) {
986 unlink_or_warn(git_path_bisect_ancestors_ok());
991 - unlink_or_warn(git_path_bisect_expected_rev());
987 + delete_ref(NULL, "BISECT_EXPECTED_REV", NULL, REF_NO_DEREF);
988 verify_expected = 0;
989 }
990 }
refs.c
+2 -1
@@ -1840,6 +1840,8 @@ static int is_special_ref(const char *refname)
1840 * There are some exceptions that you might expect to see on this list
1841 * but which are handled exclusively via the reference backend:
1842 *
1843 + * - BISECT_EXPECTED_REV
1844 + *
1845 * - CHERRY_PICK_HEAD
1846 *
1847 * - HEAD
@@ -1857,7 +1859,6 @@ static int is_special_ref(const char *refname)
1859 */
1860 static const char * const special_refs[] = {
1861 "AUTO_MERGE",
1860 - "BISECT_EXPECTED_REV",
1862 "FETCH_HEAD",
1863 "MERGE_AUTOSTASH",
1864 "MERGE_HEAD",
t/t6030-bisect-porcelain.sh
+1 -1
@@ -1176,7 +1176,7 @@ test_expect_success 'git bisect reset cleans bisection state properly' '
1176 git bisect bad $HASH4 &&
1177 git bisect reset &&
1178 test -z "$(git for-each-ref "refs/bisect/*")" &&
1179 - test_path_is_missing ".git/BISECT_EXPECTED_REV" &&
1179 + test_ref_missing BISECT_EXPECTED_REV &&
1180 test_path_is_missing ".git/BISECT_ANCESTORS_OK" &&
1181 test_path_is_missing ".git/BISECT_LOG" &&
1182 test_path_is_missing ".git/BISECT_RUN" &&