Re: [PATCH] tools/mvtest: ensure testcase is executable (755)

[Shiyang Ruan <ruansy.fnst@fujitsu.com>]

在 2023/9/7 21:54, Zorro Lang 写道:
> On Wed, Sep 06, 2023 at 03:13:49PM +0800, Shiyang Ruan wrote:
>> Some test cases lack executable permission ('x'). Before running each
>> test case, `./check` checks and grants them 'x' permission. However,
>> this always leads to a dirty git repo. And the absence of 'x' permission
>> in test cases is often overlooked during reviews.
>>
>> Since maintainers use mvtest to assign new case, add this change for
>> convenience of maintainers.
>>
>> Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com>
>> ---
>>   tools/mvtest | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/tools/mvtest b/tools/mvtest
>> index 99b154142..e839f0256 100755
>> --- a/tools/mvtest
>> +++ b/tools/mvtest
>> @@ -28,6 +28,8 @@ append() {
>>   test "${src}" != "${dest}" || die "Test \"${src}\" is the same as dest."
>>   test -e "tests/${src}" || die "Test \"${src}\" does not exist."
>>   test ! -e "tests/${dest}" || die "Test \"${src}\" already exists."
>> +# make sure testcase is executable
>> +test `stat -c '%a' tests/${src}` == 755 || chmod 755 "tests/${src}"
> 
> (Weird, I thought I've replied this patch, but I can't find my reply anywhere.)
> 
> I said we can check and set the permission on the ${dest} file, after the
> move operation is done. Due to we actually want to get a 755 ${dest} file
> by running the mvtest.

It seems that maillist server was down at that time.  But I received 
your reply:

 > I think we can check and set the file permission on the target file
 > after the move operation done, not the source file.
 >
 > And we recommend using $() to replace ``, if both of them work.

Then I have sent v2[1] which is using $() instead of `` and doing the 
check-then-set tests/${dest} after `git mv`, as you suggested.

[1] 
https://lore.kernel.org/fstests/20230907113501.4119112-1-ruansy.fnst@fujitsu.com/


--
Thanks,
Ruan.

> 
> Thanks,
> Zorro
> 
>>   
>>   sid="$(basename "${src}")"
>>   did="$(basename "${dest}")"
>> -- 
>> 2.42.0
>>
>