infra: fix dollar signs; remove some undefined versions
PATCH v4 1/1 infra: fix dollar signs; remove some undefined versions
This patch series fixes the double sign problem in the inner-xxx-targets (see description in the patch itself), and makes the following semi-related changes:
- convert toolchain and toolchain-buildroot package to virtual-package
- change makedevs and mkpasswd to have a 'buildroot' version
The changes are related in the sense that the double dollar signs were needed to correctly implement the subsequent patches.
I think that the series is ready to be merged now, with Reviews/Acks on all patches.
As follow-up I plan to do something about the toolchain-external version which is still 'undefined', and add some extra comments in pkg-generic.mk.
---
v4: rebase, remove change to toolchain-external from this series
v3: rebase and implement comments on first patch
v2: increase consistency of rules (first two patches)
PATCH v4 1/1 infra: consistently use double dollar signs inside inner-xxx-targets
The inner-xxx-targets in the buildroot package infrastructures are evaluated using $(eval) which causes variable references to be a bit different than in regular make code. As we want most references to be expanded only at the time of the $(eval) we should not use standard references $(VAR) but rather use double dollar signs $$(VAR). This includes function references like $(call), $(subst), etc. The only exception is the reference to pkgdir/pkgname and numbered variables, which are parameters to the inner block: $(1), $(2), etc.
This patch introduces consistent usage of double-dollar signs throughout the different inner-xxx-targets blocks.
In some cases, this would potentially cause circular references, in particular when the value of HOST_FOO_VAR would be obtained from the corresponding FOO_VAR if HOST_FOO_VAR is not defined. In these cases, a test is added to check for a host package (the only case where such constructions are relevant; these are not circular).
Benefits of these changes are:
- behavior of variables is now again as expected. For example, setting $(2)_VERSION = virtual in pkg-virtual.mk will effectively work, while originally it would cause very odd results.
- The output of 'make printvars' is now much more useful. This target shows the value of all variables, and the expression that led to that value. However, if the expression was coming from an inner-xxx-targets block, and was using single dollar signs, it would show in printvars as VAR = value (value)
while if double dollar signs are used, it would effectively look like
VAR = value (actual expression)
as is intended.
This improvement is for example effective for FOO_DL_VERSION, FOO_RAWNAME, FOO_SITE_METHOD and FOO_MAKE.
The correctness of this patch has been verified using 'make printvars', 'make manual' and 'make legal-info' before and after applying this patch, and comparing the output.
---
v4: rebase
v3: (rebase + implement comments from Arnout)
- add comment to manual.mk and fs/common.mk
- replace the $(origin) stuff with if ($(4),host) instead
- revert changes to outer targets
- remove comment in legal info ('double dollar signs are really needed here because...')
- revert changes to caseconvert-helper
v2:
- extend change to manual, fs/common and caseconvert-helper (Yann)
- increase consistency by also changing $(Q) in $$(Q) and the UPPERCASE calls in the xxx-targets (outer) block. (Yann)
docs/manual/manual.mk | 21 +++++---- fs/common.mk | 18 ++++--- package/pkg-autotools.mk | 30 ++++++++------ package/pkg-cmake.mk | 34 ++++++++------- package/pkg-generic.mk | 89 ++++++++++++++++++++--------------------- package/pkg-luarocks.mk | 10 ++-- package/pkg-perl.mk | 44 ++++++++++---------- package/pkg-python.mk | 30 +++++++------ package/pkg-virtual.mk | 12 +++-- 9 files changed, 151 insertions(+), 137 deletions(-)
Note: the 'make -qp' suggestion by Arnout produced a lot of output due to the fact that the variables are now no longer expanded early. For printvars I could easily work around this by patching it to only print the value, not the rule, and then comparing. For 'make -qp' it is not possible. I did not take on the task of comparing these files of 500k lines, but am trusting the output of printvars along with the comparison of legal-info and manual output.
docs/manual/manual.mk +12 -9
@@ -33,8 +33,8 @@ fi ################################################################################ -# GENDOC -- generates the make targets needed to build a specific type of -# asciidoc documentation. +# GENDOC_INNER -- generates the make targets needed to build a specific type of +# asciidoc documentation. # # argument 1 is the name of the document and must be a subdirectory of docs/; # the top-level asciidoc file must have the same name
@@ -45,6 +45,9 @@ # argument 6 (optional) are extra arguments for a2x # # The variable <DOCUMENT_NAME>_SOURCES defines the dependencies. +# +# Since this function will be called from within an $(eval ...) +# all variable references except the arguments must be $$-quoted. ################################################################################ define GENDOC_INNER $(1): $(1)-$(3)
@@ -54,16 +57,16 @@ manual-check-dependencies-$(3): $$(O)/docs/$(1)/$(1).$(4): docs/$(1)/$(1).txt \ - $$($(call UPPERCASE,$(1))_SOURCES) \ + $$($$(call UPPERCASE,$(1))_SOURCES) \ manual-check-dependencies \ manual-check-dependencies-$(3) \ manual-update-lists - $(Q)$(call MESSAGE,"Generating $(5) $(1)...") - $(Q)mkdir -p $$(@D)/.build - $(Q)rsync -au docs/$(1)/*.txt $$(@D)/.build - $(Q)a2x $(6) -f $(2) -d book -L -r $(TOPDIR)/docs/images \ + $$(Q)$$(call MESSAGE,"Generating $(5) $(1)...") + $$(Q)mkdir -p $$(@D)/.build + $$(Q)rsync -au docs/$(1)/*.txt $$(@D)/.build + $$(Q)a2x $(6) -f $(2) -d book -L -r $$(TOPDIR)/docs/images \ -D $$(@D) $$(@D)/.build/$(1).txt - -$(Q)rm -rf $$(@D)/.build + -$$(Q)rm -rf $$(@D)/.build endef ################################################################################
@@ -82,7 +85,7 @@ $(call GENDOC_INNER,$(1),epub,epub,epub,ePUB) clean: $(1)-clean $(1)-clean: - $(Q)$(RM) -rf $(O)/docs/$(1) + $$(Q)$$(RM) -rf $$(O)/docs/$(1) .PHONY: $(1) $(1)-clean manual-update-lists endef
fs/common.mk +10 -8
@@ -35,11 +35,13 @@ USERS_TABLE = $(BUILD_DIR)/_users_table.txt ROOTFS_USERS_TABLES = $(call qstrip,$(BR2_ROOTFS_USERS_TABLES)) +# Since this function will be called from within an $(eval ...) +# all variable references except the arguments must be $$-quoted. define ROOTFS_TARGET_INTERNAL # extra deps ROOTFS_$(2)_DEPENDENCIES += host-fakeroot host-makedevs \ - $(if $(PACKAGES_USERS),host-mkpasswd) + $$(if $$(PACKAGES_USERS),host-mkpasswd) ifeq ($$(BR2_TARGET_ROOTFS_$(2)_GZIP),y) ROOTFS_$(2)_COMPRESS_EXT = .gz
@@ -69,7 +71,7 @@ $$(foreach hook,$$(ROOTFS_$(2)_PRE_GEN_HOOKS),$$(call $$(hook))$$(sep)) rm -f $$(FAKEROOT_SCRIPT) rm -f $$(TARGET_DIR_WARNING_FILE) - rm -f $(USERS_TABLE) + rm -f $$(USERS_TABLE) echo "chown -h -R 0:0 $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT) ifneq ($$(ROOTFS_DEVICE_TABLES),) cat $$(ROOTFS_DEVICE_TABLES) > $$(FULL_DEVICE_TABLE)
@@ -80,17 +82,17 @@ echo "$$(HOST_DIR)/usr/bin/makedevs -d $$(FULL_DEVICE_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT) endif ifneq ($$(ROOTFS_USERS_TABLES),) - cat $$(ROOTFS_USERS_TABLES) >> $(USERS_TABLE) + cat $$(ROOTFS_USERS_TABLES) >> $$(USERS_TABLE) endif - printf '$(subst $(sep),\n,$(PACKAGES_USERS))' >> $(USERS_TABLE) - PATH=$(BR_PATH) $(TOPDIR)/support/scripts/mkusers $(USERS_TABLE) $(TARGET_DIR) >> $(FAKEROOT_SCRIPT) + printf '$$(subst $$(sep),\n,$$(PACKAGES_USERS))' >> $$(USERS_TABLE) + PATH=$$(BR_PATH) $$(TOPDIR)/support/scripts/mkusers $$(USERS_TABLE) $$(TARGET_DIR) >> $$(FAKEROOT_SCRIPT) echo "$$(ROOTFS_$(2)_CMD)" >> $$(FAKEROOT_SCRIPT) chmod a+x $$(FAKEROOT_SCRIPT) - PATH=$(BR_PATH) $$(HOST_DIR)/usr/bin/fakeroot -- $$(FAKEROOT_SCRIPT) - $(INSTALL) -m 0644 support/misc/target-dir-warning.txt $$(TARGET_DIR_WARNING_FILE) + PATH=$$(BR_PATH) $$(HOST_DIR)/usr/bin/fakeroot -- $$(FAKEROOT_SCRIPT) + $$(INSTALL) -m 0644 support/misc/target-dir-warning.txt $$(TARGET_DIR_WARNING_FILE) - at rm -f $$(FAKEROOT_SCRIPT) $$(FULL_DEVICE_TABLE) ifneq ($$(ROOTFS_$(2)_COMPRESS_CMD),) - PATH=$(BR_PATH) $$(ROOTFS_$(2)_COMPRESS_CMD) $$@ > $$@$$(ROOTFS_$(2)_COMPRESS_EXT) + PATH=$$(BR_PATH) $$(ROOTFS_$(2)_COMPRESS_CMD) $$@ > $$@$$(ROOTFS_$(2)_COMPRESS_EXT) endif rootfs-$(1)-show-depends:
package/pkg-autotools.mk +17 -13
@@ -65,7 +65,7 @@ ifndef $(2)_LIBTOOL_PATCH ifdef $(3)_LIBTOOL_PATCH - $(2)_LIBTOOL_PATCH = $($(3)_LIBTOOL_PATCH) + $(2)_LIBTOOL_PATCH = $$($(3)_LIBTOOL_PATCH) else $(2)_LIBTOOL_PATCH ?= YES endif
@@ -73,25 +73,28 @@ ifndef $(2)_MAKE ifdef $(3)_MAKE - $(2)_MAKE = $($(3)_MAKE) + $(2)_MAKE = $$($(3)_MAKE) else - $(2)_MAKE ?= $(MAKE) + $(2)_MAKE ?= $$(MAKE) endif endif ifndef $(2)_AUTORECONF ifdef $(3)_AUTORECONF - $(2)_AUTORECONF = $($(3)_AUTORECONF) + $(2)_AUTORECONF = $$($(3)_AUTORECONF) else $(2)_AUTORECONF ?= NO endif endif +ifeq ($(4),host) + $(2)_AUTORECONF_OPT ?= $$($(3)_AUTORECONF_OPT) +endif + $(2)_CONF_ENV ?= $(2)_CONF_OPT ?= $(2)_MAKE_ENV ?= $(2)_MAKE_OPT ?= -$(2)_AUTORECONF_OPT ?= $($(3)_AUTORECONF_OPT) $(2)_INSTALL_OPT ?= install $(2)_INSTALL_STAGING_OPT ?= DESTDIR=$$(STAGING_DIR) install $(2)_INSTALL_TARGET_OPT ?= DESTDIR=$$(TARGET_DIR) install
@@ -175,7 +178,7 @@ # define LIBTOOL_PATCH_HOOK @$$(call MESSAGE,"Patching libtool") - $(Q)if test "$$($$(PKG)_LIBTOOL_PATCH)" = "YES" \ + $$(Q)if test "$$($$(PKG)_LIBTOOL_PATCH)" = "YES" \ -a "$$($$(PKG)_AUTORECONF)" != "YES"; then \ for i in `find $$($$(PKG)_SRCDIR) -name ltmain.sh`; do \ ltmain_version=`sed -n '/^[ ]*VERSION=/{s/^[ ]*VERSION=//;p;q;}' $$$$i | \
@@ -201,8 +204,8 @@ # define AUTORECONF_HOOK @$$(call MESSAGE,"Autoreconfiguring") - $(Q)cd $$($$(PKG)_SRCDIR) && $(AUTORECONF) $$($$(PKG)_AUTORECONF_OPT) - $(Q)if test "$$($$(PKG)_LIBTOOL_PATCH)" = "YES"; then \ + $$(Q)cd $$($$(PKG)_SRCDIR) && $$(AUTORECONF) $$($$(PKG)_AUTORECONF_OPT) + $$(Q)if test "$$($$(PKG)_LIBTOOL_PATCH)" = "YES"; then \ for i in `find $$($$(PKG)_SRCDIR) -name ltmain.sh`; do \ ltmain_version=`sed -n '/^[ ]*VERSION=/{s/^[ ]*VERSION=//;p;q;}' $$$$i | \ sed -e 's/\([0-9].[0-9]*\).*/\1/' -e 's/\"//'`; \
@@ -220,10 +223,11 @@ # This must be repeated from inner-generic-package, otherwise we get an empty # _DEPENDENCIES if _AUTORECONF is YES. Also filter the result of _AUTORECONF # away from the non-host rule -$(2)_DEPENDENCIES ?= $(filter-out host-automake host-autoconf host-libtool \ +ifeq ($(4),host) +$(2)_DEPENDENCIES ?= $$(filter-out host-automake host-autoconf host-libtool \ host-toolchain $(1),\ - $(patsubst host-host-%,host-%,$(addprefix host-,$($(3)_DEPENDENCIES)))) - + $$(patsubst host-host-%,host-%,$$(addprefix host-,$$($(3)_DEPENDENCIES)))) +endif ifeq ($$($(2)_AUTORECONF),YES) $(2)_PRE_CONFIGURE_HOOKS += AUTORECONF_HOOK
@@ -263,9 +267,9 @@ ifndef $(2)_INSTALL_STAGING_CMDS define $(2)_INSTALL_STAGING_CMDS $$(TARGET_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_INSTALL_STAGING_OPT) -C $$($$(PKG)_SRCDIR) - for i in $$$$(find $(STAGING_DIR)/usr/lib* -name "*.la"); do \ + for i in $$$$(find $$(STAGING_DIR)/usr/lib* -name "*.la"); do \ cp -f $$$$i $$$$i~; \ - $$(SED) "s:\(['= ]\)/usr:\\1$(STAGING_DIR)/usr:g" $$$$i; \ + $$(SED) "s:\(['= ]\)/usr:\\1$$(STAGING_DIR)/usr:g" $$$$i; \ done endef endif
package/pkg-cmake.mk +18 -16
@@ -38,14 +38,14 @@ $(2)_CONF_ENV ?= $(2)_CONF_OPT ?= -$(2)_MAKE ?= $(MAKE) +$(2)_MAKE ?= $$(MAKE) $(2)_MAKE_ENV ?= $(2)_MAKE_OPT ?= $(2)_INSTALL_HOST_OPT ?= install $(2)_INSTALL_STAGING_OPT ?= DESTDIR=$$(STAGING_DIR) install $(2)_INSTALL_TARGET_OPT ?= DESTDIR=$$(TARGET_DIR) install -$(2)_SRCDIR = $$($(2)_DIR)/$($(2)_SUBDIR) +$(2)_SRCDIR = $$($(2)_DIR)/$$($(2)_SUBDIR) $(2)_BUILDDIR = $$($(2)_SRCDIR) #
@@ -60,15 +60,15 @@ define $(2)_CONFIGURE_CMDS (cd $$($$(PKG)_BUILDDIR) && \ rm -f CMakeCache.txt && \ - PATH=$(BR_PATH) \ - $$($$(PKG)_CONF_ENV) $(HOST_DIR)/usr/bin/cmake $$($$(PKG)_SRCDIR) \ + PATH=$$(BR_PATH) \ + $$($$(PKG)_CONF_ENV) $$(HOST_DIR)/usr/bin/cmake $$($$(PKG)_SRCDIR) \ -DCMAKE_TOOLCHAIN_FILE="$$(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake" \ - -DCMAKE_BUILD_TYPE=$(if $(BR2_ENABLE_DEBUG),Debug,Release) \ + -DCMAKE_BUILD_TYPE=$$(if $$(BR2_ENABLE_DEBUG),Debug,Release) \ -DCMAKE_INSTALL_PREFIX="/usr" \ -DCMAKE_COLOR_MAKEFILE=OFF \ -DBUILD_TESTING=OFF \ - -DBUILD_SHARED_LIBS=$(if $(BR2_PREFER_STATIC_LIB),OFF,ON) \ - -DUSE_CCACHE=$(if $(BR2_CCACHE),ON,OFF) \ + -DBUILD_SHARED_LIBS=$$(if $$(BR2_PREFER_STATIC_LIB),OFF,ON) \ + -DUSE_CCACHE=$$(if $$(BR2_CCACHE),ON,OFF) \ $$($$(PKG)_CONF_OPT) \ ) endef
@@ -78,15 +78,15 @@ define $(2)_CONFIGURE_CMDS (cd $$($$(PKG)_BUILDDIR) && \ rm -f CMakeCache.txt && \ - PATH=$(BR_PATH) \ - $(HOST_DIR)/usr/bin/cmake $$($$(PKG)_SRCDIR) \ + PATH=$$(BR_PATH) \ + $$(HOST_DIR)/usr/bin/cmake $$($$(PKG)_SRCDIR) \ -DCMAKE_INSTALL_SO_NO_EXE=0 \ -DCMAKE_FIND_ROOT_PATH="$$(HOST_DIR)" \ -DCMAKE_FIND_ROOT_PATH_MODE_PROGRAM="BOTH" \ -DCMAKE_FIND_ROOT_PATH_MODE_LIBRARY="BOTH" \ -DCMAKE_FIND_ROOT_PATH_MODE_INCLUDE="BOTH" \ -DCMAKE_INSTALL_PREFIX="$$(HOST_DIR)/usr" \ - -DUSE_CCACHE=$(if $(BR2_CCACHE),ON,OFF) \ + -DUSE_CCACHE=$$(if $$(BR2_CCACHE),ON,OFF) \ -DBUILD_TESTING=OFF \ $$($$(PKG)_CONF_OPT) \ )
@@ -96,7 +96,9 @@ # This must be repeated from inner-generic-package, otherwise we only get # host-cmake in _DEPENDENCIES because of the following line -$(2)_DEPENDENCIES ?= $(filter-out host-toolchain $(1),$(patsubst host-host-%,host-%,$(addprefix host-,$($(3)_DEPENDENCIES)))) +ifeq ($(4),host) +$(2)_DEPENDENCIES ?= $$(filter-out host-toolchain $(1),$$(patsubst host-host-%,host-%,$$(addprefix host-,$$($(3)_DEPENDENCIES)))) +endif $(2)_DEPENDENCIES += host-cmake
@@ -107,11 +109,11 @@ ifndef $(2)_BUILD_CMDS ifeq ($(4),target) define $(2)_BUILD_CMDS - $(TARGET_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPT) -C $$($$(PKG)_BUILDDIR) + $$(TARGET_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPT) -C $$($$(PKG)_BUILDDIR) endef else define $(2)_BUILD_CMDS - $(HOST_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPT) -C $$($$(PKG)_BUILDDIR) + $$(HOST_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPT) -C $$($$(PKG)_BUILDDIR) endef endif endif
@@ -122,7 +124,7 @@ # ifndef $(2)_INSTALL_CMDS define $(2)_INSTALL_CMDS - $(HOST_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPT) $$($$(PKG)_INSTALL_HOST_OPT) -C $$($$(PKG)_BUILDDIR) + $$(HOST_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPT) $$($$(PKG)_INSTALL_HOST_OPT) -C $$($$(PKG)_BUILDDIR) endef endif
@@ -132,7 +134,7 @@ # ifndef $(2)_INSTALL_STAGING_CMDS define $(2)_INSTALL_STAGING_CMDS - $(TARGET_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPT) $$($$(PKG)_INSTALL_STAGING_OPT) -C $$($$(PKG)_BUILDDIR) + $$(TARGET_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPT) $$($$(PKG)_INSTALL_STAGING_OPT) -C $$($$(PKG)_BUILDDIR) endef endif
@@ -142,7 +144,7 @@ # ifndef $(2)_INSTALL_TARGET_CMDS define $(2)_INSTALL_TARGET_CMDS - $(TARGET_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPT) $$($$(PKG)_INSTALL_TARGET_OPT) -C $$($$(PKG)_BUILDDIR) + $$(TARGET_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPT) $$($$(PKG)_INSTALL_TARGET_OPT) -C $$($$(PKG)_BUILDDIR) endef endif
package/pkg-generic.mk +44 -45
220 lines not on this page · raw message
package/pkg-luarocks.mk +5 -5
@@ -34,10 +34,10 @@ define inner-luarocks-package $(2)_BUILD_OPT ?= -$(2)_SUBDIR ?= $(1)-$(shell echo "$($(3)_VERSION)" | sed -e "s/-[0-9]$$//") -$(2)_ROCKSPEC ?= $(1)-$($(3)_VERSION).rockspec -$(2)_SOURCE ?= $(1)-$($(3)_VERSION).src.rock -$(2)_SITE ?= $(call qstrip,$(BR2_LUAROCKS_MIRROR)) +$(2)_SUBDIR ?= $(1)-$$(shell echo "$$($(3)_VERSION)" | sed -e "s/-[0-9]$$$$//") +$(2)_ROCKSPEC ?= $(1)-$$($(3)_VERSION).rockspec +$(2)_SOURCE ?= $(1)-$$($(3)_VERSION).src.rock +$(2)_SITE ?= $$(call qstrip,$$(BR2_LUAROCKS_MIRROR)) # Since we do not support host-luarocks-package, we know this is # a target package, and can just add the required dependencies
@@ -49,7 +49,7 @@ ifndef $(2)_EXTRACT_CMDS define $(2)_EXTRACT_CMDS cd $$($(2)_DIR)/.. && \ - $$(LUAROCKS_RUN) unpack --force $(DL_DIR)/$$($(2)_SOURCE) + $$(LUAROCKS_RUN) unpack --force $$(DL_DIR)/$$($(2)_SOURCE) endef endif
package/pkg-perl.mk +22 -22
86 lines not on this page · raw message
package/pkg-python.mk +16 -14
@@ -82,7 +82,7 @@ define inner-python-package -$(2)_SRCDIR = $$($(2)_DIR)/$($(2)_SUBDIR) +$(2)_SRCDIR = $$($(2)_DIR)/$$($(2)_SUBDIR) $(2)_BUILDDIR = $$($(2)_SRCDIR) $(2)_ENV ?=
@@ -91,7 +91,7 @@ ifndef $(2)_SETUP_TYPE ifdef $(3)_SETUP_TYPE - $(2)_SETUP_TYPE = $($(3)_SETUP_TYPE) + $(2)_SETUP_TYPE = $$($(3)_SETUP_TYPE) else $$(error "$(2)_SETUP_TYPE must be set") endif
@@ -138,7 +138,9 @@ # depending on the package characteristics, and shouldn't be derived # automatically from the dependencies of the corresponding target # package. -$(2)_DEPENDENCIES ?= $(filter-out host-python host-python3 host-python-setuptools host-toolchain $(1),$(patsubst host-host-%,host-%,$(addprefix host-,$($(3)_DEPENDENCIES)))) +ifeq ($(4),host) +$(2)_DEPENDENCIES ?= $$(filter-out host-python host-python3 host-python-setuptools host-toolchain $(1),$$(patsubst host-host-%,host-%,$$(addprefix host-,$$($(3)_DEPENDENCIES)))) +endif # Target packages need both the python interpreter on the target (for # runtime) and the python interpreter on the host (for
@@ -155,19 +157,19 @@ # - otherwise, we depend on the one requested by *_NEEDS_HOST_PYTHON. # ifeq ($(4),target) -$(2)_DEPENDENCIES += $(if $(BR2_PACKAGE_PYTHON3),host-python3 python3,host-python python) +$(2)_DEPENDENCIES += $$(if $$(BR2_PACKAGE_PYTHON3),host-python3 python3,host-python python) else -ifeq ($($(2)_NEEDS_HOST_PYTHON),) -$(2)_DEPENDENCIES += $(if $(BR2_PACKAGE_PYTHON3),host-python3,host-python) +ifeq ($$($(2)_NEEDS_HOST_PYTHON),) +$(2)_DEPENDENCIES += $$(if $$(BR2_PACKAGE_PYTHON3),host-python3,host-python) else -ifeq ($($(2)_NEEDS_HOST_PYTHON),python2) +ifeq ($$($(2)_NEEDS_HOST_PYTHON),python2) $(2)_DEPENDENCIES += host-python -else ifeq ($($(2)_NEEDS_HOST_PYTHON),python3) +else ifeq ($$($(2)_NEEDS_HOST_PYTHON),python3) $(2)_DEPENDENCIES += host-python3 else -$$(error Incorrect value '$($(2)_NEEDS_HOST_PYTHON)' for $(2)_NEEDS_HOST_PYTHON) +$$(error Incorrect value '$$($(2)_NEEDS_HOST_PYTHON)' for $(2)_NEEDS_HOST_PYTHON) endif -endif # ($($(2)_NEEDS_HOST_PYTHON),) +endif # ($$($(2)_NEEDS_HOST_PYTHON),) endif # ($(4),target) # Setuptools based packages will need host-python-setuptools (both
@@ -196,12 +198,12 @@ # - otherwise, we use the one requested by *_NEEDS_HOST_PYTHON. # ifeq ($(4),target) -$(2)_PYTHON_INTERPRETER = $(HOST_DIR)/usr/bin/python +$(2)_PYTHON_INTERPRETER = $$(HOST_DIR)/usr/bin/python else -ifeq ($($(2)_NEEDS_HOST_PYTHON),) -$(2)_PYTHON_INTERPRETER = $(HOST_DIR)/usr/bin/python +ifeq ($$($(2)_NEEDS_HOST_PYTHON),) +$(2)_PYTHON_INTERPRETER = $$(HOST_DIR)/usr/bin/python else -$(2)_PYTHON_INTERPRETER = $(HOST_DIR)/usr/bin/$($(2)_NEEDS_HOST_PYTHON) +$(2)_PYTHON_INTERPRETER = $$(HOST_DIR)/usr/bin/$$($(2)_NEEDS_HOST_PYTHON) endif endif
package/pkg-virtual.mk +7 -5
@@ -35,8 +35,8 @@ define inner-virtual-package # Ensure the virtual package has an implementation defined. -ifeq ($(BR2_PACKAGE_HAS_$(2)),y) -ifeq ($(call qstrip,$(BR2_PACKAGE_PROVIDES_$(2))),) +ifeq ($$(BR2_PACKAGE_HAS_$(2)),y) +ifeq ($$(call qstrip,$$(BR2_PACKAGE_PROVIDES_$(2))),) $$(error No implementation selected for virtual package $(1). Configuration error) endif endif
@@ -50,11 +50,13 @@ # This must be repeated from inner-generic-package, otherwise we get an empty # _DEPENDENCIES -$(2)_DEPENDENCIES ?= $(filter-out host-toolchain $(1),\ - $(patsubst host-host-%,host-%,$(addprefix host-,$($(3)_DEPENDENCIES)))) +ifeq ($(4),host) +$(2)_DEPENDENCIES ?= $$(filter-out host-toolchain $(1),\ + $$(patsubst host-host-%,host-%,$$(addprefix host-,$$($(3)_DEPENDENCIES)))) +endif # Add dependency against the provider -$(2)_DEPENDENCIES += $(call qstrip,$(BR2_PACKAGE_PROVIDES_$(2))) +$(2)_DEPENDENCIES += $$(call qstrip,$$(BR2_PACKAGE_PROVIDES_$(2))) # Call the generic package infrastructure to generate the necessary # make targets
PATCH v4 1/1 infra: add comment describing single/double dollar-sign rules
In reply to Thomas De SchampheleireAs the rules with respect to variable and function references and the need for single or double dollar signs are not trivial, add a comment in pkg-generic.mk describing them.
---
v4: no changes
v3: expand exception for pkgdir and pkgname
v2: clarify exception for pkgdir and pkgname
package/pkg-generic.mk | 23 +++++++++++++++++++++++ 1 files changed, 23 insertions(+), 0 deletions(-)
package/pkg-generic.mk +23 -0
@@ -269,6 +269,29 @@ # argument 3 is the uppercase package name, without the HOST_ prefix # for host packages # argument 4 is the type (target or host) +# +# Note about variable and function references: inside all blocks that are +# evaluated with $(eval), which includes all 'inner-xxx-package' blocks, +# specific rules apply with respect to variable and function references. +# - Numbered variables (parameters to the block) can be referenced with a single +# dollar sign: $(1), $(2), $(3), etc. +# - pkgdir and pkgname should be referenced with a single dollar sign too. These +# functions rely on 'the most recently parsed makefile' which is supposed to +# be the package .mk file. If we defer the evaluation of these functions using +# double dollar signs, then they may be evaluated too late, when other +# makefiles have already been parsed. One specific case is when $$(pkgdir) is +# assigned to a variable using deferred evaluation with '=' and this variable +# is used in a target rule outside the eval'ed inner block. In this case, the +# pkgdir will be that of the last makefile parsed by buildroot, which is not +# the expected value. This mechanism is for example used for the TARGET_PATCH +# rule. +# - All other variables should be referenced with a double dollar sign: +# $$(TARGET_DIR), $$($(2)_VERSION), etc. Also all make functions should be +# referenced with a double dollar sign: $$(subst), $$(call), $$(filter-out), +# etc. This rule ensures that these variables and functions are only expanded +# during the $(eval) step, and not earlier. Otherwise, unintuitive and +# undesired behavior occurs with respect to these variables and functions. +# ################################################################################ define inner-generic-package
PATCH v4 1/1 pkg-virtual: simplify definition of FOO_VERSION to 'virtual'
In reply to Thomas De SchampheleireAs mentioned in the e-mail accompanying the introduction of the pkg-virtual infrastructure [1], the definition of FOO_VERSION is 'strange'.
After the cleanup of single/double dollar signs in inner-generic-package, the special construction in pkg-virtual is no longer needed and can be simplified.
[1] http://lists.busybox.net/pipermail/buildroot/2014-April/093670.html
---
v4: no changes
v3: no changes
v2: no changes
Note: this patch should only be applied when the first patch in this series is.
package/pkg-virtual.mk | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
package/pkg-virtual.mk +1 -2
@@ -45,8 +45,7 @@ $(2)_SOURCE = # Fake a version string, so it looks nicer in the build log -$(3)_VERSION = virtual -HOST_$(3)_VERSION = virtual +$(2)_VERSION = virtual # This must be repeated from inner-generic-package, otherwise we get an empty # _DEPENDENCIES
PATCH v4 1/1 toolchain/toolchain-buildroot: migrate to virtual package infrastructure
In reply to Thomas De SchampheleireThis patch migrates the toolchain and toolchain-buildroot packages to the virtual package infrastructure, causing the log messages to change from:
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
toolchain undefined Downloading
toolchain undefined Extracting
...
to
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
toolchain virtual Downloading
toolchain virtual Extracting
...
and similar for 'toolchain-buildroot', simply because it looks nicer.
At the same time, the directory names also become toolchain-virtual, toolchain-buildroot-virtual instead of the corresponding 'undefined' variants.
---
v4: no changes
v3: no changes
v2: no changes
Note: this patch should only be applied when the first patch in this series is.
toolchain/toolchain-buildroot/toolchain-buildroot.mk | 4 +--- toolchain/toolchain/toolchain.mk | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-)
toolchain/toolchain-buildroot/toolchain-buildroot.mk +1 -3
@@ -4,8 +4,6 @@ # ################################################################################ -TOOLCHAIN_BUILDROOT_SOURCE = - BR_LIBC = $(call qstrip,$(BR2_TOOLCHAIN_BUILDROOT_LIBC)) # Triggering the build of the host-gcc-final will automatically do the
@@ -16,4 +14,4 @@ TOOLCHAIN_BUILDROOT_ADD_TOOLCHAIN_DEPENDENCY = NO -$(eval $(generic-package)) +$(eval $(virtual-package))
toolchain/toolchain/toolchain.mk +1 -3
@@ -4,8 +4,6 @@ # ################################################################################ -TOOLCHAIN_SOURCE = - ifeq ($(BR2_TOOLCHAIN_BUILDROOT),y) TOOLCHAIN_DEPENDENCIES += toolchain-buildroot else ifeq ($(BR2_TOOLCHAIN_EXTERNAL),y)
@@ -14,6 +12,6 @@ TOOLCHAIN_ADD_TOOLCHAIN_DEPENDENCY = NO -$(eval $(generic-package)) +$(eval $(virtual-package)) toolchain: $(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake
PATCH v4 1/1 makedevs: change version from 'undefined' to 'buildroot-$(BR2_VERSION)'
In reply to Thomas De SchampheleireThe sources of the makedevs package are shipped with Buildroot, rather than downloaded from an external location. As a result, no explicit version is defined, causing build messages and build directory to show 'undefined' as version.
This patch sets the version for makedevs to 'buildroot-$(BR2_VERSION), which would for example expand to 'buildroot-2014.05'.
---
v4: no changes
v3: no changes
v2: no changes
package/makedevs/makedevs.mk | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
package/makedevs/makedevs.mk +2 -0
@@ -8,6 +8,8 @@ MAKEDEVS_SOURCE = HOST_MAKEDEVS_SOURCE = +MAKEDEVS_VERSION = buildroot-$(BR2_VERSION) + define MAKEDEVS_BUILD_CMDS $(TARGET_CC) $(TARGET_CFLAGS) $(TARGET_LDFLAGS) \ package/makedevs/makedevs.c -o $(@D)/makedevs
PATCH v4 1/1 mkpasswd: change version from 'undefined' to 'buildroot-$(BR2_VERSION)'
In reply to Thomas De SchampheleireThe sources of the mkpasswd package are shipped with Buildroot, rather than downloaded from an external location. As a result, no explicit version is defined, causing build messages and build directory to show 'undefined' as version.
This patch sets the version for mkpasswd to 'buildroot-$(BR2_VERSION), which would for example expand to 'buildroot-2014.05'.
---
v4: no changes
v3: no changes
v2: no changes
package/mkpasswd/mkpasswd.mk | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
package/mkpasswd/mkpasswd.mk +2 -0
@@ -10,6 +10,8 @@ HOST_MKPASSWD_SOURCE = HOST_MKPASSWD_LICENSE = GPLv2+ +HOST_MKPASSWD_VERSION = buildroot-$(BR2_VERSION) + define HOST_MKPASSWD_BUILD_CMDS $(HOSTCC) $(HOST_CFLAGS) $(HOST_LDFLAGS) \ package/mkpasswd/mkpasswd.c package/mkpasswd/utils.c \
PATCH v4 1/1 infra: fix dollar signs; remove some undefined versions
In reply to Thomas De SchampheleireDear Thomas De Schampheleire,
Quoted 10 lines · Thomas De Schampheleire
This patch series fixes the double sign problem in the inner-xxx-targets (see description in the patch itself), and makes the following semi-related changes:
- convert toolchain and toolchain-buildroot package to virtual-package
- change makedevs and mkpasswd to have a 'buildroot' versionThe changes are related in the sense that the double dollar signs were needed to correctly implement the subsequent patches.
Quoted 7 lines
I think that the series is ready to be merged now, with Reviews/Acks on all patches.
As follow-up I plan to do something about the toolchain-external version which is still 'undefined', and add some extra comments in pkg-generic.mk.
Signed-off-by Thomas De Schampheleire <[email protected]>
Series applied, thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com