From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
Eric Sunshine <sunshine@sunshineco.com>,
Han-Wen Nienhuys <hanwen@google.com>
Subject: [PATCH v2 00/12] show-ref: introduce mode to check for ref existence
Date: Thu, 26 Oct 2023 11:56:16 +0200 [thread overview]
Message-ID: <cover.1698314128.git.ps@pks.im> (raw)
In-Reply-To: <cover.1698152926.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 14503 bytes --]
Hi,
this is the second version of my patch series that introduces a new `git
show-ref --exists` mode to check for reference existence.
Changes compared to v1:
- Various typo fixes in commit messages.
- Stopped using non-constant designated initializers.
- Renamed a varible from `matchlen` to `patternlen` to match another
renamed variable better.
- Improved the error message for mutually exclusive modes to be
easier to handle for translators.
- "--quiet" is not advertised for the pattern-based mode of
git-show-ref(1) anymore.
- Rephrased "error code" to "exit code" in the documentation.
Thanks for the feedback so far!
Patrick
Patrick Steinhardt (12):
builtin/show-ref: convert pattern to a local variable
builtin/show-ref: split up different subcommands
builtin/show-ref: fix leaking string buffer
builtin/show-ref: fix dead code when passing patterns
builtin/show-ref: refactor `--exclude-existing` options
builtin/show-ref: stop using global variable to count matches
builtin/show-ref: stop using global vars for `show_one()`
builtin/show-ref: refactor options for patterns subcommand
builtin/show-ref: ensure mutual exclusiveness of subcommands
builtin/show-ref: explicitly spell out different modes in synopsis
builtin/show-ref: add new mode to check for reference existence
t: use git-show-ref(1) to check for ref existence
Documentation/git-show-ref.txt | 20 ++-
builtin/show-ref.c | 280 ++++++++++++++++++++++-----------
t/t1403-show-ref.sh | 70 +++++++++
t/t1430-bad-ref-name.sh | 27 ++--
t/t3200-branch.sh | 33 ++--
t/t5521-pull-options.sh | 4 +-
t/t5605-clone-local.sh | 2 +-
t/test-lib-functions.sh | 55 +++++++
8 files changed, 369 insertions(+), 122 deletions(-)
Range-diff against v1:
1: 78163accbd2 = 1: 78163accbd2 builtin/show-ref: convert pattern to a local variable
2: 7e6ab5dee23 ! 2: 9a234622d99 builtin/show-ref: split up different subcommands
@@ builtin/show-ref.c: static int exclude_existing(const char *match)
+
+static int cmd_show_ref__patterns(const char **patterns)
+{
-+ struct show_ref_data show_ref_data = {
-+ .patterns = (patterns && *patterns) ? patterns : NULL,
-+ };
++ struct show_ref_data show_ref_data = {0};
++
++ if (patterns && *patterns)
++ show_ref_data.patterns = patterns;
+
+ if (show_head)
+ head_ref(show_ref, &show_ref_data);
3: ae2e401fbd8 = 3: bb0d656a0b4 builtin/show-ref: fix leaking string buffer
4: 29c0c0c6c97 ! 4: 87afcee830c builtin/show-ref: fix dead code when passing patterns
@@ Commit message
builtin/show-ref: fix dead code when passing patterns
When passing patterns to `git show-ref` we have some code that will
- cause us to die of `verify && !quiet` is true. But because `verify`
+ cause us to die if `verify && !quiet` is true. But because `verify`
indicates a different subcommand of git-show-ref(1) that causes us to
execute `cmd_show_ref__verify()` and not `cmd_show_ref__patterns()`, the
condition cannot ever be true.
5: 8d0b0b5700c ! 5: bed2a8a0769 builtin/show-ref: refactor `--exclude-existing` options
@@ Commit message
builtin/show-ref: refactor `--exclude-existing` options
It's not immediately obvious options which options are applicable to
- what subcommand ni git-show-ref(1) because all options exist as global
+ what subcommand in git-show-ref(1) because all options exist as global
state. This can easily cause confusion for the reader.
Refactor options for the `--exclude-existing` subcommand to be contained
in a separate structure. This structure is stored on the stack and
- passed down as required. Consequentially, it clearly delimits the scope
- of those options and requires the reader to worry less about global
- state.
+ passed down as required. Consequently, it clearly delimits the scope of
+ those options and requires the reader to worry less about global state.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
@@ builtin/show-ref.c: static int add_existing(const char *refname,
struct string_list existing_refs = STRING_LIST_INIT_DUP;
char buf[1024];
- int matchlen = match ? strlen(match) : 0;
-+ int matchlen = opts->pattern ? strlen(opts->pattern) : 0;
++ int patternlen = opts->pattern ? strlen(opts->pattern) : 0;
for_each_ref(add_existing, &existing_refs);
while (fgets(buf, sizeof(buf), stdin)) {
@@ builtin/show-ref.c: static int cmd_show_ref__exclude_existing(const char *match)
- if (match) {
+ if (opts->pattern) {
int reflen = buf + len - ref;
- if (reflen < matchlen)
+- if (reflen < matchlen)
++ if (reflen < patternlen)
continue;
- if (strncmp(ref, match, matchlen))
-+ if (strncmp(ref, opts->pattern, matchlen))
++ if (strncmp(ref, opts->pattern, patternlen))
continue;
}
if (check_refname_format(ref, 0)) {
6: 6e0f3d4e104 = 6: d52a5e8ced2 builtin/show-ref: stop using global variable to count matches
7: 2da1e65dd8f ! 7: 63f1dadf4c2 builtin/show-ref: stop using global vars for `show_one()`
@@ builtin/show-ref.c: static int cmd_show_ref__verify(const char **refs)
+static int cmd_show_ref__patterns(const struct show_one_options *show_one_opts,
+ const char **patterns)
{
- struct show_ref_data show_ref_data = {
+- struct show_ref_data show_ref_data = {0};
++ struct show_ref_data show_ref_data = {
+ .show_one_opts = show_one_opts,
- .patterns = (patterns && *patterns) ? patterns : NULL,
- };
++ };
+ if (patterns && *patterns)
+ show_ref_data.patterns = patterns;
@@ builtin/show-ref.c: static int cmd_show_ref__patterns(const char **patterns)
static int hash_callback(const struct option *opt, const char *arg, int unset)
8: 805889eda4c ! 8: 88dfeaa4871 builtin/show-ref: refactor options for patterns subcommand
@@ builtin/show-ref.c: static int cmd_show_ref__verify(const struct show_one_option
struct show_ref_data show_ref_data = {
.show_one_opts = show_one_opts,
+ .show_head = opts->show_head,
- .patterns = (patterns && *patterns) ? patterns : NULL,
};
+ if (patterns && *patterns)
+ show_ref_data.patterns = patterns;
+
- if (show_head)
+ if (opts->show_head)
head_ref(show_ref, &show_ref_data);
9: d0a991cf4f8 ! 9: 5ba566723e8 builtin/show-ref: ensure mutual exclusiveness of subcommands
@@ builtin/show-ref.c: int cmd_show_ref(int argc, const char **argv, const char *pr
show_ref_usage, 0);
+ if ((!!exclude_existing_opts.enabled + !!verify) > 1)
-+ die(_("only one of --exclude-existing or --verify can be given"));
++ die(_("only one of '%s' or '%s' can be given"),
++ "--exclude-existing", "--verify");
+
if (exclude_existing_opts.enabled)
return cmd_show_ref__exclude_existing(&exclude_existing_opts);
@@ t/t1403-show-ref.sh: test_expect_success 'show-ref --verify with dangling ref' '
+test_expect_success 'show-ref sub-modes are mutually exclusive' '
+ test_must_fail git show-ref --verify --exclude-existing 2>err &&
-+ grep "only one of --exclude-existing or --verify can be given" err
++ grep "only one of ${SQ}--exclude-existing${SQ} or ${SQ}--verify${SQ} can be given" err
+'
+
test_done
10: adcfa7a6a9d ! 10: b78ccc5f692 builtin/show-ref: explicitly spell out different modes in synopsis
@@ Commit message
Split up the synopsis for these two modes such that we can disambiguate
those differences.
+ While at it, drop "--quiet" from the pattern mode's synopsis. It does
+ not make a lot of sense to list patterns, but squelch the listing output
+ itself. The description for "--quiet" is adapted accordingly.
+
Signed-off-by: Patrick Steinhardt <ps@pks.im>
## Documentation/git-show-ref.txt ##
@@ Documentation/git-show-ref.txt: git-show-ref - List references in a local reposi
--------
[verse]
-'git show-ref' [-q | --quiet] [--verify] [--head] [-d | --dereference]
-+'git show-ref' [-q | --quiet] [--head] [-d | --dereference]
++'git show-ref' [--head] [-d | --dereference]
[-s | --hash[=<n>]] [--abbrev[=<n>]] [--tags]
[--heads] [--] [<pattern>...]
+'git show-ref' --verify [-q | --quiet] [-d | --dereference]
@@ Documentation/git-show-ref.txt: git-show-ref - List references in a local reposi
'git show-ref' --exclude-existing[=<pattern>]
DESCRIPTION
+@@ Documentation/git-show-ref.txt: OPTIONS
+ -q::
+ --quiet::
+
+- Do not print any results to stdout. When combined with `--verify`, this
+- can be used to silently check if a reference exists.
++ Do not print any results to stdout. Can be used with `--verify` to
++ silently check if a reference exists.
+
+ --exclude-existing[=<pattern>]::
+
## builtin/show-ref.c ##
@@
@@ builtin/show-ref.c
static const char * const show_ref_usage[] = {
- N_("git show-ref [-q | --quiet] [--verify] [--head] [-d | --dereference]\n"
-+ N_("git show-ref [-q | --quiet] [--head] [-d | --dereference]\n"
++ N_("git show-ref [--head] [-d | --dereference]\n"
" [-s | --hash[=<n>]] [--abbrev[=<n>]] [--tags]\n"
" [--heads] [--] [<pattern>...]"),
+ N_("git show-ref --verify [-q | --quiet] [-d | --dereference]\n"
11: 2f876e61dd3 ! 11: 327942b1162 builtin/show-ref: add new mode to check for reference existence
@@ Commit message
- Dangling symrefs can be resolved via git-symbolic-ref(1), but this
requires the caller to special case existence checks depending on
- whteher or not a reference is symbolic or direct.
+ whether or not a reference is symbolic or direct.
Furthermore, git-rev-list(1) and other commands do not let the caller
distinguish easily between an actually missing reference and a generic
error.
- Taken together, this gseems like sufficient motivation to introduce a
+ Taken together, this seems like sufficient motivation to introduce a
separate plumbing command to explicitly check for the existence of a
reference without trying to resolve its contents.
This new command comes in the form of `git show-ref --exists`. This
new mode will exit successfully when the reference exists, with a
- specific error code of 2 when it does not exist, or with 1 when there
+ specific exit code of 2 when it does not exist, or with 1 when there
has been a generic error.
Note that the only way to properly implement this command is by using
@@ Documentation/git-show-ref.txt: OPTIONS
+--exists::
+
-+ Check whether the given reference exists. Returns an error code of 0 if
-+ it does, 2 if it is missing, and 128 in case looking up the reference
++ Check whether the given reference exists. Returns an exit code of 0 if
++ it does, 2 if it is missing, and 1 in case looking up the reference
+ failed with an error other than the reference being missing.
+
--abbrev[=<n>]::
@@ builtin/show-ref.c: static int cmd_show_ref__patterns(const struct patterns_opti
+ unsigned int unused_type;
+ int failure_errno = 0;
+ const char *ref;
-+ int ret = 1;
++ int ret = 0;
+
+ if (!refs || !*refs)
+ die("--exists requires a reference");
@@ builtin/show-ref.c: static int cmd_show_ref__patterns(const struct patterns_opti
+ error(_("reference does not exist"));
+ ret = 2;
+ } else {
-+ error(_("failed to look up reference: %s"), strerror(failure_errno));
++ errno = failure_errno;
++ error_errno(_("failed to look up reference"));
++ ret = 1;
+ }
+
+ goto out;
+ }
+
-+ ret = 0;
-+
+out:
+ strbuf_release(&unused_referent);
+ return ret;
@@ builtin/show-ref.c: int cmd_show_ref(int argc, const char **argv, const char *pr
show_ref_usage, 0);
- if ((!!exclude_existing_opts.enabled + !!verify) > 1)
-- die(_("only one of --exclude-existing or --verify can be given"));
+- die(_("only one of '%s' or '%s' can be given"),
+- "--exclude-existing", "--verify");
+ if ((!!exclude_existing_opts.enabled + !!verify + !!exists) > 1)
-+ die(_("only one of --exclude-existing, --exists or --verify can be given"));
++ die(_("only one of '%s', '%s' or '%s' can be given"),
++ "--exclude-existing", "--verify", "--exists");
if (exclude_existing_opts.enabled)
return cmd_show_ref__exclude_existing(&exclude_existing_opts);
@@ t/t1403-show-ref.sh: test_expect_success 'show-ref --verify with dangling ref' '
test_expect_success 'show-ref sub-modes are mutually exclusive' '
+ cat >expect <<-EOF &&
-+ fatal: only one of --exclude-existing, --exists or --verify can be given
++ fatal: only one of ${SQ}--exclude-existing${SQ}, ${SQ}--verify${SQ} or ${SQ}--exists${SQ} can be given
+ EOF
+
test_must_fail git show-ref --verify --exclude-existing 2>err &&
-- grep "only one of --exclude-existing or --verify can be given" err
+- grep "only one of ${SQ}--exclude-existing${SQ} or ${SQ}--verify${SQ} can be given" err
+ test_cmp expect err &&
+
+ test_must_fail git show-ref --verify --exists 2>err &&
12: a3a43d82e1f = 12: 226731c5f18 t: use git-show-ref(1) to check for ref existence
base-commit: a9ecda2788e229afc9b611acaa26d0d9d4da53ed
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2023-10-26 9:56 UTC|newest]
Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-24 13:10 [PATCH 00/12] show-ref: introduce mode to check for ref existence Patrick Steinhardt
2023-10-24 13:10 ` [PATCH 01/12] builtin/show-ref: convert pattern to a local variable Patrick Steinhardt
2023-10-24 13:10 ` [PATCH 02/12] builtin/show-ref: split up different subcommands Patrick Steinhardt
2023-10-24 17:55 ` Eric Sunshine
2023-10-24 13:10 ` [PATCH 03/12] builtin/show-ref: fix leaking string buffer Patrick Steinhardt
2023-10-24 13:10 ` [PATCH 04/12] builtin/show-ref: fix dead code when passing patterns Patrick Steinhardt
2023-10-24 18:02 ` Eric Sunshine
2023-10-24 13:10 ` [PATCH 05/12] builtin/show-ref: refactor `--exclude-existing` options Patrick Steinhardt
2023-10-24 18:48 ` Eric Sunshine
2023-10-25 11:50 ` Patrick Steinhardt
2023-10-24 13:11 ` [PATCH 06/12] builtin/show-ref: stop using global variable to count matches Patrick Steinhardt
2023-10-24 13:11 ` [PATCH 07/12] builtin/show-ref: stop using global vars for `show_one()` Patrick Steinhardt
2023-10-24 13:11 ` [PATCH 08/12] builtin/show-ref: refactor options for patterns subcommand Patrick Steinhardt
2023-10-24 13:11 ` [PATCH 09/12] builtin/show-ref: ensure mutual exclusiveness of subcommands Patrick Steinhardt
2023-10-24 19:25 ` Eric Sunshine
2023-10-24 13:11 ` [PATCH 10/12] builtin/show-ref: explicitly spell out different modes in synopsis Patrick Steinhardt
2023-10-24 19:39 ` Eric Sunshine
2023-10-25 11:50 ` Patrick Steinhardt
2023-10-24 13:11 ` [PATCH 11/12] builtin/show-ref: add new mode to check for reference existence Patrick Steinhardt
2023-10-24 21:01 ` Eric Sunshine
2023-10-25 11:50 ` Patrick Steinhardt
2023-10-24 13:11 ` [PATCH 12/12] t: use git-show-ref(1) to check for ref existence Patrick Steinhardt
2023-10-24 19:17 ` [PATCH 00/12] show-ref: introduce mode " Junio C Hamano
2023-10-25 14:26 ` Han-Wen Nienhuys
2023-10-25 14:44 ` Phillip Wood
2023-10-26 9:48 ` Patrick Steinhardt
2023-10-27 13:06 ` Phillip Wood
2023-10-26 9:44 ` Patrick Steinhardt
2023-10-26 9:56 ` Patrick Steinhardt [this message]
2023-10-26 9:56 ` [PATCH v2 01/12] builtin/show-ref: convert pattern to a local variable Patrick Steinhardt
2023-10-26 9:56 ` [PATCH v2 02/12] builtin/show-ref: split up different subcommands Patrick Steinhardt
2023-10-26 9:56 ` [PATCH v2 03/12] builtin/show-ref: fix leaking string buffer Patrick Steinhardt
2023-10-30 18:10 ` Taylor Blau
2023-10-26 9:56 ` [PATCH v2 04/12] builtin/show-ref: fix dead code when passing patterns Patrick Steinhardt
2023-10-30 18:24 ` Taylor Blau
2023-10-26 9:56 ` [PATCH v2 05/12] builtin/show-ref: refactor `--exclude-existing` options Patrick Steinhardt
2023-10-30 18:37 ` Taylor Blau
2023-10-31 8:10 ` Patrick Steinhardt
2023-10-30 18:55 ` Taylor Blau
2023-10-31 8:10 ` Patrick Steinhardt
2023-10-26 9:56 ` [PATCH v2 06/12] builtin/show-ref: stop using global variable to count matches Patrick Steinhardt
2023-10-30 19:14 ` Taylor Blau
2023-10-26 9:56 ` [PATCH v2 07/12] builtin/show-ref: stop using global vars for `show_one()` Patrick Steinhardt
2023-10-26 9:56 ` [PATCH v2 08/12] builtin/show-ref: refactor options for patterns subcommand Patrick Steinhardt
2023-10-26 9:56 ` [PATCH v2 09/12] builtin/show-ref: ensure mutual exclusiveness of subcommands Patrick Steinhardt
2023-10-30 19:31 ` Taylor Blau
2023-10-31 8:10 ` Patrick Steinhardt
2023-10-26 9:57 ` [PATCH v2 10/12] builtin/show-ref: explicitly spell out different modes in synopsis Patrick Steinhardt
2023-10-26 9:57 ` [PATCH v2 11/12] builtin/show-ref: add new mode to check for reference existence Patrick Steinhardt
2023-10-26 9:57 ` [PATCH v2 12/12] t: use git-show-ref(1) to check for ref existence Patrick Steinhardt
2023-10-30 19:32 ` [PATCH v2 00/12] show-ref: introduce mode " Taylor Blau
2023-10-31 2:26 ` Junio C Hamano
2023-10-31 8:16 ` [PATCH v3 00/12] builtin/show-ref: " Patrick Steinhardt
2023-10-31 8:16 ` [PATCH v3 01/12] builtin/show-ref: convert pattern to a local variable Patrick Steinhardt
2023-10-31 8:16 ` [PATCH v3 02/12] builtin/show-ref: split up different subcommands Patrick Steinhardt
2023-10-31 8:16 ` [PATCH v3 03/12] builtin/show-ref: fix leaking string buffer Patrick Steinhardt
2023-10-31 8:16 ` [PATCH v3 04/12] builtin/show-ref: fix dead code when passing patterns Patrick Steinhardt
2023-10-31 8:16 ` [PATCH v3 05/12] builtin/show-ref: refactor `--exclude-existing` options Patrick Steinhardt
2023-10-31 8:16 ` [PATCH v3 06/12] builtin/show-ref: stop using global variable to count matches Patrick Steinhardt
2023-10-31 8:16 ` [PATCH v3 07/12] builtin/show-ref: stop using global vars for `show_one()` Patrick Steinhardt
2023-10-31 8:16 ` [PATCH v3 08/12] builtin/show-ref: refactor options for patterns subcommand Patrick Steinhardt
2023-10-31 8:16 ` [PATCH v3 09/12] builtin/show-ref: ensure mutual exclusiveness of subcommands Patrick Steinhardt
2023-10-31 8:16 ` [PATCH v3 10/12] builtin/show-ref: explicitly spell out different modes in synopsis Patrick Steinhardt
2023-10-31 8:16 ` [PATCH v3 11/12] builtin/show-ref: add new mode to check for reference existence Patrick Steinhardt
2023-10-31 8:16 ` [PATCH v3 12/12] t: use git-show-ref(1) to check for ref existence Patrick Steinhardt
2023-10-31 19:06 ` [PATCH v3 00/12] builtin/show-ref: introduce mode " Taylor Blau
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=cover.1698314128.git.ps@pks.im \
--to=ps@pks.im \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=hanwen@google.com \
--cc=sunshine@sunshineco.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).