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
next prev parent 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).