Hi Karthik
On 19/01/2024 14:27, Karthik Nayak wrote:
The `is_pseudoref_syntax()` function is used to determine if a
particular refname follows the pseudoref syntax. The pseudoref syntax is
loosely defined at this instance as all refs which are in caps and use
underscores. Most of the pseudorefs also have the "HEAD" suffix.
Using this information we make the `is_pseudoref_syntax()` function
stricter, by adding the check for "HEAD" suffix and for refs which don't
end with the HEAD suffix, matching them with a predetermined list.
This requires fixing up t1407 to use the "HEAD" suffix for creation of
pseudorefs.
I'm concerned that this change is a regression. is_pseudoref_syntax() is
used by is_current_worktree_ref() and so scripts that create pseudorefs
that do not conform to your new rules will break as git will no-longer
consider the pseudorefs they create to be worktree specific. The list of
hard coded exceptions also looks quite short, I can see MERGE_AUTOSTASH
and BISECT_START are missing and there are probably others I've not
thought of.
The commit message would be a good place to discuss why you're making
this change, the implications of the change and any alternative
approaches that you considered. As I understand it you're tying to get
round the problem that the files backend stores pseudorefs mixed up with
other non-ref files in $GIT_DIR. Another approach would be to read all
the files whose name matches the pseudoref syntax and see if its
contents looks like a valid ref skipping names like COMMIT_EDITMSG that
we know are not pseudorefs.
Best Wishes
Phillip
Signed-off-by: Karthik Nayak <karthik.188@xxxxxxxxx>
---
refs.c | 21 ++++++++++++++++++---
t/t1407-worktree-ref-store.sh | 12 ++++++------
2 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index 5999605230..b84e173762 100644
--- a/refs.c
+++ b/refs.c
@@ -829,6 +829,14 @@ int is_per_worktree_ref(const char *refname)
int is_pseudoref_syntax(const char *refname)
{
+ /* TODO: move these pseudorefs to have _HEAD suffix */
+ static const char *const irregular_pseudorefs[] = {
+ "BISECT_EXPECTED_REV",
+ "NOTES_MERGE_PARTIAL",
+ "NOTES_MERGE_REF",
+ "AUTO_MERGE"
+ };
+ size_t i;
const char *c;
for (c = refname; *c; c++) {
@@ -837,10 +845,17 @@ int is_pseudoref_syntax(const char *refname)
}
/*
- * HEAD is not a pseudoref, but it certainly uses the
- * pseudoref syntax.
+ * Most pseudorefs end with _HEAD. HEAD itself is not a
+ * pseudoref, but it certainly uses the pseudoref syntax.
*/
- return 1;
+ if (ends_with(refname, "HEAD"))
+ return 1;
+
+ for (i = 0; i < ARRAY_SIZE(irregular_pseudorefs); i++)
+ if (!strcmp(refname, irregular_pseudorefs[i]))
+ return 1;
+
+ return 0;
}
static int is_current_worktree_ref(const char *ref) {
diff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh
index 05b1881c59..53592c95f3 100755
--- a/t/t1407-worktree-ref-store.sh
+++ b/t/t1407-worktree-ref-store.sh
@@ -61,18 +61,18 @@ test_expect_success 'create_symref(FOO, refs/heads/main)' '
# PSEUDO-WT and refs/bisect/random do not create reflogs by default, so it is
# not testing a realistic scenario.
test_expect_success REFFILES 'for_each_reflog()' '
- echo $ZERO_OID > .git/logs/PSEUDO-MAIN &&
+ echo $ZERO_OID >.git/logs/PSEUDO_MAIN_HEAD &&
mkdir -p .git/logs/refs/bisect &&
- echo $ZERO_OID > .git/logs/refs/bisect/random &&
+ echo $ZERO_OID >.git/logs/refs/bisect/random &&
- echo $ZERO_OID > .git/worktrees/wt/logs/PSEUDO-WT &&
+ echo $ZERO_OID >.git/worktrees/wt/logs/PSEUDO_WT_HEAD &&
mkdir -p .git/worktrees/wt/logs/refs/bisect &&
- echo $ZERO_OID > .git/worktrees/wt/logs/refs/bisect/wt-random &&
+ echo $ZERO_OID >.git/worktrees/wt/logs/refs/bisect/wt-random &&
$RWT for-each-reflog | cut -d" " -f 2- | sort >actual &&
cat >expected <<-\EOF &&
HEAD 0x1
- PSEUDO-WT 0x0
+ PSEUDO_WT_HEAD 0x0
refs/bisect/wt-random 0x0
refs/heads/main 0x0
refs/heads/wt-main 0x0
@@ -82,7 +82,7 @@ test_expect_success REFFILES 'for_each_reflog()' '
$RMAIN for-each-reflog | cut -d" " -f 2- | sort >actual &&
cat >expected <<-\EOF &&
HEAD 0x1
- PSEUDO-MAIN 0x0
+ PSEUDO_MAIN_HEAD 0x0
refs/bisect/random 0x0
refs/heads/main 0x0
refs/heads/wt-main 0x0
