Subject: [PATCH v2 0/4] refs: improve handling of special refs

From: Patrick Steinhardt <[email-masked]>
Date: Tue, 12 Dec 2023 08:18:44 +0100

Hi,

this is the second version of my patch series that aims to improve our
handling of special refs. This patch series is in preparation for the
upcoming reftable backend.

Changes compared to v1:

  - Patch 1: Stop needlessly resetting `errno` before calling
    `strbuf_read_file()`. Also, clean up state we create in our test
    repos properly.

  - Patch 3: The discussion with Phillip made me reevaluate my claim
    that "rebase-apply/" and "rebase-merge/" contain special refs.
    Indeed they don't, all files in there seem to be exclusively
    read and written via the filesystem without ever going via the
    refdb. I've thus dropped them from the set of special refs.

  - Patch 4: The array of static refs is now static.

Thanks for your reviews!

Patrick

Patrick Steinhardt (4):
  wt-status: read HEAD and ORIG_HEAD via the refdb
  refs: propagate errno when reading special refs fails
  refs: complete list of special refs
  bisect: consistently write BISECT_EXPECTED_REV via the refdb

 bisect.c                    | 25 +++-------------
 builtin/bisect.c            |  8 ++---
 refs.c                      | 59 +++++++++++++++++++++++++++++++++++--
 t/t1403-show-ref.sh         | 10 +++++++
 t/t6030-bisect-porcelain.sh |  2 +-
 wt-status.c                 | 17 ++++++-----
 6 files changed, 82 insertions(+), 39 deletions(-)

Range-diff against v1:
1:  35b74eb972 = 1:  1db3eb3945 wt-status: read HEAD and ORIG_HEAD via the refdb
2:  691552a17e ! 2:  24032a62e5 refs: propagate errno when reading special refs fails
    @@ refs.c: static int refs_read_special_head(struct ref_store *ref_store,
      	strbuf_addf(&full_path, "%s/%s", ref_store->gitdir, refname);
      
     -	if (strbuf_read_file(&content, full_path.buf, 0) < 0)
    -+	errno = 0;
    -+
     +	if (strbuf_read_file(&content, full_path.buf, 0) < 0) {
     +		*failure_errno = errno;
      		goto done;
    @@ t/t1403-show-ref.sh: test_expect_success '--exists with directory fails with gen
     +'
     +
     +test_expect_success '--exists with existing special ref' '
    ++	test_when_finished "rm .git/FETCH_HEAD" &&
     +	git rev-parse HEAD >.git/FETCH_HEAD &&
     +	git show-ref --exists FETCH_HEAD
     +'
3:  0e38103114 ! 3:  3dd9089fd5 refs: complete list of special refs
    @@ refs.c: static int refs_read_special_head(struct ref_store *ref_store,
     +	 * - MERGE_HEAD may contain multiple object IDs when merging multiple
     +	 *   heads.
     +	 *
    -+	 * - "rebase-apply/" and "rebase-merge/" contain all of the state for
    -+	 *   rebases, where keeping it closely together feels sensible.
    -+	 *
     +	 * There are some exceptions that you might expect to see on this list
     +	 * but which are handled exclusively via the reference backend:
     +	 *
     +	 * - CHERRY_PICK_HEAD
    ++	 *
     +	 * - HEAD
    ++	 *
     +	 * - ORIG_HEAD
     +	 *
    ++	 * - "rebase-apply/" and "rebase-merge/" contain all of the state for
    ++	 *   rebases, including some reference-like files. These are
    ++	 *   exclusively read and written via the filesystem and never go
    ++	 *   through the refdb.
    ++	 *
     +	 * Writing or deleting references must consistently go either through
     +	 * the filesystem (special refs) or through the reference backend
     +	 * (normal ones).
     +	 */
    -+	const char * const special_refs[] = {
    ++	static const char * const special_refs[] = {
     +		"AUTO_MERGE",
     +		"BISECT_EXPECTED_REV",
     +		"FETCH_HEAD",
     +		"MERGE_AUTOSTASH",
     +		"MERGE_HEAD",
     +	};
    -+	int i;
    ++	size_t i;
     +
     +	for (i = 0; i < ARRAY_SIZE(special_refs); i++)
     +		if (!strcmp(refname, special_refs[i]))
     +			return 1;
     +
    -+	/*
    -+	 * git-rebase(1) stores its state in `rebase-apply/` or
    -+	 * `rebase-merge/`, including various reference-like bits.
    -+	 */
    -+	if (starts_with(refname, "rebase-apply/") ||
    -+	    starts_with(refname, "rebase-merge/"))
    -+		return 1;
    -+
     +	return 0;
     +}
     +
4:  c7ab26fb7e ! 4:  4a4447a3f5 bisect: consistently write BISECT_EXPECTED_REV via the refdb
    @@ refs.c: static int is_special_ref(const char *refname)
      	 * but which are handled exclusively via the reference backend:
      	 *
     +	 * - BISECT_EXPECTED_REV
    ++	 *
      	 * - CHERRY_PICK_HEAD
    + 	 *
      	 * - HEAD
    - 	 * - ORIG_HEAD
     @@ refs.c: static int is_special_ref(const char *refname)
      	 */
    - 	const char * const special_refs[] = {
    + 	static const char * const special_refs[] = {
      		"AUTO_MERGE",
     -		"BISECT_EXPECTED_REV",
      		"FETCH_HEAD",

base-commit: 1a87c842ece327d03d08096395969aca5e0a6996
-- 
2.43.GIT