git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Marko Kungla <marko.kungla@gmail.com>
Cc: git@vger.kernel.org, Jonathan Nieder <jrnieder@gmail.com>
Subject: Re: bug with check-ref-format outside of repository
Date: Fri, 14 Jul 2017 14:03:13 -0400	[thread overview]
Message-ID: <20170714180313.apsnbnw7no2nvtf5@sigill.intra.peff.net> (raw)
In-Reply-To: <CAKY_R-uk9hpR2hbkPsw2cqoMo6bQKoyp6cWTO20L3fOWfLW2-Q@mail.gmail.com>

On Sat, Jul 08, 2017 at 03:17:32AM +0200, Marko Kungla wrote:

> As contrived e.g: if I have in my "workspace" sub directories mostly
> git repositories, but some not and if I exec,
> "for d in */ ; do (cd $d && git check-ref-format --branch @{-1});
> done" then I get 3 possible responses.
> 
> git version: 2.13.0
> 1. Valid branch name
> 2. fatal: '@{-1}' is not a valid branch name
> 3. fatal: BUG: setup_git_env called without repository
> 
> git version 2.13.2.915.ga9c46e097
> 1. Valid branch name
> 2. fatal: '@{-1}' is not a valid branch name
> 3. BUG: environment.c:178: git environment hasn't been setup

Thanks for the report. It's right to return an error, but obviously we
should never hit the BUG() in the lazy-setup code.

I think "check-ref-format --branch" is inherently a repository-only
operation, since its purpose is to look in the reflog with @{-1} and
similar branch-substitutions. Technically you can do:

  $ cd /not/a/git/repo
  $ git check-ref-format --branch 'refs/heads/foo'

but I'm not sure why you would want to. So I think the patch below is
probably the right direction. The other alternative is for
interpret_nth_prior_checkout() to detect the "not in a repo" case and
quietly return "nope, we couldn't find such a reflog entry".

+cc Jonathan, who added the original call in 49cc460d8 (Allow
"check-ref-format --branch" from subdirectory, 2010-08-05). That commit
message doesn't give any indication of why we used the gently form.
Looking back at the original thread[1], I suspect it was mostly out of
conservatism.

[1] https://public-inbox.org/git/20100806033922.GS22369@burratino/

---
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index eac499450..1e5f9835f 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -39,9 +39,8 @@ static char *collapse_slashes(const char *refname)
 static int check_ref_format_branch(const char *arg)
 {
 	struct strbuf sb = STRBUF_INIT;
-	int nongit;
 
-	setup_git_directory_gently(&nongit);
+	setup_git_directory();
 	if (strbuf_check_branch_ref(&sb, arg))
 		die("'%s' is not a valid branch name", arg);
 	printf("%s\n", sb.buf + 11);
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 0790edf60..1674b061e 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -161,6 +161,10 @@ test_expect_success 'check-ref-format --branch from subdir' '
 	test "$refname" = "$sha1"
 '
 
+test_expect_success 'check-ref-format --branch from non-repo' '
+	test_must_fail nongit git check-ref-format --branch @{-1}
+'
+
 valid_ref_normalized() {
 	prereq=
 	case $1 in

  reply	other threads:[~2017-07-14 18:03 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-08  1:17 bug with check-ref-format outside of repository Marko Kungla
2017-07-14 18:03 ` Jeff King [this message]
2017-07-14 18:18   ` [PATCH] check-ref-format: require a repository for --branch Jeff King
2017-07-17 17:27     ` Jonathan Nieder
2017-07-17 21:18       ` Marko Kungla
2017-08-17 10:23         ` Jeff King
2017-08-17 10:22       ` Jeff King
2017-08-17 21:30         ` Junio C Hamano
2017-08-18  6:20           ` Jeff King
2017-08-18  7:45             ` Junio C Hamano
2017-10-16  6:44         ` Junio C Hamano
2017-10-16 10:45           ` Junio C Hamano
2017-10-16 22:45             ` Jeff King
2017-10-16 23:00               ` Jonathan Nieder
2017-10-17  1:22               ` Junio C Hamano
2017-10-17  2:42                 ` Jeff King
2017-10-17  3:33                   ` Junio C Hamano
2017-10-17  4:44                     ` Junio C Hamano
2017-10-17  7:06                       ` [PATCH 0/3] " Jonathan Nieder
2017-10-17  7:08                         ` [PATCH 1/3] check-ref-format --branch: do not expand @{...} outside repository Jonathan Nieder
2017-10-17  7:10                         ` [PATCH 2/3] check-ref-format --branch: strip refs/heads/ using skip_prefix Jonathan Nieder
2017-10-17  7:12                         ` [PATCH 0/3] Re: [PATCH] check-ref-format: require a repository for --branch Junio C Hamano
2017-10-17  7:17                           ` Jonathan Nieder
2017-10-17  8:21                             ` Junio C Hamano
2017-10-17  7:12                         ` [PATCH 3/3] check-ref-format doc: --branch validates and expands <branch> Jonathan Nieder
2017-10-17 20:55                           ` Junio C Hamano
2017-10-17 21:06                             ` Jonathan Nieder
2017-10-18  5:10                             ` Jeff King
2017-10-17  1:27             ` [PATCH] check-ref-format: require a repository for --branch Kevin Daudt
2017-10-17  2:40               ` Junio C Hamano
2017-10-17  4:30                 ` Kevin Daudt
2017-10-16 22:42           ` Jeff King
2017-10-17  4:41           ` Jonathan Nieder
2017-10-17  7:05             ` Junio C Hamano
2017-10-17  7:25               ` Jonathan Nieder
2017-10-17  7:34               ` Jonathan Nieder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170714180313.apsnbnw7no2nvtf5@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=marko.kungla@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).