| Message ID | 20240331203354.815139-1-yann.morin.1998@free.fr |
|---|---|
| State | Accepted |
| Headers | show |
| Series | utils/check-package: emit library name along with check function name | expand |
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)
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.
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 >
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)
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(-)