git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / 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: [PATCH] check-ref-format: require a repository for --branch
Date: Fri, 14 Jul 2017 14:18:31 -0400	[thread overview]
Message-ID: <20170714181831.fvi2coppzhm747mk@sigill.intra.peff.net> (raw)
In-Reply-To: <20170714180313.apsnbnw7no2nvtf5@sigill.intra.peff.net>

On Fri, Jul 14, 2017 at 02:03:13PM -0400, Jeff King wrote:

> So I think the patch below is probably the right direction.

And here it is with a real commit message, if this is what we want to
do.

I do feel like "check-ref-format --branch" is a misfeature. The
operation seems to fit better with "rev-parse" (which yes, is a kitchen
sink, but it does all sorts of similar resolution operations). I think
"git rev-parse --symbolic-full-name @{-1}" is basically the same thing
(modulo the refs/heads/ shortening).

-- >8 --
Subject: [PATCH] check-ref-format: require a repository for --branch

When the user asks "--branch" to interpret a branch name
like "@{-1}", we have to dig the answer out of the HEAD
reflog. We can obviously only do that if we have a
repository, and indeed, running it outside a repository
causes us to hit a BUG().

We basically have two options:

  1. We can define "@{-N}" outside of a repository as "no
     such branch" and die with "not a valid brach name".

  2. We can just declare that "--branch" must be run inside
     a repository, in which case we die with "not a git
     repository".

The effect is more or less the same for "@{-N}".
Technically one can use "--branch" outside of a repository
as long as you don't use any names that actually need
interpreting. But since intrpreting is the option's
documented purpose, there doesn't seem any point in trying
to resolve vanilla names like "foo" (which we end up just
printing to stdout verbatim).

So let's go with option 2.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/check-ref-format.c  | 3 +--
 t/t1402-check-ref-format.sh | 4 ++++
 2 files changed, 5 insertions(+), 2 deletions(-)

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
-- 
2.14.0.rc0.457.g08326e0ba


  reply	other threads:[~2017-07-14 18:18 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
2017-07-14 18:18   ` Jeff King [this message]
2017-07-17 17:27     ` [PATCH] check-ref-format: require a repository for --branch 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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20170714181831.fvi2coppzhm747mk@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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).