Subject: Re: [PATCH 3/8] ahead-behind: implement --ignore-missing option

From: Taylor Blau <[email-masked]>
Date: Mon, 6 Mar 2023 19:46:42 -0500

On Mon, Mar 06, 2023 at 02:06:33PM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <[email-masked]>
>
> 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 :-).

> 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.

>  	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