Re: [PATCH] init: don't reset core.filemode on git-new-workdirs.

From: Junio C Hamano <gitster@pobox.com>
To: Madhu <enometh@meer.net>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] init: don't reset core.filemode on git-new-workdirs.
Date: Mon, 22 Mar 2021 11:02:42 -0700	[thread overview]
Message-ID: <xmqqr1k76p8d.fsf@gitster.g> (raw)
In-Reply-To: <20210322.143437.212295420302618690.enometh@meer.net> (Madhu's message of "Mon, 22 Mar 2021 14:34:37 +0530 (IST)")

Madhu <enometh@meer.net> writes:

> *  Junio C Hamano <gitster@pobox.com> <xmqq7dlz94by.fsf@gitster.g>
>> ...
>> And the symlink check is never done in "reinit" case, so perhaps
>> when "git init" is run again in an already functioning repository,
>> we should not muck with the filemode, either.
>
> I'd think so (on the last point)....

So, assuming that you are with me to think that reinit should not
touch the filemode thing, ...

>> A natural conclusion of the line of thought is that we can move the
>> "check filemode trustability" block (from the comment to concluding
>> git_config_set()) inside the "if (!reinit)" that happens a bit later
>> and we'd be fine---as an existing normal repository, as well as what
>> new-workdir creates, won't have to do the "let's chmod +x/-x the
>> config file and see what happens" code at all (perhaps the attached
>> patch, which hasn't even been compile tested).
>> ...

... wouldn't the illustration patch I gave, which removed the "check
filemode" bit from the main codepath and moved it to inside an if
block that is executed only when "if (!reinit)" is true, "skip" the
problematic "check if config is a regular file whose executable bit
can be flipped and flopped" code in your use case, i.e. in an existing
repository?

> I don't think the posted patch (snipped) would work as reinit is
> always 1 and we are always a candidate for reiniting - I may be
> missing something.

In other words, yes, the illustration patch you are responding to
assumes that the "reinit" variable is set correctly (i.e. the HEAD
exists and sensibly readable if you run "git init" in an already
functioning working tree) and we can use it to avoid the filemode
check.

> Using a new file for the filemode test would be a natural
> improvement. 

That becomes necessary only if we want to futz with core.filemode
while doing "reinit", as .git/config can be a symlink.  When we are
creating a repository from scratch, we always create a regular file
to prepare .git/config, and there is no need to do that, if we are
happy to set core.filemode the same way as core.symlinks, i.e. only
check once when the repository is created.  No?

Thanks.