pkg-generic: detect incorrectly used package
RFC PATCH v2 1/1 pkg-generic: detect incorrectly used package
In Buildroot, the selection of a package from a Config.in level and from a Makefile level are completely disconnected. This can lead to issues where the build of a package is triggered at the Makefile level due to the package being listed in another package <pkg>_DEPENDENCIES variable, even if that package is not enabled in the configuration.
This has for example been the case recently with python-can having 'python' in its <pkg>_DEPENDENCIES, while python-can could be enabled when Python 3.x is used, in which case the 'python' package should not be built.
To detect such issues more easily, this patch adds a check in the package infrastructure. When the build process of a package is being triggered, we verify that the package is enabled in the configuration. We do this check in the "configure" step, since this step is the first common step between the normal download case and the "local site method" / "package override" case.
This requires passing two new target variables to the configure target:
- TYPE, which is either host or target. This is needed since the test should only be done on target packages (most host packages don't have a Config.in option)
- KCONFIG_VAR, which is the name of the Config.in variable corresponding to the package being built. For most packages, it's BR2_PACKAGE_<pkg>, but not for toolchain packages, bootloaders or linux.
---
Still sent as an RFC. Maybe we should make this optional, since it prevents from doing "make <pkg>" when "<pkg>" is not selected in menuconfig, which can be seen as annoying for power users. So maybe we could enable this only in the autobuilders? But then newcomers writing new packages would not notice their potential mistake.
Changes since v1:
- Use KCONFIG_VAR in order to make the thing work for toolchain packages, bootloaders and Linux. Issue reported by Vicente.
--- package/pkg-generic.mk | 6 ++++++ 1 file changed, 6 insertions(+)
package/pkg-generic.mk +6 -0
@@ -143,6 +143,10 @@ $(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\ # Configure $(BUILD_DIR)/%/.stamp_configured: + if test "$(TYPE)" = "target" -a -z "$($(KCONFIG_VAR))" ; then \ + echo "ERROR: package $(PKG) is being built but is not enabled in the configuration." ; \ + exit 1 ; \ + fi @$(call step_start,configure) @$(call MESSAGE,"Configuring") $(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
@@ -664,6 +668,8 @@ $$($(2)_TARGET_INSTALL_IMAGES): PKG=$(2) $$($(2)_TARGET_INSTALL_HOST): PKG=$(2) $$($(2)_TARGET_BUILD): PKG=$(2) $$($(2)_TARGET_CONFIGURE): PKG=$(2) +$$($(2)_TARGET_CONFIGURE): TYPE=$(4) +$$($(2)_TARGET_CONFIGURE): KCONFIG_VAR=$$($(2)_KCONFIG_VAR) $$($(2)_TARGET_RSYNC): SRCDIR=$$($(2)_OVERRIDE_SRCDIR) $$($(2)_TARGET_RSYNC): PKG=$(2) $$($(2)_TARGET_PATCH): PKG=$(2)
Signature · 1 line
2.5.0
RFC PATCH v2 1/1 pkg-generic: detect incorrectly used package
Quoted 7 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Thomas" == Thomas Petazzoni <[email protected]> writes:
In Buildroot, the selection of a package from a Config.in level and from a
Makefilelevel are completely disconnected. This can lead to issues where the build of a package is triggered at theMakefilelevel due to the package being listed in another package <pkg>_DEPENDENCIES variable, even if that package is not enabled in the configuration.
[snip]
Quoted 6 lines
---
Still sent as an RFC. Maybe we should make this optional, since it prevents from doing "make <pkg>" when "<pkg>" is not selected in menuconfig, which can be seen as annoying for power users. So maybe we could enable this only in the autobuilders? But then newcomers writing new packages would not notice their potential mistake.
Yes, that's indeed something I do very often. Alternatively we can perhaps extend the logic to also check MAKECMDGOALS and only trigger for empty / default builds.
package/pkg-generic.mk $(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\ · line 149
Quoted 11 lines
index 6a7d97e..7db5ab8 100644
package/pkg-generic.mk +4
-0@@ -143,6 +143,10 @@ $(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\ # Configure $(BUILD_DIR)/%/.stamp_configured: + if test "$(TYPE)" = "target" -a -z "$($(KCONFIG_VAR))" ; then \ + echo "ERROR: package $(PKG) is being built but is not enabled in the configuration." ; \ + exit 1 ; \ + fi
This should imho also use @.
It would be good if the error message would be more clear about what the problem is and how to fix it. Something like:
A package must have added $(PKG) to its _DEPENDENCIES line but forgot to add the corresponding select / depends on $(KCONFIG_VAR). Please review your packages and try again.
Signature · 1 line
Bye, Peter Korsgaard
RFC PATCH v2 1/1 pkg-generic: detect incorrectly used package
Dear Peter Korsgaard,
Quoted 9 lines · Peter Korsgaard
Quoted 5 lines
Still sent as an RFC. Maybe we should make this optional, since it prevents from doing "make <pkg>" when "<pkg>" is not selected in menuconfig, which can be seen as annoying for power users. So maybe we could enable this only in the autobuilders? But then newcomers writing new packages would not notice their potential mistake.
Yes, that's indeed something I do very often. Alternatively we can perhaps extend the logic to also check MAKECMDGOALS and only trigger for empty / default builds.
That could be an option, indeed. Only enable this check if MAKECMDGOALS is empty or "all" ?
Quoted 5 lines
Quoted 4 lines
index 6a7d97e..7db5ab8 100644
package/pkg-generic.mk +0
-0@@ -143,6 +143,10 @@ $(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\
Quoted 8 lines
package/pkg-generic.mk $(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\ · line 149
Quoted 6 lines
package/pkg-generic.mk +4
-0# Configure $(BUILD_DIR)/%/.stamp_configured: + if test "$(TYPE)" = "target" -a -z "$($(KCONFIG_VAR))" ; then \ + echo "ERROR: package $(PKG) is being built but is not enabled in the configuration." ; \ + exit 1 ; \ + fiThis should imho also use @.
Aah, yes. I actually had a @ at some point during my development, until the point where I had to debug this :)
Quoted 6 lines
It would be good if the error message would be more clear about what the problem is and how to fix it. Something like:
A package must have added $(PKG) to its
_DEPENDENCIESline but forgot to add the corresponding select / depends on $(KCONFIG_VAR). Please review your packages and try again.
Will fix.
Thanks for the suggestions!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com