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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 22, 2021, 18:02 UTC
Message-ID: <xmqqr1k76p8d.fsf@gitster.g>
In-Reply-To: <20210322.143437.212295420302618690.enometh@meer.net>

Madhu <enometh@meer.net> writes:

Show 7 quoted lines

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

Show 8 quoted lines

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