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

Patrick Steinhardt
2024-05-10T08:48:08Z

[-- Attachment #1: Type: text/plain, Size: 9595 bytes --]

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 <ps@pks.im>

## 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
--
2.45.0