From: Junio C Hamano <gitster@pobox•com>
To: Jeff King <peff@peff•net>, Jonathan Nieder <jrnieder@gmail•com>
Cc: Marko Kungla <marko.kungla@gmail•com>, git@vger•kernel.org
Subject: Re: [PATCH] check-ref-format: require a repository for --branch
Date: Mon, 16 Oct 2017 19:45:46 +0900 [thread overview]
Message-ID: <xmqq1sm3s751.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <xmqqinffsibr.fsf@gitster.mtv.corp.google.com> (Junio C. Hamano's message of "Mon, 16 Oct 2017 15:44:08 +0900")
Junio C Hamano <gitster@pobox•com> writes:
> Having said that, there may still be a use case where a Porcelain
> script wants a way to see if a $name it has is appropriate as a
> branch name before it has a repository (e.g. a wrapper to "git
> clone" that wants to verify the name it is going to give to the "-b"
> option), and a check desired in such a context is different from
> (and is stricter than) feeding refs/heads/$name to the same command
> without the "--branch" option.
>
> So I think the right endgame in the longer term is:
> ...
Here is to illustrate what I mean in a patch form. It resurrects
the gentle setup, and uses a purely textual format check when we are
outside the repository, while bypassing the @{magic} interpolation
codepath that requires us to be in a repository. When we are in a
repository, we operate the same way as before.
Designed to be applied directly on top of 4d03f955
("check-ref-format: require a repository for --branch", 2017-07-14),
so it is missing the "'HEAD' is not a good branch name" I showed a
few days ago.
builtin/check-ref-format.c | 16 +++++++++++++---
cache.h | 14 ++++++++++++++
sha1_name.c | 22 +++++++++++++++++++---
strbuf.h | 6 ++++++
t/t1402-check-ref-format.sh | 10 +++++++++-
5 files changed, 61 insertions(+), 7 deletions(-)
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 1e5f9835f0..4e62852089 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -38,12 +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;
+ const char *name = arg;
- setup_git_directory();
- if (strbuf_check_branch_ref(&sb, arg))
+ setup_git_directory_gently(&nongit);
+
+ if (!nongit)
+ malformed = (strbuf_check_branch_ref(&sb, arg) ||
+ !skip_prefix(sb.buf, "refs/heads/", &name));
+ else
+ malformed = check_branch_ref_format(arg);
+
+ if (malformed)
die("'%s' is not a valid branch name", arg);
- printf("%s\n", sb.buf + 11);
+ printf("%s\n", name);
+ strbuf_release(&sb);
return 0;
}
diff --git a/cache.h b/cache.h
index 52b91f5b64..3815925122 100644
--- 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
+ */
+
+/*
+ * Check if a 'name' is suitable to be used as a branch name; this can
+ * be and is stricter than what check_refname_format() returns for a
+ * string that is a concatenation of "name" after "refs/heads/"; a
+ * name that begins with "-" is not allowed, for example.
+ */
+extern int check_branch_ref_format(const char *name);
+
extern int get_oid_mb(const char *str, struct object_id *oid);
extern int validate_headref(const char *ref);
diff --git a/sha1_name.c b/sha1_name.c
index 5e2ec37b65..c95080258f 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1319,15 +1319,31 @@ void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)
strbuf_add(sb, name + used, len - used);
}
-int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
+static int strbuf_check_branch_ref_format(struct strbuf *sb)
{
- strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
- if (name[0] == '-')
+ if (*sb->buf == '-')
return -1;
strbuf_splice(sb, 0, 0, "refs/heads/", 11);
return check_refname_format(sb->buf, 0);
}
+int check_branch_ref_format(const char *name)
+{
+ struct strbuf sb = STRBUF_INIT;
+ int result;
+
+ strbuf_addstr(&sb, name);
+ result = strbuf_check_branch_ref_format(&sb);
+ strbuf_release(&sb);
+ return result;
+}
+
+int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
+{
+ strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
+ return strbuf_check_branch_ref_format(sb);
+}
+
/*
* This is like "get_sha1_basic()", except it allows "sha1 expressions",
* notably "xyz^" for "parent of xyz"
diff --git a/strbuf.h b/strbuf.h
index d785258649..3da95685b2 100644
--- 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
+ */
+
/*
* Copy "name" to "sb", expanding any special @-marks as handled by
* interpret_branch_name(). The result is a non-qualified branch name
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 1674b061e2..2ddefb4bd1 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -161,10 +161,18 @@ test_expect_success 'check-ref-format --branch from subdir' '
test "$refname" = "$sha1"
'
-test_expect_success 'check-ref-format --branch from non-repo' '
+test_expect_success 'check-ref-format --branch @{-1} from non-repo' '
test_must_fail nongit git check-ref-format --branch @{-1}
'
+test_expect_success 'check-ref-format --branch master in non-repo' '
+ nongit git check-ref-format --branch master
+'
+
+test_expect_success 'check-ref-format --branch -naster in repo' '
+ test_must_fail git check-ref-format --branch -naster
+'
+
valid_ref_normalized() {
prereq=
case $1 in
next prev parent reply other threads:[~2017-10-16 10:45 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 ` [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 [this message]
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=xmqq1sm3s751.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=jrnieder@gmail$(echo .)com \
--cc=marko.kungla@gmail$(echo .)com \
--cc=peff@peff$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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