Re: [PATCH 3/8] ahead-behind: implement --ignore-missing option
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Mar 7, 2023, 00:46 UTC
- Message-ID
- <ZAaJckdkz6zaF6lw@nand.local>
- In-Reply-To
- <b1d022c7cac5aed6e2d64b45f20dba5b3297536c.1678111599.git.gitgitgadget@gmail.com>
On Mon, Mar 06, 2023 at 02:06:33PM +0000, Derrick Stolee via GitGitGadget wrote:
Show 13 quoted lines
> From: Derrick Stolee <derrickstolee@github.com> > > When parsing the tip revisions from the ahead-behind inputs, it is > important to check that those tips exist before adding them to the list > for computation. The previous change caused the builtin to return with > errors if the revisions could not be resolved. > > However, when running 'git ahead-behind' in an environment with > concurrent edits, such as a Git server, then the references could be > deleted from underneath the caller between reading the reference list > and starting the 'git ahead-behind' process. Avoid this race by allowing > the caller to specify '--ignore-missing' and continue using the > information that is still available.
Well explained, thanks for writing this all out :-).
Show 21 quoted lines
> diff --git a/builtin/ahead-behind.c b/builtin/ahead-behind.c
> index c1212cc8d46..e4f65fc0548 100644
> --- a/builtin/ahead-behind.c
> +++ b/builtin/ahead-behind.c
> @@ -8,13 +8,18 @@ static const char * const ahead_behind_usage[] = {
> NULL
> };
>
> +static int ignore_missing;
> +
> static int handle_arg(struct string_list *tips, const char *arg)
> {
> struct string_list_item *item;
> struct commit *c = lookup_commit_reference_by_name(arg);
>
> - if (!c)
> + if (!c) {
> + if (ignore_missing)
> + return 0;
> return error(_("could not resolve '%s'"), arg);
> + }And the diff makes complete sense here, too.
Show 7 quoted lines
> item = string_list_append(tips, arg);
> item->util = c;
> @@ -30,6 +35,7 @@ int cmd_ahead_behind(int argc, const char **argv, const char *prefix)
> struct option ahead_behind_opts[] = {
> OPT_STRING('b', "base", &base_ref, N_("base"), N_("base reference to process")),
> OPT_BOOL(0 , "stdin", &from_stdin, N_("read rev names from stdin")),
> + OPT_BOOL(0 , "ignore-missing", &ignore_missing, N_("ignore missing tip references")),The spacing between "0" and the comma and "ignore-missing" (as well with "stdin" above, though I didn't notice it when reading the previous patch) is a little funky.
I suspect that it is carried over from some historical typo from many years ago, but probably worth fixing while we're thinking about it.
Thanks, Taylor