From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 25 May 2016 07:51:42 GMT
Subject: Re: [PATCH] add: add --chmod=+x / --chmod=-x options
Message-ID: <xmqq1t4q378x.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160525020609.GA20123@zoidberg>

Edward Thomson <ethomson@edwardthomson.com> writes:

>  	if (trust_executable_bit && has_symlinks)
>  		ce->ce_mode = create_ce_mode(st_mode);
> +	else if (force_executable)
> +		ce->ce_mode = create_ce_mode(0777);
> +	else if (force_notexecutable)
> +		ce->ce_mode = create_ce_mode(0666);
>  	else {
>  		/* If there is an existing entry, pick the mode bits and type
>  		 * from it, otherwise assume unexecutable regular file.

I would rather do this part more like:

	if (S_ISREG(st_mode) && (force_executable || force_nonexecuable)) {
        	if (force_executable)
			ce->ce_mode = create_ce_mode(0777);
		else
			ce->ce_mode = create_ce_mode(0666);
	} else if (trust_executable_bit && has_symlinks) {
        	ce->ce_mode = create_ce_mode(st_mode);
	} else {
        	... carry the existing mode over ...

which would make sure that the new code will not interfere with
symbolic links and that forcing will be honored even on filesystems
whose executable bit can be trusted (i.e. "can be trusted" does not
have to mean "must be trusted").