All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: "Chandra Pratap via GitGitGadget" <gitgitgadget@gmail.com>,
	git@vger.kernel.org, "Torsten Bögershausen" <tboegi@web.de>,
	"Chandra Pratap" <chandrapratap376@gmail.com>,
	"Chandra Pratap" <chandrapratap3519@gmail.com>
Subject: Re: [PATCH v3] Teach git apply to respect core.fileMode settings
Date: Tue, 26 Dec 2023 12:10:08 -0800	[thread overview]
Message-ID: <xmqqo7echgin.fsf@gitster.g> (raw)
In-Reply-To: <fb022e3c-adb5-e341-9fd0-9c5311abe908@gmx.de> (Johannes Schindelin's message of "Tue, 26 Dec 2023 20:18:58 +0100 (CET)")

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

>> 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.

  reply	other threads:[~2023-12-26 20:10 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-18 14:09 [PATCH] Teach git apply to respect core.fileMode settings Chandra Pratap via GitGitGadget
2023-12-18 18:04 ` Junio C Hamano
2023-12-18 18:10   ` Junio C Hamano
2023-12-19 17:07     ` Chandra Pratap
2023-12-19 18:30 ` [PATCH v2] " Chandra Pratap via GitGitGadget
2023-12-19 20:46   ` Torsten Bögershausen
2023-12-19 22:21   ` Junio C Hamano
2023-12-20 10:08   ` [PATCH v3] " Chandra Pratap via GitGitGadget
2023-12-24 13:42     ` Johannes Schindelin
2023-12-26 17:35       ` Junio C Hamano
2023-12-26 19:18         ` Johannes Schindelin
2023-12-26 20:10           ` Junio C Hamano [this message]
2023-12-26 20:58         ` Junio C Hamano
2023-12-26 21:35           ` Junio C Hamano
2023-12-26 23:32     ` [PATCH v4 0/3] apply with core.filemode=false Junio C Hamano
2023-12-26 23:32       ` [PATCH v4 1/3] apply: ignore working tree filemode when !core.filemode Junio C Hamano
2023-12-26 23:32       ` [PATCH v4 2/3] apply: correctly reverse patch's pre- and post-image mode bits Junio C Hamano
2023-12-26 23:32       ` [PATCH v4 3/3] apply: code simplification Junio C Hamano
2024-02-07 22:15       ` [PATCH v4 0/3] apply with core.filemode=false Junio C Hamano
2024-02-18 22:38         ` Johannes Schindelin
2024-02-19 21:24           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqo7echgin.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=chandrapratap3519@gmail.com \
    --cc=chandrapratap376@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=tboegi@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.