patch, 4 partsforbid HEAD as a tagname

builtin/tag.c, object-name.c, refs.c, refs.h, strbuf.h

diff --git a/builtin/tag.c b/builtin/tag.c
index 93d10d5915..8279dccbe0 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -447,17 +447,6 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
-static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
-{
-	if (name[0] == '-')
-		return -1;
-
-	strbuf_reset(sb);
-	strbuf_addf(sb, "refs/tags/%s", name);
-
-	return check_refname_format(sb->buf, 0);
-}
-
 int cmd_tag(int argc,
 	    const char **argv,
 	    const char *prefix,
diff --git a/object-name.c b/object-name.c
index c892fbe80a..9f2ae164e4 100644
--- a/object-name.c
+++ b/object-name.c
@@ -1734,42 +1734,6 @@ int repo_interpret_branch_name(struct repository *r,
 	return -1;
 }
 
-void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)
-{
-	int len = strlen(name);
-	struct interpret_branch_name_options options = {
-		.allowed = allowed
-	};
-	int used = repo_interpret_branch_name(the_repository, name, len, sb,
-					      &options);
-
-	if (used < 0)
-		used = 0;
-	strbuf_add(sb, name + used, len - used);
-}
-
-int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
-{
-	if (startup_info->have_repository)
-		strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
-	else
-		strbuf_addstr(sb, name);
-
-	/*
-	 * This splice must be done even if we end up rejecting the
-	 * name; builtin/branch.c::copy_or_rename_branch() still wants
-	 * to see what the name expanded to so that "branch -m" can be
-	 * used as a tool to correct earlier mistakes.
-	 */
-	strbuf_splice(sb, 0, 0, "refs/heads/", 11);
-
-	if (*name == '-' ||
-	    !strcmp(sb->buf, "refs/heads/HEAD"))
-		return -1;
-
-	return check_refname_format(sb->buf, 0);
-}
-
 void object_context_release(struct object_context *ctx)
 {
 	free(ctx->path);
diff --git a/refs.c b/refs.c
index 5f729ed412..59a9223d4c 100644
--- a/refs.c
+++ b/refs.c
@@ -697,6 +697,53 @@ static char *substitute_branch_name(struct repository *r,
 	return NULL;
 }
 
+void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)
+{
+	int len = strlen(name);
+	struct interpret_branch_name_options options = {
+		.allowed = allowed
+	};
+	int used = repo_interpret_branch_name(the_repository, name, len, sb,
+					      &options);
+
+	if (used < 0)
+		used = 0;
+	strbuf_add(sb, name + used, len - used);
+}
+
+int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
+{
+	if (startup_info->have_repository)
+		strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
+	else
+		strbuf_addstr(sb, name);
+
+	/*
+	 * This splice must be done even if we end up rejecting the
+	 * name; builtin/branch.c::copy_or_rename_branch() still wants
+	 * to see what the name expanded to so that "branch -m" can be
+	 * used as a tool to correct earlier mistakes.
+	 */
+	strbuf_splice(sb, 0, 0, "refs/heads/", 11);
+
+	if (*name == '-' ||
+	    !strcmp(sb->buf, "refs/heads/HEAD"))
+		return -1;
+
+	return check_refname_format(sb->buf, 0);
+}
+
+int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
+{
+	if (name[0] == '-')
+		return -1;
+
+	strbuf_reset(sb);
+	strbuf_addf(sb, "refs/tags/%s", name);
+
+	return check_refname_format(sb->buf, 0);
+}
+
 int repo_dwim_ref(struct repository *r, const char *str, int len,
 		  struct object_id *oid, char **ref, int nonfatal_dangling_mark)
 {
diff --git a/refs.h b/refs.h
index 108dfc93b3..f19b0ad92f 100644
--- a/refs.h
+++ b/refs.h
@@ -180,6 +180,35 @@ int repo_dwim_log(struct repository *r, const char *str, int len, struct object_
  */
 char *repo_default_branch_name(struct repository *r, int quiet);
 
+/*
+ * Copy "name" to "sb", expanding any special @-marks as handled by
+ * repo_interpret_branch_name(). The result is a non-qualified branch name
+ * (so "foo" or "origin/master" instead of "refs/heads/foo" or
+ * "refs/remotes/origin/master").
+ *
+ * Note that the resulting name may not be a syntactically valid refname.
+ *
+ * If "allowed" is non-zero, restrict the set of allowed expansions. See
+ * repo_interpret_branch_name() for details.
+ */
+void strbuf_branchname(struct strbuf *sb, const char *name,
+		       unsigned allowed);
+
+/*
+ * Like strbuf_branchname() above, but confirm that the result is
+ * syntactically valid to be used as a local branch name in refs/heads/.
+ *
+ * The return value is "0" if the result is valid, and "-1" otherwise.
+ */
+int strbuf_check_branch_ref(struct strbuf *sb, const char *name);
+
+/*
+ * Similar for a tag name in refs/tags/.
+ *
+ * The return value is "0" if the result is valid, and "-1" otherwise.
+ */
+int strbuf_check_tag_ref(struct strbuf *sb, const char *name);
+
 /*
  * A ref_transaction represents a collection of reference updates that
  * should succeed or fail together.
diff --git a/strbuf.h b/strbuf.h
index 003f880ff7..4dc05b4ba7 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -637,28 +637,6 @@ static inline void strbuf_complete_line(struct strbuf *sb)
 	strbuf_complete(sb, '\n');
 }
 
-/*
- * Copy "name" to "sb", expanding any special @-marks as handled by
- * repo_interpret_branch_name(). The result is a non-qualified branch name
- * (so "foo" or "origin/master" instead of "refs/heads/foo" or
- * "refs/remotes/origin/master").
- *
- * Note that the resulting name may not be a syntactically valid refname.
- *
- * If "allowed" is non-zero, restrict the set of allowed expansions. See
- * repo_interpret_branch_name() for details.
- */
-void strbuf_branchname(struct strbuf *sb, const char *name,
-		       unsigned allowed);
-
-/*
- * Like strbuf_branchname() above, but confirm that the result is
- * syntactically valid to be used as a local branch name in refs/heads/.
- *
- * The return value is "0" if the result is valid, and "-1" otherwise.
- */
-int strbuf_check_branch_ref(struct strbuf *sb, const char *name);
-
 typedef int (*char_predicate)(char ch);
 
 void strbuf_addstr_urlencode(struct strbuf *sb, const char *name,

branch.c, builtin/branch.c, builtin/check-ref-format.c, builtin/checkout.c, builtin/merge.c, builtin/tag.c, builtin/worktree.c, gitweb/gitweb.perl, refs.c, refs.h

diff --git a/branch.c b/branch.c
index 08fa4094d2..58b61831af 100644
--- a/branch.c
+++ b/branch.c
@@ -372,7 +372,7 @@ int read_branch_desc(struct strbuf *buf, const char *branch_name)
  */
 int validate_branchname(const char *name, struct strbuf *ref)
 {
-	if (strbuf_check_branch_ref(ref, name)) {
+	if (check_branch_ref(ref, name)) {
 		int code = die_message(_("'%s' is not a valid branch name"), name);
 		advise_if_enabled(ADVICE_REF_SYNTAX,
 				  _("See `man git check-ref-format`"));
diff --git a/builtin/branch.c b/builtin/branch.c
index fd1611ebf5..17acf598d2 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -257,7 +257,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 		char *target = NULL;
 		int flags = 0;
 
-		strbuf_branchname(&bname, argv[i], allowed_interpret);
+		copy_branchname(&bname, argv[i], allowed_interpret);
 		free(name);
 		name = mkpathdup(fmt, bname.buf);
 
@@ -579,7 +579,7 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
 	int recovery = 0, oldref_usage = 0;
 	struct worktree **worktrees = get_worktrees();
 
-	if (strbuf_check_branch_ref(&oldref, oldname)) {
+	if (check_branch_ref(&oldref, oldname)) {
 		/*
 		 * Bad name --- this could be an attempt to rename a
 		 * ref that we used to allow to be created by accident.
@@ -894,7 +894,7 @@ int cmd_branch(int argc,
 				die(_("cannot give description to detached HEAD"));
 			branch_name = head;
 		} else if (argc == 1) {
-			strbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
+			copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
 			branch_name = buf.buf;
 		} else {
 			die(_("cannot edit description of more than one branch"));
@@ -933,7 +933,7 @@ int cmd_branch(int argc,
 		if (!argc)
 			branch = branch_get(NULL);
 		else if (argc == 1) {
-			strbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
+			copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
 			branch = branch_get(buf.buf);
 		} else
 			die(_("too many arguments to set new upstream"));
@@ -963,7 +963,7 @@ int cmd_branch(int argc,
 		if (!argc)
 			branch = branch_get(NULL);
 		else if (argc == 1) {
-			strbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
+			copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
 			branch = branch_get(buf.buf);
 		} else
 			die(_("too many arguments to unset upstream"));
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index e86d8ef980..cef1ffe3ce 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -42,7 +42,7 @@ static int check_ref_format_branch(const char *arg)
 	int nongit;
 
 	setup_git_directory_gently(&nongit);
-	if (strbuf_check_branch_ref(&sb, arg) ||
+	if (check_branch_ref(&sb, arg) ||
 	    !skip_prefix(sb.buf, "refs/heads/", &name))
 		die("'%s' is not a valid branch name", arg);
 	printf("%s\n", name);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index c449558e66..5e5afa0f26 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -742,7 +742,7 @@ static void setup_branch_path(struct branch_info *branch)
 			   &branch->oid, &branch->refname, 0))
 		repo_get_oid_committish(the_repository, branch->name, &branch->oid);
 
-	strbuf_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL);
+	copy_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL);
 	if (strcmp(buf.buf, branch->name)) {
 		free(branch->name);
 		branch->name = xstrdup(buf.buf);
diff --git a/builtin/merge.c b/builtin/merge.c
index 84d0f3604b..d0c31d7714 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -498,7 +498,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
 	char *found_ref = NULL;
 	int len, early;
 
-	strbuf_branchname(&bname, remote, 0);
+	copy_branchname(&bname, remote, 0);
 	remote = bname.buf;
 
 	oidclr(&branch_head, the_repository->hash_algo);
diff --git a/builtin/tag.c b/builtin/tag.c
index 8279dccbe0..670e564178 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -639,7 +639,7 @@ int cmd_tag(int argc,
 	if (repo_get_oid(the_repository, object_ref, &object))
 		die(_("Failed to resolve '%s' as a valid ref."), object_ref);
 
-	if (strbuf_check_tag_ref(&ref, tag))
+	if (check_tag_ref(&ref, tag))
 		die(_("'%s' is not a valid tag name."), tag);
 
 	if (refs_read_ref(get_main_ref_store(the_repository), ref.buf, &prev))
diff --git a/builtin/worktree.c b/builtin/worktree.c
index fc31d072a6..c68f601358 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -432,7 +432,7 @@ static int add_worktree(const char *path, const char *refname,
 	worktrees = NULL;
 
 	/* is 'refname' a branch or commit? */
-	if (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&
+	if (!opts->detach && !check_branch_ref(&symref, refname) &&
 	    refs_ref_exists(get_main_ref_store(the_repository), symref.buf)) {
 		is_branch = 1;
 		if (!opts->force)
@@ -604,7 +604,7 @@ static void print_preparing_worktree_line(int detach,
 		fprintf_ln(stderr, _("Preparing worktree (new branch '%s')"), new_branch);
 	} else {
 		struct strbuf s = STRBUF_INIT;
-		if (!detach && !strbuf_check_branch_ref(&s, branch) &&
+		if (!detach && !check_branch_ref(&s, branch) &&
 		    refs_ref_exists(get_main_ref_store(the_repository), s.buf))
 			fprintf_ln(stderr, _("Preparing worktree (checking out '%s')"),
 				  branch);
@@ -745,7 +745,7 @@ static char *dwim_branch(const char *path, char **new_branch)
 	char *branchname = xstrndup(s, n);
 	struct strbuf ref = STRBUF_INIT;
 
-	branch_exists = !strbuf_check_branch_ref(&ref, branchname) &&
+	branch_exists = !check_branch_ref(&ref, branchname) &&
 			refs_ref_exists(get_main_ref_store(the_repository),
 					ref.buf);
 	strbuf_release(&ref);
@@ -838,7 +838,7 @@ static int add(int ac, const char **av, const char *prefix)
 		new_branch = new_branch_force;
 
 		if (!opts.force &&
-		    !strbuf_check_branch_ref(&symref, new_branch) &&
+		    !check_branch_ref(&symref, new_branch) &&
 		    refs_ref_exists(get_main_ref_store(the_repository), symref.buf))
 			die_if_checked_out(symref.buf, 0);
 		strbuf_release(&symref);
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index b09a8d0523..8cdb0d9b9f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2094,7 +2094,7 @@ sub format_log_line_html {
         (
             # The output of "git describe", e.g. v2.10.0-297-gf6727b0
             # or hadoop-20160921-113441-20-g094fb7d
-            (?<!-) # see strbuf_check_tag_ref(). Tags can't start with -
+            (?<!-) # see check_tag_ref(). Tags can't start with -
             [A-Za-z0-9.-]+
             (?!\.) # refs can't end with ".", see check_refname_format()
             -g$regex
diff --git a/refs.c b/refs.c
index 59a9223d4c..a24bfe3845 100644
--- a/refs.c
+++ b/refs.c
@@ -697,7 +697,7 @@ static char *substitute_branch_name(struct repository *r,
 	return NULL;
 }
 
-void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)
+void copy_branchname(struct strbuf *sb, const char *name, unsigned allowed)
 {
 	int len = strlen(name);
 	struct interpret_branch_name_options options = {
@@ -711,10 +711,10 @@ void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)
 	strbuf_add(sb, name + used, len - used);
 }
 
-int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
+int check_branch_ref(struct strbuf *sb, const char *name)
 {
 	if (startup_info->have_repository)
-		strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
+		copy_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
 	else
 		strbuf_addstr(sb, name);
 
@@ -733,7 +733,7 @@ int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
 	return check_refname_format(sb->buf, 0);
 }
 
-int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
+int check_tag_ref(struct strbuf *sb, const char *name)
 {
 	if (name[0] == '-')
 		return -1;
diff --git a/refs.h b/refs.h
index f19b0ad92f..c5280477f0 100644
--- a/refs.h
+++ b/refs.h
@@ -191,23 +191,23 @@ char *repo_default_branch_name(struct repository *r, int quiet);
  * If "allowed" is non-zero, restrict the set of allowed expansions. See
  * repo_interpret_branch_name() for details.
  */
-void strbuf_branchname(struct strbuf *sb, const char *name,
+void copy_branchname(struct strbuf *sb, const char *name,
 		       unsigned allowed);
 
 /*
- * Like strbuf_branchname() above, but confirm that the result is
+ * Like copy_branchname() above, but confirm that the result is
  * syntactically valid to be used as a local branch name in refs/heads/.
  *
  * The return value is "0" if the result is valid, and "-1" otherwise.
  */
-int strbuf_check_branch_ref(struct strbuf *sb, const char *name);
+int check_branch_ref(struct strbuf *sb, const char *name);
 
 /*
  * Similar for a tag name in refs/tags/.
  *
  * The return value is "0" if the result is valid, and "-1" otherwise.
  */
-int strbuf_check_tag_ref(struct strbuf *sb, const char *name);
+int check_tag_ref(struct strbuf *sb, const char *name);
 
 /*
  * A ref_transaction represents a collection of reference updates that

diff --git a/t/t5604-clone-reference.sh b/t/t5604-clone-reference.sh
index 9b32db8478..5f5c650ff8 100755
--- a/t/t5604-clone-reference.sh
+++ b/t/t5604-clone-reference.sh
@@ -131,7 +131,7 @@ test_expect_success 'cloning with multiple references drops duplicates' '
 
 test_expect_success 'clone with reference from a tagged repository' '
 	(
-		cd A && git tag -a -m tagged HEAD
+		cd A && git tag -a -m tagged foo
 	) &&
 	git clone --reference=A A I
 '
@@ -156,10 +156,10 @@ test_expect_success 'fetch with incomplete alternates' '
 		git remote add J "file://$base_dir/J" &&
 		GIT_TRACE_PACKET=$U.K git fetch J
 	) &&
-	main_object=$(cd A && git for-each-ref --format="%(objectname)" refs/heads/main) &&
+	main_object=$(git -C A rev-parse --verify refs/heads/main) &&
 	test -s "$U.K" &&
 	! grep " want $main_object" "$U.K" &&
-	tag_object=$(cd A && git for-each-ref --format="%(objectname)" refs/tags/HEAD) &&
+	tag_object=$(git -C A rev-parse --verify refs/tags/foo) &&
 	! grep " want $tag_object" "$U.K"
 '
 

refs.c, t/t7004-tag.sh

diff --git a/refs.c b/refs.c
index a24bfe3845..01ef2a3093 100644
--- a/refs.c
+++ b/refs.c
@@ -735,7 +735,7 @@ int check_branch_ref(struct strbuf *sb, const char *name)
 
 int check_tag_ref(struct strbuf *sb, const char *name)
 {
-	if (name[0] == '-')
+	if (name[0] == '-' || !strcmp(name, "HEAD"))
 		return -1;
 
 	strbuf_reset(sb);
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index b1316e62f4..2082ce63f7 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -91,6 +91,12 @@ test_expect_success 'creating a tag using default HEAD should succeed' '
 	test_must_fail git reflog exists refs/tags/mytag
 '
 
+test_expect_success 'HEAD is forbidden as a tagname' '
+	test_when_finished "git tag -d HEAD || :" &&
+	test_must_fail git tag HEAD &&
+	test_must_fail git tag -a -m "useless" HEAD
+'
+
 test_expect_success 'creating a tag with --create-reflog should create reflog' '
 	git log -1 \
 		--format="format:tag: tagging %h (%s, %cd)%n" \

builtin/tag.c, t/t7004-tag.sh

diff --git a/builtin/tag.c b/builtin/tag.c
index 670e564178..b65f56e5b4 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -88,6 +88,8 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn,
 
        for (p = argv; *p; p++) {
                strbuf_reset(&ref);
+               if (!strcmp(*p, "HEAD"))
+                     die("Hi!");
                strbuf_addf(&ref, "refs/tags/%s", *p);
                if (refs_read_ref(get_main_ref_store(the_repository), ref.buf, &oid)) {
                        error(_("tag '%s' not found."), *p);

Therefore, if the previous seems reasonable, perhaps we should add a
test like:

--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -97,6 +97,11 @@ test_expect_success 'HEAD is forbidden as a tagname' '
 	test_must_fail git tag -a -m "useless" HEAD
 '
 
+test_expect_success 'allow deleting a tag named HEAD' '
+	git update-ref refs/tags/HEAD HEAD &&
+	git tag -d HEAD
+'
+
 test_expect_success 'creating a tag with --create-reflog should create reflog' '
 	git log -1 \
 		--format="format:tag: tagging %h (%s, %cd)%n" \

> 
> Helped-by: Jeff King <peff@peff.net>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  refs.c         | 2 +-
>  t/t7004-tag.sh | 6 ++++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/refs.c b/refs.c
> index a24bfe3845..01ef2a3093 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -735,7 +735,7 @@ int check_branch_ref(struct strbuf *sb, const char *name)
>  
>  int check_tag_ref(struct strbuf *sb, const char *name)
>  {
> -	if (name[0] == '-')
> +	if (name[0] == '-' || !strcmp(name, "HEAD"))
>  		return -1;
>  
>  	strbuf_reset(sb);
> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index b1316e62f4..2082ce63f7 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -91,6 +91,12 @@ test_expect_success 'creating a tag using default HEAD should succeed' '
>  	test_must_fail git reflog exists refs/tags/mytag
>  '
>  
> +test_expect_success 'HEAD is forbidden as a tagname' '
> +	test_when_finished "git tag -d HEAD || :" &&

I'm not considering this as a test for it :-)

> +	test_must_fail git tag HEAD &&
> +	test_must_fail git tag -a -m "useless" HEAD
> +'
> +
>  test_expect_success 'creating a tag with --create-reflog should create reflog' '
>  	git log -1 \
>  		--format="format:tag: tagging %h (%s, %cd)%n" \
> -- 
> 2.47.1-514-g9b43e7ecc4
>