Re: [PATCH] add: add --chmod=+x / --chmod=-x options

From: Junio C Hamano <gitster@pobox.com>
Date: May 25, 2016, 07:51 UTC
Message-ID: <xmqq1t4q378x.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160525020609.GA20123@zoidberg>

Edward Thomson <ethomson@edwardthomson.com> writes:

Show 9 quoted lines

>  	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").