[Buildroot] [PATCH 0/4] support ignore-indent for check-package configs

[Buildroot] [PATCH 0/4] support ignore-indent for check-package configs

[James Knight]

The following series brings support for having a linter hint in
configuration files to avoid having `check-package` warn about bad
indentations for container-like configurations.

Buildroot ignores indentations for the root package configurations as
well as kodi/x11r7 packages. These configurations reference other
configurations, which makes it somewhat preferred to indent packages
under a menu option. For example:

    menu "<category>"
        source "package/<ref>/Config.in"
        ...
    endmenu

To prevent `check-package` from triggering a warning, an internal list
inside `lib_config.py` is maintained. This is less than ideal since (1)
it requires adjusting the library script if future "category"
configurations are added/removed, and (2) does not provide a way for
br2-external trees to utilize the same exceptions. For example, if a
br2-external tree manages a `Config.in` with pending upstream packages:

    menu "Target packages (Queued upstream)"
        source "$BR2_EXTERNAL_BAR_42_PATH/newpkg1/Config.in"
        source "$BR2_EXTERNAL_BAR_42_PATH/newpkg2/Config.in"
    endmenu

Utilizing Buildroot's `check-package` call on this file will generate an
indentation warning.

What this series brings is the ability to add a `ignore-indent` flag
into configuration scripts to avoid `check-package` from generating
errors for select files. Continuing with the above example, adding the
flag as follows will no longer generate an indentation error:

    # noqa: ignore-indent
    menu "Target packages (Queued upstream)"
        source "$BR2_EXTERNAL_BAR_42_PATH/newpkg1/Config.in"
        source "$BR2_EXTERNAL_BAR_42_PATH/newpkg2/Config.in"
    endmenu

This change allows dropping the internal list of configuration
exceptions (for package/Config.in, package/Config.in.host, kodi and
x11r7), to instead use the `ignore-indent` flag in the respective files.

James Knight (4):
  utils/check-package: cleanup line reading
  utils/check-package: support a file-state instance for library types
  utils/check-package: support ignore-indent flag for configurations
  utils/check-package: utilize ignore-indent flag for special configs

 package/Config.in                   |  2 ++
 package/Config.in.host              |  2 ++
 package/kodi/Config.in              |  2 ++
 package/x11r7/Config.in             |  2 ++
 utils/check-package                 | 30 ++++++++++++++++++---------
 utils/checkpackagelib/base.py       |  2 +-
 utils/checkpackagelib/lib.py        | 10 ++++-----
 utils/checkpackagelib/lib_config.py | 32 ++++++++++++++++++++---------
 utils/checkpackagelib/lib_hash.py   |  6 +++---
 utils/checkpackagelib/lib_mk.py     | 22 ++++++++++----------
 utils/checkpackagelib/lib_patch.py  |  6 +++---
 utils/checkpackagelib/lib_sysv.py   |  4 ++--
 utils/checkpackagelib/test_util.py  |  4 ++--
 13 files changed, 77 insertions(+), 47 deletions(-)

--
2.40.1.windows.1

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

Re: [Buildroot] [PATCH 0/4] support ignore-indent for check-package configs

[Thomas Petazzoni via buildroot]

Hello Ricardo,

Since you're the main maintainer of the check-package logic, I really
would like to have your opinion about the approach taken in this patch
series. I think the feature makes sense, but I don't know if the
implementation is really in line with the philosophy of the existing
code. If you could give your opinion, it would be very useful.

Thanks!

Thomas

On Sat, 29 Apr 2023 14:12:01 -0400
James Knight <james.d.knight@live.com> wrote:

> The following series brings support for having a linter hint in
> configuration files to avoid having `check-package` warn about bad
> indentations for container-like configurations.
> 
> Buildroot ignores indentations for the root package configurations as
> well as kodi/x11r7 packages. These configurations reference other
> configurations, which makes it somewhat preferred to indent packages
> under a menu option. For example:
> 
>     menu "<category>"
>         source "package/<ref>/Config.in"
>         ...
>     endmenu
> 
> To prevent `check-package` from triggering a warning, an internal list
> inside `lib_config.py` is maintained. This is less than ideal since (1)
> it requires adjusting the library script if future "category"
> configurations are added/removed, and (2) does not provide a way for
> br2-external trees to utilize the same exceptions. For example, if a
> br2-external tree manages a `Config.in` with pending upstream packages:
> 
>     menu "Target packages (Queued upstream)"
>         source "$BR2_EXTERNAL_BAR_42_PATH/newpkg1/Config.in"
>         source "$BR2_EXTERNAL_BAR_42_PATH/newpkg2/Config.in"
>     endmenu
> 
> Utilizing Buildroot's `check-package` call on this file will generate an
> indentation warning.
> 
> What this series brings is the ability to add a `ignore-indent` flag
> into configuration scripts to avoid `check-package` from generating
> errors for select files. Continuing with the above example, adding the
> flag as follows will no longer generate an indentation error:
> 
>     # noqa: ignore-indent
>     menu "Target packages (Queued upstream)"
>         source "$BR2_EXTERNAL_BAR_42_PATH/newpkg1/Config.in"
>         source "$BR2_EXTERNAL_BAR_42_PATH/newpkg2/Config.in"
>     endmenu
> 
> This change allows dropping the internal list of configuration
> exceptions (for package/Config.in, package/Config.in.host, kodi and
> x11r7), to instead use the `ignore-indent` flag in the respective files.
> 
> James Knight (4):
>   utils/check-package: cleanup line reading
>   utils/check-package: support a file-state instance for library types
>   utils/check-package: support ignore-indent flag for configurations
>   utils/check-package: utilize ignore-indent flag for special configs
> 
>  package/Config.in                   |  2 ++
>  package/Config.in.host              |  2 ++
>  package/kodi/Config.in              |  2 ++
>  package/x11r7/Config.in             |  2 ++
>  utils/check-package                 | 30 ++++++++++++++++++---------
>  utils/checkpackagelib/base.py       |  2 +-
>  utils/checkpackagelib/lib.py        | 10 ++++-----
>  utils/checkpackagelib/lib_config.py | 32 ++++++++++++++++++++---------
>  utils/checkpackagelib/lib_hash.py   |  6 +++---
>  utils/checkpackagelib/lib_mk.py     | 22 ++++++++++----------
>  utils/checkpackagelib/lib_patch.py  |  6 +++---
>  utils/checkpackagelib/lib_sysv.py   |  4 ++--
>  utils/checkpackagelib/test_util.py  |  4 ++--
>  13 files changed, 77 insertions(+), 47 deletions(-)
> 
> --
> 2.40.1.windows.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot



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

Re: [Buildroot] [PATCH 0/4] support ignore-indent for check-package configs

[Ricardo Martincoski]

[-- Attachment #1: Type: text/plain, Size: 3535 bytes --]

Hello,

On Sat, Aug 26, 2023 at 07:27 PM, Thomas Petazzoni via buildroot wrote:

> Hello Ricardo,
> 
> Since you're the main maintainer of the check-package logic, I really
> would like to have your opinion about the approach taken in this patch
> series. I think the feature makes sense, but I don't know if the
> implementation is really in line with the philosophy of the existing
> code. If you could give your opinion, it would be very useful.

Sure.
Sorry the long delay.

> On Sat, 29 Apr 2023 14:12:01 -0400
> James Knight <james.d.knight@live.com> wrote:
> 
>> The following series brings support for having a linter hint in
>> configuration files to avoid having `check-package` warn about bad
>> indentations for container-like configurations.
>> 
>> Buildroot ignores indentations for the root package configurations as
>> well as kodi/x11r7 packages. These configurations reference other
>> configurations, which makes it somewhat preferred to indent packages
>> under a menu option. For example:
>> 
>>     menu "<category>"
>>         source "package/<ref>/Config.in"
>>         ...
>>     endmenu
>> 
>> To prevent `check-package` from triggering a warning, an internal list
>> inside `lib_config.py` is maintained. This is less than ideal since (1)
>> it requires adjusting the library script if future "category"
>> configurations are added/removed, and (2) does not provide a way for
>> br2-external trees to utilize the same exceptions. For example, if a
>> br2-external tree manages a `Config.in` with pending upstream packages:
>> 
>>     menu "Target packages (Queued upstream)"
>>         source "$BR2_EXTERNAL_BAR_42_PATH/newpkg1/Config.in"
>>         source "$BR2_EXTERNAL_BAR_42_PATH/newpkg2/Config.in"
>>     endmenu
>> 
>> Utilizing Buildroot's `check-package` call on this file will generate an
>> indentation warning.
>> 
>> What this series brings is the ability to add a `ignore-indent` flag
>> into configuration scripts to avoid `check-package` from generating
>> errors for select files. Continuing with the above example, adding the
>> flag as follows will no longer generate an indentation error:
>> 
>>     # noqa: ignore-indent
>>     menu "Target packages (Queued upstream)"
>>         source "$BR2_EXTERNAL_BAR_42_PATH/newpkg1/Config.in"
>>         source "$BR2_EXTERNAL_BAR_42_PATH/newpkg2/Config.in"
>>     endmenu
>> 
>> This change allows dropping the internal list of configuration
>> exceptions (for package/Config.in, package/Config.in.host, kodi and
>> x11r7), to instead use the `ignore-indent` flag in the respective files.

This is a great idea.
I have a few implementation comments that I will provide in patches 2
and 3.

While reviewing the series I also came across an improvement in the code
that I hope can make easier to add such support.
I will sent a patch shortly with this improvement.

As a tip for further development in check-package (to catch for instance
any issue while splitting patches, like it occurred in patch 1 and is
already fixed in 'next' branch) I usually use below commands while
developing patches:

$ utils/docker-run utils/check-package -vv --include-only Indent package/example1/Config.in
$ utils/docker-run python3 -m pytest -vv utils/checkpackagelib/
$ utils/docker-run support/testing/run-tests -s -o o -d d -k tests.utils

together to the more obvious one:

$ utils/docker-run make check-package

utils/docker-run is not really needed. I use it only to provide
reproducible environment without effort of installing dependencies.

Regards,
Ricardo

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

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