[PATCH 0/4] builtin/refs: add ability to write references

[PATCH 0/4] builtin/refs: add ability to write references

[Patrick Steinhardt]

Hi,

Reference-related functionality in Git is currently spread across many
different commands: git-update-ref(1), git-for-each-ref(1),
git-show-ref(1), git-pack-refs(1) and git-symbolic-ref(1). This makes it
hard for users to discover what functionality we have available to work
with references.

We have thus started to consolidate this functionality into git-refs(1),
which is a toolbox of everything related to references. Until now, the
command doesn't handle functionality of git-update-ref(1).

This patch series backfills most of the functionality by introducing
three new commands:

  - `git refs delete` to delete references. This is the equivalent of
    `git update-ref -d`.

  - `git refs update` to update references. This is the equivalent of
    `git update-ref <refname> <oldvalue> <newvalue>`.

  - `git refs rename` to rename a reference, including its reflog. This
    does not have an equivalent in git-update-ref(1), but is inspired by
    and supersedes [1].

Thanks!

Patrick

[1]: <xmqqv7brz9ba.fsf@gitster.g>

---
Patrick Steinhardt (4):
      builtin/refs: drop `the_repository`
      builtin/refs: add "delete" subcommand
      builtin/refs: add "update" subcommand
      builtin/refs: add "rename" subcommand

 Documentation/git-refs.adoc |  34 +++++++++
 builtin/refs.c              | 153 +++++++++++++++++++++++++++++++++++--
 t/meson.build               |   3 +
 t/t1464-refs-delete.sh      | 133 ++++++++++++++++++++++++++++++++
 t/t1465-refs-update.sh      | 179 ++++++++++++++++++++++++++++++++++++++++++++
 t/t1466-refs-rename.sh      | 131 ++++++++++++++++++++++++++++++++
 6 files changed, 625 insertions(+), 8 deletions(-)


---
base-commit: 700432b2ba22603a0bcb71475c9c333d17c9b0d1
change-id: 20260616-pks-refs-writing-subcommands-7a77be5bda9b

[PATCH 1/4] builtin/refs: drop `the_repository`

[Patrick Steinhardt]

We still have a couple of uses of `the_repository` in "builtin/refs.c".
All of those are trivial to convert though as the command always
requires a repository to exist.

Convert them to use the passed-in repository and drop
`USE_THE_REPOSITORY_VARIABLE`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/refs.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/builtin/refs.c b/builtin/refs.c
index e3125bc61b..f0faabf45a 100644
--- a/builtin/refs.c
+++ b/builtin/refs.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
 #include "builtin.h"
 #include "config.h"
 #include "fsck.h"
@@ -23,7 +22,7 @@
 	N_("git refs optimize " PACK_REFS_OPTS)
 
 static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
-			    struct repository *repo UNUSED)
+			    struct repository *repo)
 {
 	const char * const migrate_usage[] = {
 		REFS_MIGRATE_USAGE,
@@ -59,13 +58,13 @@ static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
 		goto out;
 	}
 
-	if (the_repository->ref_storage_format == format) {
+	if (repo->ref_storage_format == format) {
 		err = error(_("repository already uses '%s' format"),
 			    ref_storage_format_to_name(format));
 		goto out;
 	}
 
-	if (repo_migrate_ref_storage_format(the_repository, format, flags, &errbuf) < 0) {
+	if (repo_migrate_ref_storage_format(repo, format, flags, &errbuf) < 0) {
 		err = error("%s", errbuf.buf);
 		goto out;
 	}
@@ -99,8 +98,8 @@ static int cmd_refs_verify(int argc, const char **argv, const char *prefix,
 	if (argc)
 		usage(_("'git refs verify' takes no arguments"));
 
-	repo_config(the_repository, git_fsck_config, &fsck_refs_options);
-	prepare_repo_settings(the_repository);
+	repo_config(repo, git_fsck_config, &fsck_refs_options);
+	prepare_repo_settings(repo);
 
 	worktrees = get_worktrees_without_reading_head();
 	for (size_t i = 0; worktrees[i]; i++)
@@ -124,7 +123,7 @@ static int cmd_refs_list(int argc, const char **argv, const char *prefix,
 }
 
 static int cmd_refs_exists(int argc, const char **argv, const char *prefix,
-			   struct repository *repo UNUSED)
+			   struct repository *repo)
 {
 	struct strbuf unused_referent = STRBUF_INIT;
 	struct object_id unused_oid;
@@ -145,7 +144,7 @@ static int cmd_refs_exists(int argc, const char **argv, const char *prefix,
 		die(_("'git refs exists' requires a reference"));
 
 	ref = *argv++;
-	if (refs_read_raw_ref(get_main_ref_store(the_repository), ref,
+	if (refs_read_raw_ref(get_main_ref_store(repo), ref,
 			      &unused_oid, &unused_referent, &unused_type,
 			      &failure_errno)) {
 		if (failure_errno == ENOENT || failure_errno == EISDIR) {

-- 
2.55.0.rc0.786.g65d90a0328.dirty

[PATCH 2/4] builtin/refs: add "delete" subcommand

[Patrick Steinhardt]

Reference-related functionality in Git is currently spread across many
different commands: git-update-ref(1), git-for-each-ref(1),
git-show-ref(1), git-pack-refs(1) and git-symbolic-ref(1). This makes it
hard for users to discover what functionality we have available to work
with references.

We have thus started to consolidate this functionality into git-refs(1),
which is a toolbox of everything related to references. Until now, the
command doesn't handle functionality of git-update-ref(1).

Fix this gap by introducing a new "delete" subcommand, which is the
equivalent of `git update-ref -d`.

Note that we're intentionally not using a generic "write" subcommand
with a "-d" flag. This is rather harder to discover, and subcommands
that are implmented as flags tend to be hard to reason about in the code
as we'd have to handle mutually-exclusive flags that stem from the other
subcommand-like modes.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/git-refs.adoc |  17 ++++++
 builtin/refs.c              |  46 +++++++++++++++
 t/meson.build               |   1 +
 t/t1464-refs-delete.sh      | 133 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 197 insertions(+)

diff --git a/Documentation/git-refs.adoc b/Documentation/git-refs.adoc
index fa33680cc7..c03e8e6ac3 100644
--- a/Documentation/git-refs.adoc
+++ b/Documentation/git-refs.adoc
@@ -20,6 +20,7 @@ git refs list [--count=<count>] [--shell|--perl|--python|--tcl]
 		   [ --stdin | (<pattern>...)]
 git refs exists <ref>
 git refs optimize [--all] [--no-prune] [--auto] [--include <pattern>] [--exclude <pattern>]
+git refs delete [--message=<reason>] [--no-deref] <ref> [<oldvalue>]
 
 DESCRIPTION
 -----------
@@ -51,6 +52,12 @@ optimize::
 	usage. This subcommand is an alias for linkgit:git-pack-refs[1] and
 	offers identical functionality.
 
+delete::
+	Delete the given reference. This subcommand mirrors `git update-ref -d`
+	(see linkgit:git-update-ref[1]). When `<oldvalue>` is given, the
+	reference is only deleted after verifying that it currently contains
+	`<oldvalue>`.
+
 OPTIONS
 -------
 
@@ -90,6 +97,16 @@ The following options are specific to 'git refs optimize':
 
 include::pack-refs-options.adoc[]
 
+The following options are specific to commands which write references:
+
+`--message=<reason>`::
+	Use the given <reason> string for the reflog entry associated with the
+	update. An empty message is rejected.
+
+`--no-deref`::
+	Operate on <ref> itself rather than the reference it points to via a
+	symbolic ref.
+
 KNOWN LIMITATIONS
 -----------------
 
diff --git a/builtin/refs.c b/builtin/refs.c
index f0faabf45a..69eb528522 100644
--- a/builtin/refs.c
+++ b/builtin/refs.c
@@ -21,6 +21,9 @@
 #define REFS_OPTIMIZE_USAGE \
 	N_("git refs optimize " PACK_REFS_OPTS)
 
+#define REFS_DELETE_USAGE \
+	N_("git refs delete [--message=<reason>] [--no-deref] <ref> [<oldvalue>]")
+
 static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
 			    struct repository *repo)
 {
@@ -175,6 +178,47 @@ 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_delete(int argc, const char **argv, const char *prefix,
+			   struct repository *repo)
+{
+	static char const * const refs_delete_usage[] = {
+		REFS_DELETE_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),
+		OPT_END(),
+	};
+	struct object_id oldoid;
+	const char *refname;
+
+	argc = parse_options(argc, argv, prefix, opts, refs_delete_usage, 0);
+	if (argc < 1 || argc > 2)
+		usage(_("delete requires reference name and an optional old object ID"));
+
+	if (message && !*message)
+		die(_("refusing to perform update with empty message"));
+
+	repo_config(repo, git_default_config, NULL);
+
+	refname = argv[0];
+	if (argc == 2) {
+		if (repo_get_oid_with_flags(repo, argv[1], &oldoid, GET_OID_SKIP_AMBIGUITY_CHECK))
+			die(_("invalid old object ID: '%s'"), argv[1]);
+		if (is_null_oid(&oldoid))
+			die(_("cannot delete object with null old object ID"));
+	}
+
+	return refs_delete_ref(get_main_ref_store(repo), message, refname,
+			       argc == 2 ? &oldoid : NULL, flags);
+}
+
 int cmd_refs(int argc,
 	     const char **argv,
 	     const char *prefix,
@@ -186,6 +230,7 @@ int cmd_refs(int argc,
 		"git refs list " COMMON_USAGE_FOR_EACH_REF,
 		REFS_EXISTS_USAGE,
 		REFS_OPTIMIZE_USAGE,
+		REFS_DELETE_USAGE,
 		NULL,
 	};
 	parse_opt_subcommand_fn *fn = NULL;
@@ -195,6 +240,7 @@ int cmd_refs(int argc,
 		OPT_SUBCOMMAND("list", &fn, cmd_refs_list),
 		OPT_SUBCOMMAND("exists", &fn, cmd_refs_exists),
 		OPT_SUBCOMMAND("optimize", &fn, cmd_refs_optimize),
+		OPT_SUBCOMMAND("delete", &fn, cmd_refs_delete),
 		OPT_END(),
 	};
 
diff --git a/t/meson.build b/t/meson.build
index c5832fee05..1ccf08a3b5 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -223,6 +223,7 @@ integration_tests = [
   't1461-refs-list.sh',
   't1462-refs-exists.sh',
   't1463-refs-optimize.sh',
+  't1464-refs-delete.sh',
   't1500-rev-parse.sh',
   't1501-work-tree.sh',
   't1502-rev-parse-parseopt.sh',
diff --git a/t/t1464-refs-delete.sh b/t/t1464-refs-delete.sh
new file mode 100755
index 0000000000..4a36d3866b
--- /dev/null
+++ b/t/t1464-refs-delete.sh
@@ -0,0 +1,133 @@
+#!/bin/sh
+
+test_description='git refs delete'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+. ./test-lib.sh
+
+setup_repo () {
+	git init "$1" &&
+	test_commit -C "$1" A &&
+	test_commit -C "$1" B
+}
+
+test_expect_success 'delete without oldvalue verification' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	A=$(git -C repo rev-parse A) &&
+	git -C repo update-ref refs/heads/foo $A &&
+	git -C repo refs delete refs/heads/foo &&
+	test_must_fail git -C repo show-ref --verify -q refs/heads/foo
+'
+
+test_expect_success 'delete with matching oldvalue' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		git update-ref refs/heads/foo $A &&
+		git refs delete refs/heads/foo $A &&
+		test_must_fail git refs exists refs/heads/foo
+	)
+'
+
+test_expect_success 'delete with stale oldvalue fails' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		B=$(git rev-parse B) &&
+		git update-ref refs/heads/foo $A &&
+		test_must_fail git refs delete refs/heads/foo $B 2>err &&
+		test_grep " but expected " err &&
+		git refs exists refs/heads/foo
+	)
+'
+
+test_expect_success 'delete with null oldvalue fails' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		git update-ref refs/heads/foo $A &&
+		test_must_fail git refs delete refs/heads/foo $ZERO_OID 2>err &&
+		test_grep "null old object ID" err &&
+		git refs exists refs/heads/foo
+	)
+'
+
+test_expect_success 'delete with invalid oldvalue fails' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		git update-ref refs/heads/foo $A &&
+		test_must_fail git refs delete refs/heads/foo invalid-oid 2>err &&
+		test_grep "invalid old object ID" err &&
+		git refs exists refs/heads/foo
+	)
+'
+
+test_expect_success 'delete symref with --no-deref leaves target intact' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		git update-ref refs/heads/foo $A &&
+		git symbolic-ref refs/heads/symref refs/heads/foo &&
+		git refs delete --no-deref refs/heads/symref &&
+		test_must_fail git refs exists refs/heads/symref &&
+		git refs exists refs/heads/foo
+	)
+'
+
+test_expect_success 'delete with message records reason in reflog' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		git update-ref refs/heads/foo $A &&
+		git symbolic-ref HEAD refs/heads/foo &&
+		git refs delete --message=delete-reason refs/heads/foo &&
+		test_must_fail git refs exists refs/heads/foo &&
+		test-tool ref-store main for-each-reflog-ent HEAD >actual &&
+		test_grep "delete-reason$" actual
+	)
+'
+
+test_expect_success 'delete with empty message fails' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		git update-ref refs/heads/foo $A &&
+		test_must_fail git refs delete --message= refs/heads/foo 2>err &&
+		test_grep "empty message" err &&
+		git refs exists refs/heads/foo
+	)
+'
+
+test_expect_success 'delete without arguments fails' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	test_must_fail git -C repo refs delete 2>err &&
+	test_grep "requires reference name" err
+'
+
+test_expect_success 'delete with too many arguments fails' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	test_must_fail git refs delete one two three 2>err &&
+	test_grep "requires reference name" err
+'
+
+test_done

-- 
2.55.0.rc0.786.g65d90a0328.dirty

[PATCH 3/4] builtin/refs: add "update" subcommand

[Patrick Steinhardt]

Add a new "update" subcommand which mirrors `git update-ref <refname>
<oldoid> <newoid>`. This follows the same reasoning as the preceding
commit.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/git-refs.adoc |   7 ++
 builtin/refs.c              |  50 +++++++++++++
 t/meson.build               |   1 +
 t/t1465-refs-update.sh      | 179 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 237 insertions(+)

diff --git a/Documentation/git-refs.adoc b/Documentation/git-refs.adoc
index c03e8e6ac3..0a887cf5e5 100644
--- a/Documentation/git-refs.adoc
+++ b/Documentation/git-refs.adoc
@@ -21,6 +21,7 @@ git refs list [--count=<count>] [--shell|--perl|--python|--tcl]
 git refs exists <ref>
 git refs optimize [--all] [--no-prune] [--auto] [--include <pattern>] [--exclude <pattern>]
 git refs delete [--message=<reason>] [--no-deref] <ref> [<oldvalue>]
+git refs update [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value> [<old-value>]
 
 DESCRIPTION
 -----------
@@ -58,6 +59,12 @@ delete::
 	reference is only deleted after verifying that it currently contains
 	`<oldvalue>`.
 
+update::
+	Update the given reference to point at `<new-value>`. This subcommand
+	mirrors `git update-ref` (see linkgit:git-update-ref[1]). When
+	`<old-value>` is given, the reference is only updated after verifying
+	that it currently contains `<old-value>`.
+
 OPTIONS
 -------
 
diff --git a/builtin/refs.c b/builtin/refs.c
index 69eb528522..3238ddf3f0 100644
--- a/builtin/refs.c
+++ b/builtin/refs.c
@@ -24,6 +24,9 @@
 #define REFS_DELETE_USAGE \
 	N_("git refs delete [--message=<reason>] [--no-deref] <ref> [<oldvalue>]")
 
+#define REFS_UPDATE_USAGE \
+	N_("git refs update [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value> [<old-value>]")
+
 static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
 			    struct repository *repo)
 {
@@ -219,6 +222,51 @@ static int cmd_refs_delete(int argc, const char **argv, const char *prefix,
 			       argc == 2 ? &oldoid : NULL, flags);
 }
 
+static int cmd_refs_update(int argc, const char **argv, const char *prefix,
+			   struct repository *repo)
+{
+	static char const * const refs_update_usage[] = {
+		REFS_UPDATE_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),
+		OPT_BIT(0, "create-reflog", &flags, N_("create a reflog"),
+			REF_FORCE_CREATE_REFLOG),
+		OPT_END(),
+	};
+	struct object_id newoid, oldoid;
+	const char *refname;
+
+	argc = parse_options(argc, argv, prefix, opts, refs_update_usage, 0);
+	if (argc < 2 || argc > 3)
+		usage(_("update requires reference name, new value and an optional old value"));
+
+	if (message && !*message)
+		die(_("refusing to perform update with empty message"));
+
+	repo_config(repo, git_default_config, NULL);
+
+	refname = argv[0];
+	if (repo_get_oid_with_flags(repo, argv[1], &newoid,
+				    GET_OID_SKIP_AMBIGUITY_CHECK))
+		die(_("invalid new object ID: %s"), argv[1]);
+	if (argc == 3 &&
+	    repo_get_oid_with_flags(repo, argv[2], &oldoid,
+				    GET_OID_SKIP_AMBIGUITY_CHECK))
+		die(_("invalid old object ID: %s"), argv[2]);
+
+	return refs_update_ref(get_main_ref_store(repo), message, refname,
+			       &newoid, argc == 3 ? &oldoid : NULL, flags,
+			       UPDATE_REFS_DIE_ON_ERR);
+}
+
 int cmd_refs(int argc,
 	     const char **argv,
 	     const char *prefix,
@@ -231,6 +279,7 @@ int cmd_refs(int argc,
 		REFS_EXISTS_USAGE,
 		REFS_OPTIMIZE_USAGE,
 		REFS_DELETE_USAGE,
+		REFS_UPDATE_USAGE,
 		NULL,
 	};
 	parse_opt_subcommand_fn *fn = NULL;
@@ -241,6 +290,7 @@ int cmd_refs(int argc,
 		OPT_SUBCOMMAND("exists", &fn, cmd_refs_exists),
 		OPT_SUBCOMMAND("optimize", &fn, cmd_refs_optimize),
 		OPT_SUBCOMMAND("delete", &fn, cmd_refs_delete),
+		OPT_SUBCOMMAND("update", &fn, cmd_refs_update),
 		OPT_END(),
 	};
 
diff --git a/t/meson.build b/t/meson.build
index 1ccf08a3b5..2063962dab 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -224,6 +224,7 @@ integration_tests = [
   't1462-refs-exists.sh',
   't1463-refs-optimize.sh',
   't1464-refs-delete.sh',
+  't1465-refs-update.sh',
   't1500-rev-parse.sh',
   't1501-work-tree.sh',
   't1502-rev-parse-parseopt.sh',
diff --git a/t/t1465-refs-update.sh b/t/t1465-refs-update.sh
new file mode 100755
index 0000000000..e7582a6195
--- /dev/null
+++ b/t/t1465-refs-update.sh
@@ -0,0 +1,179 @@
+#!/bin/sh
+
+test_description='git refs update'
+
+. ./test-lib.sh
+
+setup_repo () {
+	git init "$1" &&
+	test_commit -C "$1" A &&
+	test_commit -C "$1" B
+}
+
+test_ref_matches () {
+	git rev-parse "$1" >expect &&
+	echo "$2" >actual &&
+	test_cmp expect actual
+}
+
+test_expect_success 'update creates a new reference' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		git refs update refs/heads/foo $A &&
+		test_ref_matches refs/heads/foo "$A"
+	)
+'
+
+test_expect_success 'update an existing reference without oldvalue' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		B=$(git rev-parse B) &&
+		git refs update refs/heads/foo $A &&
+		git refs update refs/heads/foo $B &&
+		test_ref_matches refs/heads/foo $B
+	)
+'
+
+test_expect_success 'update with matching oldvalue' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		B=$(git rev-parse B) &&
+		git refs update refs/heads/foo $A &&
+		git refs update refs/heads/foo $B $A &&
+		test_ref_matches refs/heads/foo $B
+	)
+'
+
+test_expect_success 'update with stale oldvalue fails' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		B=$(git rev-parse B) &&
+		git refs update refs/heads/foo $A &&
+		test_must_fail git refs update refs/heads/foo $B $B 2>err &&
+		test_grep " but expected " err &&
+		test_ref_matches refs/heads/foo $A
+	)
+'
+
+test_expect_success 'update with invalid new value fails' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		test_must_fail git refs update refs/heads/foo invalid-oid 2>err &&
+		test_grep "invalid new object ID" err &&
+		test_must_fail git refs exists refs/heads/foo
+	)
+'
+
+test_expect_success 'update with invalid old value fails' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		B=$(git rev-parse B) &&
+		git refs update refs/heads/foo $A &&
+		test_must_fail git refs update refs/heads/foo $B invalid-oid 2>err &&
+		test_grep "invalid old object ID" err &&
+		test_ref_matches refs/heads/foo $A
+	)
+'
+
+test_expect_success 'update --no-deref rewrites the symref itself' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		B=$(git rev-parse B) &&
+		git refs update refs/heads/foo $A &&
+		git symbolic-ref refs/heads/symref refs/heads/foo &&
+		git refs update --no-deref refs/heads/symref $B &&
+		test_must_fail git symbolic-ref refs/heads/symref &&
+		test_ref_matches refs/heads/symref $B &&
+		test_ref_matches refs/heads/foo $A
+	)
+'
+
+test_expect_success 'update does not create a reflog by default' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		git refs update refs/foo $A &&
+		test_must_fail git reflog exists refs/foo
+	)
+'
+
+test_expect_success 'update creates a reflog with --create-reflog' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		git refs update --create-reflog refs/foo $A &&
+		git reflog exists refs/foo
+	)
+'
+
+test_expect_success 'update with message records reason in reflog' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		B=$(git rev-parse B) &&
+		git refs update refs/heads/foo $A &&
+		git refs update --message=update-reason refs/heads/foo $B &&
+		git reflog show refs/heads/foo >actual &&
+		test_grep "update-reason$" actual
+	)
+'
+
+test_expect_success 'update with empty message fails' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		B=$(git rev-parse B) &&
+		git refs update refs/heads/foo $A &&
+		test_must_fail git refs update --message= refs/heads/foo $B 2>err &&
+		test_grep "empty message" err
+	)
+'
+
+test_expect_success 'update with too few arguments fails' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	test_must_fail git -C repo refs update refs/heads/foo 2>err &&
+	test_grep "requires reference name, new value" err
+'
+
+test_expect_success 'update with too many arguments fails' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		B=$(git rev-parse B) &&
+		test_must_fail git refs update refs/heads/foo $A $B extra 2>err &&
+		test_grep "requires reference name, new value" err
+	)
+'
+
+test_done

-- 
2.55.0.rc0.786.g65d90a0328.dirty

[PATCH 4/4] builtin/refs: add "rename" subcommand

[Patrick Steinhardt]

Add a "rename" subcommand to git-refs(1) with the syntax:

  $ git refs rename <oldref> <newref>

It renames <oldref> together with its reflog to <newref>; even when used
on a local branch ref, the current value and the reflog of the ref are
the only things that are renamed. Document it and redirect casual users
to "git branch -m" if that is what they wanted to do.

Co-authored-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/git-refs.adoc |  10 ++++
 builtin/refs.c              |  42 ++++++++++++++
 t/meson.build               |   1 +
 t/t1466-refs-rename.sh      | 131 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 184 insertions(+)

diff --git a/Documentation/git-refs.adoc b/Documentation/git-refs.adoc
index 0a887cf5e5..85eb100205 100644
--- a/Documentation/git-refs.adoc
+++ b/Documentation/git-refs.adoc
@@ -22,6 +22,7 @@ git refs exists <ref>
 git refs optimize [--all] [--no-prune] [--auto] [--include <pattern>] [--exclude <pattern>]
 git refs delete [--message=<reason>] [--no-deref] <ref> [<oldvalue>]
 git refs update [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value> [<old-value>]
+git refs rename [--message=<reason>] <oldref> <newref>
 
 DESCRIPTION
 -----------
@@ -65,6 +66,11 @@ update::
 	`<old-value>` is given, the reference is only updated after verifying
 	that it currently contains `<old-value>`.
 
+rename::
+	Rename the reference `<oldref>` to `<newref>`. The old reference must
+	exist and the new reference must not yet exist, and both must have a
+	well-formed name (see linkgit:git-check-ref-format[1]).
+
 OPTIONS
 -------
 
@@ -106,6 +112,10 @@ include::pack-refs-options.adoc[]
 
 The following options are specific to commands which write references:
 
+`--create-reflog`::
+	Create a reflog for the reference even if one would not ordinarily be
+	created.
+
 `--message=<reason>`::
 	Use the given <reason> string for the reflog entry associated with the
 	update. An empty message is rejected.
diff --git a/builtin/refs.c b/builtin/refs.c
index 3238ddf3f0..b90baf5633 100644
--- a/builtin/refs.c
+++ b/builtin/refs.c
@@ -27,6 +27,9 @@
 #define REFS_UPDATE_USAGE \
 	N_("git refs update [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value> [<old-value>]")
 
+#define REFS_RENAME_USAGE \
+	N_("git refs rename [--message=<reason>] <oldref> <newref>")
+
 static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
 			    struct repository *repo)
 {
@@ -267,6 +270,43 @@ static int cmd_refs_update(int argc, const char **argv, const char *prefix,
 			       UPDATE_REFS_DIE_ON_ERR);
 }
 
+static int cmd_refs_rename(int argc, const char **argv, const char *prefix,
+			   struct repository *repo)
+{
+	static char const * const refs_rename_usage[] = {
+		REFS_RENAME_USAGE,
+		NULL
+	};
+	const char *message = NULL;
+	struct option opts[] = {
+		OPT_STRING(0, "message", &message, N_("reason"),
+			   N_("reason of the update")),
+		OPT_END(),
+	};
+	const char *oldref, *newref;
+
+	argc = parse_options(argc, argv, prefix, opts, refs_rename_usage, 0);
+	if (argc != 2)
+		usage(_("rename requires old and new reference name"));
+	if (message && !*message)
+		die(_("refusing to perform update with empty message"));
+
+	oldref = argv[0];
+	newref = argv[1];
+
+	if (check_refname_format(oldref, 0))
+		die(_("invalid ref format: %s"), oldref);
+	if (check_refname_format(newref, 0))
+		die(_("invalid ref format: %s"), newref);
+
+	if (!refs_ref_exists(get_main_ref_store(repo), oldref))
+		die(_("reference does not exist: '%s'"), oldref);
+	if (refs_ref_exists(get_main_ref_store(repo), newref))
+		die(_("reference already exists: '%s'"), newref);
+
+	return refs_rename_ref(get_main_ref_store(repo), oldref, newref, message);
+}
+
 int cmd_refs(int argc,
 	     const char **argv,
 	     const char *prefix,
@@ -280,6 +320,7 @@ int cmd_refs(int argc,
 		REFS_OPTIMIZE_USAGE,
 		REFS_DELETE_USAGE,
 		REFS_UPDATE_USAGE,
+		REFS_RENAME_USAGE,
 		NULL,
 	};
 	parse_opt_subcommand_fn *fn = NULL;
@@ -291,6 +332,7 @@ int cmd_refs(int argc,
 		OPT_SUBCOMMAND("optimize", &fn, cmd_refs_optimize),
 		OPT_SUBCOMMAND("delete", &fn, cmd_refs_delete),
 		OPT_SUBCOMMAND("update", &fn, cmd_refs_update),
+		OPT_SUBCOMMAND("rename", &fn, cmd_refs_rename),
 		OPT_END(),
 	};
 
diff --git a/t/meson.build b/t/meson.build
index 2063962dab..a1a6880fe6 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -225,6 +225,7 @@ integration_tests = [
   't1463-refs-optimize.sh',
   't1464-refs-delete.sh',
   't1465-refs-update.sh',
+  't1466-refs-rename.sh',
   't1500-rev-parse.sh',
   't1501-work-tree.sh',
   't1502-rev-parse-parseopt.sh',
diff --git a/t/t1466-refs-rename.sh b/t/t1466-refs-rename.sh
new file mode 100755
index 0000000000..f80d58e0f4
--- /dev/null
+++ b/t/t1466-refs-rename.sh
@@ -0,0 +1,131 @@
+#!/bin/sh
+
+test_description='git refs rename'
+
+. ./test-lib.sh
+
+setup_repo () {
+	git init "$1" &&
+	test_commit -C "$1" A &&
+	test_commit -C "$1" B
+}
+
+test_ref_matches () {
+	git rev-parse "$1" >expect &&
+	echo "$2" >actual &&
+	test_cmp expect actual
+}
+
+test_expect_success 'rename an existing reference' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		git refs update refs/heads/foo $A &&
+		git refs rename refs/heads/foo refs/heads/bar &&
+		test_must_fail git refs exists refs/heads/foo &&
+		test_ref_matches refs/heads/bar $A
+	)
+'
+
+test_expect_success 'rename moves the reflog along with the reference' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		git refs update --message="rename me" refs/heads/foo $A &&
+		git refs rename refs/heads/foo refs/heads/bar &&
+		git reflog show refs/heads/bar >reflog &&
+		test_grep "rename me" reflog &&
+		test_must_fail git reflog exists refs/heads/foo
+	)
+'
+
+test_expect_success 'rename with message records reason in reflog' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		git refs update refs/heads/foo $A &&
+		git refs rename --message="rename reason" refs/heads/foo refs/heads/bar &&
+		git reflog show refs/heads/bar >actual &&
+		test_grep "rename reason" actual
+	)
+'
+
+test_expect_success 'rename a nonexistent reference fails' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		test_must_fail git refs rename refs/heads/foo refs/heads/bar 2>err &&
+		test_grep "reference does not exist" err
+	)
+'
+
+test_expect_success 'rename to an existing reference fails' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		B=$(git rev-parse B) &&
+		git refs update refs/heads/foo $A &&
+		git refs update refs/heads/bar $B &&
+		test_must_fail git refs rename refs/heads/foo refs/heads/bar 2>err &&
+		test_grep "reference already exists" err
+	)
+'
+
+test_expect_success 'rename with empty message fails' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		git refs update refs/heads/foo $A &&
+		test_must_fail git refs rename --message= refs/heads/foo refs/heads/bar 2>err &&
+		test_grep "empty message" err
+	)
+'
+
+test_expect_success 'rename with invalid old reference name fails' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		test_must_fail git refs rename "refs/heads/foo..bar" refs/heads/bar 2>err &&
+		test_grep "invalid ref format" err
+	)
+'
+
+test_expect_success 'rename with invalid new reference name fails' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	(
+		cd repo &&
+		A=$(git rev-parse A) &&
+		git refs update refs/heads/foo $A &&
+		test_must_fail git refs rename refs/heads/foo "refs/heads/bar..baz" 2>err &&
+		test_grep "invalid ref format" err
+	)
+'
+
+test_expect_success 'rename with too few arguments fails' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	test_must_fail git -C repo refs rename refs/heads/foo 2>err &&
+	test_grep "requires old and new reference name" err
+'
+
+test_expect_success 'rename with too many arguments fails' '
+	test_when_finished "rm -rf repo" &&
+	setup_repo repo &&
+	test_must_fail git -C repo refs rename refs/heads/foo refs/heads/bar refs/heads/baz 2>err &&
+	test_grep "requires old and new reference name" err
+'
+
+test_done

-- 
2.55.0.rc0.786.g65d90a0328.dirty

Re: [PATCH 3/4] builtin/refs: add "update" subcommand

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> Add a new "update" subcommand which mirrors `git update-ref <refname>
> <oldoid> <newoid>`. This follows the same reasoning as the preceding
> commit.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  Documentation/git-refs.adoc |   7 ++
>  builtin/refs.c              |  50 +++++++++++++
>  t/meson.build               |   1 +
>  t/t1465-refs-update.sh      | 179 ++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 237 insertions(+)

I do not offhand know (and I am still away by 2 hours from the time
I wake up and start functioning) if update-ref shares the same
issue, but with "delete, update, rename" combo, lack of "create"
feels a bit annoying.  Wouldn't we want to offer an option to users
who want to ensure that the refs they create are truly new and they
are not overwriting a ref somebody has created?  Either (1) drop
"delete" and take a special value (e.g. "") as <newvalue> to signal
deletion and make the same special value used as <oldvalue> signals
creation, or (2) add "create" and insist that "update" takes only an
existing ref, would make the annoyance go away, I guess.

> +test_expect_success 'update creates a new reference' '
> +	test_when_finished "rm -rf repo" &&
> +	setup_repo repo &&
> +	(
> +		cd repo &&
> +		A=$(git rev-parse A) &&
> +		git refs update refs/heads/foo $A &&
> +		test_ref_matches refs/heads/foo "$A"
> +	)
> +'

Here we cannot test (and I strongly suspect that "git refs update"
and "git update-ref" lack ability to do so) a case where a creation
is attempted on an existing ref and fails.

Re: [PATCH 3/4] builtin/refs: add "update" subcommand

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

>  git refs delete [--message=<reason>] [--no-deref] <ref> [<oldvalue>]
> +git refs update [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value> [<old-value>]

"<old-value> vs <new-value>" is good, we should update "delete" to
use "<old-value>" to match.


>  DESCRIPTION
>  -----------
> @@ -58,6 +59,12 @@ delete::
>  	reference is only deleted after verifying that it currently contains
>  	`<oldvalue>`.
>  
> +update::
> +	Update the given reference to point at `<new-value>`. This subcommand
> +	mirrors `git update-ref` (see linkgit:git-update-ref[1]). When
> +	`<old-value>` is given, the reference is only updated after verifying
> +	that it currently contains `<old-value>`.

As to the lack of "create", among the two potential changes, I have
a slight preference for adding "create" and failing "update" that
does not refer to an existing ref.  If we go that route, the
"--create-reflog" option should move to "create", as "update" will
never be used to create a new ref.


> +	if (repo_get_oid_with_flags(repo, argv[1], &newoid,
> +				    GET_OID_SKIP_AMBIGUITY_CHECK))
> +		die(_("invalid new object ID: %s"), argv[1]);
> +	if (argc == 3 &&
> +	    repo_get_oid_with_flags(repo, argv[2], &oldoid,
> +				    GET_OID_SKIP_AMBIGUITY_CHECK))
> +		die(_("invalid old object ID: %s"), argv[2]);

On the "delete" side, these messages quote the object name, i.e.,

			die(_("invalid old object ID: '%s'"), argv[1]);

We should be consistent.

Re: [PATCH 4/4] builtin/refs: add "rename" subcommand

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> +static int cmd_refs_rename(int argc, const char **argv, const char *prefix,
> +			   struct repository *repo)
> +{
> +	static char const * const refs_rename_usage[] = {
> +		REFS_RENAME_USAGE,
> +		NULL
> +	};
> +	const char *message = NULL;
> +	struct option opts[] = {
> +		OPT_STRING(0, "message", &message, N_("reason"),
> +			   N_("reason of the update")),
> +		OPT_END(),
> +	};
> +	const char *oldref, *newref;
> +
> +	argc = parse_options(argc, argv, prefix, opts, refs_rename_usage, 0);
> +	if (argc != 2)
> +		usage(_("rename requires old and new reference name"));
> +	if (message && !*message)
> +		die(_("refusing to perform update with empty message"));
> +
> +	oldref = argv[0];
> +	newref = argv[1];
> +
> +	if (check_refname_format(oldref, 0))
> +		die(_("invalid ref format: %s"), oldref);
> +	if (check_refname_format(newref, 0))
> +		die(_("invalid ref format: %s"), newref);

Do we want to quote the value?  What other subcommands do in "git refs"?

> +	if (!refs_ref_exists(get_main_ref_store(repo), oldref))
> +		die(_("reference does not exist: '%s'"), oldref);
> +	if (refs_ref_exists(get_main_ref_store(repo), newref))
> +		die(_("reference already exists: '%s'"), newref);
> +
> +	return refs_rename_ref(get_main_ref_store(repo), oldref, newref, message);
> +}

I suspect that my version shared the same issue, but doesn't
refs_rename_ref() return -1 for failure, which we may want to turn
to positive 1 before returning?

This is a tangent but git.c:handle_builtin() that calls
git.c:run_builtin() may want to do the "negative return? flip the
polarity" conversion to make this worry go away.  I dunno what such
a change would break, though.

If we rename a ref that does not have a reflog, would it leave the
ref under the new name without reflog, or would we get a reflog with
a single entry that marks the fact the old ref was renamed into the
new ref?  Should that be controlled via --create-reflog option?