Re: [PATCH 2/3] refs: do not label special refs as pseudo refs

[Justin Tobler <jltobler@gmail.com>]

On 24/04/29 03:41PM, Patrick Steinhardt wrote:
> diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
> index d71b199955..4275918fa0 100644
> --- a/Documentation/glossary-content.txt
> +++ b/Documentation/glossary-content.txt
> @@ -497,20 +497,28 @@ exclude;;
>  	unusual refs.
>  
>  [[def_pseudoref]]pseudoref::
> -	Pseudorefs are a class of files under `$GIT_DIR` which behave
> -	like refs for the purposes of rev-parse, but which are treated
> -	specially by git.  Pseudorefs both have names that are all-caps,
> -	and always start with a line consisting of a
> -	<<def_SHA1,SHA-1>> followed by whitespace.  So, HEAD is not a
> -	pseudoref, because it is sometimes a symbolic ref.  They might

We remove the example here about HEAD not being a pseudoref. This
example seems helpful to indicate that a pseudoref cannot be a symbolic
ref. Is this no longer the case and the change intended?

> -	optionally contain some additional data.  `MERGE_HEAD` and
> -	`CHERRY_PICK_HEAD` are examples.  Unlike
> -	<<def_per_worktree_ref,per-worktree refs>>, these files cannot
> -	be symbolic refs, and never have reflogs.  They also cannot be
> -	updated through the normal ref update machinery.  Instead,
> -	they are updated by directly writing to the files.  However,
> -	they can be read as if they were refs, so `git rev-parse
> -	MERGE_HEAD` will work.
> +	Pseudorefs are references that live in the root of the reference
> +	hierarchy, outside of the usual "refs/" hierarchy. Pseudorefs have an
> +	all-uppercase name and must end with a "_HEAD" suffix, for example
> +	"`BISECT_HEAD`". Other than that, pseudorefs behave the exact same as
> +	any other reference and can be both read and written via regular Git
> +	tooling.

Pseudorefs behaving the same and using the same tooling seems to
contridict the previous documentation. I assume the previous information
was out-of-date, but it might be nice to explain this in the message.

> ++
> +<<def_special_ref>,Special refs>> are not pseudorefs.
> ++
> +Due to historic reasons, Git has several irregular pseudo refs that do not
> +follow above rules. The following list of irregular pseudo refs is exhaustive

We seem to be inconsistent between using "pseudoref" and "pseudo ref".
Not sure it we want to be consistent here. 

-Justin

> +and shall not be extended in the future:
> +
> + - "`AUTO_MERGE`"
> +
> + - "`BISECT_EXPECTED_REV`"
> +
> + - "`NOTES_MERGE_PARTIAL`"
> +
> + - "`NOTES_MERGE_REF`"
> +
> + - "`MERGE_AUTOSTASH`"
>  
>  [[def_pull]]pull::
>  	Pulling a <<def_branch,branch>> means to <<def_fetch,fetch>> it and
> diff --git a/refs.c b/refs.c
> index c64f66bff9..567c6fc6ff 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -905,6 +905,8 @@ int is_pseudoref(struct ref_store *refs, const char *refname)
>  
>  	if (!is_pseudoref_syntax(refname))
>  		return 0;
> +	if (is_special_ref(refname))
> +		return 0;
>  
>  	if (ends_with(refname, "_HEAD")) {
>  		refs_resolve_ref_unsafe(refs, refname,
> diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
> index 948f1bb5f4..8c92fbde79 100755
> --- a/t/t6302-for-each-ref-filter.sh
> +++ b/t/t6302-for-each-ref-filter.sh
> @@ -52,6 +52,23 @@ test_expect_success '--include-root-refs pattern prints pseudorefs' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success '--include-root-refs pattern does not print special refs' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit initial &&
> +		git rev-parse HEAD >.git/MERGE_HEAD &&
> +		git for-each-ref --format="%(refname)" --include-root-refs >actual &&
> +		cat >expect <<-EOF &&
> +		HEAD
> +		$(git symbolic-ref HEAD)
> +		refs/tags/initial
> +		EOF
> +		test_cmp expect actual
> +	)
> +'
> +
>  test_expect_success '--include-root-refs with other patterns' '
>  	cat >expect <<-\EOF &&
>  	HEAD
> -- 
> 2.45.0-rc1
>