[Buildroot] [PATCH 0 of 7 v2] infra: fix dollar signs; remove some undefined versions

Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed

* [Buildroot] [PATCH 0 of 7 v2] infra: fix dollar signs; remove some undefined versions
@ 2014-05-12 14:44 Thomas De Schampheleire
  2014-05-12 14:44 ` [Buildroot] [PATCH 1 of 7 v2] infra: consistently use double dollar signs inside inner-xxx-targets Thomas De Schampheleire
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Thomas De Schampheleire @ 2014-05-12 14:44 UTC (permalink / raw)
  To: buildroot


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 toolchain-external package to have a virtual version
- 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.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

---
v2: increase consistency of rules (first two patches)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Buildroot] [PATCH 1 of 7 v2] infra: consistently use double dollar signs inside inner-xxx-targets
  2014-05-12 14:44 [Buildroot] [PATCH 0 of 7 v2] infra: fix dollar signs; remove some undefined versions Thomas De Schampheleire
@ 2014-05-12 14:44 ` Thomas De Schampheleire
  2014-05-13  9:25   ` Arnout Vandecappelle
  2014-05-12 14:44 ` [Buildroot] [PATCH 2 of 7 v2] infra: add comment describing single/double dollar-sign rules Thomas De Schampheleire
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Thomas De Schampheleire @ 2014-05-12 14:44 UTC (permalink / raw)
  To: buildroot

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, an
immediate assignment using := is necessary instead of a deferred assignment.
using =.
When such a circular assignment occurs in a variable defined with '?=',
a special construction is necessary, as make does not have a corresponding
'?:=' assignment. As 'FOO ?= bar' is equivalent to
  ifeq ($(origin FOO),undefined)
    FOO = bar
  endif
we can replace such constructions with
  ifeq ($$(origin FOO),undefined)
    FOO := bar
  endif


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 can be verified by comparing 'make printvars'
before and after applying this patch.

Insight-provided-by: Arnout Vandecappelle <arnout@mind.be>
Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

---
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    |  14 +++---
 fs/common.mk             |  14 +++---
 package/pkg-autotools.mk |  34 +++++++++-------
 package/pkg-cmake.mk     |  32 ++++++++-------
 package/pkg-generic.mk   |  90 +++++++++++++++++++++---------------------
 package/pkg-luarocks.mk  |  12 ++--
 package/pkg-perl.mk      |  48 +++++++++++-----------
 package/pkg-python.mk    |  34 ++++++++-------
 package/pkg-utils.mk     |   8 +-
 package/pkg-virtual.mk   |  16 ++++---
 10 files changed, 157 insertions(+), 145 deletions(-)

Note: in addition to using 'make printvars' to verify this patch, a 'make
randpackageconfig' was performed successfully.

diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk
--- a/docs/manual/manual.mk
+++ b/docs/manual/manual.mk
@@ -54,16 +54,16 @@ define GENDOC_INNER
 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 +82,7 @@ define GENDOC
 $(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
 
diff --git a/fs/common.mk b/fs/common.mk
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -39,7 +39,7 @@ 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 +69,7 @@ endif
 	$$(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 -R 0:0 $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)
 ifneq ($$(ROOTFS_DEVICE_TABLES),)
 	cat $$(ROOTFS_DEVICE_TABLES) > $$(FULL_DEVICE_TABLE)
@@ -80,14 +80,14 @@ endif
 	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),)
 	$$(ROOTFS_$(2)_COMPRESS_CMD) $$@ > $$@$$(ROOTFS_$(2)_COMPRESS_EXT)
diff --git a/package/pkg-autotools.mk b/package/pkg-autotools.mk
--- a/package/pkg-autotools.mk
+++ b/package/pkg-autotools.mk
@@ -65,7 +65,7 @@ define inner-autotools-package
 
 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 @@ endif
 
 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 ($$(origin $(2)_AUTORECONF_OPT),undefined)
+ $(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 @@ endef
 #
 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 @@ endif
 #
 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 @@ endef
 # 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 ($$(origin $(2)_DEPENDENCIES),undefined)
+$(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 @@ endif
 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
@@ -290,5 +294,5 @@ endef
 # autotools-package -- the target generator macro for autotools packages
 ################################################################################
 
-autotools-package = $(call inner-autotools-package,$(pkgname),$(call UPPERCASE,$(pkgname)),$(call UPPERCASE,$(pkgname)),target)
-host-autotools-package = $(call inner-autotools-package,host-$(pkgname),$(call UPPERCASE,host-$(pkgname)),$(call UPPERCASE,$(pkgname)),host)
+autotools-package = $(call inner-autotools-package,$(pkgname),$$(call UPPERCASE,$(pkgname)),$$(call UPPERCASE,$(pkgname)),target)
+host-autotools-package = $(call inner-autotools-package,host-$(pkgname),$$(call UPPERCASE,host-$(pkgname)),$$(call UPPERCASE,$(pkgname)),host)
diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
--- a/package/pkg-cmake.mk
+++ b/package/pkg-cmake.mk
@@ -38,14 +38,14 @@ define inner-cmake-package
 
 $(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,12 +60,12 @@ ifeq ($(4),target)
 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_INSTALL_PREFIX="/usr" \
 		-DCMAKE_COLOR_MAKEFILE=OFF \
-		-DBUILD_SHARED_LIBS=$(if $(BR2_PREFER_STATIC_LIB),OFF,ON) \
+		-DBUILD_SHARED_LIBS=$$(if $$(BR2_PREFER_STATIC_LIB),OFF,ON) \
 		$$($$(PKG)_CONF_OPT) \
 	)
 endef
@@ -75,8 +75,8 @@ else
 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" \
@@ -91,7 +91,9 @@ endif
 
 # 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 ($$(origin $(2)_DEPENDENCIES),undefined)
+$(2)_DEPENDENCIES := $$(filter-out host-toolchain $(1),$$(patsubst host-host-%,host-%,$$(addprefix host-,$$($(3)_DEPENDENCIES))))
+endif
 
 $(2)_DEPENDENCIES += host-cmake
 
@@ -102,11 +104,11 @@ endif
 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
@@ -117,7 +119,7 @@ endif
 #
 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
 
@@ -127,7 +129,7 @@ endif
 #
 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
 
@@ -137,7 +139,7 @@ endif
 #
 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
 
@@ -151,8 +153,8 @@ endef
 # cmake-package -- the target generator macro for CMake packages
 ################################################################################
 
-cmake-package = $(call inner-cmake-package,$(pkgname),$(call UPPERCASE,$(pkgname)),$(call UPPERCASE,$(pkgname)),target)
-host-cmake-package = $(call inner-cmake-package,host-$(pkgname),$(call UPPERCASE,host-$(pkgname)),$(call UPPERCASE,$(pkgname)),host)
+cmake-package = $(call inner-cmake-package,$(pkgname),$$(call UPPERCASE,$(pkgname)),$$(call UPPERCASE,$(pkgname)),target)
+host-cmake-package = $(call inner-cmake-package,host-$(pkgname),$$(call UPPERCASE,host-$(pkgname)),$$(call UPPERCASE,$(pkgname)),host)
 
 ################################################################################
 # Generation of the CMake toolchain file
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -263,7 +263,7 @@ define inner-generic-package
 
 $(2)_TYPE                       =  $(4)
 $(2)_NAME			=  $(1)
-$(2)_RAWNAME			=  $(patsubst host-%,%,$(1))
+$(2)_RAWNAME			=  $$(patsubst host-%,%,$(1))
 
 # Keep the package version that may contain forward slashes in the _DL_VERSION
 # variable, then replace all forward slashes ('/') by underscores ('_') to
@@ -272,15 +272,15 @@ define inner-generic-package
 # version control system branch or tag, for example remotes/origin/1_10_stable.
 ifndef $(2)_VERSION
  ifdef $(3)_VERSION
-  $(2)_DL_VERSION = $($(3)_VERSION)
-  $(2)_VERSION = $(subst /,_,$($(3)_VERSION))
+  $(2)_DL_VERSION = $$($(3)_VERSION)
+  $(2)_VERSION := $$(subst /,_,$$($(3)_VERSION))
  else
   $(2)_VERSION = undefined
   $(2)_DL_VERSION = undefined
  endif
 else
-  $(2)_DL_VERSION = $($(2)_VERSION)
-  $(2)_VERSION = $(subst /,_,$($(2)_VERSION))
+  $(2)_DL_VERSION = $$($(2)_VERSION)
+  $(2)_VERSION := $$(subst /,_,$$($(2)_VERSION))
 endif
 
 $(2)_BASE_NAME	=  $(1)-$$($(2)_VERSION)
@@ -304,7 +304,7 @@ endif
 
 ifndef $(2)_SOURCE
  ifdef $(3)_SOURCE
-  $(2)_SOURCE = $($(3)_SOURCE)
+  $(2)_SOURCE = $$($(3)_SOURCE)
  else
   $(2)_SOURCE			?= $$($(2)_RAWNAME)-$$($(2)_VERSION).tar.gz
  endif
@@ -312,22 +312,22 @@ endif
 
 ifndef $(2)_PATCH
  ifdef $(3)_PATCH
-  $(2)_PATCH = $($(3)_PATCH)
+  $(2)_PATCH = $$($(3)_PATCH)
  endif
 endif
 
 ifndef $(2)_SITE
  ifdef $(3)_SITE
-  $(2)_SITE = $($(3)_SITE)
+  $(2)_SITE = $$($(3)_SITE)
  endif
 endif
 
 ifndef $(2)_SITE_METHOD
  ifdef $(3)_SITE_METHOD
-  $(2)_SITE_METHOD = $($(3)_SITE_METHOD)
+  $(2)_SITE_METHOD = $$($(3)_SITE_METHOD)
  else
 	# Try automatic detection using the scheme part of the URI
-	$(2)_SITE_METHOD = $(call geturischeme,$($(2)_SITE))
+	$(2)_SITE_METHOD = $$(call geturischeme,$$($(2)_SITE))
  endif
 endif
 
@@ -339,7 +339,7 @@ endif
 
 ifndef $(2)_LICENSE
  ifdef $(3)_LICENSE
-  $(2)_LICENSE = $($(3)_LICENSE)
+  $(2)_LICENSE = $$($(3)_LICENSE)
  endif
 endif
 
@@ -347,13 +347,13 @@ endif
 
 ifndef $(2)_LICENSE_FILES
  ifdef $(3)_LICENSE_FILES
-  $(2)_LICENSE_FILES = $($(3)_LICENSE_FILES)
+  $(2)_LICENSE_FILES = $$($(3)_LICENSE_FILES)
  endif
 endif
 
 ifndef $(2)_REDISTRIBUTE
  ifdef $(3)_REDISTRIBUTE
-  $(2)_REDISTRIBUTE = $($(3)_REDISTRIBUTE)
+  $(2)_REDISTRIBUTE = $$($(3)_REDISTRIBUTE)
  endif
 endif
 
@@ -364,8 +364,10 @@ endif
 # dependency
 $(2)_ADD_TOOLCHAIN_DEPENDENCY	?= YES
 
-$(2)_DEPENDENCIES ?= $(filter-out  host-toolchain $(1),\
-	$(patsubst host-host-%,host-%,$(addprefix host-,$($(3)_DEPENDENCIES))))
+ifeq ($$(origin $(2)_DEPENDENCIES),undefined)
+$(2)_DEPENDENCIES := $$(filter-out  host-toolchain $(1),\
+	$$(patsubst host-host-%,host-%,$$(addprefix host-,$$($(3)_DEPENDENCIES))))
+endif
 ifeq ($(4),target)
 ifeq ($$($(2)_ADD_TOOLCHAIN_DEPENDENCY),YES)
 $(2)_DEPENDENCIES += toolchain
@@ -392,8 +394,8 @@ endif
 
 # default extract command
 $(2)_EXTRACT_CMDS ?= \
-	$$(if $$($(2)_SOURCE),$$(INFLATE$$(suffix $$($(2)_SOURCE))) $(DL_DIR)/$$($(2)_SOURCE) | \
-	$(TAR) $(TAR_STRIP_COMPONENTS)=1 -C $$($(2)_DIR) $(TAR_OPTIONS) -)
+	$$(if $$($(2)_SOURCE),$$(INFLATE$$(suffix $$($(2)_SOURCE))) $$(DL_DIR)/$$($(2)_SOURCE) | \
+	$$(TAR) $$(TAR_STRIP_COMPONENTS)=1 -C $$($(2)_DIR) $$(TAR_OPTIONS) -)
 
 # pre/post-steps hooks
 $(2)_PRE_DOWNLOAD_HOOKS         ?=
@@ -467,7 +469,7 @@ endif
 $$($(2)_TARGET_CONFIGURE):	| $$($(2)_DEPENDENCIES)
 
 $$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | dirs prepare
-ifeq ($(filter $(1),$(DEPENDENCIES_HOST_PREREQ)),)
+ifeq ($$(filter $(1),$$(DEPENDENCIES_HOST_PREREQ)),)
 $$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | dependencies
 endif
 
@@ -510,11 +512,11 @@ endif
 			@echo $$($(2)_DEPENDENCIES)
 
 $(1)-graph-depends:
-			@$(INSTALL) -d $(O)/graphs
-			@cd "$(CONFIG_DIR)"; \
-			$(TOPDIR)/support/scripts/graph-depends -p $(1) -d $(BR_GRAPH_DEPTH) \
-			|tee $(O)/graphs/$$(@).dot \
-			|dot -T$(BR_GRAPH_OUT) -o $(O)/graphs/$$(@).$(BR_GRAPH_OUT)
+			@$$(INSTALL) -d $$(O)/graphs
+			@cd "$$(CONFIG_DIR)"; \
+			$$(TOPDIR)/support/scripts/graph-depends -p $(1) -d $$(BR_GRAPH_DEPTH) \
+			|tee $$(O)/graphs/$$(@).dot \
+			|dot -T$$(BR_GRAPH_OUT) -o $$(O)/graphs/$$(@).$$(BR_GRAPH_OUT)
 
 $(1)-dirclean:		$$($(2)_TARGET_DIRCLEAN)
 
@@ -548,7 +550,7 @@ endif
 $$($(2)_TARGET_RSYNC_SOURCE):		SRCDIR=$$($(2)_OVERRIDE_SRCDIR)
 $$($(2)_TARGET_RSYNC_SOURCE):		PKG=$(2)
 $$($(2)_TARGET_PATCH):			PKG=$(2)
-$$($(2)_TARGET_PATCH):			RAWNAME=$(patsubst host-%,%,$(1))
+$$($(2)_TARGET_PATCH):			RAWNAME=$$(patsubst host-%,%,$(1))
 $$($(2)_TARGET_PATCH):			PKGDIR=$(pkgdir)
 $$($(2)_TARGET_EXTRACT):		PKG=$(2)
 $$($(2)_TARGET_SOURCE):			PKG=$(2)
@@ -559,9 +561,9 @@ endif
 # kernel case, the bootloaders case, and the normal packages case.
 ifeq ($(1),linux)
 $(2)_KCONFIG_VAR = BR2_LINUX_KERNEL
-else ifneq ($(filter boot/%,$(pkgdir)),)
+else ifneq ($$(filter boot/%,$(pkgdir)),)
 $(2)_KCONFIG_VAR = BR2_TARGET_$(2)
-else ifneq ($(filter toolchain/%,$(pkgdir)),)
+else ifneq ($$(filter toolchain/%,$(pkgdir)),)
 $(2)_KCONFIG_VAR = BR2_$(2)
 else
 $(2)_KCONFIG_VAR = BR2_PACKAGE_$(2)
@@ -577,7 +579,7 @@ ifeq ($$($(2)_REDISTRIBUTE),YES)
 ifneq ($$($(2)_SITE_METHOD),local)
 ifneq ($$($(2)_SITE_METHOD),override)
 # Packages that have a tarball need it downloaded and extracted beforehand
-$(1)-legal-info: $(1)-extract $(REDIST_SOURCES_DIR_$(call UPPERCASE,$(4)))
+$(1)-legal-info: $(1)-extract $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))
 $(2)_MANIFEST_TARBALL = $$($(2)_SOURCE)
 endif
 endif
@@ -587,40 +589,40 @@ endif
 # legal-info: produce legally relevant info.
 $(1)-legal-info:
 # Packages without a source are assumed to be part of Buildroot, skip them.
-	$(foreach hook,$($(2)_PRE_LEGAL_INFO_HOOKS),$(call $(hook))$(sep))
-ifneq ($(call qstrip,$$($(2)_SOURCE)),)
+	$$(foreach hook,$$($(2)_PRE_LEGAL_INFO_HOOKS),$$(call $$(hook))$$(sep))
+ifneq ($$(call qstrip,$$($(2)_SOURCE)),)
 
 ifeq ($$($(2)_SITE_METHOD),local)
 # Packages without a tarball: don't save and warn
-	@$(call legal-warning-pkg-savednothing,$$($(2)_RAWNAME),local)
+	@$$(call legal-warning-pkg-savednothing,$$($(2)_RAWNAME),local)
 
 else ifneq ($$($(2)_OVERRIDE_SRCDIR),)
-	@$(call legal-warning-pkg-savednothing,$$($(2)_RAWNAME),override)
+	@$$(call legal-warning-pkg-savednothing,$$($(2)_RAWNAME),override)
 
 else
 # Other packages
 
 # Save license files if defined
-ifeq ($(call qstrip,$$($(2)_LICENSE_FILES)),)
-	@$(call legal-license-nofiles,$$($(2)_RAWNAME),$(call UPPERCASE,$(4)))
-	@$(call legal-warning-pkg,$$($(2)_RAWNAME),cannot save license ($(2)_LICENSE_FILES not defined))
+ifeq ($$(call qstrip,$$($(2)_LICENSE_FILES)),)
+	@$$(call legal-license-nofiles,$$($(2)_RAWNAME),$$(call UPPERCASE,$(4)))
+	@$$(call legal-warning-pkg,$$($(2)_RAWNAME),cannot save license ($(2)_LICENSE_FILES not defined))
 else
 # Double dollar signs are really needed here, to catch host packages
 # without explicit HOST_FOO_LICENSE_FILES assignment, also in case they
 # have multiple license files.
-	@$$(foreach F,$$($(2)_LICENSE_FILES),$$(call legal-license-file,$$($(2)_RAWNAME),$$(F),$$($(2)_DIR)/$$(F),$(call UPPERCASE,$(4)))$$(sep))
+	@$$(foreach F,$$($(2)_LICENSE_FILES),$$(call legal-license-file,$$($(2)_RAWNAME),$$(F),$$($(2)_DIR)/$$(F),$$(call UPPERCASE,$(4)))$$(sep))
 endif # license files
 
 ifeq ($$($(2)_REDISTRIBUTE),YES)
 # Copy the source tarball (just hardlink if possible)
-	@cp -l $(DL_DIR)/$$($(2)_SOURCE) $(REDIST_SOURCES_DIR_$(call UPPERCASE,$(4))) 2>/dev/null || \
-	   cp $(DL_DIR)/$$($(2)_SOURCE) $(REDIST_SOURCES_DIR_$(call UPPERCASE,$(4)))
+	@cp -l $$(DL_DIR)/$$($(2)_SOURCE) $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4))) 2>/dev/null || \
+	   cp $$(DL_DIR)/$$($(2)_SOURCE) $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))
 endif # redistribute
 
 endif # other packages
-	@$(call legal-manifest,$$($(2)_RAWNAME),$$($(2)_VERSION),$$($(2)_LICENSE),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_MANIFEST_TARBALL),$(call UPPERCASE,$(4)))
-endif # ifneq ($(call qstrip,$$($(2)_SOURCE)),)
-	$(foreach hook,$($(2)_POST_LEGAL_INFO_HOOKS),$(call $(hook))$(sep))
+	@$$(call legal-manifest,$$($(2)_RAWNAME),$$($(2)_VERSION),$$($(2)_LICENSE),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_MANIFEST_TARBALL),$$(call UPPERCASE,$(4)))
+endif # ifneq ($$(call qstrip,$$($(2)_SOURCE)),)
+	$$(foreach hook,$$($(2)_POST_LEGAL_INFO_HOOKS),$$(call $$(hook))$$(sep))
 
 # add package to the general list of targets if requested by the buildroot
 # configuration
@@ -649,8 +651,8 @@ endif # SITE_METHOD
 # ZCAT="gzip -d -c", and to check for the dependency we only want 'gzip'.
 # Do not add xzcat to the list of required dependencies, as it gets built
 # automatically if it isn't found.
-ifneq ($(call suitable-extractor,$($(2)_SOURCE)),$(XZCAT))
-DL_TOOLS_DEPENDENCIES += $(firstword $(call suitable-extractor,$($(2)_SOURCE)))
+ifneq ($$(call suitable-extractor,$$($(2)_SOURCE)),$$(XZCAT))
+DL_TOOLS_DEPENDENCIES += $$(firstword $$(call suitable-extractor,$$($(2)_SOURCE)))
 endif
 
 endif # $(2)_KCONFIG_VAR
@@ -661,8 +663,8 @@ endef # inner-generic-package
 ################################################################################
 
 # In the case of target packages, keep the package name "pkg"
-generic-package = $(call inner-generic-package,$(pkgname),$(call UPPERCASE,$(pkgname)),$(call UPPERCASE,$(pkgname)),target)
+generic-package = $(call inner-generic-package,$(pkgname),$$(call UPPERCASE,$(pkgname)),$$(call UPPERCASE,$(pkgname)),target)
 # In the case of host packages, turn the package name "pkg" into "host-pkg"
-host-generic-package = $(call inner-generic-package,host-$(pkgname),$(call UPPERCASE,host-$(pkgname)),$(call UPPERCASE,$(pkgname)),host)
+host-generic-package = $(call inner-generic-package,host-$(pkgname),$$(call UPPERCASE,host-$(pkgname)),$$(call UPPERCASE,$(pkgname)),host)
 
 # :mode=makefile:
diff --git a/package/pkg-luarocks.mk b/package/pkg-luarocks.mk
--- a/package/pkg-luarocks.mk
+++ b/package/pkg-luarocks.mk
@@ -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 @@ define inner-luarocks-package
 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
 
@@ -77,5 +77,5 @@ endef
 # luarocks-package -- the target generator macro for LuaRocks packages
 ################################################################################
 
-luarocks-package = $(call inner-luarocks-package,$(pkgname),$(call UPPERCASE,$(pkgname)),$(call UPPERCASE,$(pkgname)),target)
+luarocks-package = $(call inner-luarocks-package,$(pkgname),$$(call UPPERCASE,$(pkgname)),$$(call UPPERCASE,$(pkgname)),target)
 # host-luarocks-package not supported
diff --git a/package/pkg-perl.mk b/package/pkg-perl.mk
--- a/package/pkg-perl.mk
+++ b/package/pkg-perl.mk
@@ -51,18 +51,18 @@ define $(2)_CONFIGURE_CMDS
 	cd $$($$(PKG)_SRCDIR) && if [ -f Build.PL ] ; then \
 		PERL_MM_USE_DEFAULT=1 \
 		perl Build.PL \
-			--config ar="$(TARGET_AR)" \
-			--config full_ar="$(TARGET_AR)" \
-			--config cc="$(TARGET_CC)" \
-			--config ccflags="$(TARGET_CFLAGS)" \
-			--config ld="$(TARGET_CC)" \
-			--config lddlflags="-shared $(TARGET_LDFLAGS)" \
-			--config ldflags="$(TARGET_LDFLAGS)" \
-			--include_dirs $$(STAGING_DIR)/usr/lib/perl5/$$(PERL_VERSION)/$(PERL_ARCHNAME)/CORE \
+			--config ar="$$(TARGET_AR)" \
+			--config full_ar="$$(TARGET_AR)" \
+			--config cc="$$(TARGET_CC)" \
+			--config ccflags="$$(TARGET_CFLAGS)" \
+			--config ld="$$(TARGET_CC)" \
+			--config lddlflags="-shared $$(TARGET_LDFLAGS)" \
+			--config ldflags="$$(TARGET_LDFLAGS)" \
+			--include_dirs $$(STAGING_DIR)/usr/lib/perl5/$$(PERL_VERSION)/$$(PERL_ARCHNAME)/CORE \
 			--destdir $$(TARGET_DIR) \
 			--installdirs vendor \
 			--install_path lib=/usr/lib/perl5/site_perl/$$(PERL_VERSION) \
-			--install_path arch=/usr/lib/perl5/site_perl/$$(PERL_VERSION)/$(PERL_ARCHNAME) \
+			--install_path arch=/usr/lib/perl5/site_perl/$$(PERL_VERSION)/$$(PERL_ARCHNAME) \
 			--install_path bin=/usr/bin \
 			--install_path script=/usr/bin \
 			--install_path bindoc=/usr/share/man/man1 \
@@ -72,17 +72,17 @@ define $(2)_CONFIGURE_CMDS
 		PERL_MM_USE_DEFAULT=1 \
 		PERL_AUTOINSTALL=--skipdeps \
 		perl Makefile.PL \
-			AR="$(TARGET_AR)" \
-			FULL_AR="$(TARGET_AR)" \
-			CC="$(TARGET_CC)" \
-			CCFLAGS="$(TARGET_CFLAGS)" \
-			LD="$(TARGET_CC)" \
-			LDDLFLAGS="-shared $(TARGET_LDFLAGS)" \
-			LDFLAGS="$(TARGET_LDFLAGS)" \
+			AR="$$(TARGET_AR)" \
+			FULL_AR="$$(TARGET_AR)" \
+			CC="$$(TARGET_CC)" \
+			CCFLAGS="$$(TARGET_CFLAGS)" \
+			LD="$$(TARGET_CC)" \
+			LDDLFLAGS="-shared $$(TARGET_LDFLAGS)" \
+			LDFLAGS="$$(TARGET_LDFLAGS)" \
 			DESTDIR=$$(TARGET_DIR) \
 			INSTALLDIRS=vendor \
 			INSTALLVENDORLIB=/usr/lib/perl5/site_perl/$$(PERL_VERSION) \
-			INSTALLVENDORARCH=/usr/lib/perl5/site_perl/$$(PERL_VERSION)/$(PERL_ARCHNAME) \
+			INSTALLVENDORARCH=/usr/lib/perl5/site_perl/$$(PERL_VERSION)/$$(PERL_ARCHNAME) \
 			INSTALLVENDORBIN=/usr/bin \
 			INSTALLVENDORSCRIPT=/usr/bin \
 			INSTALLVENDORMAN1DIR=/usr/share/man/man1 \
@@ -125,8 +125,8 @@ define $(2)_BUILD_CMDS
 	cd $$($$(PKG)_SRCDIR) && if [ -f Build.PL ] ; then \
 		perl Build $$($(2)_BUILD_OPT) build; \
 	else \
-		$(MAKE1) \
-			PERL_INC=$$(STAGING_DIR)/usr/lib/perl5/$$(PERL_VERSION)/$(PERL_ARCHNAME)/CORE \
+		$$(MAKE1) \
+			PERL_INC=$$(STAGING_DIR)/usr/lib/perl5/$$(PERL_VERSION)/$$(PERL_ARCHNAME)/CORE \
 			$$($(2)_BUILD_OPT) pure_all; \
 	fi
 endef
@@ -137,7 +137,7 @@ define $(2)_BUILD_CMDS
 	cd $$($$(PKG)_SRCDIR) && if [ -f Build.PL ] ; then \
 		perl Build $$($(2)_BUILD_OPT) build; \
 	else \
-		$(MAKE1) $$($(2)_BUILD_OPT) pure_all; \
+		$$(MAKE1) $$($(2)_BUILD_OPT) pure_all; \
 	fi
 endef
 endif
@@ -152,7 +152,7 @@ define $(2)_INSTALL_CMDS
 	cd $$($$(PKG)_SRCDIR) && if [ -f Build.PL ] ; then \
 		perl Build $$($(2)_INSTALL_TARGET_OPT) install; \
 	else \
-		$(MAKE1) $$($(2)_INSTALL_TARGET_OPT) pure_install; \
+		$$(MAKE1) $$($(2)_INSTALL_TARGET_OPT) pure_install; \
 	fi
 endef
 endif
@@ -166,7 +166,7 @@ define $(2)_INSTALL_TARGET_CMDS
 	cd $$($$(PKG)_SRCDIR) && if [ -f Build.PL ] ; then \
 		perl Build $$($(2)_INSTALL_TARGET_OPT) install; \
 	else \
-		$(MAKE1) $$($(2)_INSTALL_TARGET_OPT) pure_install; \
+		$$(MAKE1) $$($(2)_INSTALL_TARGET_OPT) pure_install; \
 	fi
 endef
 endif
@@ -181,5 +181,5 @@ endef
 # perl-package -- the target generator macro for Perl packages
 ################################################################################
 
-perl-package = $(call inner-perl-package,$(pkgname),$(call UPPERCASE,$(pkgname)),$(call UPPERCASE,$(pkgname)),target)
-host-perl-package = $(call inner-perl-package,host-$(pkgname),$(call UPPERCASE,host-$(pkgname)),$(call UPPERCASE,$(pkgname)),host)
+perl-package = $(call inner-perl-package,$(pkgname),$$(call UPPERCASE,$(pkgname)),$$(call UPPERCASE,$(pkgname)),target)
+host-perl-package = $(call inner-perl-package,host-$(pkgname),$$(call UPPERCASE,host-$(pkgname)),$$(call UPPERCASE,$(pkgname)),host)
diff --git a/package/pkg-python.mk b/package/pkg-python.mk
--- a/package/pkg-python.mk
+++ b/package/pkg-python.mk
@@ -82,7 +82,7 @@ HOST_PKG_PYTHON_SETUPTOOLS_INSTALL_OPT =
 
 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 @@ define inner-python-package
 
 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 @@ endif
 # 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 ($$(origin $(2)_DEPENDENCIES),undefined)
+$(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 @@ endif
 #   - 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 @@ endif
 #   - 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
 
@@ -255,5 +257,5 @@ endef
 # python-package -- the target generator macro for Python packages
 ################################################################################
 
-python-package = $(call inner-python-package,$(pkgname),$(call UPPERCASE,$(pkgname)),$(call UPPERCASE,$(pkgname)),target)
-host-python-package = $(call inner-python-package,host-$(pkgname),$(call UPPERCASE,host-$(pkgname)),$(call UPPERCASE,$(pkgname)),host)
+python-package = $(call inner-python-package,$(pkgname),$$(call UPPERCASE,$(pkgname)),$$(call UPPERCASE,$(pkgname)),target)
+host-python-package = $(call inner-python-package,host-$(pkgname),$$(call UPPERCASE,host-$(pkgname)),$$(call UPPERCASE,$(pkgname)),host)
diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
--- a/package/pkg-utils.mk
+++ b/package/pkg-utils.mk
@@ -19,13 +19,13 @@
 define caseconvert-helper
 $(1) = $$(strip \
 	$$(eval __tmp := $$(1))\
-	$(foreach c, $(2),\
-		$$(eval __tmp := $$(subst $(word 1,$(subst :, ,$c)),$(word 2,$(subst :, ,$c)),$$(__tmp))))\
+	$$(foreach c, $(2),\
+		$$(eval __tmp := $$(subst $$(word 1,$$(subst :, ,$$(c))),$$(word 2,$$(subst :, ,$$(c))),$$(__tmp))))\
 	$$(__tmp))
 endef
 
-$(eval $(call caseconvert-helper,UPPERCASE,$(join $(addsuffix :,$([FROM])),$([TO]))))
-$(eval $(call caseconvert-helper,LOWERCASE,$(join $(addsuffix :,$([TO])),$([FROM]))))
+$(eval $(call caseconvert-helper,UPPERCASE,$$(join $$(addsuffix :,$$([FROM])),$$([TO]))))
+$(eval $(call caseconvert-helper,LOWERCASE,$$(join $$(addsuffix :,$$([TO])),$$([FROM]))))
 
 #
 # Manipulation of .config files based on the Kconfig
diff --git a/package/pkg-virtual.mk b/package/pkg-virtual.mk
--- a/package/pkg-virtual.mk
+++ b/package/pkg-virtual.mk
@@ -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 @@ HOST_$(3)_VERSION = virtual
 
 # 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 ($$(origin $(2)_DEPENDENCIES),undefined)
+$(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
@@ -66,5 +68,5 @@ endef
 # virtual-package -- the target generator macro for virtual packages
 ################################################################################
 
-virtual-package = $(call inner-virtual-package,$(pkgname),$(call UPPERCASE,$(pkgname)),$(call UPPERCASE,$(pkgname)),target)
-host-virtual-package = $(call inner-virtual-package,host-$(pkgname),$(call UPPERCASE,host-$(pkgname)),$(call UPPERCASE,$(pkgname)),host)
+virtual-package = $(call inner-virtual-package,$(pkgname),$$(call UPPERCASE,$(pkgname)),$$(call UPPERCASE,$(pkgname)),target)
+host-virtual-package = $(call inner-virtual-package,host-$(pkgname),$$(call UPPERCASE,host-$(pkgname)),$$(call UPPERCASE,$(pkgname)),host)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Buildroot] [PATCH 2 of 7 v2] infra: add comment describing single/double dollar-sign rules
  2014-05-12 14:44 [Buildroot] [PATCH 0 of 7 v2] infra: fix dollar signs; remove some undefined versions Thomas De Schampheleire
  2014-05-12 14:44 ` [Buildroot] [PATCH 1 of 7 v2] infra: consistently use double dollar signs inside inner-xxx-targets Thomas De Schampheleire
@ 2014-05-12 14:44 ` Thomas De Schampheleire
  2014-05-12 14:44 ` [Buildroot] [PATCH 3 of 7 v2] pkg-virtual: simplify definition of FOO_VERSION to 'virtual' Thomas De Schampheleire
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Thomas De Schampheleire @ 2014-05-12 14:44 UTC (permalink / raw)
  To: buildroot

As 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.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

---
v2: clarify exception for pkgdir and pkgname

 package/pkg-generic.mk |  14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -250,6 +250,20 @@ endif
 #  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. Special variables $(pkgname) and $(pkgdir)
+# should also be referenced with a single dollar sign. 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.
+# These rules ensure 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Buildroot] [PATCH 3 of 7 v2] pkg-virtual: simplify definition of FOO_VERSION to 'virtual'
  2014-05-12 14:44 [Buildroot] [PATCH 0 of 7 v2] infra: fix dollar signs; remove some undefined versions Thomas De Schampheleire
  2014-05-12 14:44 ` [Buildroot] [PATCH 1 of 7 v2] infra: consistently use double dollar signs inside inner-xxx-targets Thomas De Schampheleire
  2014-05-12 14:44 ` [Buildroot] [PATCH 2 of 7 v2] infra: add comment describing single/double dollar-sign rules Thomas De Schampheleire
@ 2014-05-12 14:44 ` Thomas De Schampheleire
  2014-05-12 14:44 ` [Buildroot] [PATCH 4 of 7 v2] toolchain/toolchain-buildroot: migrate to virtual package infrastructure Thomas De Schampheleire
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Thomas De Schampheleire @ 2014-05-12 14:44 UTC (permalink / raw)
  To: buildroot

As 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.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

[1] http://lists.busybox.net/pipermail/buildroot/2014-April/093670.html

---
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(-)

diff --git a/package/pkg-virtual.mk b/package/pkg-virtual.mk
--- a/package/pkg-virtual.mk
+++ b/package/pkg-virtual.mk
@@ -45,8 +45,7 @@ endif
 $(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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Buildroot] [PATCH 4 of 7 v2] toolchain/toolchain-buildroot: migrate to virtual package infrastructure
  2014-05-12 14:44 [Buildroot] [PATCH 0 of 7 v2] infra: fix dollar signs; remove some undefined versions Thomas De Schampheleire
                   ` (2 preceding siblings ...)
  2014-05-12 14:44 ` [Buildroot] [PATCH 3 of 7 v2] pkg-virtual: simplify definition of FOO_VERSION to 'virtual' Thomas De Schampheleire
@ 2014-05-12 14:44 ` Thomas De Schampheleire
  2014-05-12 14:44 ` [Buildroot] [PATCH 5 of 7 v2] toolchain-external: change version from 'undefined' to 'virtual' Thomas De Schampheleire
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Thomas De Schampheleire @ 2014-05-12 14:44 UTC (permalink / raw)
  To: buildroot

This patch migrates the toolchain and toolchain-buildroot packages to the
virtual package infrastructure, causing the log messages to change from:

>>> toolchain undefined Downloading
>>> toolchain undefined Extracting
...

to

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

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

---
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(-)

diff --git a/toolchain/toolchain-buildroot/toolchain-buildroot.mk b/toolchain/toolchain-buildroot/toolchain-buildroot.mk
--- a/toolchain/toolchain-buildroot/toolchain-buildroot.mk
+++ b/toolchain/toolchain-buildroot/toolchain-buildroot.mk
@@ -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_DEPENDENCIES = host-
 
 TOOLCHAIN_BUILDROOT_ADD_TOOLCHAIN_DEPENDENCY = NO
 
-$(eval $(generic-package))
+$(eval $(virtual-package))
diff --git a/toolchain/toolchain/toolchain.mk b/toolchain/toolchain/toolchain.mk
--- a/toolchain/toolchain/toolchain.mk
+++ b/toolchain/toolchain/toolchain.mk
@@ -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 @@ endif
 
 TOOLCHAIN_ADD_TOOLCHAIN_DEPENDENCY = NO
 
-$(eval $(generic-package))
+$(eval $(virtual-package))
 
 toolchain: $(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Buildroot] [PATCH 5 of 7 v2] toolchain-external: change version from 'undefined' to 'virtual'
  2014-05-12 14:44 [Buildroot] [PATCH 0 of 7 v2] infra: fix dollar signs; remove some undefined versions Thomas De Schampheleire
                   ` (3 preceding siblings ...)
  2014-05-12 14:44 ` [Buildroot] [PATCH 4 of 7 v2] toolchain/toolchain-buildroot: migrate to virtual package infrastructure Thomas De Schampheleire
@ 2014-05-12 14:44 ` Thomas De Schampheleire
  2014-05-12 14:44 ` [Buildroot] [PATCH 6 of 7 v2] makedevs: change version from 'undefined' to 'buildroot-$(BR2_VERSION)' Thomas De Schampheleire
  2014-05-12 14:44 ` [Buildroot] [PATCH 7 of 7 v2] mkpasswd: " Thomas De Schampheleire
  6 siblings, 0 replies; 13+ messages in thread
From: Thomas De Schampheleire @ 2014-05-12 14:44 UTC (permalink / raw)
  To: buildroot

The toolchain-external package displays the version 'undefined' in the build
messages and the directory in output/build, which is not very nice.
This patch sets the version to 'virtual', in analogy to the toolchain and
toolchain-buildroot packages (which use the virtual-package infrastructure).

Although toolchain-external is not strictly a virtual package, since it uses
the generic-package infrastructure, it can be considered as a virtual
package in the sense that it does not have a fixed version or source (they
depend on the selected external toolchain).

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

---
v2: no changes

 toolchain/toolchain-external/toolchain-external.mk |  2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/toolchain/toolchain-external/toolchain-external.mk b/toolchain/toolchain-external/toolchain-external.mk
--- a/toolchain/toolchain-external/toolchain-external.mk
+++ b/toolchain/toolchain-external/toolchain-external.mk
@@ -410,6 +410,8 @@ TOOLCHAIN_EXTERNAL_SITE =
 TOOLCHAIN_EXTERNAL_SOURCE =
 endif

+TOOLCHAIN_EXTERNAL_VERSION = virtual
+
 TOOLCHAIN_EXTERNAL_ADD_TOOLCHAIN_DEPENDENCY = NO

 TOOLCHAIN_EXTERNAL_INSTALL_STAGING = YES

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Buildroot] [PATCH 6 of 7 v2] makedevs: change version from 'undefined' to 'buildroot-$(BR2_VERSION)'
  2014-05-12 14:44 [Buildroot] [PATCH 0 of 7 v2] infra: fix dollar signs; remove some undefined versions Thomas De Schampheleire
                   ` (4 preceding siblings ...)
  2014-05-12 14:44 ` [Buildroot] [PATCH 5 of 7 v2] toolchain-external: change version from 'undefined' to 'virtual' Thomas De Schampheleire
@ 2014-05-12 14:44 ` Thomas De Schampheleire
  2014-05-12 14:44 ` [Buildroot] [PATCH 7 of 7 v2] mkpasswd: " Thomas De Schampheleire
  6 siblings, 0 replies; 13+ messages in thread
From: Thomas De Schampheleire @ 2014-05-12 14:44 UTC (permalink / raw)
  To: buildroot

The 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'.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

---
v2: no changes

 package/makedevs/makedevs.mk |  2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/package/makedevs/makedevs.mk b/package/makedevs/makedevs.mk
--- a/package/makedevs/makedevs.mk
+++ b/package/makedevs/makedevs.mk
@@ -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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Buildroot] [PATCH 7 of 7 v2] mkpasswd: change version from 'undefined' to 'buildroot-$(BR2_VERSION)'
  2014-05-12 14:44 [Buildroot] [PATCH 0 of 7 v2] infra: fix dollar signs; remove some undefined versions Thomas De Schampheleire
                   ` (5 preceding siblings ...)
  2014-05-12 14:44 ` [Buildroot] [PATCH 6 of 7 v2] makedevs: change version from 'undefined' to 'buildroot-$(BR2_VERSION)' Thomas De Schampheleire
@ 2014-05-12 14:44 ` Thomas De Schampheleire
  6 siblings, 0 replies; 13+ messages in thread
From: Thomas De Schampheleire @ 2014-05-12 14:44 UTC (permalink / raw)
  To: buildroot

The 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'.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

---
v2: no changes

 package/mkpasswd/mkpasswd.mk |  2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/package/mkpasswd/mkpasswd.mk b/package/mkpasswd/mkpasswd.mk
--- a/package/mkpasswd/mkpasswd.mk
+++ b/package/mkpasswd/mkpasswd.mk
@@ -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 \

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Buildroot] [PATCH 1 of 7 v2] infra: consistently use double dollar signs inside inner-xxx-targets
  2014-05-12 14:44 ` [Buildroot] [PATCH 1 of 7 v2] infra: consistently use double dollar signs inside inner-xxx-targets Thomas De Schampheleire
@ 2014-05-13  9:25   ` Arnout Vandecappelle
  2014-05-13 10:59     ` Thomas De Schampheleire
  2014-05-21 13:14     ` Thomas De Schampheleire
  0 siblings, 2 replies; 13+ messages in thread
From: Arnout Vandecappelle @ 2014-05-13  9:25 UTC (permalink / raw)
  To: buildroot

 Hi Thomas,

 Thanks for taking upon you this gruelling task :-)

On 12/05/14 16:44, Thomas De Schampheleire wrote:
> 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

 I don't see why references to pkgdir/pkgname should be an exception. See my
comments below. So the original was perfect. (sorry, Yann :-)

> 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, an
> immediate assignment using := is necessary instead of a deferred assignment.
> using =.
> When such a circular assignment occurs in a variable defined with '?=',
> a special construction is necessary, as make does not have a corresponding
> '?:=' assignment. As 'FOO ?= bar' is equivalent to
>   ifeq ($(origin FOO),undefined)
>     FOO = bar
>   endif
> we can replace such constructions with
>   ifeq ($$(origin FOO),undefined)
>     FOO := bar
>   endif
> 
> 
> 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 can be verified by comparing 'make printvars'
> before and after applying this patch.
> 
> Insight-provided-by: Arnout Vandecappelle <arnout@mind.be>
> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

 In case it wasn't obvious: this series is _not_ for 2014.05.

> 
> ---
> 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    |  14 +++---
>  fs/common.mk             |  14 +++---
>  package/pkg-autotools.mk |  34 +++++++++-------
>  package/pkg-cmake.mk     |  32 ++++++++-------
>  package/pkg-generic.mk   |  90 +++++++++++++++++++++---------------------
>  package/pkg-luarocks.mk  |  12 ++--
>  package/pkg-perl.mk      |  48 +++++++++++-----------
>  package/pkg-python.mk    |  34 ++++++++-------
>  package/pkg-utils.mk     |   8 +-
>  package/pkg-virtual.mk   |  16 ++++---
>  10 files changed, 157 insertions(+), 145 deletions(-)
> 
> Note: in addition to using 'make printvars' to verify this patch, a 'make
> randpackageconfig' was performed successfully.

 Perhaps you should do a 'make manual' as well.

> 
> diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk
> --- a/docs/manual/manual.mk
> +++ b/docs/manual/manual.mk
> @@ -54,16 +54,16 @@ define GENDOC_INNER

 I would add a brief comment in the beginning of GENDOC_INNER:

# Since this function will be called from within an $(eval ...)
# all variable references except the arguments must be $$-quoted.

>  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 +82,7 @@ define GENDOC
>  $(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
>  
> diff --git a/fs/common.mk b/fs/common.mk
> --- a/fs/common.mk
> +++ b/fs/common.mk
> @@ -39,7 +39,7 @@ define ROOTFS_TARGET_INTERNAL

 Same here: add comment.

>  
>  # 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 +69,7 @@ endif
>  	$$(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 -R 0:0 $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)
>  ifneq ($$(ROOTFS_DEVICE_TABLES),)
>  	cat $$(ROOTFS_DEVICE_TABLES) > $$(FULL_DEVICE_TABLE)
> @@ -80,14 +80,14 @@ endif
>  	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),)
>  	$$(ROOTFS_$(2)_COMPRESS_CMD) $$@ > $$@$$(ROOTFS_$(2)_COMPRESS_EXT)
> diff --git a/package/pkg-autotools.mk b/package/pkg-autotools.mk
> --- a/package/pkg-autotools.mk
> +++ b/package/pkg-autotools.mk
> @@ -65,7 +65,7 @@ define inner-autotools-package
>  
>  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 @@ endif
>  
>  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 ($$(origin $(2)_AUTORECONF_OPT),undefined)
> + $(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)

 Wouldn't it be clearer to keep this as a ?= but put it inside an
ifeq ($(4),host) ? I haven't tested whether that works, however.

>  $(2)_INSTALL_OPT                ?= install
>  $(2)_INSTALL_STAGING_OPT	?= DESTDIR=$$(STAGING_DIR) install
>  $(2)_INSTALL_TARGET_OPT		?= DESTDIR=$$(TARGET_DIR)  install
> @@ -175,7 +178,7 @@ endef
>  #
>  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 @@ endif
>  #
>  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 @@ endef
>  # 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 ($$(origin $(2)_DEPENDENCIES),undefined)

ifeq ($(4),host)
 $(2)_DEPENDENCIES ?= ....

 This one I tested with printvars, and it seems to work.

> +$(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 @@ endif
>  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
> @@ -290,5 +294,5 @@ endef
>  # autotools-package -- the target generator macro for autotools packages
>  ################################################################################
>  
> -autotools-package = $(call inner-autotools-package,$(pkgname),$(call UPPERCASE,$(pkgname)),$(call UPPERCASE,$(pkgname)),target)
> -host-autotools-package = $(call inner-autotools-package,host-$(pkgname),$(call UPPERCASE,host-$(pkgname)),$(call UPPERCASE,$(pkgname)),host)
> +autotools-package = $(call inner-autotools-package,$(pkgname),$$(call UPPERCASE,$(pkgname)),$$(call UPPERCASE,$(pkgname)),target)
> +host-autotools-package = $(call inner-autotools-package,host-$(pkgname),$$(call UPPERCASE,host-$(pkgname)),$$(call UPPERCASE,$(pkgname)),host)

 The reason not to double-$ $(pkgname) is not that it doesn't work (at least it
works for me with printvars), but that it increases the make parsing time with a
factor four (because $(pkgname) gets evaluated very often).

 IMHO, mixing $ and $$ doesn't make things clearer. Therefore, I would leave
everything single-$ here and add a comment

# Using $$ for $(pkgname) is bad for performance because it is evaluated so
# often. Therefore, we use single $ for the arguments here.

(and same for all other outer functions).

> diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
> --- a/package/pkg-cmake.mk
> +++ b/package/pkg-cmake.mk
> @@ -38,14 +38,14 @@ define inner-cmake-package
>  
>  $(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,12 +60,12 @@ ifeq ($(4),target)
>  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_INSTALL_PREFIX="/usr" \
>  		-DCMAKE_COLOR_MAKEFILE=OFF \
> -		-DBUILD_SHARED_LIBS=$(if $(BR2_PREFER_STATIC_LIB),OFF,ON) \
> +		-DBUILD_SHARED_LIBS=$$(if $$(BR2_PREFER_STATIC_LIB),OFF,ON) \
>  		$$($$(PKG)_CONF_OPT) \
>  	)
>  endef
> @@ -75,8 +75,8 @@ else
>  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" \
> @@ -91,7 +91,9 @@ endif
>  
>  # 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 ($$(origin $(2)_DEPENDENCIES),undefined)

ifeq ($(4),host)

> +$(2)_DEPENDENCIES := $$(filter-out host-toolchain $(1),$$(patsubst host-host-%,host-%,$$(addprefix host-,$$($(3)_DEPENDENCIES))))
> +endif
>  
>  $(2)_DEPENDENCIES += host-cmake
>  
> @@ -102,11 +104,11 @@ endif
>  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)

 Unrelated to this patch, does anyone know why we use $(PKG) here instead of $(2) ?

>  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
> @@ -117,7 +119,7 @@ endif
>  #
>  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
>  
> @@ -127,7 +129,7 @@ endif
>  #
>  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
>  
> @@ -137,7 +139,7 @@ endif
>  #
>  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
>  
> @@ -151,8 +153,8 @@ endef
>  # cmake-package -- the target generator macro for CMake packages
>  ################################################################################
>  
> -cmake-package = $(call inner-cmake-package,$(pkgname),$(call UPPERCASE,$(pkgname)),$(call UPPERCASE,$(pkgname)),target)
> -host-cmake-package = $(call inner-cmake-package,host-$(pkgname),$(call UPPERCASE,host-$(pkgname)),$(call UPPERCASE,$(pkgname)),host)
> +cmake-package = $(call inner-cmake-package,$(pkgname),$$(call UPPERCASE,$(pkgname)),$$(call UPPERCASE,$(pkgname)),target)
> +host-cmake-package = $(call inner-cmake-package,host-$(pkgname),$$(call UPPERCASE,host-$(pkgname)),$$(call UPPERCASE,$(pkgname)),host)

 Same here: keep single $, add comment.

>  
>  ################################################################################
>  # Generation of the CMake toolchain file
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -263,7 +263,7 @@ define inner-generic-package
>  
>  $(2)_TYPE                       =  $(4)
>  $(2)_NAME			=  $(1)
> -$(2)_RAWNAME			=  $(patsubst host-%,%,$(1))
> +$(2)_RAWNAME			=  $$(patsubst host-%,%,$(1))
>  
>  # Keep the package version that may contain forward slashes in the _DL_VERSION
>  # variable, then replace all forward slashes ('/') by underscores ('_') to
> @@ -272,15 +272,15 @@ define inner-generic-package
>  # version control system branch or tag, for example remotes/origin/1_10_stable.
>  ifndef $(2)_VERSION
>   ifdef $(3)_VERSION
> -  $(2)_DL_VERSION = $($(3)_VERSION)
> -  $(2)_VERSION = $(subst /,_,$($(3)_VERSION))
> +  $(2)_DL_VERSION = $$($(3)_VERSION)
> +  $(2)_VERSION := $$(subst /,_,$$($(3)_VERSION))
>   else
>    $(2)_VERSION = undefined
>    $(2)_DL_VERSION = undefined
>   endif
>  else
> -  $(2)_DL_VERSION = $($(2)_VERSION)
> -  $(2)_VERSION = $(subst /,_,$($(2)_VERSION))
> +  $(2)_DL_VERSION = $$($(2)_VERSION)
> +  $(2)_VERSION := $$(subst /,_,$$($(2)_VERSION))
>  endif
>  
>  $(2)_BASE_NAME	=  $(1)-$$($(2)_VERSION)
> @@ -304,7 +304,7 @@ endif
>  
>  ifndef $(2)_SOURCE
>   ifdef $(3)_SOURCE
> -  $(2)_SOURCE = $($(3)_SOURCE)
> +  $(2)_SOURCE = $$($(3)_SOURCE)
>   else
>    $(2)_SOURCE			?= $$($(2)_RAWNAME)-$$($(2)_VERSION).tar.gz
>   endif
> @@ -312,22 +312,22 @@ endif
>  
>  ifndef $(2)_PATCH
>   ifdef $(3)_PATCH
> -  $(2)_PATCH = $($(3)_PATCH)
> +  $(2)_PATCH = $$($(3)_PATCH)
>   endif
>  endif
>  
>  ifndef $(2)_SITE
>   ifdef $(3)_SITE
> -  $(2)_SITE = $($(3)_SITE)
> +  $(2)_SITE = $$($(3)_SITE)
>   endif
>  endif
>  
>  ifndef $(2)_SITE_METHOD
>   ifdef $(3)_SITE_METHOD
> -  $(2)_SITE_METHOD = $($(3)_SITE_METHOD)
> +  $(2)_SITE_METHOD = $$($(3)_SITE_METHOD)
>   else
>  	# Try automatic detection using the scheme part of the URI
> -	$(2)_SITE_METHOD = $(call geturischeme,$($(2)_SITE))
> +	$(2)_SITE_METHOD = $$(call geturischeme,$$($(2)_SITE))
>   endif
>  endif
>  
> @@ -339,7 +339,7 @@ endif
>  
>  ifndef $(2)_LICENSE
>   ifdef $(3)_LICENSE
> -  $(2)_LICENSE = $($(3)_LICENSE)
> +  $(2)_LICENSE = $$($(3)_LICENSE)
>   endif
>  endif
>  
> @@ -347,13 +347,13 @@ endif
>  
>  ifndef $(2)_LICENSE_FILES
>   ifdef $(3)_LICENSE_FILES
> -  $(2)_LICENSE_FILES = $($(3)_LICENSE_FILES)
> +  $(2)_LICENSE_FILES = $$($(3)_LICENSE_FILES)
>   endif
>  endif
>  
>  ifndef $(2)_REDISTRIBUTE
>   ifdef $(3)_REDISTRIBUTE
> -  $(2)_REDISTRIBUTE = $($(3)_REDISTRIBUTE)
> +  $(2)_REDISTRIBUTE = $$($(3)_REDISTRIBUTE)
>   endif
>  endif
>  
> @@ -364,8 +364,10 @@ endif
>  # dependency
>  $(2)_ADD_TOOLCHAIN_DEPENDENCY	?= YES
>  
> -$(2)_DEPENDENCIES ?= $(filter-out  host-toolchain $(1),\
> -	$(patsubst host-host-%,host-%,$(addprefix host-,$($(3)_DEPENDENCIES))))
> +ifeq ($$(origin $(2)_DEPENDENCIES),undefined)

 ifeq ($(4),host)

> +$(2)_DEPENDENCIES := $$(filter-out  host-toolchain $(1),\
> +	$$(patsubst host-host-%,host-%,$$(addprefix host-,$$($(3)_DEPENDENCIES))))
> +endif
>  ifeq ($(4),target)
>  ifeq ($$($(2)_ADD_TOOLCHAIN_DEPENDENCY),YES)
>  $(2)_DEPENDENCIES += toolchain
> @@ -392,8 +394,8 @@ endif
>  
>  # default extract command
>  $(2)_EXTRACT_CMDS ?= \
> -	$$(if $$($(2)_SOURCE),$$(INFLATE$$(suffix $$($(2)_SOURCE))) $(DL_DIR)/$$($(2)_SOURCE) | \
> -	$(TAR) $(TAR_STRIP_COMPONENTS)=1 -C $$($(2)_DIR) $(TAR_OPTIONS) -)
> +	$$(if $$($(2)_SOURCE),$$(INFLATE$$(suffix $$($(2)_SOURCE))) $$(DL_DIR)/$$($(2)_SOURCE) | \
> +	$$(TAR) $$(TAR_STRIP_COMPONENTS)=1 -C $$($(2)_DIR) $$(TAR_OPTIONS) -)
>  
>  # pre/post-steps hooks
>  $(2)_PRE_DOWNLOAD_HOOKS         ?=
> @@ -467,7 +469,7 @@ endif
>  $$($(2)_TARGET_CONFIGURE):	| $$($(2)_DEPENDENCIES)
>  
>  $$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | dirs prepare
> -ifeq ($(filter $(1),$(DEPENDENCIES_HOST_PREREQ)),)
> +ifeq ($$(filter $(1),$$(DEPENDENCIES_HOST_PREREQ)),)
>  $$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | dependencies
>  endif
>  
> @@ -510,11 +512,11 @@ endif
>  			@echo $$($(2)_DEPENDENCIES)
>  
>  $(1)-graph-depends:
> -			@$(INSTALL) -d $(O)/graphs
> -			@cd "$(CONFIG_DIR)"; \
> -			$(TOPDIR)/support/scripts/graph-depends -p $(1) -d $(BR_GRAPH_DEPTH) \
> -			|tee $(O)/graphs/$$(@).dot \
> -			|dot -T$(BR_GRAPH_OUT) -o $(O)/graphs/$$(@).$(BR_GRAPH_OUT)
> +			@$$(INSTALL) -d $$(O)/graphs
> +			@cd "$$(CONFIG_DIR)"; \
> +			$$(TOPDIR)/support/scripts/graph-depends -p $(1) -d $$(BR_GRAPH_DEPTH) \
> +			|tee $$(O)/graphs/$$(@).dot \
> +			|dot -T$$(BR_GRAPH_OUT) -o $$(O)/graphs/$$(@).$$(BR_GRAPH_OUT)
>  
>  $(1)-dirclean:		$$($(2)_TARGET_DIRCLEAN)
>  
> @@ -548,7 +550,7 @@ endif
>  $$($(2)_TARGET_RSYNC_SOURCE):		SRCDIR=$$($(2)_OVERRIDE_SRCDIR)
>  $$($(2)_TARGET_RSYNC_SOURCE):		PKG=$(2)
>  $$($(2)_TARGET_PATCH):			PKG=$(2)
> -$$($(2)_TARGET_PATCH):			RAWNAME=$(patsubst host-%,%,$(1))
> +$$($(2)_TARGET_PATCH):			RAWNAME=$$(patsubst host-%,%,$(1))
>  $$($(2)_TARGET_PATCH):			PKGDIR=$(pkgdir)
>  $$($(2)_TARGET_EXTRACT):		PKG=$(2)
>  $$($(2)_TARGET_SOURCE):			PKG=$(2)
> @@ -559,9 +561,9 @@ endif
>  # kernel case, the bootloaders case, and the normal packages case.
>  ifeq ($(1),linux)
>  $(2)_KCONFIG_VAR = BR2_LINUX_KERNEL
> -else ifneq ($(filter boot/%,$(pkgdir)),)
> +else ifneq ($$(filter boot/%,$(pkgdir)),)

 $$(pkgdir) works fine here. Also, it gets evaluated only once anyway, so
there's no speedup from using a single $.

>  $(2)_KCONFIG_VAR = BR2_TARGET_$(2)
> -else ifneq ($(filter toolchain/%,$(pkgdir)),)
> +else ifneq ($$(filter toolchain/%,$(pkgdir)),)
>  $(2)_KCONFIG_VAR = BR2_$(2)
>  else
>  $(2)_KCONFIG_VAR = BR2_PACKAGE_$(2)
> @@ -577,7 +579,7 @@ ifeq ($$($(2)_REDISTRIBUTE),YES)
>  ifneq ($$($(2)_SITE_METHOD),local)
>  ifneq ($$($(2)_SITE_METHOD),override)
>  # Packages that have a tarball need it downloaded and extracted beforehand
> -$(1)-legal-info: $(1)-extract $(REDIST_SOURCES_DIR_$(call UPPERCASE,$(4)))
> +$(1)-legal-info: $(1)-extract $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))
>  $(2)_MANIFEST_TARBALL = $$($(2)_SOURCE)
>  endif
>  endif
> @@ -587,40 +589,40 @@ endif
>  # legal-info: produce legally relevant info.
>  $(1)-legal-info:

 Have you also tested a 'make legal-info'? This whole thing is rather fragile so
we may easily miss something by just reviewing. Also, printvars doesn't show the
contents of this rule (because it's not a variable). 'make -qp' does, however,
so that would also be a good check (it may reorder things, though, so it may not
be easy to diff).

>  # Packages without a source are assumed to be part of Buildroot, skip them.
> -	$(foreach hook,$($(2)_PRE_LEGAL_INFO_HOOKS),$(call $(hook))$(sep))
> -ifneq ($(call qstrip,$$($(2)_SOURCE)),)
> +	$$(foreach hook,$$($(2)_PRE_LEGAL_INFO_HOOKS),$$(call $$(hook))$$(sep))
> +ifneq ($$(call qstrip,$$($(2)_SOURCE)),)
>  
>  ifeq ($$($(2)_SITE_METHOD),local)
>  # Packages without a tarball: don't save and warn
> -	@$(call legal-warning-pkg-savednothing,$$($(2)_RAWNAME),local)
> +	@$$(call legal-warning-pkg-savednothing,$$($(2)_RAWNAME),local)
>  
>  else ifneq ($$($(2)_OVERRIDE_SRCDIR),)
> -	@$(call legal-warning-pkg-savednothing,$$($(2)_RAWNAME),override)
> +	@$$(call legal-warning-pkg-savednothing,$$($(2)_RAWNAME),override)
>  
>  else
>  # Other packages
>  
>  # Save license files if defined
> -ifeq ($(call qstrip,$$($(2)_LICENSE_FILES)),)

 Huh, this original wouldn't even have worked (because it would strip the quotes
from the variable name, not from its contents).

> -	@$(call legal-license-nofiles,$$($(2)_RAWNAME),$(call UPPERCASE,$(4)))
> -	@$(call legal-warning-pkg,$$($(2)_RAWNAME),cannot save license ($(2)_LICENSE_FILES not defined))
> +ifeq ($$(call qstrip,$$($(2)_LICENSE_FILES)),)
> +	@$$(call legal-license-nofiles,$$($(2)_RAWNAME),$$(call UPPERCASE,$(4)))
> +	@$$(call legal-warning-pkg,$$($(2)_RAWNAME),cannot save license ($(2)_LICENSE_FILES not defined))
>  else
>  # Double dollar signs are really needed here, to catch host packages
>  # without explicit HOST_FOO_LICENSE_FILES assignment, also in case they
>  # have multiple license files.

 I think you can remove this comment now :-)

> -	@$$(foreach F,$$($(2)_LICENSE_FILES),$$(call legal-license-file,$$($(2)_RAWNAME),$$(F),$$($(2)_DIR)/$$(F),$(call UPPERCASE,$(4)))$$(sep))
> +	@$$(foreach F,$$($(2)_LICENSE_FILES),$$(call legal-license-file,$$($(2)_RAWNAME),$$(F),$$($(2)_DIR)/$$(F),$$(call UPPERCASE,$(4)))$$(sep))
>  endif # license files
>  
>  ifeq ($$($(2)_REDISTRIBUTE),YES)
>  # Copy the source tarball (just hardlink if possible)
> -	@cp -l $(DL_DIR)/$$($(2)_SOURCE) $(REDIST_SOURCES_DIR_$(call UPPERCASE,$(4))) 2>/dev/null || \
> -	   cp $(DL_DIR)/$$($(2)_SOURCE) $(REDIST_SOURCES_DIR_$(call UPPERCASE,$(4)))
> +	@cp -l $$(DL_DIR)/$$($(2)_SOURCE) $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4))) 2>/dev/null || \
> +	   cp $$(DL_DIR)/$$($(2)_SOURCE) $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))
>  endif # redistribute
>  
>  endif # other packages
> -	@$(call legal-manifest,$$($(2)_RAWNAME),$$($(2)_VERSION),$$($(2)_LICENSE),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_MANIFEST_TARBALL),$(call UPPERCASE,$(4)))
> -endif # ifneq ($(call qstrip,$$($(2)_SOURCE)),)
> -	$(foreach hook,$($(2)_POST_LEGAL_INFO_HOOKS),$(call $(hook))$(sep))
> +	@$$(call legal-manifest,$$($(2)_RAWNAME),$$($(2)_VERSION),$$($(2)_LICENSE),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_MANIFEST_TARBALL),$$(call UPPERCASE,$(4)))
> +endif # ifneq ($$(call qstrip,$$($(2)_SOURCE)),)
> +	$$(foreach hook,$$($(2)_POST_LEGAL_INFO_HOOKS),$$(call $$(hook))$$(sep))
>  
>  # add package to the general list of targets if requested by the buildroot
>  # configuration
> @@ -649,8 +651,8 @@ endif # SITE_METHOD
>  # ZCAT="gzip -d -c", and to check for the dependency we only want 'gzip'.
>  # Do not add xzcat to the list of required dependencies, as it gets built
>  # automatically if it isn't found.
> -ifneq ($(call suitable-extractor,$($(2)_SOURCE)),$(XZCAT))
> -DL_TOOLS_DEPENDENCIES += $(firstword $(call suitable-extractor,$($(2)_SOURCE)))
> +ifneq ($$(call suitable-extractor,$$($(2)_SOURCE)),$$(XZCAT))
> +DL_TOOLS_DEPENDENCIES += $$(firstword $$(call suitable-extractor,$$($(2)_SOURCE)))
>  endif
>  
>  endif # $(2)_KCONFIG_VAR
> @@ -661,8 +663,8 @@ endef # inner-generic-package
>  ################################################################################
>  
>  # In the case of target packages, keep the package name "pkg"
> -generic-package = $(call inner-generic-package,$(pkgname),$(call UPPERCASE,$(pkgname)),$(call UPPERCASE,$(pkgname)),target)
> +generic-package = $(call inner-generic-package,$(pkgname),$$(call UPPERCASE,$(pkgname)),$$(call UPPERCASE,$(pkgname)),target)
>  # In the case of host packages, turn the package name "pkg" into "host-pkg"
> -host-generic-package = $(call inner-generic-package,host-$(pkgname),$(call UPPERCASE,host-$(pkgname)),$(call UPPERCASE,$(pkgname)),host)
> +host-generic-package = $(call inner-generic-package,host-$(pkgname),$$(call UPPERCASE,host-$(pkgname)),$$(call UPPERCASE,$(pkgname)),host)

 Keep original, add comment.

>  
>  # :mode=makefile:
> diff --git a/package/pkg-luarocks.mk b/package/pkg-luarocks.mk
> --- a/package/pkg-luarocks.mk
> +++ b/package/pkg-luarocks.mk
> @@ -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 @@ define inner-luarocks-package
>  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
>  
> @@ -77,5 +77,5 @@ endef
>  # luarocks-package -- the target generator macro for LuaRocks packages
>  ################################################################################
>  
> -luarocks-package = $(call inner-luarocks-package,$(pkgname),$(call UPPERCASE,$(pkgname)),$(call UPPERCASE,$(pkgname)),target)
> +luarocks-package = $(call inner-luarocks-package,$(pkgname),$$(call UPPERCASE,$(pkgname)),$$(call UPPERCASE,$(pkgname)),target)

 Keep original, add comment.

>  # host-luarocks-package not supported
> diff --git a/package/pkg-perl.mk b/package/pkg-perl.mk
> --- a/package/pkg-perl.mk
> +++ b/package/pkg-perl.mk
> @@ -51,18 +51,18 @@ define $(2)_CONFIGURE_CMDS
>  	cd $$($$(PKG)_SRCDIR) && if [ -f Build.PL ] ; then \
>  		PERL_MM_USE_DEFAULT=1 \
>  		perl Build.PL \
> -			--config ar="$(TARGET_AR)" \
> -			--config full_ar="$(TARGET_AR)" \
> -			--config cc="$(TARGET_CC)" \
> -			--config ccflags="$(TARGET_CFLAGS)" \
> -			--config ld="$(TARGET_CC)" \
> -			--config lddlflags="-shared $(TARGET_LDFLAGS)" \
> -			--config ldflags="$(TARGET_LDFLAGS)" \
> -			--include_dirs $$(STAGING_DIR)/usr/lib/perl5/$$(PERL_VERSION)/$(PERL_ARCHNAME)/CORE \
> +			--config ar="$$(TARGET_AR)" \
> +			--config full_ar="$$(TARGET_AR)" \
> +			--config cc="$$(TARGET_CC)" \
> +			--config ccflags="$$(TARGET_CFLAGS)" \
> +			--config ld="$$(TARGET_CC)" \
> +			--config lddlflags="-shared $$(TARGET_LDFLAGS)" \
> +			--config ldflags="$$(TARGET_LDFLAGS)" \
> +			--include_dirs $$(STAGING_DIR)/usr/lib/perl5/$$(PERL_VERSION)/$$(PERL_ARCHNAME)/CORE \
>  			--destdir $$(TARGET_DIR) \
>  			--installdirs vendor \
>  			--install_path lib=/usr/lib/perl5/site_perl/$$(PERL_VERSION) \
> -			--install_path arch=/usr/lib/perl5/site_perl/$$(PERL_VERSION)/$(PERL_ARCHNAME) \
> +			--install_path arch=/usr/lib/perl5/site_perl/$$(PERL_VERSION)/$$(PERL_ARCHNAME) \
>  			--install_path bin=/usr/bin \
>  			--install_path script=/usr/bin \
>  			--install_path bindoc=/usr/share/man/man1 \
> @@ -72,17 +72,17 @@ define $(2)_CONFIGURE_CMDS
>  		PERL_MM_USE_DEFAULT=1 \
>  		PERL_AUTOINSTALL=--skipdeps \
>  		perl Makefile.PL \
> -			AR="$(TARGET_AR)" \
> -			FULL_AR="$(TARGET_AR)" \
> -			CC="$(TARGET_CC)" \
> -			CCFLAGS="$(TARGET_CFLAGS)" \
> -			LD="$(TARGET_CC)" \
> -			LDDLFLAGS="-shared $(TARGET_LDFLAGS)" \
> -			LDFLAGS="$(TARGET_LDFLAGS)" \
> +			AR="$$(TARGET_AR)" \
> +			FULL_AR="$$(TARGET_AR)" \
> +			CC="$$(TARGET_CC)" \
> +			CCFLAGS="$$(TARGET_CFLAGS)" \
> +			LD="$$(TARGET_CC)" \
> +			LDDLFLAGS="-shared $$(TARGET_LDFLAGS)" \
> +			LDFLAGS="$$(TARGET_LDFLAGS)" \
>  			DESTDIR=$$(TARGET_DIR) \
>  			INSTALLDIRS=vendor \
>  			INSTALLVENDORLIB=/usr/lib/perl5/site_perl/$$(PERL_VERSION) \
> -			INSTALLVENDORARCH=/usr/lib/perl5/site_perl/$$(PERL_VERSION)/$(PERL_ARCHNAME) \
> +			INSTALLVENDORARCH=/usr/lib/perl5/site_perl/$$(PERL_VERSION)/$$(PERL_ARCHNAME) \
>  			INSTALLVENDORBIN=/usr/bin \
>  			INSTALLVENDORSCRIPT=/usr/bin \
>  			INSTALLVENDORMAN1DIR=/usr/share/man/man1 \
> @@ -125,8 +125,8 @@ define $(2)_BUILD_CMDS
>  	cd $$($$(PKG)_SRCDIR) && if [ -f Build.PL ] ; then \
>  		perl Build $$($(2)_BUILD_OPT) build; \
>  	else \
> -		$(MAKE1) \
> -			PERL_INC=$$(STAGING_DIR)/usr/lib/perl5/$$(PERL_VERSION)/$(PERL_ARCHNAME)/CORE \
> +		$$(MAKE1) \
> +			PERL_INC=$$(STAGING_DIR)/usr/lib/perl5/$$(PERL_VERSION)/$$(PERL_ARCHNAME)/CORE \
>  			$$($(2)_BUILD_OPT) pure_all; \
>  	fi
>  endef
> @@ -137,7 +137,7 @@ define $(2)_BUILD_CMDS
>  	cd $$($$(PKG)_SRCDIR) && if [ -f Build.PL ] ; then \
>  		perl Build $$($(2)_BUILD_OPT) build; \
>  	else \
> -		$(MAKE1) $$($(2)_BUILD_OPT) pure_all; \
> +		$$(MAKE1) $$($(2)_BUILD_OPT) pure_all; \
>  	fi
>  endef
>  endif
> @@ -152,7 +152,7 @@ define $(2)_INSTALL_CMDS
>  	cd $$($$(PKG)_SRCDIR) && if [ -f Build.PL ] ; then \
>  		perl Build $$($(2)_INSTALL_TARGET_OPT) install; \
>  	else \
> -		$(MAKE1) $$($(2)_INSTALL_TARGET_OPT) pure_install; \
> +		$$(MAKE1) $$($(2)_INSTALL_TARGET_OPT) pure_install; \
>  	fi
>  endef
>  endif
> @@ -166,7 +166,7 @@ define $(2)_INSTALL_TARGET_CMDS
>  	cd $$($$(PKG)_SRCDIR) && if [ -f Build.PL ] ; then \
>  		perl Build $$($(2)_INSTALL_TARGET_OPT) install; \
>  	else \
> -		$(MAKE1) $$($(2)_INSTALL_TARGET_OPT) pure_install; \
> +		$$(MAKE1) $$($(2)_INSTALL_TARGET_OPT) pure_install; \
>  	fi
>  endef
>  endif
> @@ -181,5 +181,5 @@ endef
>  # perl-package -- the target generator macro for Perl packages
>  ################################################################################
>  
> -perl-package = $(call inner-perl-package,$(pkgname),$(call UPPERCASE,$(pkgname)),$(call UPPERCASE,$(pkgname)),target)
> -host-perl-package = $(call inner-perl-package,host-$(pkgname),$(call UPPERCASE,host-$(pkgname)),$(call UPPERCASE,$(pkgname)),host)
> +perl-package = $(call inner-perl-package,$(pkgname),$$(call UPPERCASE,$(pkgname)),$$(call UPPERCASE,$(pkgname)),target)
> +host-perl-package = $(call inner-perl-package,host-$(pkgname),$$(call UPPERCASE,host-$(pkgname)),$$(call UPPERCASE,$(pkgname)),host)

 Keep original, add comment.

> diff --git a/package/pkg-python.mk b/package/pkg-python.mk
> --- a/package/pkg-python.mk
> +++ b/package/pkg-python.mk
> @@ -82,7 +82,7 @@ HOST_PKG_PYTHON_SETUPTOOLS_INSTALL_OPT =
>  
>  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 @@ define inner-python-package
>  
>  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 @@ endif
>  # 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 ($$(origin $(2)_DEPENDENCIES),undefined)

ifeq($(4),host)

 But maybe that doesn't work here. Or maybe you do need the := here. Although I
doubt it.

> +$(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 @@ endif
>  #   - 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 @@ endif
>  #   - 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
>  
> @@ -255,5 +257,5 @@ endef
>  # python-package -- the target generator macro for Python packages
>  ################################################################################
>  
> -python-package = $(call inner-python-package,$(pkgname),$(call UPPERCASE,$(pkgname)),$(call UPPERCASE,$(pkgname)),target)
> -host-python-package = $(call inner-python-package,host-$(pkgname),$(call UPPERCASE,host-$(pkgname)),$(call UPPERCASE,$(pkgname)),host)
> +python-package = $(call inner-python-package,$(pkgname),$$(call UPPERCASE,$(pkgname)),$$(call UPPERCASE,$(pkgname)),target)
> +host-python-package = $(call inner-python-package,host-$(pkgname),$$(call UPPERCASE,host-$(pkgname)),$$(call UPPERCASE,$(pkgname)),host)
> diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
> --- a/package/pkg-utils.mk
> +++ b/package/pkg-utils.mk
> @@ -19,13 +19,13 @@
>  define caseconvert-helper
>  $(1) = $$(strip \
>  	$$(eval __tmp := $$(1))\
> -	$(foreach c, $(2),\
> -		$$(eval __tmp := $$(subst $(word 1,$(subst :, ,$c)),$(word 2,$(subst :, ,$c)),$$(__tmp))))\
> +	$$(foreach c, $(2),\
> +		$$(eval __tmp := $$(subst $$(word 1,$$(subst :, ,$$(c))),$$(word 2,$$(subst :, ,$$(c))),$$(__tmp))))\
>  	$$(__tmp))
>  endef
>  
> -$(eval $(call caseconvert-helper,UPPERCASE,$(join $(addsuffix :,$([FROM])),$([TO]))))
> -$(eval $(call caseconvert-helper,LOWERCASE,$(join $(addsuffix :,$([TO])),$([FROM]))))
> +$(eval $(call caseconvert-helper,UPPERCASE,$$(join $$(addsuffix :,$$([FROM])),$$([TO]))))
> +$(eval $(call caseconvert-helper,LOWERCASE,$$(join $$(addsuffix :,$$([TO])),$$([FROM]))))

 Actually, here the single $-es are intentional. It is chosen in order to
maximize the speed-up, that's why you see this counter-intuitive mix of $ and
$$. Maybe you can extend the comment above a little to explain that a little
better. Or I can do that as well if you prefer.

>  
>  #
>  # Manipulation of .config files based on the Kconfig
> diff --git a/package/pkg-virtual.mk b/package/pkg-virtual.mk
> --- a/package/pkg-virtual.mk
> +++ b/package/pkg-virtual.mk
> @@ -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 @@ HOST_$(3)_VERSION = virtual
>  
>  # 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 ($$(origin $(2)_DEPENDENCIES),undefined)

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
> @@ -66,5 +68,5 @@ endef
>  # virtual-package -- the target generator macro for virtual packages
>  ################################################################################
>  
> -virtual-package = $(call inner-virtual-package,$(pkgname),$(call UPPERCASE,$(pkgname)),$(call UPPERCASE,$(pkgname)),target)
> -host-virtual-package = $(call inner-virtual-package,host-$(pkgname),$(call UPPERCASE,host-$(pkgname)),$(call UPPERCASE,$(pkgname)),host)
> +virtual-package = $(call inner-virtual-package,$(pkgname),$$(call UPPERCASE,$(pkgname)),$$(call UPPERCASE,$(pkgname)),target)
> +host-virtual-package = $(call inner-virtual-package,host-$(pkgname),$$(call UPPERCASE,host-$(pkgname)),$$(call UPPERCASE,$(pkgname)),host)
> 

 Keep original + comment.


 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Buildroot] [PATCH 1 of 7 v2] infra: consistently use double dollar signs inside inner-xxx-targets
  2014-05-13  9:25   ` Arnout Vandecappelle
@ 2014-05-13 10:59     ` Thomas De Schampheleire
  2014-05-13 19:33       ` Arnout Vandecappelle
  2014-05-21 13:14     ` Thomas De Schampheleire
  1 sibling, 1 reply; 13+ messages in thread
From: Thomas De Schampheleire @ 2014-05-13 10:59 UTC (permalink / raw)
  To: buildroot

Hi Arnout,

On Tue, May 13, 2014 at 11:25 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
>  Hi Thomas,
>
>  Thanks for taking upon you this gruelling task :-)

Thanks for the review!

>
> On 12/05/14 16:44, Thomas De Schampheleire wrote:
[..]
>>
>> Note: in addition to using 'make printvars' to verify this patch, a 'make
>> randpackageconfig' was performed successfully.
>
>  Perhaps you should do a 'make manual' as well.

Will do (I did it already, but did not diffed the output with the orig
version, it just 'looked' ok).

>
>>
>> diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk
>> --- a/docs/manual/manual.mk
>> +++ b/docs/manual/manual.mk
>> @@ -54,16 +54,16 @@ define GENDOC_INNER
>
>  I would add a brief comment in the beginning of GENDOC_INNER:
>
> # Since this function will be called from within an $(eval ...)
> # all variable references except the arguments must be $$-quoted.
>

Ok (same for similar comments below)

[..]

>>
>> +ifeq ($$(origin $(2)_AUTORECONF_OPT),undefined)
>> + $(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)
>
>  Wouldn't it be clearer to keep this as a ?= but put it inside an
> ifeq ($(4),host) ? I haven't tested whether that works, however.

It's probably clearer (if it works, to test).
But then all of the 'use-target-var-if-host-var-is-not-provided' could
be reworked using this same mechanism, and make the entire inner
target more clear, right?

[..]

>>
>> -autotools-package = $(call inner-autotools-package,$(pkgname),$(call UPPERCASE,$(pkgname)),$(call UPPERCASE,$(pkgname)),target)
>> -host-autotools-package = $(call inner-autotools-package,host-$(pkgname),$(call UPPERCASE,host-$(pkgname)),$(call UPPERCASE,$(pkgname)),host)
>> +autotools-package = $(call inner-autotools-package,$(pkgname),$$(call UPPERCASE,$(pkgname)),$$(call UPPERCASE,$(pkgname)),target)
>> +host-autotools-package = $(call inner-autotools-package,host-$(pkgname),$$(call UPPERCASE,host-$(pkgname)),$$(call UPPERCASE,$(pkgname)),host)
>
>  The reason not to double-$ $(pkgname) is not that it doesn't work (at least it
> works for me with printvars), but that it increases the make parsing time with a
> factor four (because $(pkgname) gets evaluated very often).

I don't agree, in my test it does _not_ work correctly with $$(pkgname).
If I make that change in pkg-generic, and try to build busybox, I get
as build log output:

tdescham at argentina ~/repo/contrib/buildroot-misc $ make busybox-dirclean busybox
rm -Rf /home/tdescham/repo/contrib/buildroot-misc/output/build/busybox-1.22.1
>>>   Extracting
>>>   Patching
>>>   Configuring
>>>   Building
>>>   Installing to target

(so without the correct pkgname) and no actual steps are performed.

>
>  IMHO, mixing $ and $$ doesn't make things clearer. Therefore, I would leave
> everything single-$ here and add a comment
>
> # Using $$ for $(pkgname) is bad for performance because it is evaluated so
> # often. Therefore, we use single $ for the arguments here.
>
> (and same for all other outer functions).

So according to me this comment is not correct.
I do agree that the mixing is not more clear than using single $ in
the outer functions, but it is less consistent with the general rules.
I have no strong opinion on what to do here.

[..]

>>
>> @@ -102,11 +104,11 @@ endif
>>  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)
>
>  Unrelated to this patch, does anyone know why we use $(PKG) here instead of $(2) ?

In pkg-generic.mk, the targets are _outside_ the
inner-generic-targets. Therefore $(2) is not available. For this
reason, PKG is set to $(2) in the context of these rules.
In the other package infrastructures, this principle is not used: the
targets are inside the inner target, so both PKG and $(2) could be
used.
My assumption is that this is due to copy/paste.

[..]

>>  # kernel case, the bootloaders case, and the normal packages case.
>>  ifeq ($(1),linux)
>>  $(2)_KCONFIG_VAR = BR2_LINUX_KERNEL
>> -else ifneq ($(filter boot/%,$(pkgdir)),)
>> +else ifneq ($$(filter boot/%,$(pkgdir)),)
>
>  $$(pkgdir) works fine here. Also, it gets evaluated only once anyway, so
> there's no speedup from using a single $.
>

What to do here given the fact that speedup is not the only issue?

[..]

>>  # legal-info: produce legally relevant info.
>>  $(1)-legal-info:
>
>  Have you also tested a 'make legal-info'? This whole thing is rather fragile so
> we may easily miss something by just reviewing. Also, printvars doesn't show the
> contents of this rule (because it's not a variable). 'make -qp' does, however,
> so that would also be a good check (it may reorder things, though, so it may not
> be easy to diff).

I ran it, and output seemed ok, but I should actually diff the output
with a known-good one to be 100% sure.
I will do that.

[..]

>>  else
>>  # Double dollar signs are really needed here, to catch host packages
>>  # without explicit HOST_FOO_LICENSE_FILES assignment, also in case they
>>  # have multiple license files.
>
>  I think you can remove this comment now :-)

OK.

[..]

>> diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
>> --- a/package/pkg-utils.mk
>> +++ b/package/pkg-utils.mk
>> @@ -19,13 +19,13 @@
>>  define caseconvert-helper
>>  $(1) = $$(strip \
>>       $$(eval __tmp := $$(1))\
>> -     $(foreach c, $(2),\
>> -             $$(eval __tmp := $$(subst $(word 1,$(subst :, ,$c)),$(word 2,$(subst :, ,$c)),$$(__tmp))))\
>> +     $$(foreach c, $(2),\
>> +             $$(eval __tmp := $$(subst $$(word 1,$$(subst :, ,$$(c))),$$(word 2,$$(subst :, ,$$(c))),$$(__tmp))))\
>>       $$(__tmp))
>>  endef
>>
>> -$(eval $(call caseconvert-helper,UPPERCASE,$(join $(addsuffix :,$([FROM])),$([TO]))))
>> -$(eval $(call caseconvert-helper,LOWERCASE,$(join $(addsuffix :,$([TO])),$([FROM]))))
>> +$(eval $(call caseconvert-helper,UPPERCASE,$$(join $$(addsuffix :,$$([FROM])),$$([TO]))))
>> +$(eval $(call caseconvert-helper,LOWERCASE,$$(join $$(addsuffix :,$$([TO])),$$([FROM]))))
>
>  Actually, here the single $-es are intentional. It is chosen in order to
> maximize the speed-up, that's why you see this counter-intuitive mix of $ and
> $$. Maybe you can extend the comment above a little to explain that a little
> better. Or I can do that as well if you prefer.

If you could write a suitable comment explaining it, that would be
great, as I don't know the ins and outs of this helper.



Best regards,
 Thomas

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Buildroot] [PATCH 1 of 7 v2] infra: consistently use double dollar signs inside inner-xxx-targets
  2014-05-13 10:59     ` Thomas De Schampheleire
@ 2014-05-13 19:33       ` Arnout Vandecappelle
  0 siblings, 0 replies; 13+ messages in thread
From: Arnout Vandecappelle @ 2014-05-13 19:33 UTC (permalink / raw)
  To: buildroot

On 13/05/14 12:59, Thomas De Schampheleire wrote:
> Hi Arnout,
> 
> On Tue, May 13, 2014 at 11:25 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
>>  Hi Thomas,
>>
>>  Thanks for taking upon you this gruelling task :-)
> 
> Thanks for the review!
> 
>>
>> On 12/05/14 16:44, Thomas De Schampheleire wrote:
> [..]
>>>
>>> Note: in addition to using 'make printvars' to verify this patch, a 'make
>>> randpackageconfig' was performed successfully.
>>
>>  Perhaps you should do a 'make manual' as well.
> 
> Will do (I did it already, but did not diffed the output with the orig
> version, it just 'looked' ok).

 If a PDF is created then I think it'll be OK.

[snip]
>>> +ifeq ($$(origin $(2)_AUTORECONF_OPT),undefined)
>>> + $(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)
>>
>>  Wouldn't it be clearer to keep this as a ?= but put it inside an
>> ifeq ($(4),host) ? I haven't tested whether that works, however.
> 
> It's probably clearer (if it works, to test).
> But then all of the 'use-target-var-if-host-var-is-not-provided' could
> be reworked using this same mechanism, and make the entire inner
> target more clear, right?

 Indeed. However, for those you don't need to change the condition tree in order
to make this $$ change, and you still need both the ifdef $(3) and the ifdef
$(2) parts. So I'm not altogether sure if adding an ifeq($(4),host) is really an
improvement.

> 
> [..]
> 
>>>
>>> -autotools-package = $(call inner-autotools-package,$(pkgname),$(call UPPERCASE,$(pkgname)),$(call UPPERCASE,$(pkgname)),target)
>>> -host-autotools-package = $(call inner-autotools-package,host-$(pkgname),$(call UPPERCASE,host-$(pkgname)),$(call UPPERCASE,$(pkgname)),host)
>>> +autotools-package = $(call inner-autotools-package,$(pkgname),$$(call UPPERCASE,$(pkgname)),$$(call UPPERCASE,$(pkgname)),target)
>>> +host-autotools-package = $(call inner-autotools-package,host-$(pkgname),$$(call UPPERCASE,host-$(pkgname)),$$(call UPPERCASE,$(pkgname)),host)
>>
>>  The reason not to double-$ $(pkgname) is not that it doesn't work (at least it
>> works for me with printvars), but that it increases the make parsing time with a
>> factor four (because $(pkgname) gets evaluated very often).
> 
> I don't agree, in my test it does _not_ work correctly with $$(pkgname).

 Strange, I do remember that I tested _something_ and it worked, it was just way
slower.

 Oh, hang on, the first $(pkgname) must not be double-$$-ed because it is used
on the left-hand side of the assignments; the other ones are on the right-hand
side or in conditions or in rules, so there a $$ is possible. In order to be
able to use $$ for the first argument as well, you have to also $$ all the
references to $$(1), $$(2) etc. in inner-generic-package.

> If I make that change in pkg-generic, and try to build busybox, I get
> as build log output:
> 
> tdescham at argentina ~/repo/contrib/buildroot-misc $ make busybox-dirclean busybox
> rm -Rf /home/tdescham/repo/contrib/buildroot-misc/output/build/busybox-1.22.1
>>>>   Extracting
>>>>   Patching
>>>>   Configuring
>>>>   Building
>>>>   Installing to target
> 
> (so without the correct pkgname) and no actual steps are performed.
> 
>>
>>  IMHO, mixing $ and $$ doesn't make things clearer. Therefore, I would leave
>> everything single-$ here and add a comment
>>
>> # Using $$ for $(pkgname) is bad for performance because it is evaluated so
>> # often. Therefore, we use single $ for the arguments here.
>>
>> (and same for all other outer functions).
> 
> So according to me this comment is not correct.

 Yeah, just drop the comment I guess. If we can't give a satisfactory and
correct explanation, it's better not to explain.

> I do agree that the mixing is not more clear than using single $ in
> the outer functions, but it is less consistent with the general rules.
> I have no strong opinion on what to do here.

 I do: since replacing some $ by $$ isn't improving anything, we shouldn't
change it.


[snip]
>>>  # kernel case, the bootloaders case, and the normal packages case.
>>>  ifeq ($(1),linux)
>>>  $(2)_KCONFIG_VAR = BR2_LINUX_KERNEL
>>> -else ifneq ($(filter boot/%,$(pkgdir)),)
>>> +else ifneq ($$(filter boot/%,$(pkgdir)),)
>>
>>  $$(pkgdir) works fine here. Also, it gets evaluated only once anyway, so
>> there's no speedup from using a single $.
>>
> 
> What to do here given the fact that speedup is not the only issue?

 I'd use $$ here as well, so things are consistent. That way, we use $$
everywhere within the inner- macros, except for $(1), $(2), etc.  That's a clear
and simple rule that we can easily use when reviewing future patches.

[snip]

>>> diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
>>> --- a/package/pkg-utils.mk
>>> +++ b/package/pkg-utils.mk
>>> @@ -19,13 +19,13 @@
>>>  define caseconvert-helper
>>>  $(1) = $$(strip \
>>>       $$(eval __tmp := $$(1))\
>>> -     $(foreach c, $(2),\
>>> -             $$(eval __tmp := $$(subst $(word 1,$(subst :, ,$c)),$(word 2,$(subst :, ,$c)),$$(__tmp))))\
>>> +     $$(foreach c, $(2),\
>>> +             $$(eval __tmp := $$(subst $$(word 1,$$(subst :, ,$$(c))),$$(word 2,$$(subst :, ,$$(c))),$$(__tmp))))\
>>>       $$(__tmp))
>>>  endef
>>>
>>> -$(eval $(call caseconvert-helper,UPPERCASE,$(join $(addsuffix :,$([FROM])),$([TO]))))
>>> -$(eval $(call caseconvert-helper,LOWERCASE,$(join $(addsuffix :,$([TO])),$([FROM]))))
>>> +$(eval $(call caseconvert-helper,UPPERCASE,$$(join $$(addsuffix :,$$([FROM])),$$([TO]))))
>>> +$(eval $(call caseconvert-helper,LOWERCASE,$$(join $$(addsuffix :,$$([TO])),$$([FROM]))))
>>
>>  Actually, here the single $-es are intentional. It is chosen in order to
>> maximize the speed-up, that's why you see this counter-intuitive mix of $ and
>> $$. Maybe you can extend the comment above a little to explain that a little
>> better. Or I can do that as well if you prefer.
> 
> If you could write a suitable comment explaining it, that would be
> great, as I don't know the ins and outs of this helper.

 I'll do that as a separate patch. Since you won't be touching this file
anymore, it will not conflict.

 Regards,
 Arnout


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Buildroot] [PATCH 1 of 7 v2] infra: consistently use double dollar signs inside inner-xxx-targets
  2014-05-13  9:25   ` Arnout Vandecappelle
  2014-05-13 10:59     ` Thomas De Schampheleire
@ 2014-05-21 13:14     ` Thomas De Schampheleire
  2014-05-22  0:02       ` Arnout Vandecappelle
  1 sibling, 1 reply; 13+ messages in thread
From: Thomas De Schampheleire @ 2014-05-21 13:14 UTC (permalink / raw)
  To: buildroot

Hi Arnout,

On Tue, May 13, 2014 at 11:25 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
[..]
>>
>> @@ -548,7 +550,7 @@ endif
>>  $$($(2)_TARGET_RSYNC_SOURCE):                SRCDIR=$$($(2)_OVERRIDE_SRCDIR)
>>  $$($(2)_TARGET_RSYNC_SOURCE):                PKG=$(2)
>>  $$($(2)_TARGET_PATCH):                       PKG=$(2)
>> -$$($(2)_TARGET_PATCH):                       RAWNAME=$(patsubst host-%,%,$(1))
>> +$$($(2)_TARGET_PATCH):                       RAWNAME=$$(patsubst host-%,%,$(1))
>>  $$($(2)_TARGET_PATCH):                       PKGDIR=$(pkgdir)
>>  $$($(2)_TARGET_EXTRACT):             PKG=$(2)
>>  $$($(2)_TARGET_SOURCE):                      PKG=$(2)
>> @@ -559,9 +561,9 @@ endif
>>  # kernel case, the bootloaders case, and the normal packages case.
>>  ifeq ($(1),linux)
>>  $(2)_KCONFIG_VAR = BR2_LINUX_KERNEL
>> -else ifneq ($(filter boot/%,$(pkgdir)),)
>> +else ifneq ($$(filter boot/%,$(pkgdir)),)
>
>  $$(pkgdir) works fine here. Also, it gets evaluated only once anyway, so
> there's no speedup from using a single $.
>

While creating v3 of this patch and retesting, I bumped into a nasty
situation regarding pkgdir: the foo-patch rule is executed with the
variable PKGDIR=$(pkgdir) which I changed to PKGDIR=$$(pkgdir).
However, in that case, the foo-patch rule, which is outside
inner-targets, gets executed with the literal $(pkgdir) as PKGDIR,
which is evaluated to docs/manual (the directory of the last makefile
parsed).
As a result, package patching does not occur.

There are two solutions:
- do not double the $$ here, so keep the originial PKGDIR=$(pkgdir)
- use :=, so PKGDIR:=$(pkgdir)

I feel that if we take the first alternative, we should consistently
use $(pkgdir) like that as it is less error-prone to have it as a
general exception, than as an
exception-only-if-you-pass-it-to-non-inner-rules.
All of this is because the pkgdir variable is rather tricky.

Thanks,
Thomas

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Buildroot] [PATCH 1 of 7 v2] infra: consistently use double dollar signs inside inner-xxx-targets
  2014-05-21 13:14     ` Thomas De Schampheleire
@ 2014-05-22  0:02       ` Arnout Vandecappelle
  0 siblings, 0 replies; 13+ messages in thread
From: Arnout Vandecappelle @ 2014-05-22  0:02 UTC (permalink / raw)
  To: buildroot

On 21/05/14 15:14, Thomas De Schampheleire wrote:
> Hi Arnout,
> 
> On Tue, May 13, 2014 at 11:25 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
> [..]
>>>
>>> @@ -548,7 +550,7 @@ endif
>>>  $$($(2)_TARGET_RSYNC_SOURCE):                SRCDIR=$$($(2)_OVERRIDE_SRCDIR)
>>>  $$($(2)_TARGET_RSYNC_SOURCE):                PKG=$(2)
>>>  $$($(2)_TARGET_PATCH):                       PKG=$(2)
>>> -$$($(2)_TARGET_PATCH):                       RAWNAME=$(patsubst host-%,%,$(1))
>>> +$$($(2)_TARGET_PATCH):                       RAWNAME=$$(patsubst host-%,%,$(1))
>>>  $$($(2)_TARGET_PATCH):                       PKGDIR=$(pkgdir)
>>>  $$($(2)_TARGET_EXTRACT):             PKG=$(2)
>>>  $$($(2)_TARGET_SOURCE):                      PKG=$(2)
>>> @@ -559,9 +561,9 @@ endif
>>>  # kernel case, the bootloaders case, and the normal packages case.
>>>  ifeq ($(1),linux)
>>>  $(2)_KCONFIG_VAR = BR2_LINUX_KERNEL
>>> -else ifneq ($(filter boot/%,$(pkgdir)),)
>>> +else ifneq ($$(filter boot/%,$(pkgdir)),)
>>
>>  $$(pkgdir) works fine here. Also, it gets evaluated only once anyway, so
>> there's no speedup from using a single $.
>>
> 
> While creating v3 of this patch and retesting, I bumped into a nasty
> situation regarding pkgdir: the foo-patch rule is executed with the
> variable PKGDIR=$(pkgdir) which I changed to PKGDIR=$$(pkgdir).
> However, in that case, the foo-patch rule, which is outside
> inner-targets, gets executed with the literal $(pkgdir) as PKGDIR,
> which is evaluated to docs/manual (the directory of the last makefile
> parsed).
> As a result, package patching does not occur.
> 
> There are two solutions:
> - do not double the $$ here, so keep the originial PKGDIR=$(pkgdir)
> - use :=, so PKGDIR:=$(pkgdir)
> 
> I feel that if we take the first alternative, we should consistently
> use $(pkgdir) like that as it is less error-prone to have it as a
> general exception, than as an
> exception-only-if-you-pass-it-to-non-inner-rules.
> All of this is because the pkgdir variable is rather tricky.

 Okay, I agree: first option is the best one, and then we should use it
everywhere for pkgdir. Also, for pkgdir, not delaying expansion (i.e. single-$)
can never be incorrect. Same goes for pkgname, obviously. Which matches your
earlier observation that pkgname is special.

 Can you add an explanation about this in the leading comment of
inner-generic-package?


 Regards,
 Arnout

> 
> Thanks,
> Thomas
> 


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2014-05-22  0:02 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-12 14:44 [Buildroot] [PATCH 0 of 7 v2] infra: fix dollar signs; remove some undefined versions Thomas De Schampheleire
2014-05-12 14:44 ` [Buildroot] [PATCH 1 of 7 v2] infra: consistently use double dollar signs inside inner-xxx-targets Thomas De Schampheleire
2014-05-13  9:25   ` Arnout Vandecappelle
2014-05-13 10:59     ` Thomas De Schampheleire
2014-05-13 19:33       ` Arnout Vandecappelle
2014-05-21 13:14     ` Thomas De Schampheleire
2014-05-22  0:02       ` Arnout Vandecappelle
2014-05-12 14:44 ` [Buildroot] [PATCH 2 of 7 v2] infra: add comment describing single/double dollar-sign rules Thomas De Schampheleire
2014-05-12 14:44 ` [Buildroot] [PATCH 3 of 7 v2] pkg-virtual: simplify definition of FOO_VERSION to 'virtual' Thomas De Schampheleire
2014-05-12 14:44 ` [Buildroot] [PATCH 4 of 7 v2] toolchain/toolchain-buildroot: migrate to virtual package infrastructure Thomas De Schampheleire
2014-05-12 14:44 ` [Buildroot] [PATCH 5 of 7 v2] toolchain-external: change version from 'undefined' to 'virtual' Thomas De Schampheleire
2014-05-12 14:44 ` [Buildroot] [PATCH 6 of 7 v2] makedevs: change version from 'undefined' to 'buildroot-$(BR2_VERSION)' Thomas De Schampheleire
2014-05-12 14:44 ` [Buildroot] [PATCH 7 of 7 v2] mkpasswd: " Thomas De Schampheleire

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox