Re: [PATCH] gnu: Add ninja.

[宋文武]

Ludovic Courtès <address@hidden> writes:

> 宋文武 <address@hidden> skribis:
>
>> Ludovic Courtès <address@hidden> writes:
>>
>>> 宋文武 <address@hidden> skribis:
>>>
>>>> * gnu/packages/ninja.scm: New file.
>>>> * gnu-system.am (GNU_SYSTEM_MODULES): Add it.
>>>
>>> [...]
>>>
>>>> +          'check
>>>> +          (lambda _
>>>> +            (and (zero? (system "./configure.py"))
>>>> +                 (zero? (system "./ninja ninja_test"))
>>>> +                 ;; SubprocessTest.InterruptChild fail when using 
>>>> 'system*'.
>>>> +                 ;; SubprocessTest.SetWithLots was skipped.
>>>> +                 ;; XXX: Raise [ulimit -n] well above 1025 to make this 
>>>> test go.
>>>
>>> Does it mean that the test is currently failing?
>> Yes, SetWithLots fail with the 'Raise ...' line.
>
> Oh, I see.  Then can you make it clearer in the comment:
>
>   ;; SubprocessTest.SetWithLots fails with:
>   ;;
>   ;;   Raise [ulimit -n] well above 1025 to make this test go.
>   ;;
>   ;; Skip it.
>
>>>> +                 (zero? (system (string-append
>>>> +                                 "./ninja_test "
>>>> +                                 "--gtest_filter="
>>>> +                                 "-SubprocessTest.SetWithLots")))))
>>>
>>> Please use ‘system*’ (with separate arguments) rather than ‘system’.
>>> The latter runs “/bin/sh -c ...” whereas the former runs the program
>>> directly.
>> Use 'system*' to run "./ninja_test" will cause InterruptChild to fail :(
>> (as I mentioned in the comment)
>
> Ah right, that’s weird, but could you mention in the comment (1) how it
> fails, and (2) that because of this, we use ‘system’ instead of
> ‘system*’?
>
> It’s important to explain both the problem and the solution/consequence,
> otherwise it can be hard by reading the comment to understand if we’re
> talking about an unfixed issue, a fixed issue, or a fix.
>
> OK to push with these changes.

0001-gnu-Add-ninja.patch
Description: Text Data

Is this OK?
>
> Thanks,
> Ludo’.