Re: [PATCH v3] perf test record.sh: Raise limit of open file descriptors

[Veronika Molnarova <vmolnaro@redhat.com>]

On 8/14/24 17:54, Arnaldo Carvalho de Melo wrote:
> On Wed, Aug 14, 2024 at 05:42:01PM +0200, Veronika Molnarova wrote:
>>
>>
>> On 8/14/24 17:35, Arnaldo Carvalho de Melo wrote:
>>> On Wed, Aug 14, 2024 at 12:32:39PM -0300, Arnaldo Carvalho de Melo wrote:
>>>> On Wed, Aug 14, 2024 at 05:17:34PM +0200, vmolnaro@redhat.com wrote:
>>>>> From: Veronika Molnarova <vmolnaro@redhat.com>
>>>>
>>>> Ok? I'm applying it with this change. Thanks,
>>>
>>> So I added this to the log message:
>>>
>>> Committer notes:
>>>
>>> Instead of disabling ShellCheck warnings all the uses of 'uname -n',
>>> i.e. those:
>>>
>>>   In tests/shell/record.sh line 35:
>>>   default_fd_limit=$(ulimit -Sn)
>>>                             ^-^ SC3045 (warning): In POSIX sh, ulimit -S is undefined.
>>>
>>> We can just switch from using '/bin/sh' to '/bin/bash' for this test, as
>>> bash _has_ 'ulimit -n', so ShellCheck will not emit that warning.
>>>
>>> There are dozens of 'perf test' shell tests that do just that,
>>> '/bin/bash' is a reasonable expectation for those tests.
>>>
>>> ----------------------------------------------------------
>>>
>>> Please let me know if you find any issues with this course of action,
>>>
>>> Thanks,
>>>
>>> - Arnaldo
>>>
>> If so then the check whether the ulimit is supported doesn't need to be done
>> as bash is given as a requirement. Thought that it should be supporting all
>> possible shells, even though couldn't find shell not supporting 'ulimit -Sn'.
>>
>> Can I send a quick fix that just changes to the '/bin/bash' so that the code
>> won't have unnecessary code?
> 
> See my last message, I went with your v2 + switch from /bin/sh to
> /bin/bash, as you suggest above.
> 
> - Arnaldo
> 

Yeah, that's what I have just sent. Hopefully all good. Either way works fine.

Thanks,
Veronika