{"thread":{"id":"62584","subject":"[PATCH 0/4] forbid HEAD as a tagname","startedAt":"2024-12-02T07:07:17Z","lastAt":"2024-12-05T20:27:46Z","messageCount":22,"participants":["Junio C Hamano","Patrick Steinhardt","Kristoffer Haugsbakk","shejialuo","Jeff King","Rubén Justo"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"508415","messageId":"20241202070714.3028549-1-gitster@pobox.com","threadId":"62584","inReplyTo":null,"subject":"[PATCH 0/4] forbid HEAD as a tagname","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-02T07:07:10Z","receivedAt":"2024-12-02T07:07:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"So, here is a cleaned up version, which ended up to be a 4-patch\nseries.\n\n - The first two steps move code around to get rid of the \"strbuf_\"\n   prefix from three misnamed helper functions that are more about\n   refs than they are about string manipulation operations.  We now\n   have:\n\n   - copy_branchname() that copies a branchname while applying\n     interpret_branch_name() like @{-1} for previous branch, @{u}\n     for upstream, etc.\n\n   - check_branch_ref() and check_tag_ref() that are allowed to be a\n     bit stricter than check_refname_format(), e.g., to reject \"HEAD\"\n     as the name of a branch or a tag.\n\n - The third step updates a test that assumes HEAD can be usable as\n   the name of a tag, but the breakage the test tries to protect is\n   not specific to any tagname.\n\n - The final step then forbids \"git tag\" from using \"HEAD\" as the\n   name of a tag.\n\nJunio C Hamano (4):\n  refs: move ref name helpers around\n  refs: drop strbuf_ prefix from helpers\n  t5604: do not expect that HEAD is a valid tagname\n  tag: \"git tag\" refuses to use HEAD as a tagname\n\n branch.c                   |  2 +-\n builtin/branch.c           | 10 ++++----\n builtin/check-ref-format.c |  2 +-\n builtin/checkout.c         |  2 +-\n builtin/merge.c            |  2 +-\n builtin/tag.c              | 13 +----------\n builtin/worktree.c         |  8 +++----\n gitweb/gitweb.perl         |  2 +-\n object-name.c              | 36 -----------------------------\n refs.c                     | 47 ++++++++++++++++++++++++++++++++++++++\n refs.h                     | 29 +++++++++++++++++++++++\n strbuf.h                   | 22 ------------------\n t/t5604-clone-reference.sh |  6 ++---\n t/t7004-tag.sh             |  6 +++++\n 14 files changed, 100 insertions(+), 87 deletions(-)\n\n-- \n2.47.1-514-g9b43e7ecc4\n\n"},{"id":"508416","messageId":"20241202070714.3028549-2-gitster@pobox.com","threadId":"62584","inReplyTo":"20241202070714.3028549-1-gitster@pobox.com","subject":"[PATCH 1/4] refs: move ref name helpers around","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-02T07:07:11Z","receivedAt":"2024-12-02T07:07:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"strbuf_branchname(), strbuf_check_{branch,tag}_ref() are helper\nfunctions to deal with branch and tag names, and the fact that they\nhappen to use strbuf to hold the name of a branch or a tag is not\nessential.  These functions fit better in the refs API than strbuf\nAPI, the latter of which is about string manipulations.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/tag.c | 11 -----------\n object-name.c | 36 ------------------------------------\n refs.c        | 47 +++++++++++++++++++++++++++++++++++++++++++++++\n refs.h        | 29 +++++++++++++++++++++++++++++\n strbuf.h      | 22 ----------------------\n 5 files changed, 76 insertions(+), 69 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 93d10d5915..8279dccbe0 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -447,17 +447,6 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset)\n \treturn 0;\n }\n \n-static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)\n-{\n-\tif (name[0] == '-')\n-\t\treturn -1;\n-\n-\tstrbuf_reset(sb);\n-\tstrbuf_addf(sb, \"refs/tags/%s\", name);\n-\n-\treturn check_refname_format(sb->buf, 0);\n-}\n-\n int cmd_tag(int argc,\n \t    const char **argv,\n \t    const char *prefix,\ndiff --git a/object-name.c b/object-name.c\nindex c892fbe80a..9f2ae164e4 100644\n--- a/object-name.c\n+++ b/object-name.c\n@@ -1734,42 +1734,6 @@ int repo_interpret_branch_name(struct repository *r,\n \treturn -1;\n }\n \n-void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)\n-{\n-\tint len = strlen(name);\n-\tstruct interpret_branch_name_options options = {\n-\t\t.allowed = allowed\n-\t};\n-\tint used = repo_interpret_branch_name(the_repository, name, len, sb,\n-\t\t\t\t\t      &options);\n-\n-\tif (used < 0)\n-\t\tused = 0;\n-\tstrbuf_add(sb, name + used, len - used);\n-}\n-\n-int strbuf_check_branch_ref(struct strbuf *sb, const char *name)\n-{\n-\tif (startup_info->have_repository)\n-\t\tstrbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);\n-\telse\n-\t\tstrbuf_addstr(sb, name);\n-\n-\t/*\n-\t * This splice must be done even if we end up rejecting the\n-\t * name; builtin/branch.c::copy_or_rename_branch() still wants\n-\t * to see what the name expanded to so that \"branch -m\" can be\n-\t * used as a tool to correct earlier mistakes.\n-\t */\n-\tstrbuf_splice(sb, 0, 0, \"refs/heads/\", 11);\n-\n-\tif (*name == '-' ||\n-\t    !strcmp(sb->buf, \"refs/heads/HEAD\"))\n-\t\treturn -1;\n-\n-\treturn check_refname_format(sb->buf, 0);\n-}\n-\n void object_context_release(struct object_context *ctx)\n {\n \tfree(ctx->path);\ndiff --git a/refs.c b/refs.c\nindex 5f729ed412..59a9223d4c 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -697,6 +697,53 @@ static char *substitute_branch_name(struct repository *r,\n \treturn NULL;\n }\n \n+void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)\n+{\n+\tint len = strlen(name);\n+\tstruct interpret_branch_name_options options = {\n+\t\t.allowed = allowed\n+\t};\n+\tint used = repo_interpret_branch_name(the_repository, name, len, sb,\n+\t\t\t\t\t      &options);\n+\n+\tif (used < 0)\n+\t\tused = 0;\n+\tstrbuf_add(sb, name + used, len - used);\n+}\n+\n+int strbuf_check_branch_ref(struct strbuf *sb, const char *name)\n+{\n+\tif (startup_info->have_repository)\n+\t\tstrbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);\n+\telse\n+\t\tstrbuf_addstr(sb, name);\n+\n+\t/*\n+\t * This splice must be done even if we end up rejecting the\n+\t * name; builtin/branch.c::copy_or_rename_branch() still wants\n+\t * to see what the name expanded to so that \"branch -m\" can be\n+\t * used as a tool to correct earlier mistakes.\n+\t */\n+\tstrbuf_splice(sb, 0, 0, \"refs/heads/\", 11);\n+\n+\tif (*name == '-' ||\n+\t    !strcmp(sb->buf, \"refs/heads/HEAD\"))\n+\t\treturn -1;\n+\n+\treturn check_refname_format(sb->buf, 0);\n+}\n+\n+int strbuf_check_tag_ref(struct strbuf *sb, const char *name)\n+{\n+\tif (name[0] == '-')\n+\t\treturn -1;\n+\n+\tstrbuf_reset(sb);\n+\tstrbuf_addf(sb, \"refs/tags/%s\", name);\n+\n+\treturn check_refname_format(sb->buf, 0);\n+}\n+\n int repo_dwim_ref(struct repository *r, const char *str, int len,\n \t\t  struct object_id *oid, char **ref, int nonfatal_dangling_mark)\n {\ndiff --git a/refs.h b/refs.h\nindex 108dfc93b3..f19b0ad92f 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -180,6 +180,35 @@ int repo_dwim_log(struct repository *r, const char *str, int len, struct object_\n  */\n char *repo_default_branch_name(struct repository *r, int quiet);\n \n+/*\n+ * Copy \"name\" to \"sb\", expanding any special @-marks as handled by\n+ * repo_interpret_branch_name(). The result is a non-qualified branch name\n+ * (so \"foo\" or \"origin/master\" instead of \"refs/heads/foo\" or\n+ * \"refs/remotes/origin/master\").\n+ *\n+ * Note that the resulting name may not be a syntactically valid refname.\n+ *\n+ * If \"allowed\" is non-zero, restrict the set of allowed expansions. See\n+ * repo_interpret_branch_name() for details.\n+ */\n+void strbuf_branchname(struct strbuf *sb, const char *name,\n+\t\t       unsigned allowed);\n+\n+/*\n+ * Like strbuf_branchname() above, but confirm that the result is\n+ * syntactically valid to be used as a local branch name in refs/heads/.\n+ *\n+ * The return value is \"0\" if the result is valid, and \"-1\" otherwise.\n+ */\n+int strbuf_check_branch_ref(struct strbuf *sb, const char *name);\n+\n+/*\n+ * Similar for a tag name in refs/tags/.\n+ *\n+ * The return value is \"0\" if the result is valid, and \"-1\" otherwise.\n+ */\n+int strbuf_check_tag_ref(struct strbuf *sb, const char *name);\n+\n /*\n  * A ref_transaction represents a collection of reference updates that\n  * should succeed or fail together.\ndiff --git a/strbuf.h b/strbuf.h\nindex 003f880ff7..4dc05b4ba7 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -637,28 +637,6 @@ static inline void strbuf_complete_line(struct strbuf *sb)\n \tstrbuf_complete(sb, '\\n');\n }\n \n-/*\n- * Copy \"name\" to \"sb\", expanding any special @-marks as handled by\n- * repo_interpret_branch_name(). The result is a non-qualified branch name\n- * (so \"foo\" or \"origin/master\" instead of \"refs/heads/foo\" or\n- * \"refs/remotes/origin/master\").\n- *\n- * Note that the resulting name may not be a syntactically valid refname.\n- *\n- * If \"allowed\" is non-zero, restrict the set of allowed expansions. See\n- * repo_interpret_branch_name() for details.\n- */\n-void strbuf_branchname(struct strbuf *sb, const char *name,\n-\t\t       unsigned allowed);\n-\n-/*\n- * Like strbuf_branchname() above, but confirm that the result is\n- * syntactically valid to be used as a local branch name in refs/heads/.\n- *\n- * The return value is \"0\" if the result is valid, and \"-1\" otherwise.\n- */\n-int strbuf_check_branch_ref(struct strbuf *sb, const char *name);\n-\n typedef int (*char_predicate)(char ch);\n \n void strbuf_addstr_urlencode(struct strbuf *sb, const char *name,\n-- \n2.47.1-514-g9b43e7ecc4\n\n"},{"id":"508417","messageId":"20241202070714.3028549-3-gitster@pobox.com","threadId":"62584","inReplyTo":"20241202070714.3028549-1-gitster@pobox.com","subject":"[PATCH 2/4] refs: drop strbuf_ prefix from helpers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-02T07:07:12Z","receivedAt":"2024-12-02T07:07:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The helper functions (strbuf_branchname, strbuf_check_branch_ref,\nand strbuf_check_tag_ref) are about handling branch and tag names,\nand it is a non-essential fact that these functions use strbuf to\nhold these names.  Rename them to make it clarify that these are\nmore about \"ref\".\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n branch.c                   |  2 +-\n builtin/branch.c           | 10 +++++-----\n builtin/check-ref-format.c |  2 +-\n builtin/checkout.c         |  2 +-\n builtin/merge.c            |  2 +-\n builtin/tag.c              |  2 +-\n builtin/worktree.c         |  8 ++++----\n gitweb/gitweb.perl         |  2 +-\n refs.c                     |  8 ++++----\n refs.h                     |  8 ++++----\n 10 files changed, 23 insertions(+), 23 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 08fa4094d2..58b61831af 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -372,7 +372,7 @@ int read_branch_desc(struct strbuf *buf, const char *branch_name)\n  */\n int validate_branchname(const char *name, struct strbuf *ref)\n {\n-\tif (strbuf_check_branch_ref(ref, name)) {\n+\tif (check_branch_ref(ref, name)) {\n \t\tint code = die_message(_(\"'%s' is not a valid branch name\"), name);\n \t\tadvise_if_enabled(ADVICE_REF_SYNTAX,\n \t\t\t\t  _(\"See `man git check-ref-format`\"));\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex fd1611ebf5..17acf598d2 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -257,7 +257,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n \t\tchar *target = NULL;\n \t\tint flags = 0;\n \n-\t\tstrbuf_branchname(&bname, argv[i], allowed_interpret);\n+\t\tcopy_branchname(&bname, argv[i], allowed_interpret);\n \t\tfree(name);\n \t\tname = mkpathdup(fmt, bname.buf);\n \n@@ -579,7 +579,7 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int\n \tint recovery = 0, oldref_usage = 0;\n \tstruct worktree **worktrees = get_worktrees();\n \n-\tif (strbuf_check_branch_ref(&oldref, oldname)) {\n+\tif (check_branch_ref(&oldref, oldname)) {\n \t\t/*\n \t\t * Bad name --- this could be an attempt to rename a\n \t\t * ref that we used to allow to be created by accident.\n@@ -894,7 +894,7 @@ int cmd_branch(int argc,\n \t\t\t\tdie(_(\"cannot give description to detached HEAD\"));\n \t\t\tbranch_name = head;\n \t\t} else if (argc == 1) {\n-\t\t\tstrbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);\n+\t\t\tcopy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);\n \t\t\tbranch_name = buf.buf;\n \t\t} else {\n \t\t\tdie(_(\"cannot edit description of more than one branch\"));\n@@ -933,7 +933,7 @@ int cmd_branch(int argc,\n \t\tif (!argc)\n \t\t\tbranch = branch_get(NULL);\n \t\telse if (argc == 1) {\n-\t\t\tstrbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);\n+\t\t\tcopy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);\n \t\t\tbranch = branch_get(buf.buf);\n \t\t} else\n \t\t\tdie(_(\"too many arguments to set new upstream\"));\n@@ -963,7 +963,7 @@ int cmd_branch(int argc,\n \t\tif (!argc)\n \t\t\tbranch = branch_get(NULL);\n \t\telse if (argc == 1) {\n-\t\t\tstrbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);\n+\t\t\tcopy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);\n \t\t\tbranch = branch_get(buf.buf);\n \t\t} else\n \t\t\tdie(_(\"too many arguments to unset upstream\"));\ndiff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c\nindex e86d8ef980..cef1ffe3ce 100644\n--- a/builtin/check-ref-format.c\n+++ b/builtin/check-ref-format.c\n@@ -42,7 +42,7 @@ static int check_ref_format_branch(const char *arg)\n \tint nongit;\n \n \tsetup_git_directory_gently(&nongit);\n-\tif (strbuf_check_branch_ref(&sb, arg) ||\n+\tif (check_branch_ref(&sb, arg) ||\n \t    !skip_prefix(sb.buf, \"refs/heads/\", &name))\n \t\tdie(\"'%s' is not a valid branch name\", arg);\n \tprintf(\"%s\\n\", name);\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex c449558e66..5e5afa0f26 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -742,7 +742,7 @@ static void setup_branch_path(struct branch_info *branch)\n \t\t\t   &branch->oid, &branch->refname, 0))\n \t\trepo_get_oid_committish(the_repository, branch->name, &branch->oid);\n \n-\tstrbuf_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL);\n+\tcopy_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL);\n \tif (strcmp(buf.buf, branch->name)) {\n \t\tfree(branch->name);\n \t\tbranch->name = xstrdup(buf.buf);\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 84d0f3604b..d0c31d7714 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -498,7 +498,7 @@ static void merge_name(const char *remote, struct strbuf *msg)\n \tchar *found_ref = NULL;\n \tint len, early;\n \n-\tstrbuf_branchname(&bname, remote, 0);\n+\tcopy_branchname(&bname, remote, 0);\n \tremote = bname.buf;\n \n \toidclr(&branch_head, the_repository->hash_algo);\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 8279dccbe0..670e564178 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -639,7 +639,7 @@ int cmd_tag(int argc,\n \tif (repo_get_oid(the_repository, object_ref, &object))\n \t\tdie(_(\"Failed to resolve '%s' as a valid ref.\"), object_ref);\n \n-\tif (strbuf_check_tag_ref(&ref, tag))\n+\tif (check_tag_ref(&ref, tag))\n \t\tdie(_(\"'%s' is not a valid tag name.\"), tag);\n \n \tif (refs_read_ref(get_main_ref_store(the_repository), ref.buf, &prev))\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex fc31d072a6..c68f601358 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -432,7 +432,7 @@ static int add_worktree(const char *path, const char *refname,\n \tworktrees = NULL;\n \n \t/* is 'refname' a branch or commit? */\n-\tif (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&\n+\tif (!opts->detach && !check_branch_ref(&symref, refname) &&\n \t    refs_ref_exists(get_main_ref_store(the_repository), symref.buf)) {\n \t\tis_branch = 1;\n \t\tif (!opts->force)\n@@ -604,7 +604,7 @@ static void print_preparing_worktree_line(int detach,\n \t\tfprintf_ln(stderr, _(\"Preparing worktree (new branch '%s')\"), new_branch);\n \t} else {\n \t\tstruct strbuf s = STRBUF_INIT;\n-\t\tif (!detach && !strbuf_check_branch_ref(&s, branch) &&\n+\t\tif (!detach && !check_branch_ref(&s, branch) &&\n \t\t    refs_ref_exists(get_main_ref_store(the_repository), s.buf))\n \t\t\tfprintf_ln(stderr, _(\"Preparing worktree (checking out '%s')\"),\n \t\t\t\t  branch);\n@@ -745,7 +745,7 @@ static char *dwim_branch(const char *path, char **new_branch)\n \tchar *branchname = xstrndup(s, n);\n \tstruct strbuf ref = STRBUF_INIT;\n \n-\tbranch_exists = !strbuf_check_branch_ref(&ref, branchname) &&\n+\tbranch_exists = !check_branch_ref(&ref, branchname) &&\n \t\t\trefs_ref_exists(get_main_ref_store(the_repository),\n \t\t\t\t\tref.buf);\n \tstrbuf_release(&ref);\n@@ -838,7 +838,7 @@ static int add(int ac, const char **av, const char *prefix)\n \t\tnew_branch = new_branch_force;\n \n \t\tif (!opts.force &&\n-\t\t    !strbuf_check_branch_ref(&symref, new_branch) &&\n+\t\t    !check_branch_ref(&symref, new_branch) &&\n \t\t    refs_ref_exists(get_main_ref_store(the_repository), symref.buf))\n \t\t\tdie_if_checked_out(symref.buf, 0);\n \t\tstrbuf_release(&symref);\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex b09a8d0523..8cdb0d9b9f 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2094,7 +2094,7 @@ sub format_log_line_html {\n         (\n             # The output of \"git describe\", e.g. v2.10.0-297-gf6727b0\n             # or hadoop-20160921-113441-20-g094fb7d\n-            (?<!-) # see strbuf_check_tag_ref(). Tags can't start with -\n+            (?<!-) # see check_tag_ref(). Tags can't start with -\n             [A-Za-z0-9.-]+\n             (?!\\.) # refs can't end with \".\", see check_refname_format()\n             -g$regex\ndiff --git a/refs.c b/refs.c\nindex 59a9223d4c..a24bfe3845 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -697,7 +697,7 @@ static char *substitute_branch_name(struct repository *r,\n \treturn NULL;\n }\n \n-void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)\n+void copy_branchname(struct strbuf *sb, const char *name, unsigned allowed)\n {\n \tint len = strlen(name);\n \tstruct interpret_branch_name_options options = {\n@@ -711,10 +711,10 @@ void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)\n \tstrbuf_add(sb, name + used, len - used);\n }\n \n-int strbuf_check_branch_ref(struct strbuf *sb, const char *name)\n+int check_branch_ref(struct strbuf *sb, const char *name)\n {\n \tif (startup_info->have_repository)\n-\t\tstrbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);\n+\t\tcopy_branchname(sb, name, INTERPRET_BRANCH_LOCAL);\n \telse\n \t\tstrbuf_addstr(sb, name);\n \n@@ -733,7 +733,7 @@ int strbuf_check_branch_ref(struct strbuf *sb, const char *name)\n \treturn check_refname_format(sb->buf, 0);\n }\n \n-int strbuf_check_tag_ref(struct strbuf *sb, const char *name)\n+int check_tag_ref(struct strbuf *sb, const char *name)\n {\n \tif (name[0] == '-')\n \t\treturn -1;\ndiff --git a/refs.h b/refs.h\nindex f19b0ad92f..c5280477f0 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -191,23 +191,23 @@ char *repo_default_branch_name(struct repository *r, int quiet);\n  * If \"allowed\" is non-zero, restrict the set of allowed expansions. See\n  * repo_interpret_branch_name() for details.\n  */\n-void strbuf_branchname(struct strbuf *sb, const char *name,\n+void copy_branchname(struct strbuf *sb, const char *name,\n \t\t       unsigned allowed);\n \n /*\n- * Like strbuf_branchname() above, but confirm that the result is\n+ * Like copy_branchname() above, but confirm that the result is\n  * syntactically valid to be used as a local branch name in refs/heads/.\n  *\n  * The return value is \"0\" if the result is valid, and \"-1\" otherwise.\n  */\n-int strbuf_check_branch_ref(struct strbuf *sb, const char *name);\n+int check_branch_ref(struct strbuf *sb, const char *name);\n \n /*\n  * Similar for a tag name in refs/tags/.\n  *\n  * The return value is \"0\" if the result is valid, and \"-1\" otherwise.\n  */\n-int strbuf_check_tag_ref(struct strbuf *sb, const char *name);\n+int check_tag_ref(struct strbuf *sb, const char *name);\n \n /*\n  * A ref_transaction represents a collection of reference updates that\n-- \n2.47.1-514-g9b43e7ecc4\n\n"},{"id":"508418","messageId":"20241202070714.3028549-4-gitster@pobox.com","threadId":"62584","inReplyTo":"20241202070714.3028549-1-gitster@pobox.com","subject":"[PATCH 3/4] t5604: do not expect that HEAD is a valid tagname","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-02T07:07:13Z","receivedAt":"2024-12-02T07:07:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"09116a1c (refs: loosen over-strict \"format\" check, 2011-11-16)\nintroduced a test piece (originally in t5700) that expects to be\nable to create a tag named \"HEAD\" and then a local clone using the\nrepository as its own reference works correctly.  Later, another\ntest piece started using this tag starting at acede2eb (t5700:\ndocument a failure of alternates to affect fetch, 2012-02-11).\n\nBut the breakage 09116a1c fixed was not specific to the tagname\nHEAD.  It would have failed exactly the same way if the tag used\nwere foo instead of HEAD.\n\nBefore forbidding \"git tag\" from creating \"refs/tags/HEAD\", update\nthese tests to use 'foo', not 'HEAD', as the name of the test tag.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t5604-clone-reference.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t5604-clone-reference.sh b/t/t5604-clone-reference.sh\nindex 9b32db8478..5f5c650ff8 100755\n--- a/t/t5604-clone-reference.sh\n+++ b/t/t5604-clone-reference.sh\n@@ -131,7 +131,7 @@ test_expect_success 'cloning with multiple references drops duplicates' '\n \n test_expect_success 'clone with reference from a tagged repository' '\n \t(\n-\t\tcd A && git tag -a -m tagged HEAD\n+\t\tcd A && git tag -a -m tagged foo\n \t) &&\n \tgit clone --reference=A A I\n '\n@@ -156,10 +156,10 @@ test_expect_success 'fetch with incomplete alternates' '\n \t\tgit remote add J \"file://$base_dir/J\" &&\n \t\tGIT_TRACE_PACKET=$U.K git fetch J\n \t) &&\n-\tmain_object=$(cd A && git for-each-ref --format=\"%(objectname)\" refs/heads/main) &&\n+\tmain_object=$(git -C A rev-parse --verify refs/heads/main) &&\n \ttest -s \"$U.K\" &&\n \t! grep \" want $main_object\" \"$U.K\" &&\n-\ttag_object=$(cd A && git for-each-ref --format=\"%(objectname)\" refs/tags/HEAD) &&\n+\ttag_object=$(git -C A rev-parse --verify refs/tags/foo) &&\n \t! grep \" want $tag_object\" \"$U.K\"\n '\n \n-- \n2.47.1-514-g9b43e7ecc4\n\n"},{"id":"508419","messageId":"20241202070714.3028549-5-gitster@pobox.com","threadId":"62584","inReplyTo":"20241202070714.3028549-1-gitster@pobox.com","subject":"[PATCH 4/4] tag: \"git tag\" refuses to use HEAD as a tagname","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-02T07:07:14Z","receivedAt":"2024-12-02T07:07:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Even though the plumbing level allows you to create refs/tags/HEAD\nand refs/heads/HEAD, doing so makes it confusing within the context\nof the UI Git Porcelain commands provides.  Just like we prevent a\nbranch from getting called \"HEAD\" at the Porcelain layer (i.e. \"git\nbranch\" command), teach \"git tag\" to refuse to create a tag \"HEAD\".\n\nHelped-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n refs.c         | 2 +-\n t/t7004-tag.sh | 6 ++++++\n 2 files changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex a24bfe3845..01ef2a3093 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -735,7 +735,7 @@ int check_branch_ref(struct strbuf *sb, const char *name)\n \n int check_tag_ref(struct strbuf *sb, const char *name)\n {\n-\tif (name[0] == '-')\n+\tif (name[0] == '-' || !strcmp(name, \"HEAD\"))\n \t\treturn -1;\n \n \tstrbuf_reset(sb);\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex b1316e62f4..2082ce63f7 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -91,6 +91,12 @@ test_expect_success 'creating a tag using default HEAD should succeed' '\n \ttest_must_fail git reflog exists refs/tags/mytag\n '\n \n+test_expect_success 'HEAD is forbidden as a tagname' '\n+\ttest_when_finished \"git tag -d HEAD || :\" &&\n+\ttest_must_fail git tag HEAD &&\n+\ttest_must_fail git tag -a -m \"useless\" HEAD\n+'\n+\n test_expect_success 'creating a tag with --create-reflog should create reflog' '\n \tgit log -1 \\\n \t\t--format=\"format:tag: tagging %h (%s, %cd)%n\" \\\n-- \n2.47.1-514-g9b43e7ecc4\n\n"},{"id":"508426","messageId":"Z02R2Puck52VetcF@pks.im","threadId":"62584","inReplyTo":"20241202070714.3028549-5-gitster@pobox.com","subject":"Re: [PATCH 4/4] tag: \"git tag\" refuses to use HEAD as a tagname","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-02T10:54:21Z","receivedAt":"2024-12-02T10:54:40Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Dec 02, 2024 at 04:07:14PM +0900, Junio C Hamano wrote:\n> diff --git a/refs.c b/refs.c\n> index a24bfe3845..01ef2a3093 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -735,7 +735,7 @@ int check_branch_ref(struct strbuf *sb, const char *name)\n>  \n>  int check_tag_ref(struct strbuf *sb, const char *name)\n>  {\n> -\tif (name[0] == '-')\n> +\tif (name[0] == '-' || !strcmp(name, \"HEAD\"))\n>  \t\treturn -1;\n>  \n>  \tstrbuf_reset(sb);\n\nI was thinking a bit about whether we can spin this a bit wider and\ndisallow creation of any refname that looks like a root ref. But I don't\nthink that would make sense, as root refs are defined as all-uppercase\nrefs living in the root, and disallowing tags that look like this would\ngo way too far.\n\nSo I then wondered what a reasonable alternative would be, and the only\nrule I could come up with was to disallow root refs with \"HEAD\" in it.\nBut even that doesn't feel reasonable to me.\n\nAll to say: singling out \"HEAD\" feels like a sensible step, and I don't\nthink we should handle root refs more generally here.\n\nThe other patches look good to me, as well. Thanks!\n\nPatrick\n"},{"id":"508444","messageId":"477f0dbd-60ed-4f73-b945-cdbdaf9f510a@app.fastmail.com","threadId":"62584","inReplyTo":"20241202070714.3028549-4-gitster@pobox.com","subject":"Re: [PATCH 3/4] t5604: do not expect that HEAD is a valid tagname","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2024-12-02T12:19:56Z","receivedAt":"2024-12-02T12:20:17Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Mon, Dec 2, 2024, at 08:07, Junio C Hamano wrote:\n> 09116a1c (refs: loosen over-strict \"format\" check, 2011-11-16)\n\nNit/confusion: the abbreviated hash is only eight hexes long.  I’m used to it\nbeing 11 for this project?\n\nDoes the age of the commit matter?\n"},{"id":"508448","messageId":"Z02voaSNYRciv38z@ArchLinux","threadId":"62584","inReplyTo":"20241202070714.3028549-5-gitster@pobox.com","subject":"Re: [PATCH 4/4] tag: \"git tag\" refuses to use HEAD as a tagname","fromName":"shejialuo","fromEmail":"shejialuo@gmail.com","sentAt":"2024-12-02T13:01:21Z","receivedAt":"2024-12-02T13:00:59Z","isPatch":true,"sender":{"key":"shejialuo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56911263?v=4"},"body":"On Mon, Dec 02, 2024 at 04:07:14PM +0900, Junio C Hamano wrote:\n\n[snip]\n\n> diff --git a/refs.c b/refs.c\n> index a24bfe3845..01ef2a3093 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -735,7 +735,7 @@ int check_branch_ref(struct strbuf *sb, const char *name)\n>  \n>  int check_tag_ref(struct strbuf *sb, const char *name)\n>  {\n> -\tif (name[0] == '-')\n> +\tif (name[0] == '-' || !strcmp(name, \"HEAD\"))\n\nI am wondering whether we should also update \"check_refname_format\"\nfunction to report \"refs/heads/HEAD\" and \"refs/tags/HEAD\" is bad ref\nname.\n\n>  \t\treturn -1;\n>  \n>  \tstrbuf_reset(sb);\n\nThanks,\nJialuo\n"},{"id":"508462","messageId":"20241202203743.GB776185@coredump.intra.peff.net","threadId":"62584","inReplyTo":"20241202070714.3028549-2-gitster@pobox.com","subject":"Re: [PATCH 1/4] refs: move ref name helpers around","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-12-02T20:37:43Z","receivedAt":"2024-12-02T20:37:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 02, 2024 at 04:07:11PM +0900, Junio C Hamano wrote:\n\n> strbuf_branchname(), strbuf_check_{branch,tag}_ref() are helper\n> functions to deal with branch and tag names, and the fact that they\n> happen to use strbuf to hold the name of a branch or a tag is not\n> essential.  These functions fit better in the refs API than strbuf\n> API, the latter of which is about string manipulations.\n\nWow, they are declared in strbuf.h but not even implemented there. So it\nwas doubly confusing. This looks like a nice cleanup.\n\n-Peff\n"},{"id":"508463","messageId":"ee2af264-8d4f-401d-893c-e08c30f5a9b6@gmail.com","threadId":"62584","inReplyTo":"20241202070714.3028549-5-gitster@pobox.com","subject":"Re: [PATCH 4/4] tag: \"git tag\" refuses to use HEAD as a tagname","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2024-12-02T20:42:56Z","receivedAt":"2024-12-02T20:42:59Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"On Mon, Dec 02, 2024 at 04:07:14PM +0900, Junio C Hamano wrote:\n> Even though the plumbing level allows you to create refs/tags/HEAD\n> and refs/heads/HEAD, doing so makes it confusing within the context\n> of the UI Git Porcelain commands provides.  Just like we prevent a\n> branch from getting called \"HEAD\" at the Porcelain layer (i.e. \"git\n> branch\" command), teach \"git tag\" to refuse to create a tag \"HEAD\".\n\nThis sounds like a good step in the right direction for me.\n\nFrom the subject in this patch, I was worried that we were also\npreventing deletion.  However, I have confirmed that we still allow\nthe intuitive deletion of a tag named 'HEAD' with \"git tag -d HEAD\";\nfor example, in repositories where such a tag already exists.\n\nPerhaps tangential, but a silly change like this hasn't broken any\ntests:\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 670e564178..b65f56e5b4 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -88,6 +88,8 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn,\n \n        for (p = argv; *p; p++) {\n                strbuf_reset(&ref);\n+               if (!strcmp(*p, \"HEAD\"))\n+                     die(\"Hi!\");\n                strbuf_addf(&ref, \"refs/tags/%s\", *p);\n                if (refs_read_ref(get_main_ref_store(the_repository), ref.buf, &oid)) {\n                        error(_(\"tag '%s' not found.\"), *p);\n\nTherefore, if the previous seems reasonable, perhaps we should add a\ntest like:\n\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -97,6 +97,11 @@ test_expect_success 'HEAD is forbidden as a tagname' '\n \ttest_must_fail git tag -a -m \"useless\" HEAD\n '\n \n+test_expect_success 'allow deleting a tag named HEAD' '\n+\tgit update-ref refs/tags/HEAD HEAD &&\n+\tgit tag -d HEAD\n+'\n+\n test_expect_success 'creating a tag with --create-reflog should create reflog' '\n \tgit log -1 \\\n \t\t--format=\"format:tag: tagging %h (%s, %cd)%n\" \\\n\n> \n> Helped-by: Jeff King <peff@peff.net>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>  refs.c         | 2 +-\n>  t/t7004-tag.sh | 6 ++++++\n>  2 files changed, 7 insertions(+), 1 deletion(-)\n> \n> diff --git a/refs.c b/refs.c\n> index a24bfe3845..01ef2a3093 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -735,7 +735,7 @@ int check_branch_ref(struct strbuf *sb, const char *name)\n>  \n>  int check_tag_ref(struct strbuf *sb, const char *name)\n>  {\n> -\tif (name[0] == '-')\n> +\tif (name[0] == '-' || !strcmp(name, \"HEAD\"))\n>  \t\treturn -1;\n>  \n>  \tstrbuf_reset(sb);\n> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\n> index b1316e62f4..2082ce63f7 100755\n> --- a/t/t7004-tag.sh\n> +++ b/t/t7004-tag.sh\n> @@ -91,6 +91,12 @@ test_expect_success 'creating a tag using default HEAD should succeed' '\n>  \ttest_must_fail git reflog exists refs/tags/mytag\n>  '\n>  \n> +test_expect_success 'HEAD is forbidden as a tagname' '\n> +\ttest_when_finished \"git tag -d HEAD || :\" &&\n\nI'm not considering this as a test for it :-)\n\n> +\ttest_must_fail git tag HEAD &&\n> +\ttest_must_fail git tag -a -m \"useless\" HEAD\n> +'\n> +\n>  test_expect_success 'creating a tag with --create-reflog should create reflog' '\n>  \tgit log -1 \\\n>  \t\t--format=\"format:tag: tagging %h (%s, %cd)%n\" \\\n> -- \n> 2.47.1-514-g9b43e7ecc4\n> \n"},{"id":"508464","messageId":"20241202205114.GC776185@coredump.intra.peff.net","threadId":"62584","inReplyTo":"20241202070714.3028549-3-gitster@pobox.com","subject":"Re: [PATCH 2/4] refs: drop strbuf_ prefix from helpers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-12-02T20:51:14Z","receivedAt":"2024-12-02T20:51:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 02, 2024 at 04:07:12PM +0900, Junio C Hamano wrote:\n\n> The helper functions (strbuf_branchname, strbuf_check_branch_ref,\n> and strbuf_check_tag_ref) are about handling branch and tag names,\n> and it is a non-essential fact that these functions use strbuf to\n> hold these names.  Rename them to make it clarify that these are\n> more about \"ref\".\n\nSounds good. I wasn't quite sure about the name copy_branchname(), since\nit actually expands/interprets the name. But the word \"interpret\" is\nalready used for another similar function, repo_interpret_branch_name().\n\nIn fact, this function is a very thin wrapper around it, which made me\nwonder if it has any value. It looks like the main useful bit is that on\nerror it will copy the name verbatim.\n\nSo I guess it is really more like copy_or_expand_branchname(). I don't\nknow if that is really adding much, though. Probably just the name\ncopy_branchname(), coupled with the documentation above the declaration,\nwill be sufficient.\n\n  As a side note, repo_interpret_branch_name() is in object-file.[ch],\n  but probably should also be in refs.[ch], as in your first patch.\n  Let's not worry about it for your series, though.\n\n-Peff\n"},{"id":"508465","messageId":"20241202205238.GD776185@coredump.intra.peff.net","threadId":"62584","inReplyTo":"20241202070714.3028549-4-gitster@pobox.com","subject":"Re: [PATCH 3/4] t5604: do not expect that HEAD is a valid tagname","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-12-02T20:52:38Z","receivedAt":"2024-12-02T20:52:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 02, 2024 at 04:07:13PM +0900, Junio C Hamano wrote:\n\n> 09116a1c (refs: loosen over-strict \"format\" check, 2011-11-16)\n> introduced a test piece (originally in t5700) that expects to be\n> able to create a tag named \"HEAD\" and then a local clone using the\n> repository as its own reference works correctly.  Later, another\n> test piece started using this tag starting at acede2eb (t5700:\n> document a failure of alternates to affect fetch, 2012-02-11).\n> \n> But the breakage 09116a1c fixed was not specific to the tagname\n> HEAD.  It would have failed exactly the same way if the tag used\n> were foo instead of HEAD.\n> \n> Before forbidding \"git tag\" from creating \"refs/tags/HEAD\", update\n> these tests to use 'foo', not 'HEAD', as the name of the test tag.\n\nYeah, I think this is worth doing independently. The patch looks good,\nthough...\n\n> @@ -131,7 +131,7 @@ test_expect_success 'cloning with multiple references drops duplicates' '\n>  \n>  test_expect_success 'clone with reference from a tagged repository' '\n>  \t(\n> -\t\tcd A && git tag -a -m tagged HEAD\n> +\t\tcd A && git tag -a -m tagged foo\n>  \t) &&\n>  \tgit clone --reference=A A I\n>  '\n> @@ -156,10 +156,10 @@ test_expect_success 'fetch with incomplete alternates' '\n>  \t\tgit remote add J \"file://$base_dir/J\" &&\n>  \t\tGIT_TRACE_PACKET=$U.K git fetch J\n>  \t) &&\n> -\tmain_object=$(cd A && git for-each-ref --format=\"%(objectname)\" refs/heads/main) &&\n> +\tmain_object=$(git -C A rev-parse --verify refs/heads/main) &&\n>  \ttest -s \"$U.K\" &&\n>  \t! grep \" want $main_object\" \"$U.K\" &&\n> -\ttag_object=$(cd A && git for-each-ref --format=\"%(objectname)\" refs/tags/HEAD) &&\n> +\ttag_object=$(git -C A rev-parse --verify refs/tags/foo) &&\n>  \t! grep \" want $tag_object\" \"$U.K\"\n>  '\n\nI notice that you swapped out \"cd A && git\" for \"git -C A\" in the second\nhunk (evne in the line which does not otherwise need to be touched). I\nthink that is good, but is it worth doing the same in the first hunk?\nThat would actually let us drop the subshell.\n\n-Peff\n"},{"id":"508466","messageId":"20241202210006.GE776185@coredump.intra.peff.net","threadId":"62584","inReplyTo":"477f0dbd-60ed-4f73-b945-cdbdaf9f510a@app.fastmail.com","subject":"Re: [PATCH 3/4] t5604: do not expect that HEAD is a valid tagname","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-12-02T21:00:06Z","receivedAt":"2024-12-02T21:00:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 02, 2024 at 01:19:56PM +0100, Kristoffer Haugsbakk wrote:\n\n> On Mon, Dec 2, 2024, at 08:07, Junio C Hamano wrote:\n> > 09116a1c (refs: loosen over-strict \"format\" check, 2011-11-16)\n> \n> Nit/confusion: the abbreviated hash is only eight hexes long.  I’m used to it\n> being 11 for this project?\n\nIt's not a fixed size. Long ago, the rule was \"enough to be unique, but\nat least 7 (or whatever you set core.abbrev to)\". These days that \"7\" is\nscaled based on the number of objects in the repo. See e6c587c733\n(abbrev: auto size the default abbreviation, 2016-09-30).\n\nSo I'd expect 10 digits in a fresh clone of git.git. It's possible Junio\nhas set core.abbrev to something fixed, though.\n\n> Does the age of the commit matter?\n\nNope, it shouldn't.\n\n-Peff\n"},{"id":"508467","messageId":"20241202210313.GF776185@coredump.intra.peff.net","threadId":"62584","inReplyTo":"20241202070714.3028549-5-gitster@pobox.com","subject":"Re: [PATCH 4/4] tag: \"git tag\" refuses to use HEAD as a tagname","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-12-02T21:03:13Z","receivedAt":"2024-12-02T21:03:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 02, 2024 at 04:07:14PM +0900, Junio C Hamano wrote:\n\n> Even though the plumbing level allows you to create refs/tags/HEAD\n> and refs/heads/HEAD, doing so makes it confusing within the context\n> of the UI Git Porcelain commands provides.  Just like we prevent a\n> branch from getting called \"HEAD\" at the Porcelain layer (i.e. \"git\n> branch\" command), teach \"git tag\" to refuse to create a tag \"HEAD\".\n\nThis looks good and mostly as expected. I do think Rubén's suggestion to\nadd an explicit deletion test might be worth having to future-proof\nthings.\n\n> @@ -91,6 +91,12 @@ test_expect_success 'creating a tag using default HEAD should succeed' '\n>  \ttest_must_fail git reflog exists refs/tags/mytag\n>  '\n>  \n> +test_expect_success 'HEAD is forbidden as a tagname' '\n> +\ttest_when_finished \"git tag -d HEAD || :\" &&\n> +\ttest_must_fail git tag HEAD &&\n> +\ttest_must_fail git tag -a -m \"useless\" HEAD\n> +'\n\nThe test_when_finished surprised me a little, just because we would not\nexpect anything to have been created. I don't think we usually bother\nwith cleaning up failure modes, as it is a losing battle (if the test\ndid not succeed you are only guessing at what mess may have been left\nbehind). But I don't think it's hurting anything, beyond a few wasted\ncycles to run what should be a noop.\n\n-Peff\n"},{"id":"508468","messageId":"be4dccb2-70f1-4687-a052-caeb86e4e1c7@app.fastmail.com","threadId":"62584","inReplyTo":"20241202210006.GE776185@coredump.intra.peff.net","subject":"Re: [PATCH 3/4] t5604: do not expect that HEAD is a valid tagname","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2024-12-02T21:09:57Z","receivedAt":"2024-12-02T21:10:20Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Mon, Dec 2, 2024, at 22:00, Jeff King wrote:\n> On Mon, Dec 02, 2024 at 01:19:56PM +0100, Kristoffer Haugsbakk wrote:\n>\n>> On Mon, Dec 2, 2024, at 08:07, Junio C Hamano wrote:\n>> > 09116a1c (refs: loosen over-strict \"format\" check, 2011-11-16)\n>>\n>> Nit/confusion: the abbreviated hash is only eight hexes long.  I’m used to it\n>> being 11 for this project?\n>\n> It's not a fixed size. Long ago, the rule was \"enough to be unique, but\n> at least 7 (or whatever you set core.abbrev to)\". These days that \"7\" is\n> scaled based on the number of objects in the repo. See e6c587c733\n> (abbrev: auto size the default abbreviation, 2016-09-30).\n\nYes.  11 was based on the output I get as well as what seemed normal in the\nrecent git log.\n\n>\n> So I'd expect 10 digits in a fresh clone of git.git. It's possible Junio\n> has set core.abbrev to something fixed, though.\n>\n>> Does the age of the commit matter?\n>\n> Nope, it shouldn't.\n\nMakes sense.  Thanks.\n\n-- \nKristoffer Haugsbakk\n"},{"id":"508488","messageId":"xmqqbjxth84y.fsf@gitster.g","threadId":"62584","inReplyTo":"20241202203743.GB776185@coredump.intra.peff.net","subject":"Re: [PATCH 1/4] refs: move ref name helpers around","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-03T01:23:25Z","receivedAt":"2024-12-03T01:23:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, Dec 02, 2024 at 04:07:11PM +0900, Junio C Hamano wrote:\n>\n>> strbuf_branchname(), strbuf_check_{branch,tag}_ref() are helper\n>> functions to deal with branch and tag names, and the fact that they\n>> happen to use strbuf to hold the name of a branch or a tag is not\n>> essential.  These functions fit better in the refs API than strbuf\n>> API, the latter of which is about string manipulations.\n>\n> Wow, they are declared in strbuf.h but not even implemented there. So it\n> was doubly confusing. This looks like a nice cleanup.\n\nYup.  Another home that may want to adopt them is the object-name\nAPI, but I think refs API is good enough, and certainly better than\nstrbuf.\n"},{"id":"508490","messageId":"xmqq34j5h7v9.fsf@gitster.g","threadId":"62584","inReplyTo":"20241202210006.GE776185@coredump.intra.peff.net","subject":"Re: [PATCH 3/4] t5604: do not expect that HEAD is a valid tagname","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-03T01:29:14Z","receivedAt":"2024-12-03T01:29:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> So I'd expect 10 digits in a fresh clone of git.git. It's possible Junio\n> has set core.abbrev to something fixed, though.\n\nThanks.\n\nI have \"git one\" (and \"git who\") aliased to this script:\n\n    $ cat $(type --path git-onewho)\n    #!/bin/sh\n    if sha1=$(git rev-parse -q --verify \"$1\")\n    then\n            git show --date=short -s --abbrev=8 --pretty='format:%h (%s, %ad)' \"$1\"\n    else\n            git log -1 --format=\"%aN <%aE>\" --author=\"$1\" --all\n    fi | tr -d \"\\012\"\n    $ git help one\n    'one' is aliased to 'onewho'\n    $ git help who\n    'who' is aliased to 'onewho'\n    \nso that I can say \"\\C-u ESC ! git one HEAD\" (or \"git one peff\")\nwhile writing a piece of e-mail.  I can drop --abbrev=8 from there\nbut the machinery knows to bust that limit if it is necessary to\nensure uniqueness, so ...\n"},{"id":"508491","messageId":"xmqqy10xft94.fsf@gitster.g","threadId":"62584","inReplyTo":"Z02voaSNYRciv38z@ArchLinux","subject":"Re: [PATCH 4/4] tag: \"git tag\" refuses to use HEAD as a tagname","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-03T01:30:15Z","receivedAt":"2024-12-03T01:30:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"shejialuo <shejialuo@gmail.com> writes:\n\n>>  int check_tag_ref(struct strbuf *sb, const char *name)\n>>  {\n>> -\tif (name[0] == '-')\n>> +\tif (name[0] == '-' || !strcmp(name, \"HEAD\"))\n>\n> I am wondering whether we should also update \"check_refname_format\"\n> function to report \"refs/heads/HEAD\" and \"refs/tags/HEAD\" is bad ref\n> name.\n\nCheck the list archive for the past few days; it was considered and\nrejected.\n\nThanks.\n"},{"id":"508492","messageId":"xmqqser5ft16.fsf@gitster.g","threadId":"62584","inReplyTo":"ee2af264-8d4f-401d-893c-e08c30f5a9b6@gmail.com","subject":"Re: [PATCH 4/4] tag: \"git tag\" refuses to use HEAD as a tagname","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-03T01:35:01Z","receivedAt":"2024-12-03T01:35:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rubén Justo <rjusto@gmail.com> writes:\n\n> --- a/t/t7004-tag.sh\n> +++ b/t/t7004-tag.sh\n> @@ -97,6 +97,11 @@ test_expect_success 'HEAD is forbidden as a tagname' '\n>  \ttest_must_fail git tag -a -m \"useless\" HEAD\n>  '\n>  \n> +test_expect_success 'allow deleting a tag named HEAD' '\n> +\tgit update-ref refs/tags/HEAD HEAD &&\n> +\tgit tag -d HEAD\n> +'\n\nThis concisely captures exactly what we want to see.  The plumbing\nis still allowed to create something that the worldview given by the\nGit Porcelain UI may not like (so that people can create different UI)\nand our tool can still be usable to recover from a mistake of using\nHEAD as the name of a tag, which we more strongly discourage.\n\nGood idea.\n\n>> +test_expect_success 'HEAD is forbidden as a tagname' '\n>> +\ttest_when_finished \"git tag -d HEAD || :\" &&\n>\n> I'm not considering this as a test for it :-)\n\nIt is not.  If \"git tag HEAD\" get broken in the future, the\nmust-fail steps below may create such a tag, and we want to remove\nit before moving along.\n"},{"id":"508687","messageId":"20241205202537.GD2629822@coredump.intra.peff.net","threadId":"62584","inReplyTo":"xmqq34j5h7v9.fsf@gitster.g","subject":"Re: [PATCH 3/4] t5604: do not expect that HEAD is a valid tagname","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-12-05T20:25:37Z","receivedAt":"2024-12-05T20:25:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 03, 2024 at 10:29:14AM +0900, Junio C Hamano wrote:\n\n> I have \"git one\" (and \"git who\") aliased to this script:\n> \n>     $ cat $(type --path git-onewho)\n>     #!/bin/sh\n>     if sha1=$(git rev-parse -q --verify \"$1\")\n>     then\n>             git show --date=short -s --abbrev=8 --pretty='format:%h (%s, %ad)' \"$1\"\n>     else\n>             git log -1 --format=\"%aN <%aE>\" --author=\"$1\" --all\n>     fi | tr -d \"\\012\"\n>     $ git help one\n>     'one' is aliased to 'onewho'\n>     $ git help who\n>     'who' is aliased to 'onewho'\n>     \n> so that I can say \"\\C-u ESC ! git one HEAD\" (or \"git one peff\")\n> while writing a piece of e-mail.  I can drop --abbrev=8 from there\n> but the machinery knows to bust that limit if it is necessary to\n> ensure uniqueness, so ...\n\nYeah, I have something similar. IMHO a manual --abbrev there is working\nagainst your goal.\n\nWe do increase that to find a unique answer, but that is not very\nfuture-proof; it is only extending by one character taking into account\nwhat objects you have _now_. It might not be true for somebody else's\nrepo with more objects, or even your own repo in the near future.\n\nThe auto-scaling of core.abbrev done by default these days also suffers\nfrom that problem (it can only count how many objects you have now, not\nhow many you expect to have a year from now). But I think our heuristics\nthere give a bit higher safety margin for future-proofing the values.\n\n-Peff\n"},{"id":"508688","messageId":"20241205202647.GE2629822@coredump.intra.peff.net","threadId":"62584","inReplyTo":"xmqqy10xft94.fsf@gitster.g","subject":"Re: [PATCH 4/4] tag: \"git tag\" refuses to use HEAD as a tagname","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-12-05T20:26:47Z","receivedAt":"2024-12-05T20:26:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 03, 2024 at 10:30:15AM +0900, Junio C Hamano wrote:\n\n> shejialuo <shejialuo@gmail.com> writes:\n> \n> >>  int check_tag_ref(struct strbuf *sb, const char *name)\n> >>  {\n> >> -\tif (name[0] == '-')\n> >> +\tif (name[0] == '-' || !strcmp(name, \"HEAD\"))\n> >\n> > I am wondering whether we should also update \"check_refname_format\"\n> > function to report \"refs/heads/HEAD\" and \"refs/tags/HEAD\" is bad ref\n> > name.\n> \n> Check the list archive for the past few days; it was considered and\n> rejected.\n\nAgreed, but maybe that is an indication we should discuss that\nalternative in the commit message. (I had looked for similar\njustification in the existing \"branch\" restriction, but couldn't find\nit, to the point that I wondered if I had hallucinated our reasoning\nback then).\n\n-Peff\n"},{"id":"508690","messageId":"20241205202744.GA3170018@coredump.intra.peff.net","threadId":"62584","inReplyTo":"20241205202647.GE2629822@coredump.intra.peff.net","subject":"Re: [PATCH 4/4] tag: \"git tag\" refuses to use HEAD as a tagname","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-12-05T20:27:44Z","receivedAt":"2024-12-05T20:27:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 05, 2024 at 03:26:47PM -0500, Jeff King wrote:\n\n> > > I am wondering whether we should also update \"check_refname_format\"\n> > > function to report \"refs/heads/HEAD\" and \"refs/tags/HEAD\" is bad ref\n> > > name.\n> > \n> > Check the list archive for the past few days; it was considered and\n> > rejected.\n> \n> Agreed, but maybe that is an indication we should discuss that\n> alternative in the commit message. (I had looked for similar\n> justification in the existing \"branch\" restriction, but couldn't find\n> it, to the point that I wondered if I had hallucinated our reasoning\n> back then).\n\nAh, nevermind. I just read your v2 and I think it covers this nicely.\n\n-Peff\n"}]}