[Buildroot] [PATCH] utils/check-package: emit library name along with check function name

[Buildroot] [PATCH] utils/check-package: emit library name along with check function name

[Yann E. MORIN]

Currently, when we generate .checkpackageignore, we store, for each
error, only the name of the function that generated that error.

Although we currently do not have two check libs that have same-name
check functions, there is nothing that would prevent that, and there
is no reason why two unrelated libs could not implement checks with
the same name.

If such a situation were to arise, we'd have no way, when parsing the
ignore list (in-tree: .checkpackageignore), to know which of the libs
the exclusion would apply to.

Fix that by storing both the library and function names together. The
leading "checkpackagelib." (with the trailing dot, 16 chars) is removed
for brevity, because it's present in all libs' names.

As a consequence, regenerate .checkpackageignore.

Note: people using that script to validate their br2-external trees will
also have to regenerate their own exclusion list if they have one.

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Ricardo Martincoski <ricardo.martincoski@datacom.com.br>

---
Note: for ease of review, .checkpackage has *not* been regenerated in
this commit; I'll re-submit the patch after reviews, or comitters can
decide to regenerate it when applying.

Note: for example, hypothetically, we could have a lib_hash (that checks
.hash files) and lib_mk (that check .mk file) that both implement a
CheckHash() function, the first to validate that hashes are of a valid
form, the second to validate that a git hash (in _VERSION) does exist in
the repository pointed to by _SITE.
---
 utils/check-package | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utils/check-package b/utils/check-package
index de41891b56..373bc63f52 100755
--- a/utils/check-package
+++ b/utils/check-package
@@ -224,7 +224,7 @@ def check_file_using_lib(fname):
         print("{}: would run: {}".format(fname, functions_to_run))
         return nwarnings, nlines
 
-    objects = [[c[0], c[1](fname, flags.manual_url)] for c in internal_functions]
+    objects = [[f"{lib.__name__[16:]}::{c[0]}", c[1](fname, flags.manual_url)] for c in internal_functions]
 
     for name, cf in objects:
         warn, fail = print_warnings(cf.before(), name in xfail)
-- 
2.44.0

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

Re: [Buildroot] [PATCH] utils/check-package: emit library name along with check function name

[Arnout Vandecappelle via buildroot]

On 31/03/2024 22:33, Yann E. MORIN wrote:
> Currently, when we generate .checkpackageignore, we store, for each
> error, only the name of the function that generated that error.
> 
> Although we currently do not have two check libs that have same-name
> check functions, there is nothing that would prevent that, and there
> is no reason why two unrelated libs could not implement checks with
> the same name.
> 
> If such a situation were to arise, we'd have no way, when parsing the
> ignore list (in-tree: .checkpackageignore), to know which of the libs
> the exclusion would apply to.
> 
> Fix that by storing both the library and function names together. The
> leading "checkpackagelib." (with the trailing dot, 16 chars) is removed
> for brevity, because it's present in all libs' names.
> 
> As a consequence, regenerate .checkpackageignore.
> 
> Note: people using that script to validate their br2-external trees will
> also have to regenerate their own exclusion list if they have one.
> 
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Ricardo Martincoski <ricardo.martincoski@datacom.com.br>
> 
> ---
> Note: for ease of review, .checkpackage has *not* been regenerated in
> this commit; I'll re-submit the patch after reviews, or comitters can
> decide to regenerate it when applying.
> 
> Note: for example, hypothetically, we could have a lib_hash (that checks
> .hash files) and lib_mk (that check .mk file) that both implement a
> CheckHash() function, the first to validate that hashes are of a valid
> form, the second to validate that a git hash (in _VERSION) does exist in
> the repository pointed to by _SITE.
> ---
>   utils/check-package | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/utils/check-package b/utils/check-package
> index de41891b56..373bc63f52 100755
> --- a/utils/check-package
> +++ b/utils/check-package
> @@ -224,7 +224,7 @@ def check_file_using_lib(fname):
>           print("{}: would run: {}".format(fname, functions_to_run))
>           return nwarnings, nlines
>   
> -    objects = [[c[0], c[1](fname, flags.manual_url)] for c in internal_functions]
> +    objects = [[f"{lib.__name__[16:]}::{c[0]}", c[1](fname, flags.manual_url)] for c in internal_functions]

  Bikeshedding time! I think, since this is Python, we shouldn't use the C++ 
convention of ::, but rather the Python convention of .

  Otherwise, you can add my Reviewed-by: Arnout Vandecappelle <arnout@mind.be>

  Regards,
  Arnout

>   
>       for name, cf in objects:
>           warn, fail = print_warnings(cf.before(), name in xfail)
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] utils/check-package: emit library name along with check function name

[Yann E. MORIN]

Arnout, All,

On 2024-04-01 15:03 +0200, Arnout Vandecappelle via buildroot spake thusly:
> On 31/03/2024 22:33, Yann E. MORIN wrote:
[--SNIP--]
> > Fix that by storing both the library and function names together. The
> > leading "checkpackagelib." (with the trailing dot, 16 chars) is removed
> > for brevity, because it's present in all libs' names.
[--SNIP--]
> > -    objects = [[c[0], c[1](fname, flags.manual_url)] for c in internal_functions]
> > +    objects = [[f"{lib.__name__[16:]}::{c[0]}", c[1](fname, flags.manual_url)] for c in internal_functions]
>  Bikeshedding time! I think, since this is Python, we shouldn't use the C++
> convention of ::, but rather the Python convention of .

Ah, I don't do C++, so that was not my reference. ;-) I am not a fan of
using a dot, because it does not denote that one part is the filename,
and the other is a function (or any other type of object). But since
this is common pythonistic usage, I'll switch to a dot.

>  Otherwise, you can add my Reviewed-by: Arnout Vandecappelle <arnout@mind.be>

Thanks.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] utils/check-package: emit library name along with check function name

[Yann E. MORIN]

All,

On 2024-03-31 22:33 +0200, Yann E. MORIN spake thusly:
> Although we currently do not have two check libs that have same-name
> check functions, there is nothing that would prevent that, and there
> is no reason why two unrelated libs could not implement checks with
> the same name.
[--SNIP--]
> Fix that by storing both the library and function names together. The
> leading "checkpackagelib." (with the trailing dot, 16 chars) is removed
> for brevity, because it's present in all libs' names.
[--SNIP--]
> diff --git a/utils/check-package b/utils/check-package
> index de41891b56..373bc63f52 100755
> --- a/utils/check-package
> +++ b/utils/check-package
> @@ -224,7 +224,7 @@ def check_file_using_lib(fname):
>          print("{}: would run: {}".format(fname, functions_to_run))
>          return nwarnings, nlines
>  
> -    objects = [[c[0], c[1](fname, flags.manual_url)] for c in internal_functions]
> +    objects = [[f"{lib.__name__[16:]}::{c[0]}", c[1](fname, flags.manual_url)] for c in internal_functions]

As suggested by Arnout, s/::/./, and applied to master.

Regards,
Yann E. MORIN.

>  
>      for name, cf in objects:
>          warn, fail = print_warnings(cf.before(), name in xfail)
> -- 
> 2.44.0
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot