Re: [Buildroot] [PATCH v2 1/6] support/scripts/pkg-stats: search only Config.in{, .host} for URL

[Fiona Klute via buildroot <buildroot@buildroot.org>]

Am 05.05.26 um 21:32 schrieb Arnout Vandecappelle:
> On 26/04/2026 17:33, Fiona Klute via buildroot wrote:
>> The previous Config.* glob also caught linux/Config.ext.in and
>> package/php/Config.ext, as well as some backup files created by
>> editors (e.g. Config.in~ after editing a Config.in file in Emacs),
>> leading to wrong results depending on directory listing order.
>>
>> Also use "with" to automatically close the file when the block is
>> left, even on error.
>>
>> Signed-off-by: Fiona Klute <fiona.klute@gmx.de>
> 
>   Applied to master, thanks.
> 
>> ---
>> This cannot solve the problem we have no structured URL information
>> and don't look at which option the URL is listed under, but at least
>> it's less wrong.
> 
>   Does either of these actually cause problems?

Rarely. If there are multiple URLs in one config file, we simply pick 
the first one, so now for linux/Config.in you always get the kernel.org 
URL added in this series, even if you selected the CIP kernel which has 
its own URL in its help text. There probably aren't many packages where 
that's an issue.

Mostly it just feels awkward to probe the help text instead of having an 
explicit definition.

>   What is still incorrect is that it doesn't report when Config.in 
> doesn't have an upstream URL while Config.in.host doesn't.

True. Though in that case we'd also have to differentiate between that 
and "there is no Config.in" for host-only packages. So far pkg-stats 
treats host and target packages as one and only reports infra separately.

>> Changes v1 -> v2:
>> * Try fixed names instead of globbing all directory entries
>>
>>   support/scripts/pkg-stats | 22 +++++++++++-----------
>>   1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
>> index 55aa63c861..efb85a7405 100755
>> --- a/support/scripts/pkg-stats
>> +++ b/support/scripts/pkg-stats
>> @@ -148,17 +148,17 @@ class Package:
>>           Fills in the .url field
>>           """
>>           self.status['url'] = ("warning", "no Config.in")
>> -        for filename in os.listdir(self.pkgdir):
>> -            if fnmatch.fnmatch(filename, 'Config.*'):
>> -                fp = open(os.path.join(self.pkgdir, filename), "r")
>> -                for config_line in fp:
>> -                    if URL_RE.match(config_line):
>> -                        self.url = config_line.strip()
>> -                        self.status['url'] = ("ok", "found")
>> -                        fp.close()
>> -                        return
>> -                self.status['url'] = ("error", "missing")
>> -                fp.close()
>> +        for filename in ('Config.in', 'Config.in.host'):
>> +            try:
>> +                with open(os.path.join(self.pkgdir, filename), "r") 
>> as fp:
>> +                    for config_line in fp:
>> +                        if URL_RE.match(config_line):
>> +                            self.url = config_line.strip()
>> +                            self.status['url'] = ("ok", "found")
>> +                            return
>> +                    self.status['url'] = ("error", "missing")
>> +            except FileNotFoundError:
> 
>   We could modernise things by switching to pathlib and using .exists() 
> a priori (and never mind the exception in case of TOCTOU or unreadable 
> file).
> 
>   But that's further improvement :-)

I agree. I kept this series to simple and hopefully uncontroversial 
fixes, didn't want to get into full refactoring right away. ;-)

Best regards,
Fiona

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot