Skip to contentSkip to conversation

core/pkg-infra: declare target variant before host variant

PATCH New Yann E. MORIN 2 participants 6 messages Apr 29, 2018 – Apr 30, 2018

lore
Share & tools
lore

Yann E. MORIN

author
#0 lore raw

cover core/pkg-infra: declare target variant before host variant

Hello All!

As Thomas puts it in his original commit logs:

Our package infrastructure uses inheritance of a number of values from the target package to the host package, which assumes the target package is defined before the host package.

This series fixes the 6 affected packages, with a first patch inverting the calls to the package macros, and a secon patch just for qemu to move around all the code.

Finally, it adds a check in the package ifnrastructure to check this condition.

Regards,
Yann E. MORIN.

The following changes since commit 16edaece366b052fe696982d816e8b042b217ad5

speex: remove useless BUILD_CMDS (2018-04-29 13:46:16 +0200)

are available in the git repository at:

git://git.buildroot.org/~ymorin/git/buildroot.git

for you to fetch changes up to f3fbc309610c9ff7df8d06ba1aad1f9369e5cdac

package/pkg-generic: add check that target variant is defined before host variant (2018-04-29 14:07:08 +0200)

----------------------------------------------------------------
Thomas Petazzoni (3):
package: declare target variant before host variant
package/qemu: declare target variant before host variant package/pkg-generic: add check that target variant is defined before host variant

 docs/manual/adding-packages-generic.txt            |   6 +-
 package/android-tools/android-tools.mk             |   2 +-
 package/gauche/gauche.mk                           |   2 +-
 package/lcms2/lcms2.mk                             |   2 +-
 .../linux-syscall-support/linux-syscall-support.mk |   2 +-
 package/pkg-generic.mk                             |   6 +
 package/pngquant/pngquant.mk                       |   2 +-
 package/qemu/qemu.mk                               | 242 ++++++++++-----------
 8 files changed, 136 insertions(+), 128 deletions(-)
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
author
#1 lore raw

PATCH 1/3 package: declare target variant before host variant

From: Thomas Petazzoni <[email protected]>

Our package infrastructure uses inheritance of a number of values from the target package to the host package, which assumes the target package is defined before the host package. In addition, future changes are going to make this requirement even more important.

Therefore, let's fix the android-tools, gauche, lcms2, linux-syscall-support and pngquant packages, so that they declare their target variant before their host variant, like all other packages in Buildroot.

Signed-off-by Thomas Petazzoni <[email protected]>
Signed-off-by "Yann E. MORIN" <[email protected]>
---
 package/android-tools/android-tools.mk                 | 2 +-
 package/gauche/gauche.mk                               | 2 +-
 package/lcms2/lcms2.mk                                 | 2 +-
 package/linux-syscall-support/linux-syscall-support.mk | 2 +-
 package/pngquant/pngquant.mk                           | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)
package/android-tools/android-tools.mk +1 -1
index 11e0a15a7f..879d788e3a 100644
@@ -81,5 +81,5 @@ define ANDROID_TOOLS_INSTALL_TARGET_CMDS
 		$(INSTALL) -D -m 0755 $(@D)/build-$(t)/$(t) $(TARGET_DIR)/usr/bin/$(t)$(sep))
 endef
 
-$(eval $(host-generic-package))
 $(eval $(generic-package))
+$(eval $(host-generic-package))
package/gauche/gauche.mk +1 -1
index 1e1ee390af..0b4185f5bf 100644
@@ -29,5 +29,5 @@ endif
 # here.
 GAUCHE_CONF_ENV = CFLAGS="$(TARGET_CFLAGS) -std=gnu99"
 
-$(eval $(host-autotools-package))
 $(eval $(autotools-package))
+$(eval $(host-autotools-package))
package/lcms2/lcms2.mk +1 -1
index 29f8ac8dee..f61563dea4 100644
@@ -31,5 +31,5 @@ else
 LCMS2_CONF_OPTS += --without-zlib
 endif
 
-$(eval $(host-autotools-package))
 $(eval $(autotools-package))
+$(eval $(host-autotools-package))
package/linux-syscall-support/linux-syscall-support.mk +1 -1
index b8145de496..050c5058ab 100644
@@ -25,5 +25,5 @@ define HOST_LINUX_SYSCALL_SUPPORT_INSTALL_CMDS
 		$(HOST_DIR)/include/linux_syscall_support.h
 endef
 
-$(eval $(host-generic-package))
 $(eval $(generic-package))
+$(eval $(host-generic-package))
package/pngquant/pngquant.mk +1 -1
index 1ce8359d40..f181d6fb47 100644
@@ -51,5 +51,5 @@ define HOST_PNGQUANT_INSTALL_CMDS
 	$(HOST_MAKE_ENV) $(MAKE) -C $(@D) install
 endef
 
-$(eval $(host-generic-package))
 $(eval $(generic-package))
+$(eval $(host-generic-package))
Signature · 1 line
2.14.1
author
#2 lore raw

PATCH 2/3 package/qemu: declare target variant before host variant

In reply to Yann E. MORIN

From: Thomas Petazzoni <[email protected]>

Our package infrastructure uses inheritance of a number of values from the target package to the host package, which assumes the target package is defined before the host package. In addition, future changes are going to make this requirement even more important.

Therefore, let's fix the qemu package so that it declares its target variant before its host variant, like all other packages in Buildroot. We handle qemu separately from other packages, because unlike other packages, it didn't had the "eval" for the host and target packages at the end of the file, but rather all variables related to the host variant first, then the call to the package infrastructure for the host variant, then the variables related to the target variant, and finally the call to the package infrastructure for the target variant. We are inverting the order of those two big parts in this commit.

Signed-off-by Thomas Petazzoni <[email protected]>
Signed-off-by "Yann E. MORIN" <[email protected]>
---
 package/qemu/qemu.mk | 242 +++++++++++++++++++++++++--------------------------
 1 file changed, 121 insertions(+), 121 deletions(-)
package/qemu/qemu.mk +121 -121
index f6f4a2b870..0a6a6c0609 100644
@@ -13,6 +13,127 @@ QEMU_LICENSE_FILES = COPYING COPYING.LIB
 #       the non-(L)GPL license texts are specified in the affected
 #       individual source files.
 
+#-------------------------------------------------------------
+# Target-qemu
+
+QEMU_DEPENDENCIES = host-pkgconf host-python libglib2 zlib pixman
+
+# Need the LIBS variable because librt and libm are
+# not automatically pulled. :-(
+QEMU_LIBS = -lrt -lm
+
+QEMU_OPTS =
+
+QEMU_VARS = \
+	LIBTOOL=$(HOST_DIR)/bin/libtool \
+	PYTHON=$(HOST_DIR)/bin/python2 \
+	PYTHONPATH=$(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR)/site-packages
+
+# If we want to specify only a subset of targets, we must still enable all
+# of them, so that QEMU properly builds its list of default targets, from
+# which it then checks if the specified sub-set is valid. That's what we
+# do in the first part of the if-clause.
+# Otherwise, if we do not want to pass a sub-set of targets, we then need
+# to either enable or disable -user and/or -system emulation appropriately.
+# That's what we do in the else-clause.
+ifneq ($(call qstrip,$(BR2_PACKAGE_QEMU_CUSTOM_TARGETS)),)
+QEMU_OPTS += --enable-system --enable-linux-user
+QEMU_OPTS += --target-list="$(call qstrip,$(BR2_PACKAGE_QEMU_CUSTOM_TARGETS))"
+else
+
+ifeq ($(BR2_PACKAGE_QEMU_SYSTEM),y)
+QEMU_OPTS += --enable-system
+else
+QEMU_OPTS += --disable-system
+endif
+
+ifeq ($(BR2_PACKAGE_QEMU_LINUX_USER),y)
+QEMU_OPTS += --enable-linux-user
+else
+QEMU_OPTS += --disable-linux-user
+endif
+
+endif
+
+ifeq ($(BR2_PACKAGE_QEMU_SDL),y)
+QEMU_OPTS += --enable-sdl
+QEMU_DEPENDENCIES += sdl
+QEMU_VARS += SDL_CONFIG=$(BR2_STAGING_DIR)/usr/bin/sdl-config
+else
+QEMU_OPTS += --disable-sdl
+endif
+
+ifeq ($(BR2_PACKAGE_QEMU_FDT),y)
+QEMU_OPTS += --enable-fdt
+QEMU_DEPENDENCIES += dtc
+else
+QEMU_OPTS += --disable-fdt
+endif
+
+ifeq ($(BR2_PACKAGE_QEMU_TOOLS),y)
+QEMU_OPTS += --enable-tools
+else
+QEMU_OPTS += --disable-tools
+endif
+
+ifeq ($(BR2_PACKAGE_LIBSSH2),y)
+QEMU_OPTS += --enable-libssh2
+QEMU_DEPENDENCIES += libssh2
+else
+QEMU_OPTS += --disable-libssh2
+endif
+
+# Override CPP, as it expects to be able to call it like it'd
+# call the compiler.
+define QEMU_CONFIGURE_CMDS
+	( cd $(@D); \
+		LIBS='$(QEMU_LIBS)' \
+		$(TARGET_CONFIGURE_OPTS) \
+		$(TARGET_CONFIGURE_ARGS) \
+		CPP="$(TARGET_CC) -E" \
+		$(QEMU_VARS) \
+		./configure \
+			--prefix=/usr \
+			--cross-prefix=$(TARGET_CROSS) \
+			--with-system-pixman \
+			--audio-drv-list= \
+			--enable-kvm \
+			--enable-attr \
+			--enable-vhost-net \
+			--disable-bsd-user \
+			--disable-xen \
+			--disable-slirp \
+			--disable-vnc \
+			--disable-virtfs \
+			--disable-brlapi \
+			--disable-curses \
+			--disable-curl \
+			--disable-bluez \
+			--disable-vde \
+			--disable-linux-aio \
+			--disable-cap-ng \
+			--disable-docs \
+			--disable-spice \
+			--disable-rbd \
+			--disable-libiscsi \
+			--disable-usb-redir \
+			--disable-strip \
+			--disable-seccomp \
+			--disable-sparse \
+			$(QEMU_OPTS) \
+	)
+endef
+
+define QEMU_BUILD_CMDS
+	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)
+endef
+
+define QEMU_INSTALL_TARGET_CMDS
+	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) $(QEMU_MAKE_ENV) DESTDIR=$(TARGET_DIR) install
+endef
+
+$(eval $(generic-package))
+
 #-------------------------------------------------------------
 # Host-qemu
 

@@ -147,124 +268,3 @@ $(eval $(host-generic-package))
 
 # variable used by other packages
 QEMU_USER = $(HOST_DIR)/bin/qemu-$(HOST_QEMU_ARCH)
-
-#-------------------------------------------------------------
-# Target-qemu
-
-QEMU_DEPENDENCIES = host-pkgconf host-python libglib2 zlib pixman
-
-# Need the LIBS variable because librt and libm are
-# not automatically pulled. :-(
-QEMU_LIBS = -lrt -lm
-
-QEMU_OPTS =
-
-QEMU_VARS = \
-	LIBTOOL=$(HOST_DIR)/bin/libtool \
-	PYTHON=$(HOST_DIR)/bin/python2 \
-	PYTHONPATH=$(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR)/site-packages
-
-# If we want to specify only a subset of targets, we must still enable all
-# of them, so that QEMU properly builds its list of default targets, from
-# which it then checks if the specified sub-set is valid. That's what we
-# do in the first part of the if-clause.
-# Otherwise, if we do not want to pass a sub-set of targets, we then need
-# to either enable or disable -user and/or -system emulation appropriately.
-# That's what we do in the else-clause.
-ifneq ($(call qstrip,$(BR2_PACKAGE_QEMU_CUSTOM_TARGETS)),)
-QEMU_OPTS += --enable-system --enable-linux-user
-QEMU_OPTS += --target-list="$(call qstrip,$(BR2_PACKAGE_QEMU_CUSTOM_TARGETS))"
-else
-
-ifeq ($(BR2_PACKAGE_QEMU_SYSTEM),y)
-QEMU_OPTS += --enable-system
-else
-QEMU_OPTS += --disable-system
-endif
-
-ifeq ($(BR2_PACKAGE_QEMU_LINUX_USER),y)
-QEMU_OPTS += --enable-linux-user
-else
-QEMU_OPTS += --disable-linux-user
-endif
-
-endif
-
-ifeq ($(BR2_PACKAGE_QEMU_SDL),y)
-QEMU_OPTS += --enable-sdl
-QEMU_DEPENDENCIES += sdl
-QEMU_VARS += SDL_CONFIG=$(BR2_STAGING_DIR)/usr/bin/sdl-config
-else
-QEMU_OPTS += --disable-sdl
-endif
-
-ifeq ($(BR2_PACKAGE_QEMU_FDT),y)
-QEMU_OPTS += --enable-fdt
-QEMU_DEPENDENCIES += dtc
-else
-QEMU_OPTS += --disable-fdt
-endif
-
-ifeq ($(BR2_PACKAGE_QEMU_TOOLS),y)
-QEMU_OPTS += --enable-tools
-else
-QEMU_OPTS += --disable-tools
-endif
-
-ifeq ($(BR2_PACKAGE_LIBSSH2),y)
-QEMU_OPTS += --enable-libssh2
-QEMU_DEPENDENCIES += libssh2
-else
-QEMU_OPTS += --disable-libssh2
-endif
-
-# Override CPP, as it expects to be able to call it like it'd
-# call the compiler.
-define QEMU_CONFIGURE_CMDS
-	( cd $(@D); \
-		LIBS='$(QEMU_LIBS)' \
-		$(TARGET_CONFIGURE_OPTS) \
-		$(TARGET_CONFIGURE_ARGS) \
-		CPP="$(TARGET_CC) -E" \
-		$(QEMU_VARS) \
-		./configure \
-			--prefix=/usr \
-			--cross-prefix=$(TARGET_CROSS) \
-			--with-system-pixman \
-			--audio-drv-list= \
-			--enable-kvm \
-			--enable-attr \
-			--enable-vhost-net \
-			--disable-bsd-user \
-			--disable-xen \
-			--disable-slirp \
-			--disable-vnc \
-			--disable-virtfs \
-			--disable-brlapi \
-			--disable-curses \
-			--disable-curl \
-			--disable-bluez \
-			--disable-vde \
-			--disable-linux-aio \
-			--disable-cap-ng \
-			--disable-docs \
-			--disable-spice \
-			--disable-rbd \
-			--disable-libiscsi \
-			--disable-usb-redir \
-			--disable-strip \
-			--disable-seccomp \
-			--disable-sparse \
-			$(QEMU_OPTS) \
-	)
-endef
-
-define QEMU_BUILD_CMDS
-	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)
-endef
-
-define QEMU_INSTALL_TARGET_CMDS
-	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) $(QEMU_MAKE_ENV) DESTDIR=$(TARGET_DIR) install
-endef
-
-$(eval $(generic-package))
Signature · 1 line
2.14.1
author
#3 lore raw

PATCH 3/3 package/pkg-generic: add check that target variant is defined before host variant

In reply to Yann E. MORIN

From: Thomas Petazzoni <[email protected]>

Update the documentation accordingly.

Signed-off-by Thomas Petazzoni <[email protected]>
[yann.morin.1998 at free.fr: slight rephrasing in error message, update manual]
Signed-off-by "Yann E. MORIN" <[email protected]>
---
 docs/manual/adding-packages-generic.txt | 6 ++++--
 package/pkg-generic.mk                  | 6 ++++++
 2 files changed, 10 insertions(+), 2 deletions(-)
docs/manual/adding-packages-generic.txt +4 -2
index 62906d92bb..7e1f246752 100644
@@ -179,8 +179,10 @@ some tools to be installed on the host. If the package name is
 variables of other packages, if they depend on +libfoo+ or
 +host-libfoo+.
 
-The call to the +generic-package+ and/or +host-generic-package+ macro *must* be
-at the end of the +.mk+ file, after all variable definitions.
+The call to the +generic-package+ and/or +host-generic-package+ macro
+*must* be at the end of the +.mk+ file, after all variable definitions.
+The call to +host-generic-package+ *must* be after the call to
++generic-package+, if any.
 
 For the target package, the +generic-package+ uses the variables defined by
 the .mk file and prefixed by the uppercased package name:
package/pkg-generic.mk +6 -0
index 1c9dd1d734..bd47ca1964 100644
@@ -646,6 +646,12 @@ $(2)_POST_LEGAL_INFO_HOOKS      ?=
 $(2)_TARGET_FINALIZE_HOOKS      ?=
 $(2)_ROOTFS_PRE_CMD_HOOKS       ?=
 
+ifeq ($$($(2)_TYPE),target)
+ifneq ($$(HOST_$(2)_KCONFIG_VAR),)
+$$(error "Package $(1) defines host variant before target variant!")
+endif
+endif
+
 # human-friendly targets and target sequencing
 $(1):			$(1)-install
Signature · 1 line
2.14.1
#4 lore raw

cover core/pkg-infra: declare target variant before host variant

In reply to Yann E. MORIN

Hello,

Quoted 4 lines · Yann E. MORIN

Thomas Petazzoni (3):
package: declare target variant before host variant
package/qemu: declare target variant before host variant package/pkg-generic: add check that target variant is defined before host variant

Series applied. Thanks for having pushed this stuff all the way to the point where it got merged!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
author
#5 lore raw

cover core/pkg-infra: declare target variant before host variant

Thomas, All,

On 2018-04-30 17:44 +0200, Thomas Petazzoni spake thusly:

Quoted 8 lines · Yann E. MORIN
Quoted 4 lines

Thomas Petazzoni (3):
package: declare target variant before host variant
package/qemu: declare target variant before host variant package/pkg-generic: add check that target variant is defined before host variant

Series applied. Thanks for having pushed this stuff all the way to the point where it got merged!

And it's only the beginning of it. Muahaha! ;-)
Search for 'yem/oot*' there: https://git.buildroot.org/~ymorin/git/buildroot/refs/heads

Thanks for the initial work, though ;-)

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'