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

Madhu [off-list ref] writes:

*  Junio C Hamano [off-list ref] <xmqq7dlz94by.fsf@gitster.g>

quoted

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

quoted

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.

*  Junio C Hamano [off-list ref] <xmqqr1k76p8d.fsf@gitster.g>
Wrote on Mon, 22 Mar 2021 11:02:42 -0700

quoted

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.

Ah yes indeed.

quoted

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?

Avoiding the filemode check completely during reinit is ok with me
because it gave me wrong results.  I can't speak for the original
author of the code - if his intention was to do it explicitly as part
of "reinitialization".

[In the latter case personally I'd want to edit the config file by hand
in response to a message from git - and I certainly do not want git to
change the config file behind my back without informing me.]

Thanks again.