* Re: [Buildroot] [PATCH 1/1] Makefile: add check-package-external target
2025-09-29 9:24 [Buildroot] [PATCH 1/1] Makefile: add check-package-external target Fiona Klute via buildroot
@ 2026-01-01 17:18 ` Thomas Petazzoni via buildroot
2026-01-01 18:04 ` Fiona Klute via buildroot
2026-01-04 10:13 ` Peter Korsgaard
2026-02-03 10:40 ` Arnout Vandecappelle via buildroot
2026-02-13 19:37 ` Thomas Perale via buildroot
2 siblings, 2 replies; 9+ messages in thread
From: Thomas Petazzoni via buildroot @ 2026-01-01 17:18 UTC (permalink / raw)
To: Fiona Klute via buildroot; +Cc: Julien Olivain, Fiona Klute, Romain Naour
Hello Fiona,
Adding other maintainers in Cc: to get some feedback. If there's no
feedback, I'll apply as I don't have a very strong feeling about this.
One thing perhaps that bothers me a bit is the naming, as from the name
I didn't really understand it was for br2-external, and I was wondering
what this concept of package-external was. But I don't really have a
good name. "make check-br2-external" lacks the relationship with "make
check-package". "make check-package-br2-external" is a bit long.
I have a suggestion below, though.
On Mon, 29 Sep 2025 11:24:31 +0200
Fiona Klute via buildroot <buildroot@buildroot.org> wrote:
> +# $(1): br2-external path
> +# $(2): br2-external description
> +define check-package-external
> + @$(call MESSAGE,"Checking packages in $(2)")
> + $(Q)if [ -r "$(1)/.checkpackageignore" ]; then \
> + ignore="--ignore-list=$(1)/.checkpackageignore" ; \
> + fi ; \
> + $(TOPDIR)/utils/check-package \
> + --br2-external $${ignore} \
> + `git -C $(1) ls-tree -r --format='$(1)/%(path)' HEAD`
Could be written in pure make I believe:
$(TOPDIR)/utils/check-package \
--br2-external \
$if ($(wildcard $(1)/.checkpackageignore),--ignore $(1)/.checkpackageignore) \
`git -C $(1) ls-tree -r --format='$(1)/%(path)' HEAD`
(completely untested, of course)
Thanks!
Thomas
--
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Buildroot] [PATCH 1/1] Makefile: add check-package-external target
2026-01-01 17:18 ` Thomas Petazzoni via buildroot
@ 2026-01-01 18:04 ` Fiona Klute via buildroot
2026-01-04 10:13 ` Peter Korsgaard
1 sibling, 0 replies; 9+ messages in thread
From: Fiona Klute via buildroot @ 2026-01-01 18:04 UTC (permalink / raw)
To: Thomas Petazzoni, Fiona Klute via buildroot; +Cc: Julien Olivain, Romain Naour
Hi Thomas!
Am 01.01.26 um 18:18 schrieb Thomas Petazzoni:
> Hello Fiona,
>
> Adding other maintainers in Cc: to get some feedback. If there's no
> feedback, I'll apply as I don't have a very strong feeling about this.
>
> One thing perhaps that bothers me a bit is the naming, as from the name
> I didn't really understand it was for br2-external, and I was wondering
> what this concept of package-external was. But I don't really have a
> good name. "make check-br2-external" lacks the relationship with "make
> check-package". "make check-package-br2-external" is a bit long.
Not sure if long is a problem, when working on individual packages make
commands can get pretty long too, e.g. "make host-uboot-tools-rebuild
world" for testing a U-Boot env file. Adding "br2" might be a little
clearer, but I don't have strong feelings either way.
> I have a suggestion below, though.
>
> On Mon, 29 Sep 2025 11:24:31 +0200
> Fiona Klute via buildroot <buildroot@buildroot.org> wrote:
>
>> +# $(1): br2-external path
>> +# $(2): br2-external description
>> +define check-package-external
>> + @$(call MESSAGE,"Checking packages in $(2)")
>> + $(Q)if [ -r "$(1)/.checkpackageignore" ]; then \
>> + ignore="--ignore-list=$(1)/.checkpackageignore" ; \
>> + fi ; \
>> + $(TOPDIR)/utils/check-package \
>> + --br2-external $${ignore} \
>> + `git -C $(1) ls-tree -r --format='$(1)/%(path)' HEAD`
>
> Could be written in pure make I believe:
>
> $(TOPDIR)/utils/check-package \
> --br2-external \
> $if ($(wildcard $(1)/.checkpackageignore),--ignore $(1)/.checkpackageignore) \
> `git -C $(1) ls-tree -r --format='$(1)/%(path)' HEAD`
>
> (completely untested, of course)
That works after moving the first parenthesis in front of the "if", and
I agree it's nicer. I'll wait a bit to see if any of the other
maintainers have comments, if not I'll respin with that change. :-)
Best regards,
Fiona
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Buildroot] [PATCH 1/1] Makefile: add check-package-external target
2026-01-01 17:18 ` Thomas Petazzoni via buildroot
2026-01-01 18:04 ` Fiona Klute via buildroot
@ 2026-01-04 10:13 ` Peter Korsgaard
2026-01-04 10:39 ` Thomas Petazzoni via buildroot
1 sibling, 1 reply; 9+ messages in thread
From: Peter Korsgaard @ 2026-01-04 10:13 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: Julien Olivain, Fiona Klute, Romain Naour,
Fiona Klute via buildroot
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:
> Hello Fiona,
> Adding other maintainers in Cc: to get some feedback. If there's no
> feedback, I'll apply as I don't have a very strong feeling about this.
> One thing perhaps that bothers me a bit is the naming, as from the name
> I didn't really understand it was for br2-external, and I was wondering
> what this concept of package-external was. But I don't really have a
> good name. "make check-br2-external" lacks the relationship with "make
> check-package". "make check-package-br2-external" is a bit long.
Sorry, didn't look at the patch yet, but could we instead make 'make
check-package' do the right thing if BR2_EXTERNAL is non-zero? E.G.
iterate over the words in it (and pass --br2-external to check-package)?
--
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Buildroot] [PATCH 1/1] Makefile: add check-package-external target
2026-01-04 10:13 ` Peter Korsgaard
@ 2026-01-04 10:39 ` Thomas Petazzoni via buildroot
2026-01-04 13:34 ` Peter Korsgaard
0 siblings, 1 reply; 9+ messages in thread
From: Thomas Petazzoni via buildroot @ 2026-01-04 10:39 UTC (permalink / raw)
To: Peter Korsgaard
Cc: Julien Olivain, Fiona Klute, Romain Naour,
Fiona Klute via buildroot
On Sun, 04 Jan 2026 11:13:10 +0100
Peter Korsgaard <peter@korsgaard.com> wrote:
> Sorry, didn't look at the patch yet, but could we instead make 'make
> check-package' do the right thing if BR2_EXTERNAL is non-zero? E.G.
> iterate over the words in it (and pass --br2-external to check-package)?
There was some rationale in Fiona's patch about why it wasn't done like
this, though maybe this is something we want to challenge.
Here is the rationale:
"""
I've considered wrapping this into the existing check-package target,
but there are a few reasons not to:
1. Some people might not want to subject their external trees to
check-package at all, and would be annoyed by the warnings.
2. BR2_EXTERNAL support for utils/docker-run is still pending (see [1]).
3. The older shellcheck and flake8 versions in the Buildroot base
container image used by utils/docker-run produce some false positives
for my external trees, so I want to keep using the (newer) local
versions for checking those, but use the container for upstream
patches. That is more convenient with separate targets.
"""
Still maybe we should consider ensuring that check-package validates
BR2_EXTERNAL, and somehow address these concerns?
I think (1) is pretty moot, if you're not concerned about check-package
warnings, don't run it at all.
Regarding (2): probably a matter of merging BR2_EXTERNAL support for
docker-run.
Regarding (3): we could also update our Docker container.
Thomas
--
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Buildroot] [PATCH 1/1] Makefile: add check-package-external target
2026-01-04 10:39 ` Thomas Petazzoni via buildroot
@ 2026-01-04 13:34 ` Peter Korsgaard
2026-01-12 21:02 ` Fiona Klute via buildroot
0 siblings, 1 reply; 9+ messages in thread
From: Peter Korsgaard @ 2026-01-04 13:34 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: Julien Olivain, Fiona Klute, Romain Naour,
Fiona Klute via buildroot
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:
Hi,
> Still maybe we should consider ensuring that check-package validates
> BR2_EXTERNAL, and somehow address these concerns?
> I think (1) is pretty moot, if you're not concerned about check-package
> warnings, don't run it at all.
Or run make BR2_EXTERNAL= check-package
> Regarding (2): probably a matter of merging BR2_EXTERNAL support for
> docker-run.
> Regarding (3): we could also update our Docker container.
Yes, agreed. I think it makes sense to move to Debian 13 now.
--
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Buildroot] [PATCH 1/1] Makefile: add check-package-external target
2026-01-04 13:34 ` Peter Korsgaard
@ 2026-01-12 21:02 ` Fiona Klute via buildroot
0 siblings, 0 replies; 9+ messages in thread
From: Fiona Klute via buildroot @ 2026-01-12 21:02 UTC (permalink / raw)
To: Peter Korsgaard, Thomas Petazzoni
Cc: Julien Olivain, Romain Naour, Fiona Klute via buildroot
Am 04.01.26 um 14:34 schrieb Peter Korsgaard:
>>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:
>
> Hi,
>
> > Still maybe we should consider ensuring that check-package validates
> > BR2_EXTERNAL, and somehow address these concerns?
>
> > I think (1) is pretty moot, if you're not concerned about check-package
> > warnings, don't run it at all.
>
> Or run make BR2_EXTERNAL= check-package
Good point. I usually use different output dirs for my targets, so I
could just run directly in the Buildroot repo to run without any
external trees.
> > Regarding (2): probably a matter of merging BR2_EXTERNAL support for
> > docker-run.
Definitely wouldn't complain about that, after almost a year and half I
was wondering if it's ever going to happen. ;-)
> > Regarding (3): we could also update our Docker container.
>
> Yes, agreed. I think it makes sense to move to Debian 13 now.
No complaints here either, but as updates come in there may be new
differences.
My main concern here is that make will fail after the first failed
command, so if there are check-package issues in Buildroot (for example
because using a different version than in the container) checks for
external trees won't run (already an issue with multiple external trees
with my original patch). I'm not aware of a built-in tool in make to
handle that (i.e., record non-zero status codes, return an error if
there are any).
Would it be acceptable to record error codes to a file instead, vaguely
similar to how legal-info does it (but probably simpler), possibly to do
a non-zero exit if there are any?
Best regards,
Fiona
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Buildroot] [PATCH 1/1] Makefile: add check-package-external target
2025-09-29 9:24 [Buildroot] [PATCH 1/1] Makefile: add check-package-external target Fiona Klute via buildroot
2026-01-01 17:18 ` Thomas Petazzoni via buildroot
@ 2026-02-03 10:40 ` Arnout Vandecappelle via buildroot
2026-02-13 19:37 ` Thomas Perale via buildroot
2 siblings, 0 replies; 9+ messages in thread
From: Arnout Vandecappelle via buildroot @ 2026-02-03 10:40 UTC (permalink / raw)
To: Fiona Klute, buildroot
Hi Fiona,
On 29/09/2025 11:24, Fiona Klute via buildroot wrote:
> The new target provides a convenient way to run utils/check-package on
> any external trees, using .checkpackageignore files from the
> respective trees if present.
>
> While .checkpackageignore should be used as little as possible, in a
> few cases adding overrides for false-positives to the affected files
> is not feasible, a practical example of this is a Markdown file
> misidentified as Python by libmagic (likely due to code blocks).
>
> Signed-off-by: Fiona Klute <fiona.klute@gmx.de>
Applied to master, thanks.
We discussed it at the BR meeting, looked at the other options (like always
checking externals as well), but ended up with the conclusion that your proposal
actually covers the likely use cases best.
We also didn't find a better name so it stays check-package-external.
There are two things still missing:
- Mentioning it in the manual (I'm not even sure if check-package is already
mentioned...)
- Adding a runtime test of the feature (including with and without the ignore file).
Also one small modification, see below.
> ---
> I've considered wrapping this into the existing check-package target,
> but there are a few reasons not to:
>
> 1. Some people might not want to subject their external trees to
> check-package at all, and would be annoyed by the warnings.
>
> 2. BR2_EXTERNAL support for utils/docker-run is still pending (see [1]).
>
> 3. The older shellcheck and flake8 versions in the Buildroot base
> container image used by utils/docker-run produce some false positives
> for my external trees, so I want to keep using the (newer) local
> versions for checking those, but use the container for upstream
> patches. That is more convenient with separate targets.
>
> If anyone's wondering about the "Markdown misidentified as Python"
> example: It looks like "__init__(self):" appearing in a code block
> makes libmagic consider the file Python.
>
> Makefile | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index b8678ea15b..cdc7ac0e4e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -125,7 +125,8 @@ endif
> noconfig_targets := menuconfig nconfig gconfig xconfig config oldconfig randconfig \
> defconfig %_defconfig allyesconfig allnoconfig alldefconfig syncconfig release \
> randpackageconfig allyespackageconfig allnopackageconfig \
> - print-version olddefconfig distclean manual manual-% check-package
> + print-version olddefconfig distclean manual manual-% check-package \
> + check-package-external
>
> # Some global targets do not trigger a build, but are used to collect
> # metadata, or do various checks. When such targets are triggered,
> @@ -1262,10 +1263,27 @@ release:
> print-version:
> @echo $(BR2_VERSION_FULL)
>
> +# $(1): br2-external path
> +# $(2): br2-external description
> +define check-package-external
> + @$(call MESSAGE,"Checking packages in $(2)")
> + $(Q)if [ -r "$(1)/.checkpackageignore" ]; then \
> + ignore="--ignore-list=$(1)/.checkpackageignore" ; \
> + fi ; \
> + $(TOPDIR)/utils/check-package \
> + --br2-external $${ignore} \
If "ignore" is set in the environment and there is no ignore file, this could
create problems. So I've added an else branch to the condition above and set to
to "".
Regards,
Arnout
> + `git -C $(1) ls-tree -r --format='$(1)/%(path)' HEAD`
> +endef
> +
> check-package:
> $(Q)./utils/check-package `git ls-tree -r --name-only HEAD` \
> --ignore-list=$(TOPDIR)/.checkpackageignore
>
> +check-package-external:
> + $(foreach name,$(BR2_EXTERNAL_NAMES),\
> + $(call check-package-external,$(BR2_EXTERNAL_$(name)_PATH),\
> + $(BR2_EXTERNAL_$(name)_DESC))$(sep))
> +
> .PHONY: .checkpackageignore
> .checkpackageignore:
> $(Q)./utils/check-package --failed-only `git ls-tree -r --name-only HEAD` \
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Buildroot] [PATCH 1/1] Makefile: add check-package-external target
2025-09-29 9:24 [Buildroot] [PATCH 1/1] Makefile: add check-package-external target Fiona Klute via buildroot
2026-01-01 17:18 ` Thomas Petazzoni via buildroot
2026-02-03 10:40 ` Arnout Vandecappelle via buildroot
@ 2026-02-13 19:37 ` Thomas Perale via buildroot
2 siblings, 0 replies; 9+ messages in thread
From: Thomas Perale via buildroot @ 2026-02-13 19:37 UTC (permalink / raw)
To: Fiona Klute; +Cc: Thomas Perale, buildroot
In reply of:
> The new target provides a convenient way to run utils/check-package on
> any external trees, using .checkpackageignore files from the
> respective trees if present.
>
> While .checkpackageignore should be used as little as possible, in a
> few cases adding overrides for false-positives to the affected files
> is not feasible, a practical example of this is a Markdown file
> misidentified as Python by libmagic (likely due to code blocks).
>
> Signed-off-by: Fiona Klute <fiona.klute@gmx.de>
Applied to 2025.02.x & 2025.11.x. Thanks
> ---
> I've considered wrapping this into the existing check-package target,
> but there are a few reasons not to:
>
> 1. Some people might not want to subject their external trees to
> check-package at all, and would be annoyed by the warnings.
>
> 2. BR2_EXTERNAL support for utils/docker-run is still pending (see [1]).
>
> 3. The older shellcheck and flake8 versions in the Buildroot base
> container image used by utils/docker-run produce some false positives
> for my external trees, so I want to keep using the (newer) local
> versions for checking those, but use the container for upstream
> patches. That is more convenient with separate targets.
>
> If anyone's wondering about the "Markdown misidentified as Python"
> example: It looks like "__init__(self):" appearing in a code block
> makes libmagic consider the file Python.
>
> Makefile | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index b8678ea15b..cdc7ac0e4e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -125,7 +125,8 @@ endif
> noconfig_targets := menuconfig nconfig gconfig xconfig config oldconfig randconfig \
> defconfig %_defconfig allyesconfig allnoconfig alldefconfig syncconfig release \
> randpackageconfig allyespackageconfig allnopackageconfig \
> - print-version olddefconfig distclean manual manual-% check-package
> + print-version olddefconfig distclean manual manual-% check-package \
> + check-package-external
>
> # Some global targets do not trigger a build, but are used to collect
> # metadata, or do various checks. When such targets are triggered,
> @@ -1262,10 +1263,27 @@ release:
> print-version:
> @echo $(BR2_VERSION_FULL)
>
> +# $(1): br2-external path
> +# $(2): br2-external description
> +define check-package-external
> + @$(call MESSAGE,"Checking packages in $(2)")
> + $(Q)if [ -r "$(1)/.checkpackageignore" ]; then \
> + ignore="--ignore-list=$(1)/.checkpackageignore" ; \
> + fi ; \
> + $(TOPDIR)/utils/check-package \
> + --br2-external $${ignore} \
> + `git -C $(1) ls-tree -r --format='$(1)/%(path)' HEAD`
> +endef
> +
> check-package:
> $(Q)./utils/check-package `git ls-tree -r --name-only HEAD` \
> --ignore-list=$(TOPDIR)/.checkpackageignore
>
> +check-package-external:
> + $(foreach name,$(BR2_EXTERNAL_NAMES),\
> + $(call check-package-external,$(BR2_EXTERNAL_$(name)_PATH),\
> + $(BR2_EXTERNAL_$(name)_DESC))$(sep))
> +
> .PHONY: .checkpackageignore
> .checkpackageignore:
> $(Q)./utils/check-package --failed-only `git ls-tree -r --name-only HEAD` \
> --
> 2.51.0
>
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 9+ messages in thread