* [PATCH v2 1/4] refs: move ref name helpers around
2024-12-03 2:32 [PATCH v2 0/4] forbid HEAD as a tagname Junio C Hamano
@ 2024-12-03 2:32 ` Junio C Hamano
2024-12-03 2:32 ` [PATCH v2 2/4] refs: drop strbuf_ prefix from helpers Junio C Hamano
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2024-12-03 2:32 UTC (permalink / raw)
To: git; +Cc: git, Jeff King, Rubén Justo
strbuf_branchname(), strbuf_check_{branch,tag}_ref() are helper
functions to deal with branch and tag names, and the fact that they
happen to use strbuf to hold the name of a branch or a tag is not
essential. These functions fit better in the refs API than strbuf
API, the latter of which is about string manipulations.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/tag.c | 11 -----------
object-name.c | 36 ------------------------------------
refs.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
refs.h | 29 +++++++++++++++++++++++++++++
strbuf.h | 22 ----------------------
5 files changed, 76 insertions(+), 69 deletions(-)
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,
--
2.47.1-515-g5132b7d2ef
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v2 2/4] refs: drop strbuf_ prefix from helpers
2024-12-03 2:32 [PATCH v2 0/4] forbid HEAD as a tagname Junio C Hamano
2024-12-03 2:32 ` [PATCH v2 1/4] refs: move ref name helpers around Junio C Hamano
@ 2024-12-03 2:32 ` Junio C Hamano
2024-12-03 2:32 ` [PATCH v2 3/4] t5604: do not expect that HEAD can be a valid tagname Junio C Hamano
2024-12-03 2:32 ` [PATCH v2 4/4] tag: "git tag" refuses to use HEAD as a tagname Junio C Hamano
3 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2024-12-03 2:32 UTC (permalink / raw)
To: git; +Cc: git, Jeff King, Rubén Justo
The helper functions (strbuf_branchname, strbuf_check_branch_ref,
and strbuf_check_tag_ref) are about handling branch and tag names,
and it is a non-essential fact that these functions use strbuf to
hold these names. Rename them to make it clarify that these are
more about "ref".
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
branch.c | 2 +-
builtin/branch.c | 10 +++++-----
builtin/check-ref-format.c | 2 +-
builtin/checkout.c | 2 +-
builtin/merge.c | 2 +-
builtin/tag.c | 2 +-
builtin/worktree.c | 8 ++++----
gitweb/gitweb.perl | 2 +-
refs.c | 8 ++++----
refs.h | 8 ++++----
10 files changed, 23 insertions(+), 23 deletions(-)
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
--
2.47.1-515-g5132b7d2ef
^ permalink raw reply related [flat|nested] 5+ messages in thread