Re: [PATCH 0/2] merge-tree: add new --mergeability-only option

From: Elijah Newren <newren@gmail.com>
Date: May 12, 2025, 18:37 UTC
Message-ID: <CABPp-BEP9Uj4o5Hd6w2N8zH3Te7tBkVWE_9ytd6y1vkKGiHZNw@mail.gmail.com>
In-Reply-To: <xmqqldr1sn0j.fsf@gitster.g>

On Mon, May 12, 2025 at 11:27 AM Junio C Hamano <gitster@pobox.com> wrote:

Show 14 quoted lines

>
> Elijah Newren <newren@gmail.com> writes:
>
> > I thought about that, but I was worried that folks would expect
> > "--dry-run" to not make any changes.  This mode does not prevent
> > writing objects to the object store, it merely avoids it in the "outer
> > layer" of the merge.
>
> I think we have already precedence to call something that creates
> new objects in the object database, as long as the resulting objects
> are not made reachable ("git fetch --dry-run" probably falls into
> that category).  The idea is that it does not make a change that is
> "observable" by end-users (and what "gc" sees is not part of what
> the users would be observaing).

Oh, I was unaware of `git fetch --dry-run` for some reason. And its documentation even states "without making any changes" despite the fact that it downloads more objects to the object store, so it indeed sounds like a good precedent.

I'll switch the flag name to --dry-run. (I have a suspicion, however, that the primary users of this new merge-tree flag will care about whether objects are created, so I still want the documentation to call it out, unlike git fetch's --dry-run option.)

Show 9 quoted lines

> We have "--check" (in "git apply"), which is an exact counterpart in
> the patch based workflow to this thing.  It reads
>
>         Instead of applying the patch, see if the patch is
>         applicable to the current working tree and/or the index
>         file and detects errors.  Turns off "apply".
>
> I feel that `apply --check` should have been `apply --dry-run`, so I
> would not recommend calling it `--check` for `merge-tree`, though.

Makes sense; thanks for the pointers.