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

Patrick Steinhardt [off-list ref] writes:

On Fri, Jul 03, 2026 at 04:19:58PM +0200, Toon Claes wrote:

quoted

Patrick Steinhardt [off-list ref] writes:

quoted

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

That makes sense. Sort of.

So passing `--no-deref` to `git refs create` in the example above would
make sense if you want creation of refs/heads/target to fail. Okay,
doesn't seem very obvious, but feels correct.

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.

Not sure how to make it better, so let's leave it like this.

quoted

quoted

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]

quoted

quoted

+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.

Understood.

quoted

quoted

+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.

Okay, I better understand now with the example above. Thanks!

-- 
Cheers,
Toon