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

[Junio C Hamano <gitster@pobox.com>]

Madhu <enometh@meer.net> writes:

> 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


> 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) &&