[PATCH 0/3] Re: [PATCH] check-ref-format: require a repository for --branch

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Oct 17, 2017, 07:06 UTC
Message-ID: <20171017070619.bdgvujmbw4uxvwle@aiede.mtv.corp.google.com>
In-Reply-To: <xmqq7evupemj.fsf@gitster.mtv.corp.google.com>

Hi,

Junio C Hamano wrote:

> Subject: [PATCH] check-ref-format: --branch cannot grok @{-1} outside a repository

How about this? It is written to be more conservative than the patch I am replying to, but except for the commit message, it should be pretty much equivalent.

[...]

Show 19 quoted lines

> --- a/builtin/check-ref-format.c
> +++ b/builtin/check-ref-format.c
> @@ -38,13 +38,22 @@ static char *collapse_slashes(const char *refname)
>  
>  static int check_ref_format_branch(const char *arg)
>  {
> +	int nongit, malformed;
>  	struct strbuf sb = STRBUF_INIT;
> -	int nongit;
> +	const char *name = arg;
>  
>  	setup_git_directory_gently(&nongit);
> -	if (strbuf_check_branch_ref(&sb, arg))
> +
> +	if (!nongit)
> +		malformed = (strbuf_check_branch_ref(&sb, arg) ||
> +			     !skip_prefix(sb.buf, "refs/heads/", &name));
> +	else
> +		malformed = check_branch_ref_format(arg);

Handles the nongit case in strbuf_check_branch_ref instead of introducing a new check_branch_ref_format helper.

[...]

Show 11 quoted lines

> --- a/cache.h
> +++ b/cache.h
> @@ -1444,6 +1444,20 @@ extern int parse_oid_hex(const char *hex, struct object_id *oid, const char **en
>  #define INTERPRET_BRANCH_HEAD (1<<2)
>  extern int interpret_branch_name(const char *str, int len, struct strbuf *,
>  				 unsigned allowed);
> +
> +/*
> + * NEEDSWORK: declare strbuf_branchname() and strbuf_check_branch_ref()
> + * here, not in strbuf.h
> + */

As a result, it doesn't touch headers. I agree that these functions don't belong in strbuf.h (sorry for not updating the headers at the same time I moved their implementations) but suspect e.g. branch.h, revision.h, or some new header like revision-syntax.h would be a better place.

[...]

Show 11 quoted lines

> --- a/strbuf.h
> +++ b/strbuf.h
> @@ -568,6 +568,12 @@ static inline void strbuf_complete_line(struct strbuf *sb)
>  	strbuf_complete(sb, '\n');
>  }
>  
> +/*
> + * NEEDSWORK: the following two functions should not be in this file;
> + * these are about refnames, and should be declared next to
> + * interpret_branch_name() in cache.h
> + */

Didn't touch headers.

[...]

Show 9 quoted lines

> --- a/t/t1402-check-ref-format.sh
> +++ b/t/t1402-check-ref-format.sh
> @@ -161,6 +161,18 @@ test_expect_success 'check-ref-format --branch from subdir' '
>  	test "$refname" = "$sha1"
>  '
>  
> +test_expect_success 'check-ref-format --branch @{-1} from non-repo' '
> +	test_must_fail nongit git check-ref-format --branch @{-1}
> +'

Swapped test_must_fail and nongit to match existing tests.

Junio C Hamano (3):
  check-ref-format --branch: do not expand @{...} outside repository
  check-ref-format --branch: strip refs/heads/ using skip_prefix
  check-ref-format doc: --branch validates and expands <branch>

 Documentation/git-check-ref-format.txt |  9 ++++++++-
 builtin/check-ref-format.c             |  6 ++++--
 sha1_name.c                            |  5 ++++-
 t/t1402-check-ref-format.sh            | 16 ++++++++++++++++
 4 files changed, 32 insertions(+), 4 deletions(-)