Re: [PATCH] init: don't reset core.filemode on git-new-workdirs.
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 21, 2021, 21:58 UTC
- Message-ID
- <xmqq1rc89nk7.fsf@gitster.g>
- In-Reply-To
- <20210321.175821.1385189088303987287.enometh@meer.net>
Madhu <enometh@meer.net> writes:
Show 6 quoted lines
> From: Madhu <enometh@net.meer> > > If the .git/config file is a symlink (as is the case of a .git created > by the contrib/workdir/git-new-workdir script) then the filemode tests > fail, and the filemode is reset to be false. To avoid this only munge > core.filemode if .git/config is a regular file.
Hmph, what's the sequence of events? You let "git new-workdir" to create a cheap copy of a working tree and then? When new-workdir returns, you already have a functional working tree with .git/ directory (in which there are many symbolic links). So who wants or needs to run "git init" there in the directory in the first place?
Is the problem being solved that running an unnecessary "git init" in an already initialized repository does an unnecessary filemode check?
If that is the case, I am not sure if asking "is it a symlink?" to avoid the filemode trustability check is a good approach. At that point in the code you are patching, we have already determined if we are running the "git init" in an already initialized repository (i.e. "reinit"), so shouldn't we be basing the decision on it instead?
I see that in a later part of the same function, we test if the filesystem supports symbolic links but do so only when we are running "git init" afresh. Perhaps the filemode trustability check and the config-set to record core.filemode should all be moved there inside the "if (!reinit)" block.
All of the above assumes that the problem being solved is about what happens when "git init" is run in an already functioning working tree. If I misread what problem you are trying to solve, then none of what I suggested in the above may apply.
> --- > builtin/init-db.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-)
Missing sign-off; please review Documentation/SubmittingPatches
Show 14 quoted lines
> diff --git a/builtin/init-db.c b/builtin/init-db.c
> index dcc45bef51..b053107336 100644
> --- a/builtin/init-db.c
> +++ b/builtin/init-db.c
> @@ -285,7 +285,8 @@ static int create_default_files(const char *template_path,
> /* Check filemode trustability */
> path = git_path_buf(&buf, "config");
> filemode = TEST_FILEMODE;
> - if (TEST_FILEMODE && !lstat(path, &st1)) {
> + if (TEST_FILEMODE && !lstat(path, &st1)
> + && (st1.st_mode & S_IFMT) == S_IFREG) {
> struct stat st2;
> filemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&
> !lstat(path, &st2) &&