Re: [PATCH v3] Teach git apply to respect core.fileMode settings

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 26, 2023, 20:10 UTC
Message-ID: <xmqqo7echgin.fsf@gitster.g>
In-Reply-To: <fb022e3c-adb5-e341-9fd0-9c5311abe908@gmx.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 19 quoted lines

>> Is it defensive or is it hiding a problematic index under the rug?
>
> I wrote this defensive code only out of habit, not because I saw a
> `ce_mode` that was 0.
>
>> If there is an index entry whose ce_mode is 0, I suspect we would
>> want to error out with a BUG(), unless it is an intent-to-add entry.
>>
>> Shouldn't it cause an error to apply a patch that mucks with
>> "newfile" after you did
>>
>> 	$ git add -N newfile
>>
>> If we allow ce_mode==0 to be propagated to st_mode, I suspect we
>> will catch such a case with the "mode is different" warning code, at
>> least.
>
> Is `ce_mode == 0` an indicator of a new file? In my tests, `git add -N`
> will add the file with a non-zero mode...

Oh, if we know nobody would assign 0 to ce_mode in a valid in-index entry, then we should (1) check and BUG() if we care there may be such a case due to a bug, or (2) assume that it never happens and omit the extra check. The third way in the patch is neither and is sweeping a potential bug ("potential" because the code apparently assumes it can happen) under the rug ("sweeping" because the code silently ignores such an abnormal case), I am afraid.