Re: git: 67ce8cec004c - main - Mk/Scripts: Fix the 'stripped' check from 'make check-plist' to report all unstripped files

From: Renato Botelho <garga_at_FreeBSD.org>
Date: Fri, 02 Jun 2023 11:54:19 UTC
On 02/06/23 04:11, Mathieu Arnold wrote:
> On Fri, Jun 02, 2023 at 06:45:31AM +0000, Yuri Victorovich wrote:
>> The branch main has been updated by yuri:
>>
>> URL: https://cgit.FreeBSD.org/ports/commit/?id=67ce8cec004c85caeee5a6e965bd10f872e1b895
>>
>> commit 67ce8cec004c85caeee5a6e965bd10f872e1b895
>> Author:     Yuri Victorovich <yuri@FreeBSD.org>
>> AuthorDate: 2023-06-02 06:40:46 +0000
>> Commit:     Yuri Victorovich <yuri@FreeBSD.org>
>> CommitDate: 2023-06-02 06:45:29 +0000
>>
>>      Mk/Scripts: Fix the 'stripped' check from 'make check-plist' to report all unstripped files
>>      
>>      Prior to this patch, the 'stripped' check always skipped the first
>>      unstripped file.
>>      
>>      It uses the "find [...] -exec sh -c 'readelf -S -- /dev/null $0 "$@" || :' -- {} +"
>>      command. When arguments are passed to shell like this:
>>      "sh -c 'script' arg1 arg2 arg3" - $@ within the script is assigned
>>      to 'arg2 arg3', and $0 is assigned to arg1. This is a quirk in
>>      how shells handle arguments in case when the script is passed
>>      using -c.
>>      
>>      This patch adds $0 to account for the first passed file.
> 
> So, you are right, when you run `sh -c 'script' arg1 arg2 arg3`, arg1 is
> in $0, and arg2 and arg3 are in $@.
> 
> Now, here, we are running `sh -c 'script' -- arg1 arg2 arg3`, so, $0
> contains `--`, and arg1-3 are in $@.

On local tests, without this patch, I confirmed first file is always 
skipped, as mentioned on ticket.  I took misc/trurl as example:

❯ find work/stage -type f ! -name '*.a' ! -name '*.o'
work/stage/usr/local/man/man1/trurl.1.gz
work/stage/usr/local/share/licenses/trurl-0.7/LICENSE
work/stage/usr/local/share/licenses/trurl-0.7/catalog.mk
work/stage/usr/local/share/licenses/trurl-0.7/MIT
work/stage/usr/local/bin/trurl

Without the patch, calling readelf I can see:

❯ find work/stage -type f ! -name '*.a' ! -name '*.o' -exec sh -c 
'readelf -S -- /dev/null "$@" || :' -- {} + 2>/dev/null |& grep '^File:'
File: /dev/null
File: work/stage/usr/local/share/licenses/trurl-0.7/LICENSE
File: work/stage/usr/local/share/licenses/trurl-0.7/catalog.mk
File: work/stage/usr/local/share/licenses/trurl-0.7/MIT
File: work/stage/usr/local/bin/trurl

And with the change:

❯ find work/stage -type f ! -name '*.a' ! -name '*.o' -exec sh -c 
'readelf -S -- /dev/null "$0" "$@" || :' -- {} + 2>/dev/null |& grep 
'^File:'
File: /dev/null
File: work/stage/usr/local/man/man1/trurl.1.gz
File: work/stage/usr/local/share/licenses/trurl-0.7/LICENSE
File: work/stage/usr/local/share/licenses/trurl-0.7/catalog.mk
File: work/stage/usr/local/share/licenses/trurl-0.7/MIT
File: work/stage/usr/local/bin/trurl

Did I miss something?

>> -	    -exec sh -c 'readelf -S -- /dev/null "$@" || :' -- {} + 2>/dev/null | awk '
>> +	    -exec sh -c 'readelf -S -- /dev/null $0 "$@" || :' -- {} + 2>/dev/null | awk '
> 
> If the patch was correct, you would need to quote "$0".

Good catch.

> But as I pointed out, it is not correct, so please revert.
> 

-- 
Renato Botelho