Re: [PATCH v3] revision: add `--ignore-missing-links` user option
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 15, 2023, 18:54 UTC
- Message-ID
- <xmqqfs3fe08e.fsf@gitster.g>
- In-Reply-To
- <20230915083415.263187-1-knayak@gitlab.com>
Karthik Nayak <karthik.188@gmail.com> writes:
Show 7 quoted lines
> From: Karthik Nayak <karthik.188@gmail.com> > > The revision backend is used by multiple porcelain commands such as > git-rev-list(1) and git-log(1). The backend currently supports ignoring > missing links by setting the `ignore_missing_links` bit. This allows the > revision walk to skip any objects links which are missing. Expose this > bit via an `--ignore-missing-links` user option.
Given the above "we merely surface a feature that already exists and supported to be used by the end users from the command line" claim ...
Show 13 quoted lines
> diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> index ff715d6918..5239d83c76 100644
> --- a/builtin/rev-list.c
> +++ b/builtin/rev-list.c
> @@ -266,7 +266,8 @@ static int finish_object(struct object *obj, const char *name UNUSED,
> {
> struct rev_list_info *info = cb_data;
> if (oid_object_info_extended(the_repository, &obj->oid, NULL, 0) < 0) {
> - finish_object__ma(obj);
> + if (!info->revs->ignore_missing_links)
> + finish_object__ma(obj);
> return 1;
> }... this hunk is a bit unexpected. As a low-level plumbing command, shouldn't it be left to the user who gives --ignore-missing-links from their command line to specify how the missing "obj" here should be dealt with by giving the "--missing=<foo>" option? While giving "allow-promisor" may not make much sense, "--missing=allow-any" may of course make sense (it is the same as hardcoding the decision not to call finish_object__ma() at all), and so may "--missing=print".
Stepping back a bit, with "--missing=print", is this change still needed? The missing objects discovered will be shown at the end, with the setting, no?
Thanks.