diff mbox series

[v2,04/12] git_main_branch_name(): optionally report the full ref name

Message ID ca1c63c3e012edde26b4f0c67175ca53f4d29e08.1592225416.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series Allow overriding the default name of the default branch | expand

Commit Message

John Passaro via GitGitGadget June 15, 2020, 12:50 p.m. UTC 
From: Johannes Schindelin <johannes.schindelin@gmx.de>

We are about to introduce the first caller of that function (`git
fast-export`) that wants a full ref name instead of the short branch
name.

To make this change easier to review, let's refactor the function
accordingly without mixing in the actual first call using the new flag.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 fmt-merge-msg.c    |  2 +-
 refs.c             | 12 ++++++++----
 refs.h             |  8 ++++++--
 send-pack.c        |  2 +-
 transport-helper.c |  2 +-
 5 files changed, 17 insertions(+), 9 deletions(-)

Comments

Phillip Wood June 15, 2020, 3:04 p.m. UTC | #1 
Hi dscho

On 15/06/2020 13:50, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> We are about to introduce the first caller of that function (`git
> fast-export`) that wants a full ref name instead of the short branch
> name.
> 
> To make this change easier to review, let's refactor the function
> accordingly without mixing in the actual first call using the new flag.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>   fmt-merge-msg.c    |  2 +-
>   refs.c             | 12 ++++++++----
>   refs.h             |  8 ++++++--
>   send-pack.c        |  2 +-
>   transport-helper.c |  2 +-
>   5 files changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c
> index 43f4f829242..03dba905643 100644
> --- a/fmt-merge-msg.c
> +++ b/fmt-merge-msg.c
> @@ -451,7 +451,7 @@ static void fmt_merge_msg_title(struct strbuf *out,
>   			strbuf_addf(out, " of %s", srcs.items[i].string);
>   	}
>   
> -	main_branch = git_main_branch_name();
> +	main_branch = git_main_branch_name(0);
>   	if (!strcmp(main_branch, current_branch))
>   		strbuf_addch(out, '\n');
>   	else
> diff --git a/refs.c b/refs.c
> index f1854cffa2f..7da3ac178c4 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -560,8 +560,9 @@ void expand_ref_prefix(struct argv_array *prefixes, const char *prefix)
>   		argv_array_pushf(prefixes, *p, len, prefix);
>   }
>   
> -char *repo_main_branch_name(struct repository *r)
> +char *repo_main_branch_name(struct repository *r, int flags)
>   {
> +	int full_name = flags & MAIN_BRANCH_FULL_NAME;
>   	const char *config_key = "core.mainbranch";
>   	const char *config_display_key = "core.mainBranch";
>   	const char *fall_back = "master";
> @@ -570,7 +571,10 @@ char *repo_main_branch_name(struct repository *r)
>   	if (repo_config_get_string(r, config_key, &name) < 0)
>   		die(_("could not retrieve `%s`"), config_display_key);
>   
> -	ret = name ? name : xstrdup(fall_back);
> +	if (full_name)
> +		ret = xstrfmt("refs/heads/%s", name ? name : fall_back);
> +	else
> +		ret = name ? name : xstrdup(fall_back);

This looks good, we always check the name before returning it and free 
name if we're returning refs/heads/<name>

>   	if (check_refname_format(ret, REFNAME_ALLOW_ONELEVEL))
>   		die(_("invalid branch name: %s = %s"),
> @@ -582,9 +586,9 @@ char *repo_main_branch_name(struct repository *r)
>   	return ret;
>   }
>   
> -char *git_main_branch_name(void)
> +char *git_main_branch_name(int flags)
>   {
> -	return repo_main_branch_name(the_repository);
> +	return repo_main_branch_name(the_repository, flags);
>   }
>   
>   /*
> diff --git a/refs.h b/refs.h
> index a207ef01348..96472f9a9f5 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -157,9 +157,13 @@ int dwim_log(const char *str, int len, struct object_id *oid, char **ref);
>   /*
>    * Retrieves the name of the main (or: primary) branch of the given
>    * repository.
> + *
> + * The result is an allocated string. Unless the flags ask for a short name, it
> + * will be prefixed with "refs/heads/".
>    */

nit pick: the flag is defined to give the fullname, to get the short 
name you just pass 0.

Best Wishes

Phillip

> -char *git_main_branch_name(void);
> -char *repo_main_branch_name(struct repository *r);
> +#define MAIN_BRANCH_FULL_NAME (1<<0)
> +char *git_main_branch_name(int flags);
> +char *repo_main_branch_name(struct repository *r, int flags);
>   
>   /*
>    * A ref_transaction represents a collection of reference updates that
> diff --git a/send-pack.c b/send-pack.c
> index 2532864c812..898720511d0 100644
> --- a/send-pack.c
> +++ b/send-pack.c
> @@ -405,7 +405,7 @@ int send_pack(struct send_pack_args *args,
>   	}
>   
>   	if (!remote_refs) {
> -		char *branch_name = git_main_branch_name();
> +		char *branch_name = git_main_branch_name(0);
>   
>   		fprintf(stderr, "No refs in common and none specified; doing nothing.\n"
>   			"Perhaps you should specify a branch such as '%s'.\n",
> diff --git a/transport-helper.c b/transport-helper.c
> index 8c8f40e322d..7a54e5b2fb2 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -1044,7 +1044,7 @@ static int push_refs(struct transport *transport,
>   	}
>   
>   	if (!remote_refs) {
> -		char *branch_name = git_main_branch_name();
> +		char *branch_name = git_main_branch_name(0);
>   
>   		fprintf(stderr,
>   			_("No refs in common and none specified; doing nothing.\n"
>
Johannes Schindelin June 23, 2020, 7:17 p.m. UTC | #2 
Hi Phillip,

On Mon, 15 Jun 2020, Phillip Wood wrote:

> On 15/06/2020 13:50, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > We are about to introduce the first caller of that function (`git
> > fast-export`) that wants a full ref name instead of the short branch
> > name.
> >
> > To make this change easier to review, let's refactor the function
> > accordingly without mixing in the actual first call using the new flag.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >   fmt-merge-msg.c    |  2 +-
> >   refs.c             | 12 ++++++++----
> >   refs.h             |  8 ++++++--
> >   send-pack.c        |  2 +-
> >   transport-helper.c |  2 +-
> >   5 files changed, 17 insertions(+), 9 deletions(-)
> >
> > diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c
> > index 43f4f829242..03dba905643 100644
> > --- a/fmt-merge-msg.c
> > +++ b/fmt-merge-msg.c
> > @@ -451,7 +451,7 @@ static void fmt_merge_msg_title(struct strbuf *out,
> >    		strbuf_addf(out, " of %s", srcs.items[i].string);
> >    }
> >   -	main_branch = git_main_branch_name();
> > +	main_branch = git_main_branch_name(0);
> >    if (!strcmp(main_branch, current_branch))
> >    	strbuf_addch(out, '\n');
> >   	else
> > diff --git a/refs.c b/refs.c
> > index f1854cffa2f..7da3ac178c4 100644
> > --- a/refs.c
> > +++ b/refs.c
> > @@ -560,8 +560,9 @@ void expand_ref_prefix(struct argv_array *prefixes,
> > const char *prefix)
> >   		argv_array_pushf(prefixes, *p, len, prefix);
> >   }
> >
> > -char *repo_main_branch_name(struct repository *r)
> > +char *repo_main_branch_name(struct repository *r, int flags)
> >   {
> > +	int full_name = flags & MAIN_BRANCH_FULL_NAME;
> >    const char *config_key = "core.mainbranch";
> >    const char *config_display_key = "core.mainBranch";
> >    const char *fall_back = "master";
> > @@ -570,7 +571,10 @@ char *repo_main_branch_name(struct repository *r)
> >    if (repo_config_get_string(r, config_key, &name) < 0)
> >     die(_("could not retrieve `%s`"), config_display_key);
> >   -	ret = name ? name : xstrdup(fall_back);
> > +	if (full_name)
> > +		ret = xstrfmt("refs/heads/%s", name ? name : fall_back);
> > +	else
> > +		ret = name ? name : xstrdup(fall_back);
>
> This looks good, we always check the name before returning it and free name if
> we're returning refs/heads/<name>
>
> >    if (check_refname_format(ret, REFNAME_ALLOW_ONELEVEL))
> >   		die(_("invalid branch name: %s = %s"),
> > @@ -582,9 +586,9 @@ char *repo_main_branch_name(struct repository *r)
> >   	return ret;
> >   }
> >
> > -char *git_main_branch_name(void)
> > +char *git_main_branch_name(int flags)
> >   {
> > -	return repo_main_branch_name(the_repository);
> > +	return repo_main_branch_name(the_repository, flags);
> >   }
> >
> >   /*
> > diff --git a/refs.h b/refs.h
> > index a207ef01348..96472f9a9f5 100644
> > --- a/refs.h
> > +++ b/refs.h
> > @@ -157,9 +157,13 @@ int dwim_log(const char *str, int len, struct object_id
> > *oid, char **ref);
> >   /*
> >    * Retrieves the name of the main (or: primary) branch of the given
> >    * repository.
> > + *
> > + * The result is an allocated string. Unless the flags ask for a short
> > name, it
> > + * will be prefixed with "refs/heads/".
> >    */
>
> nit pick: the flag is defined to give the fullname, to get the short name you
> just pass 0.

I decided to drop the flag and always return the name, not the full ref.
It makes the code _slightly_ less efficient, but easier to follow.

Ciao,
Dscho

>
> Best Wishes
>
> Phillip
>
> > -char *git_main_branch_name(void);
> > -char *repo_main_branch_name(struct repository *r);
> > +#define MAIN_BRANCH_FULL_NAME (1<<0)
> > +char *git_main_branch_name(int flags);
> > +char *repo_main_branch_name(struct repository *r, int flags);
> >
> >   /*
> >    * A ref_transaction represents a collection of reference updates that
> > diff --git a/send-pack.c b/send-pack.c
> > index 2532864c812..898720511d0 100644
> > --- a/send-pack.c
> > +++ b/send-pack.c
> > @@ -405,7 +405,7 @@ int send_pack(struct send_pack_args *args,
> >    }
> >
> >   	if (!remote_refs) {
> > -		char *branch_name = git_main_branch_name();
> > +		char *branch_name = git_main_branch_name(0);
> >
> >     fprintf(stderr, "No refs in common and none specified; doing nothing.\n"
> >   			"Perhaps you should specify a branch such as '%s'.\n",
> > diff --git a/transport-helper.c b/transport-helper.c
> > index 8c8f40e322d..7a54e5b2fb2 100644
> > --- a/transport-helper.c
> > +++ b/transport-helper.c
> > @@ -1044,7 +1044,7 @@ static int push_refs(struct transport *transport,
> >    }
> >
> >   	if (!remote_refs) {
> > -		char *branch_name = git_main_branch_name();
> > +		char *branch_name = git_main_branch_name(0);
> >
> >     fprintf(stderr,
> >      _("No refs in common and none specified; doing nothing.\n"
> >
>
diff mbox series

Patch

diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c
index 43f4f829242..03dba905643 100644
--- a/fmt-merge-msg.c
+++ b/fmt-merge-msg.c
@@ -451,7 +451,7 @@  static void fmt_merge_msg_title(struct strbuf *out,
 			strbuf_addf(out, " of %s", srcs.items[i].string);
 	}
 
-	main_branch = git_main_branch_name();
+	main_branch = git_main_branch_name(0);
 	if (!strcmp(main_branch, current_branch))
 		strbuf_addch(out, '\n');
 	else
diff --git a/refs.c b/refs.c
index f1854cffa2f..7da3ac178c4 100644
--- a/refs.c
+++ b/refs.c
@@ -560,8 +560,9 @@  void expand_ref_prefix(struct argv_array *prefixes, const char *prefix)
 		argv_array_pushf(prefixes, *p, len, prefix);
 }
 
-char *repo_main_branch_name(struct repository *r)
+char *repo_main_branch_name(struct repository *r, int flags)
 {
+	int full_name = flags & MAIN_BRANCH_FULL_NAME;
 	const char *config_key = "core.mainbranch";
 	const char *config_display_key = "core.mainBranch";
 	const char *fall_back = "master";
@@ -570,7 +571,10 @@  char *repo_main_branch_name(struct repository *r)
 	if (repo_config_get_string(r, config_key, &name) < 0)
 		die(_("could not retrieve `%s`"), config_display_key);
 
-	ret = name ? name : xstrdup(fall_back);
+	if (full_name)
+		ret = xstrfmt("refs/heads/%s", name ? name : fall_back);
+	else
+		ret = name ? name : xstrdup(fall_back);
 
 	if (check_refname_format(ret, REFNAME_ALLOW_ONELEVEL))
 		die(_("invalid branch name: %s = %s"),
@@ -582,9 +586,9 @@  char *repo_main_branch_name(struct repository *r)
 	return ret;
 }
 
-char *git_main_branch_name(void)
+char *git_main_branch_name(int flags)
 {
-	return repo_main_branch_name(the_repository);
+	return repo_main_branch_name(the_repository, flags);
 }
 
 /*
diff --git a/refs.h b/refs.h
index a207ef01348..96472f9a9f5 100644
--- a/refs.h
+++ b/refs.h
@@ -157,9 +157,13 @@  int dwim_log(const char *str, int len, struct object_id *oid, char **ref);
 /*
  * Retrieves the name of the main (or: primary) branch of the given
  * repository.
+ *
+ * The result is an allocated string. Unless the flags ask for a short name, it
+ * will be prefixed with "refs/heads/".
  */
-char *git_main_branch_name(void);
-char *repo_main_branch_name(struct repository *r);
+#define MAIN_BRANCH_FULL_NAME (1<<0)
+char *git_main_branch_name(int flags);
+char *repo_main_branch_name(struct repository *r, int flags);
 
 /*
  * A ref_transaction represents a collection of reference updates that
diff --git a/send-pack.c b/send-pack.c
index 2532864c812..898720511d0 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -405,7 +405,7 @@  int send_pack(struct send_pack_args *args,
 	}
 
 	if (!remote_refs) {
-		char *branch_name = git_main_branch_name();
+		char *branch_name = git_main_branch_name(0);
 
 		fprintf(stderr, "No refs in common and none specified; doing nothing.\n"
 			"Perhaps you should specify a branch such as '%s'.\n",
diff --git a/transport-helper.c b/transport-helper.c
index 8c8f40e322d..7a54e5b2fb2 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -1044,7 +1044,7 @@  static int push_refs(struct transport *transport,
 	}
 
 	if (!remote_refs) {
-		char *branch_name = git_main_branch_name();
+		char *branch_name = git_main_branch_name(0);
 
 		fprintf(stderr,
 			_("No refs in common and none specified; doing nothing.\n"