Skip to contentSkip to conversation

pkg-generic: detect incorrectly used package

RFC PATCH New Thomas Petazzoni 2 participants 5 messages Aug 28, 2015

lore
Share & tools
lore
#0 lore raw

RFC PATCH 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.

Since this check should only be done for target packages (most of the host packages don't have Config.in options), we pass the type of the package from inner-generic-package down to the configure rule, through a target variable named 'TYPE'.

---
This is labeled as RFC because I have only quickly tested it, and I haven't thought about all the possible corner cases that may make my simplistic approach invalid. Comments and reviews are welcome.

---
 package/pkg-generic.mk | 5 +++++
 1 file changed, 5 insertions(+)
package/pkg-generic.mk +5 -0
index 6a7d97e..fa66be8 100644
@@ -143,6 +143,10 @@ $(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\
 
 # Configure
 $(BUILD_DIR)/%/.stamp_configured:
+	$(Q)if test "$(TYPE)" = "target" -a -z "$(BR2_PACKAGE_$(PKG))" ; 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,7 @@ $$($(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_RSYNC):                  SRCDIR=$$($(2)_OVERRIDE_SRCDIR)
 $$($(2)_TARGET_RSYNC):                  PKG=$(2)
 $$($(2)_TARGET_PATCH):			PKG=$(2)
Signature · 1 line
2.5.0
#1 lore raw

RFC PATCH 1/1 pkg-generic: detect incorrectly used package

Dear Thomas Petazzoni,

I tried to test your patch and I have found this:

ERROR: package TOOLCHAIN_EXTERNAL is being built but is not enabled in the configuration.

Regards,

Vincent.

Quoted 56 lines · Thomas Petazzoni

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.

Since this check should only be done for target packages (most of the host packages don't have Config.in options), we pass the type of the package from inner-generic-package down to the configure rule, through a target variable named 'TYPE'.

---
This is labeled as RFC because I have only quickly tested it, and I haven't thought about all the possible corner cases that may make my simplistic approach invalid. Comments and reviews are welcome.

---
 package/pkg-generic.mk | 5 +++++
 1 file changed, 5 insertions(+)
package/pkg-generic.mk +5 -0
index 6a7d97e..fa66be8 100644
@@ -143,6 +143,10 @@ $(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\
 
 # Configure
 $(BUILD_DIR)/%/.stamp_configured:
+	$(Q)if test "$(TYPE)" = "target" -a -z "$(BR2_PACKAGE_$(PKG))" ; 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,7 @@ $$($(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_RSYNC):                  SRCDIR=$$($(2)_OVERRIDE_SRCDIR)
 $$($(2)_TARGET_RSYNC):                  PKG=$(2)
 $$($(2)_TARGET_PATCH):			PKG=$(2)
#2 lore raw

RFC PATCH 1/1 pkg-generic: detect incorrectly used package

Dear Vicente Olivert Riera,

Quoted 4 lines · Vicente Olivert Riera

I tried to test your patch and I have found this:

ERROR: package TOOLCHAIN_EXTERNAL is being built but is not enabled in the configuration.

Right, my patch is wrong for the toolchain, bootloaders and linux packages.

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#3 lore raw

RFC PATCH 1/1 pkg-generic: detect incorrectly used package

In reply to Vicente Olivert Riera

Dear Thomas Petazzoni,

since there are certain packages which don't start by BR2_PACKAGE_ (i.e. BR2_TOOLCHAIN_EXTERNAL), I would suggest you to make this change:

Quoted 1 line · Vicente Olivert Riera

package/pkg-generic.mk $(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\ · line 146

Quoted 1 line
package/pkg-generic.mk +1 -0
+	$(Q)if test "$(TYPE)" = "target" -a -z "$(BR2_PACKAGE_$(PKG))" ; then \

to:

+ $(Q)if test "$(TYPE)" = "target" -a -z "$(BR2_PACKAGE_$(PKG))" \
+ -a -z "$(BR2_$(PKG))" ; then \

Regards,

Vincent.

#4 lore raw

RFC PATCH 1/1 pkg-generic: detect incorrectly used package

Dear Vicente Olivert Riera,

Review

Quoted 10 lines · Vicente Olivert Riera

since there are certain packages which don't start by BR2_PACKAGE_ (i.e. BR2_TOOLCHAIN_EXTERNAL), I would suggest you to make this change:

Quoted 1 line · Vicente Olivert Riera

package/pkg-generic.mk $(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\ · line 146

Quoted 1 line
package/pkg-generic.mk +1 -0
+	$(Q)if test "$(TYPE)" = "target" -a -z "$(BR2_PACKAGE_$(PKG))" ; then \

to:

(quoted diff) +2 -0
+       $(Q)if test "$(TYPE)" = "target" -a -z "$(BR2_PACKAGE_$(PKG))" \
+                                        -a -z "$(BR2_$(PKG))" ; then \

This only fixes the toolchain case, but not the Linux kernel case, or the bootloader case. I've sent a v2 of the patch that re-uses some existing mechanism of the package infrastructure to known the Config.in option associated to the package being built. I believe it's nicer.

Thanks!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com