Re: [PATCH v3 4/5] builtin/refs: add "create" subcommand

[Patrick Steinhardt <ps@pks.im>]

On Fri, Jul 03, 2026 at 04:19:58PM +0200, Toon Claes wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/Documentation/git-refs.adoc b/Documentation/git-refs.adoc
> > index 6475bdcc62..e6a3528349 100644
> > --- a/Documentation/git-refs.adoc
> > +++ b/Documentation/git-refs.adoc
> > @@ -181,6 +184,53 @@ static int cmd_refs_optimize(int argc, const char **argv, const char *prefix,
> >  	return pack_refs_core(argc, argv, prefix, repo, refs_optimize_usage);
> >  }
> >  
> > +static int cmd_refs_create(int argc, const char **argv, const char *prefix,
> > +			   struct repository *repo)
> > +{
> > +	static char const * const refs_create_usage[] = {
> > +		REFS_CREATE_USAGE,
> > +		NULL
> > +	};
> > +	const char *message = NULL;
> > +	unsigned flags = 0;
> > +	struct option opts[] = {
> > +		OPT_STRING(0, "message", &message, N_("reason"),
> > +			   N_("reason of the update")),
> > +		OPT_BIT(0 ,"no-deref", &flags,
> > +			N_("update <refname> not the one it points to"),
> > +			REF_NO_DEREF),
> 
> Can `git refs create --no-deref` be used to create symrefs? Should we
> add a test for that? Or can it not
> 
> I understand the symmetry, but does it make sense to ask the user to
> create symrefs with `--no-deref`? Feels a bit obscure. The docs say:
> 
> `--no-deref`::
> 	Operate on <ref> itself rather than the reference it points to via a
> 	symbolic ref.
> 
> That's far from obvious for a user to realize they need to pass that
> option if they want to create a symref.

It doesn't cause them to create a symref. What this flag controls is
whether the command would fail when the refname exists already as a
symbolic ref. That is:

    $ git symbolic-ref refs/heads/symref refs/heads/target
    $ git refs create refs/heads/symref $OID
    $ git refs exists refs/heads/target

The git-refs(1) command would have created "refs/heads/target" in this
case, and by passing "--no-deref" you'd instead make it fail.

This flag is somewhat weird. Having it is probably a sensible think to
do, but now that I think about it I wonder whether the default makes all
that much sense in the first place. That being said, _if_ we want to
change it then we should change it for all subcommands.

> > diff --git a/t/t1466-refs-create.sh b/t/t1466-refs-create.sh
> > new file mode 100755
> > index 0000000000..cfb21bf863
> > --- /dev/null
> > +++ b/t/t1466-refs-create.sh
> > @@ -0,0 +1,151 @@
[snip]
> > +test_expect_success 'create fails when the reference already exists' '
> > +	test_when_finished "rm -rf repo" &&
> > +	setup_repo repo &&
> > +	(
> > +		cd repo &&
> > +		A=$(git rev-parse A) &&
> > +		B=$(git rev-parse B) &&
> > +		git refs create refs/heads/foo $A &&
> > +		test_must_fail git refs create refs/heads/foo $B 2>err &&
> > +		test_grep "reference already exists" err &&
> > +		test_ref_matches refs/heads/foo "$A"
> > +	)
> > +'
> 
> I was curious about this test:
> 
> 	test_expect_success 'create succeed when the reference exists with the same value' '
> 		test_when_finished "rm -rf repo" &&
> 		setup_repo repo &&
> 		(
> 			cd repo &&
> 			A=$(git rev-parse A) &&
> 			git refs create refs/heads/foo $A &&
> 			git refs create refs/heads/foo $A &&
> 			test_ref_matches refs/heads/foo "$A"
> 		)
> 	'
> 
> That fails. It that intentional?

Yes, this is intentional. We didn't end up creating the reference, which
is what the user has asked us to do, and hence we fail.

[snip]
> > +test_expect_success 'create with symref target and --no-deref refuses to create reference' '
> > +	test_when_finished "rm -rf repo" &&
> > +	setup_repo repo &&
> > +	(
> > +		cd repo &&
> > +		A=$(git rev-parse A) &&
> > +		git symbolic-ref refs/heads/symref refs/heads/target &&
> > +		test_must_fail git refs create --no-deref refs/heads/symref $A 2>err &&
> > +		test_grep "dangling symref already exists" err &&
> > +		test_must_fail git reflog exists refs/heads/target
> > +	)
> > +'
> 
> Would it make sense to add this test:
> 
> 	test_expect_success 'create with symref target with --no-deref' '
> 		test_when_finished "rm -rf repo" &&
> 		setup_repo repo &&
> 		(
> 			cd repo &&
> 			A=$(git rev-parse A) &&
> 			git refs create refs/heads/target $A &&
> 			git refs create --no-deref refs/heads/symref refs/heads/target &&
> 			git reflog exists refs/heads/symref && false
> 		)
> 	'
> 
> But that makes me think, this option `--no-deref` is pretty obscure for
> use with `git refs create`. There are two situations:
> 
> * The symref doesn't exists: so --no-deref basically is forcing the
>   command to create a symref. That's confusing

No, it's not. It tells us that we only want to create the reference if
it doesn't exist and is not a symref. Otherwise, we'd potentially create
the reference that the symref is pointing to.

Patrick