Re: [PATCH] format-patch: cancel useAutoBase if base is invalid

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 18, 2020, 00:04 UTC
Message-ID: <xmqqsgbgne6d.fsf@gitster.c.googlers.com>
In-Reply-To: <CA+P7+xrvbwVevFaCB=cm4mZB4=nFAL2rs4gFgYF85qSopq+dwg@mail.gmail.com>

Jacob Keller <jacob.keller@gmail.com> writes:

Show 25 quoted lines

> On Wed, Sep 16, 2020 at 5:26 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Jacob Keller <jacob.e.keller@intel.com> writes:
>>
>> > Make get_base_commit detect when useAutoBase is set, and avoid failing
>> > if the base commit is picked up automatically. We still attempt to fail
>> > if --base=auto is explicitly requested on the command line.
>>
>> Makes sense.  I also think we should fail in such a broken base is
>> chosen, when useAutoBase is set by configuration and is not
>> overriden from the command line with an explicit use of --no-base
>> option, because the end-user expects an appropriate base to be used
>> that is computed automatically, but we are failing to find such a
>> base---going ahead silently in such a case would be wrong.
>>
>
> I am not sure if I follow here. The whole point of this patch is that
>
> git config format.useAutoBase true
> git format-patch -1 <old id>
>
> causes failure that is very unexpected, especially if it's been a long
> time since you set useAutoBase.
>
> I do want git format-patch --base=auto <old id> to fail, certainly.

I understand. And further, I do not think it is a good idea to silently ignore the configured format.useAutoBase when there is no command line override. IOW, we want both to fail, but with a better message (e.g. "appropriate base not found").

> I wonder if there's a way we can tell when the format patch revisions
> in question make no sense with the automatic base.

Sorry, I don't quite know what you mean---the fact that you are already getting a cryptic error message means the existing code already knows, no?