Thomas Weißschuh <thomas@xxxxxxxx> writes:
> Introduce the builtin variable GIT_DEFAULT_BRANCH which represents the
"builtin" -> "logical", as that is how "git-var" describes these things.
It is totally outside the scope of this patch, but I think we'd
better think of a way to make it clear to the readers of the
documentation that it would not do anything if they did something
like:
$ GIT_DEFAULT_BRANCH=foobar git init
I say this is outside the scope because there are other existing
logical variables that are different from the environment variables
that can affect the behaviour of git.
> the default branch name that will be used by git-init.
"git-init" -> "git init", or inside a pair of backquotes, i.e. "`git init`".
> Currently this variable is equivalent to
> git config init.defaultbranch || 'master'
>
> This however will break if at one point the default branch is changed as
> indicated by `default_branch_name_advice` in `refs.c`.
>
> By providing this command ahead of time users of git can make their
> code forward-compatible.
Makes sense. Thanks for cleanly explaining the motivation.
> Co-developed-by: Johannes Schindelin <Johannes.Schindelin@xxxxxx>
I would use "Helped-by:" here, as I do not want to see one-off
trailers invented left and right.
> diff --git a/builtin/var.c b/builtin/var.c
> index 6c6f46b4ae..d1d82b6c93 100644
> --- a/builtin/var.c
> +++ b/builtin/var.c
> @@ -5,6 +5,7 @@
> */
> #include "builtin.h"
> #include "config.h"
> +#include "refs.h"
>
> static const char var_usage[] = "git var (-l | <variable>)";
>
> @@ -27,6 +28,17 @@ static const char *pager(int flag)
> return pgm;
> }
>
> +static const char *default_branch(int flag)
> +{
> + const char *name = repo_default_branch_name(the_repository, 1);
Calling
git_default_branch_name(1)
is much shorter and clear. It's not like using the_repository is
always better. For a single and simple purpose command like "git
var" that does not run around multiple repositories and do things
in them, sticking to the "we work in _the_ repository given to us"
simple API is better.
> + if (!name)
> + die("could not determine the default branch name");
> +
> + return name;
Should we even die? What does "init" and "clone" do when they ask
for the same information and get a NULL pointer?
... goes and looks ...
They know the call cannot fail that way. So I would do either
(1) follow suit and just return whatever we get back from the API
call to the caller (which knows how to handle a NULL return); or
(2) call BUG("...") instead of die(). The name being NULL at this
point means that git_default_branch_name() returned NULL, which
the callers do not allow to happen, so it is a BUG for it to
return NULL, and this caller noticed it.
I only raise the latter as a possibility. I think just assuming
that name is never NULL like other callers is fine.
Thanks.
