Re: [PATCH v2 1/2] create_default_files(): don't set u+x bit on $GIT_DIR/config

[Michael Haggerty <mhagger@alum.mit.edu>]

On 11/17/2014 04:42 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> This seems like a one-off bug caused by a specific instance of odd code.
>> It could only recur if somebody were to remove the line that I added,
>> which would be a *very* odd mistake to make given that its purpose is
>> pretty obvious.
> 
> Or some other code that comes _after_ your new code touches the
> file.
> 
> You cannot anticipate what mistake others make.

And yet we do so all the time, adding tests for the things we consider
most likely to break in the future and omitting them for breakages that
seem unlikely. Otherwise, what frees us from the obligation to add a '!
test -x "$GIT_DIR/config"' following every single git operation?

But it's OK with me, we can add a test.

> Shouldn't something like this be sufficient?
> 
>  t/t0001-init.sh | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> index e62c0ff..acdc1df 100755
> --- a/t/t0001-init.sh
> +++ b/t/t0001-init.sh
> @@ -12,6 +12,13 @@ check_config () {
>  		echo "expected a directory $1, a file $1/config and $1/refs"
>  		return 1
>  	fi
> +
> +	if ! test_have_prereq POSIXPERM || test -x "$1/config"
> +	then
> +		echo "$1/config is executable?"
> +		return 1
> +	fi
> +
>  	bare=$(cd "$1" && git config --bool core.bare)
>  	worktree=$(cd "$1" && git config core.worktree) ||
>  	worktree=unset
> 

I think the logic should be

	if test_have_prereq POSIXPERM && test -x "$1/config"

, right? If the system doesn't have POSIXPERM, then aren't all bets off
regarding file permissions?

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu