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
* 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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.