[v4,00/10] Clarify pseudo-ref terminology

Message

Patrick Steinhardt May 10, 2024, 8:48 a.m. UTC

Hi,

thi sis the fourth version of my patch series that aims to clarify the
pseudo-ref terminology. Changes compared to v3:

  - Render refs in "Documentation/glossary-context.txt" more
    consistently with backticks, only.

  - Reorder patches 6 and 7 such that we first correct `is_root_ref()`,
    and then adapt `is_headref()`.

  - Furthermore, I have inlined `is_headref()` into `is_root_ref()`
    completely now as it didn't have any users anymore.

Thanks!

Patrick

Patrick Steinhardt (10):
  Documentation/glossary: redefine pseudorefs as special refs
  Documentation/glossary: clarify limitations of pseudorefs
  Documentation/glossary: define root refs as refs
  refs: rename `is_pseudoref()` to `is_root_ref()`
  refs: refname `is_special_ref()` to `is_pseudo_ref()`
  refs: root refs can be symbolic refs
  refs: classify HEAD as a root ref
  refs: pseudorefs are no refs
  ref-filter: properly distinuish pseudo and root refs
  refs: refuse to write pseudorefs

 Documentation/glossary-content.txt |  72 +++++++++----------
 builtin/for-each-ref.c             |   2 +-
 ref-filter.c                       |  16 +++--
 ref-filter.h                       |   4 +-
 refs.c                             | 108 ++++++++++++++---------------
 refs.h                             |  48 ++++++++++++-
 refs/files-backend.c               |   3 +-
 refs/reftable-backend.c            |   3 +-
 t/t5510-fetch.sh                   |   6 +-
 t/t6302-for-each-ref-filter.sh     |  34 +++++++++
 10 files changed, 185 insertions(+), 111 deletions(-)

Range-diff against v3:
 1:  e651bae690 !  1:  b1fc4c1ac7 Documentation/glossary: redefine pseudorefs as special refs
    @@ Documentation/glossary-content.txt: exclude;;
     ++
     +The following pseudorefs are known to Git:
     +
    -+ - "`FETCH_HEAD`" is written by linkgit:git-fetch[1] or linkgit:git-pull[1]. It
    ++ - `FETCH_HEAD` is written by linkgit:git-fetch[1] or linkgit:git-pull[1]. It
     +   may refer to multiple object IDs. Each object ID is annotated with metadata
     +   indicating where it was fetched from and its fetch status.
     +
    -+ - "`MERGE_HEAD`" is written by linkgit:git-merge[1] when resolving merge
    ++ - `MERGE_HEAD` is written by linkgit:git-merge[1] when resolving merge
     +   conflicts. It contains all commit IDs which are being merged.
      
      [[def_pull]]pull::
 2:  66ac046132 =  2:  dce3a0fa7e Documentation/glossary: clarify limitations of pseudorefs
 3:  243d616101 !  3:  79249962f5 Documentation/glossary: define root refs as refs
    @@ Documentation/glossary-content.txt: The following pseudorefs are known to Git:
     +match these rules. The following list is exhaustive and shall not be
     +extended in the future:
     ++
    -+ - AUTO_MERGE
    ++ - `AUTO_MERGE`
     +
    -+ - BISECT_EXPECTED_REV
    ++ - `BISECT_EXPECTED_REV`
     +
    -+ - NOTES_MERGE_PARTIAL
    ++ - `NOTES_MERGE_PARTIAL`
     +
    -+ - NOTES_MERGE_REF
    ++ - `NOTES_MERGE_REF`
     +
    -+ - MERGE_AUTOSTASH
    ++ - `MERGE_AUTOSTASH`
     ++
     +Different subhierarchies are used for different purposes. For example,
     +the `refs/heads/` hierarchy is used to represent local branches whereas
 4:  0a116f9d11 =  4:  ee2b090f75 refs: rename `is_pseudoref()` to `is_root_ref()`
 5:  484a0856bc =  5:  2c09bc7690 refs: refname `is_special_ref()` to `is_pseudo_ref()`
 7:  92a71222e1 !  6:  5e402811a6 refs: root refs can be symbolic refs
    @@ Commit message
         does not seem reasonable at all and I very much doubt that it results in
         anything sane.
     
    -    Furthermore, the behaviour is different to `is_headref()`, which only
    -    checks for the ref to exist. While that is in line with our glossary,
    -    this inconsistency only adds to the confusion.
    -
         Last but not least, the current behaviour can actually lead to a
         segfault when calling `is_root_ref()` with a reference that either does
         not exist or that is a symbolic ref because we never initialized `oid`.
    @@ refs.c: int is_root_ref(struct ref_store *refs, const char *refname)
      	size_t i;
      
      	if (!is_root_ref_syntax(refname))
    -@@ refs.c: int is_root_ref(struct ref_store *refs, const char *refname)
    - 	if (is_headref(refs, refname))
    - 		return 1;
    + 		return 0;
      
     +	/*
     +	 * Note that we cannot use `refs_ref_exists()` here because that also
    @@ refs.c: int is_root_ref(struct ref_store *refs, const char *refname)
      }
      
      int is_headref(struct ref_store *refs, const char *refname)
    - {
    --	if (!strcmp(refname, "HEAD"))
    --		return refs_ref_exists(refs, refname);
    -+	struct strbuf referent = STRBUF_INIT;
    -+	struct object_id oid = { 0 };
    -+	int failure_errno, ret = 0;
    -+	unsigned int flags;
    - 
    --	return 0;
    -+	/*
    -+	 * Note that we cannot use `refs_ref_exists()` here because that also
    -+	 * checks whether its target ref exists in case refname is a symbolic
    -+	 * ref.
    -+	 */
    -+	if (!strcmp(refname, "HEAD")) {
    -+		ret = !refs_read_raw_ref(refs, refname, &oid, &referent,
    -+					 &flags, &failure_errno);
    -+	}
    -+
    -+	strbuf_release(&referent);
    -+	return ret;
    - }
    +
    + ## refs.h ##
    +@@ refs.h: extern struct ref_namespace_info ref_namespace[NAMESPACE__COUNT];
    + void update_ref_namespace(enum ref_namespace namespace, char *ref);
      
    - static int is_current_worktree_ref(const char *ref) {
    + /*
    +- * Check whether the reference is an existing root reference.
    ++ * Check whether the reference is an existing root reference. A root reference
    ++ * that is a dangling symbolic ref is considered to exist.
    +  *
    +  * A root ref is a reference that lives in the root of the reference hierarchy.
    +  * These references must conform to special syntax:
     
      ## t/t6302-for-each-ref-filter.sh ##
     @@ t/t6302-for-each-ref-filter.sh: test_expect_success '--include-root-refs with other patterns' '
 6:  c196fe3c45 !  7:  b32c56afcb refs: classify HEAD as a root ref
    @@ Commit message
         - The "files" and "reftable" backends explicitly called both
           `is_root_ref()` and `is_headref()`.
     
    -    This change should thus essentially be a no-op.
    +    This also aligns behaviour or `is_root_ref()` and `is_headref()` such
    +    that we also return a trueish value when the ref is a dangling symbolic
    +    ref. As there are no callers of `is_headref()` left afer the refactoring
    +    we absorb it completely into `is_root_ref()`.
     
         Signed-off-by: Patrick Steinhardt <[e-mail]>
     
      ## refs.c ##
    +@@ refs.c: static int is_root_ref_syntax(const char *refname)
    + int is_root_ref(struct ref_store *refs, const char *refname)
    + {
    + 	static const char *const irregular_root_refs[] = {
    ++		"HEAD",
    + 		"AUTO_MERGE",
    + 		"BISECT_EXPECTED_REV",
    + 		"NOTES_MERGE_PARTIAL",
     @@ refs.c: int is_root_ref(struct ref_store *refs, const char *refname)
    + 	return ret;
    + }
      
    - 	if (!is_root_ref_syntax(refname))
    - 		return 0;
    -+	if (is_headref(refs, refname))
    -+		return 1;
    - 
    - 	if (ends_with(refname, "_HEAD")) {
    - 		refs_resolve_ref_unsafe(refs, refname,
    +-int is_headref(struct ref_store *refs, const char *refname)
    +-{
    +-	if (!strcmp(refname, "HEAD"))
    +-		return refs_ref_exists(refs, refname);
    +-
    +-	return 0;
    +-}
    +-
    + static int is_current_worktree_ref(const char *ref) {
    + 	return is_root_ref_syntax(ref) || is_per_worktree_ref(ref);
    + }
     
      ## refs.h ##
     @@ refs.h: void update_ref_namespace(enum ref_namespace namespace, char *ref);
    @@ refs.h: void update_ref_namespace(enum ref_namespace namespace, char *ref);
       */
      int is_root_ref(struct ref_store *refs, const char *refname);
      
    -+/*
    -+ * Check whether the reference is "HEAD" and whether it exists.
    -+ */
    - int is_headref(struct ref_store *refs, const char *refname);
    - 
    +-int is_headref(struct ref_store *refs, const char *refname);
    +-
      #endif /* REFS_H */
     
      ## refs/files-backend.c ##
 8:  8bd52e5363 !  8:  19af8c754c refs: pseudorefs are no refs
    @@ refs.c: int is_root_ref(struct ref_store *refs, const char *refname)
     +	if (!is_root_ref_syntax(refname) ||
     +	    is_pseudo_ref(refname))
      		return 0;
    - 	if (is_headref(refs, refname))
    - 		return 1;
    + 
    + 	/*
     @@ refs.c: static int refs_read_special_head(struct ref_store *ref_store,
      	return result;
      }
 9:  cd6d745a01 !  9:  86f7f2d2d8 ref-filter: properly distinuish pseudo and root refs
    @@ refs.c: int is_per_worktree_ref(const char *refname)
      		"MERGE_HEAD",
     
      ## refs.h ##
    -@@ refs.h: int is_root_ref(struct ref_store *refs, const char *refname);
    +@@ refs.h: void update_ref_namespace(enum ref_namespace namespace, char *ref);
       */
    - int is_headref(struct ref_store *refs, const char *refname);
    + int is_root_ref(struct ref_store *refs, const char *refname);
      
     +/*
     + * Pseudorefs are refs that have different semantics compared to
10:  6956fccced = 10:  640d3b169f refs: refuse to write pseudorefs

base-commit: 0f3415f1f8478b05e64db11eb8aaa2915e48fef6

Comments

Junio C Hamano May 10, 2024, 6:59 p.m. UTC | #1

Patrick Steinhardt <[e-mail]> writes:

>   - Reorder patches 6 and 7 such that we first correct `is_root_ref()`,
>     and then adapt `is_headref()`.
>
>   - Furthermore, I have inlined `is_headref()` into `is_root_ref()`
>     completely now as it didn't have any users anymore.

This does look like a good change, relative to the previous
iteration.  The code paths gets greatly simplified.