Request for Review: Execute the build logger with the help of a shell if the x permission is not set

[Dmitry Samersoff]

Magnus,

I'm second to Kelly.
We shouldn't have executable scripts in repository.

-Dmitry


On 2012-04-23 10:16, Kelly O'Hair wrote:
>
> If this file can never really have execute permission in the repositories, then encouraging people to put
> execute permission on it creates a risk that they might try to commit that change, and then their changeset would get
> rejected. A big annoyance.
>
> So why not just always run $(SH) and skip the 'if -x' test?  Isn't that simplier? And will always work?
>
> -kto
>
> On Apr 23, 2012, at 3:54 AM, David Holmes wrote:
>
>> On 21/04/2012 12:14 AM, Magnus Ihse Bursie wrote:
>>> In the build-infra hg repo, we allowed execute permission to be set on
>>> files, including common/bin/logger.sh, which is called by the Makefile.
>>>
>>> The official hg repos strip this bit, so when integrating to the build
>>> forest, this was lost, causing the Makefile to fail since it can't
>>> execute logger.sh.
>>>
>>> This fix looks if the x permission is there, otherwise it calls
>>> logger.sh using the shell.
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~ihse/logger.sh-missing-x-permission/webrev.00/
>>
>> You could save yourself a bit of duplication using:
>>
>> BUILD_LOG_WRAPPER='$(SRC_ROOT)/common/bin/logger.sh $(BUILD_LOG)'
>> if test ! -x $SRC_ROOT/common/bin/logger.sh ; then
>>    BUILD_LOG_WRAPPER='$(SH) $(BUILD_LOG_WRAPPER)'
>> fi
>>
>> or even factor out
>>
>> LOGGER_SH=$(SRC_ROOT)/common/bin/logger.sh
>>
>> David
>> -----
>>
>>> /Magnus
>>>
>


-- 
Dmitry Samersoff
Java Hotspot development team, SPB04
* There will come soft rains ...