Re: [Buildroot] [PATCH v2 01/14] utils/check-package: decouple adding rules from fixing all intree files

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

On Sun, 31 Jul 2022 16:35:08 -0300
Ricardo Martincoski <ricardo.martincoski@gmail.com> wrote:

> When a new check_function is added to check-package, often there are
> files in the tree that would generate warnings.
> 
> An example is the Sob check_function for patch files:
> | $ ./utils/check-package --i Sob $(git ls-files) >/dev/null
> | 369301 lines processed
> | 46 warnings generated
> Currently these warnings are listed when calling check-package directly,
> and also at the output of pkg-stats, but the check_function does not run
> on 'make check-package' (that is used to catch regressions on GitLab CI
> 'check-package' job) until all warnings in the tree are fixed.
> This (theoretically) allows new .patch files be added without SoB,
> without the GitLab CI catching it.
> 
> So add a way to check-package itself ignore current warnings, while
> still catching new files that do not follow that new check_function.
> 
> Add a file named .checkpackageignore to the buildroot topdir.
> It contains the list of check_functions that are expected to fail for
> each given intree file tested by check-package.
> Each entries is in the format:
> <filename> <check_function> [<check_function> ...]
> 
> These are 2 examples of possible entries:
> package/initscripts/init.d/rcK ConsecutiveEmptyLines EmptyLastLine Shellcheck
> utils/test-pkg Shellcheck
> 
> Keeping such a list allows us to have fine-grained control over which
> warning to ignore.
> 
> In order to avoid this list to grow indefinitely, containing entries for
> files that are already fixed, make each entry an 'expected to fail'
> instead of just an 'ignore', and generate a warning if a check_function
> that was expect to fail for a given files does not generate that
> warning.
> Unfortunately one case that do not generate warning is an entry for a
> file that is deleted in a later commit.
> 
> Add also a new option --ignore-list that can be used:
> - by pkg-stats
> - by developers willing to reduce the intree ignore list
> - by developers of br2-external trees that want its own ignore list
> - for debug and tests purposes
> 
> When using for a br2-external each entry in the ignore file is relative
> to the directory that contains the ignore file. Since calling
> check-package -b already uses relative paths to the directory it was
> called from, when a developer wants a ignore file for a br2-external
> he/she must call check-package from the directory that contains it.
> For instance, with these files:
>  /path/to/br2-external/.checkpackageignore
>  /path/to/br2-external/package/mypackage/mypackage.mk
> The file .checkpackageignore must contain entries in the format:
>  package/mypackage/mypackage.mk EmptyLastLine
> and the developer must call check-package this way:
>  $ cd /path/to/br2-external/ && \
>    /otherpath/to/check-package \
>     --ignore-list=.checkpackageignore \
>      -b package/mypackage/mypackage.mk
> 
> This is one more step towards standardizing the use of just
> 'make check-package' before submitting patches to the list.
> 
> Cc: Sen Hastings <sen@phobosdpl.com>
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> ---
> Changes v1 -> v2:
>   - rename the patch

We finally applied this, but we did one change: now by default
check-package does not ignore any error. You have to explicitly pass it
an ignore list for it to ignore some errors. This way by default
check-package is noisy, and you need to do some action to silence some
of the warnings.

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot