Re: [PATCH 3/3] rev-list: add --allow-missing-tips to be used with --missing=...

[Christian Couder <christian.couder@gmail.com>]

On Wed, Feb 7, 2024 at 10:57 AM Linus Arver <linusa@google.com> wrote:
>
> Christian Couder <christian.couder@gmail.com> writes:
>
> > On Thu, Feb 1, 2024 at 10:27 PM Junio C Hamano <gitster@pobox.com> wrote:
> >>
> >> Christian Couder <christian.couder@gmail.com> writes:
> >>
> >> > When such a command is used to find the dependencies of some objects,
> >> > for example the dependencies of quarantined objects, it would be
> >> > better if the command would instead consider such missing objects,
> >> > especially commits, in the same way as other missing objects.
> >> >
> >> > If, for example `--missing=print` is used, it would be nice for some
> >> > use cases if the missing tips passed as arguments were reported in
> >> > the same way as other missing objects instead of the command just
> >> > failing.
> >> >
> >> > Let's introduce a new `--allow-missing-tips` option to make it work
> >> > like this.
> >>
> >> An obvious question is if this even needs to be a new option.  What
> >> are expected use cases where --missing=print without this option is
> >> beneficial?
> >
> > I am not sure if such a case is really beneficial but some
> > people/script/forges might rely on an error from `git rev-list
> > --missing=print` to propagate back an error to some user interface.
>
> I currently learn toward just making the new flag's behavior be absorved
> into the existing "--missing=..." flag.

Ok, then I am going to implement that in the next version I will send.

> Nevertheless, you raise an
> interesting concern.
>
> Perhaps a compromise would be to make "--missing=..." learn the new
> behavior of this patch as Junio suggested, but to introduce a new flag,
> something like "--fail-on-missing-tips" to fail early if any of the tip
> commits' objects are missing? That way we could keep the current
> "strict" behavior of complaining if we feed rev-list any tips whose
> objects are missing. And for the vast majority of cases the
> "--missing=..." flag could (intuitively) gracefully handle tips with
> missing objects and you wouldn't have to pass in the additional flag.
>
> IOW, make the minority (certainly not majority, I think?) of users who
> really need the error propagation use the (new) extra flag, while the
> rest of us (including the version of you who was surprised by the
> limited behavior of "--missing=...", enough to write this series) don't
> have to.

I agree that the majority of users would prefer `git rev-list` with
"--missing=<arg>" (when <arg> is not "error") not to error out when
one of the tips is missing. And yeah, indeed at GitLab, we are among
this majority of users. I was worried about a small minority relying
on the fact that it would error out in such case. But maybe we don't
need to care unless it appears that this minority actually exists.