Re: [PATCH 1/1] Introduce new build variables INSTALL_MODE_EXECUTABLE and INSTALL_MODE_DATA.

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 25, 2013, 06:54 UTC
Message-ID: <7vliac2898.fsf@alter.siamese.dyndns.org>
In-Reply-To: <510AA1E7.9070704@iam.tj>

TJ <git@iam.tj> writes:

Show 29 quoted lines

> On 31/01/13 15:51, Junio C Hamano wrote:
>> TJ <git@iam.tj> writes:
>> 
>>> +	$(INSTALL) -d -m $(INSTALL_MODE_EXECUTABLE) $(DESTDIR)$(man1dir)
>>> +	$(INSTALL) -d -m $(INSTALL_MODE_EXECUTABLE) $(DESTDIR)$(man5dir)
>>> +	$(INSTALL) -d -m $(INSTALL_MODE_EXECUTABLE) $(DESTDIR)$(man7dir)
>>> +	$(INSTALL) -m $(INSTALL_MODE_DATA) $(DOC_MAN1) $(DESTDIR)$(man1dir)
>>> +	$(INSTALL) -m $(INSTALL_MODE_DATA) $(DOC_MAN5) $(DESTDIR)$(man5dir)
>>> +	$(INSTALL) -m $(INSTALL_MODE_DATA) $(DOC_MAN7) $(DESTDIR)$(man7dir)
>> 
>> I'm tempted to suggest
>> 
>>     INSTALL_DIR = $(INSTALL) -d -m 755
>>     INSTALL_DATA = $(INSTALL) -m 644
>>     INSTALL_PROGRAM = $(INSTALL) -m 755
>> 
>> The number of lines the patch needs to touch will be the same, but
>> the resulting lines will not have many $(INSTALL_MODE_BLAH) shouting
>> at us.
>
> I did contemplate that but was concerned it might be seen as interfering unduly with
> the tool name/path settings, as opposed to their options.
>
>> Besides, you would want to differentiate the two kinds of 755 anyway
>> (I'd prefer INSTALL_PROGRAM to use -m 555 personally, for example).
>
> Yes, I think I lost that one in the mists of sed-land when making the changes :)
>
> I'll revise the patch based on received comments and post the revision tomorrow.

Did anything come out of this discussion?