pkg-infra: use ?= instead of ifdef
PATCH 1/1 pkg-infra: use ?= instead of ifdef
ifdef considers a variable undefined if it is defined as empty. In the GENTARGETS/AUTOTARGETS/CMAKETARGETS infrastructure, the ifdef construct is often used to default to the value from the target-variable if the host-variable is not defined. However, this makes it impossible to override the host-variable as empty. Therefore, use ?= instead to assign the defaults.
---
I didn't touch the _VERSION infrastructure because I'm afraid to break it. In fact, I think it already is broken (I don't see how that /-to-_ substitution can work for host-packages).
package/Makefile.autotools.in | 28 +++++++++------------------- package/Makefile.cmake.in | 8 +------- package/Makefile.package.in | 38 ++++++++++++++------------------------ 3 files changed, 24 insertions(+), 50 deletions(-)
package/Makefile.autotools.in +9 -19
@@ -57,28 +57,18 @@ endef define AUTOTARGETS_INNER # define package-specific variables to default values -ifndef $(2)_SUBDIR - ifdef $(3)_SUBDIR - $(2)_SUBDIR = $($(3)_SUBDIR) - else - $(2)_SUBDIR ?= - endif -endif +$(2)_SUBDIR ?= $($(3)_SUBDIR) -ifndef $(2)_LIBTOOL_PATCH - ifdef $(3)_LIBTOOL_PATCH - $(2)_LIBTOOL_PATCH = $($(3)_LIBTOOL_PATCH) - else - $(2)_LIBTOOL_PATCH ?= YES - endif +ifdef $(3)_LIBTOOL_PATCH + $(2)_LIBTOOL_PATCH ?= $($(3)_LIBTOOL_PATCH) +else + $(2)_LIBTOOL_PATCH ?= YES endif -ifndef $(2)_MAKE - ifdef $(3)_MAKE - $(2)_MAKE = $($(3)_MAKE) - else - $(2)_MAKE ?= $(MAKE) - endif +ifdef $(3)_MAKE + $(2)_MAKE ?= $($(3)_MAKE) +else + $(2)_MAKE ?= $(MAKE) endif $(2)_CONF_ENV ?=
package/Makefile.cmake.in +1 -7
@@ -38,13 +38,7 @@ define CMAKETARGETS_INNER # define package-specific variables to default values -ifndef $(2)_SUBDIR - ifdef $(3)_SUBDIR - $(2)_SUBDIR = $($(3)_SUBDIR) - else - $(2)_SUBDIR ?= - endif -endif +$(2)_SUBDIR ?= $($(3)_SUBDIR) $(2)_CONF_ENV ?= $(2)_CONF_OPT ?=
package/Makefile.package.in +14 -24
@@ -501,36 +501,26 @@ ifneq ($$($(2)_OVERRIDE_SRCDIR),) $(2)_VERSION = custom endif -ifndef $(2)_SOURCE - ifdef $(3)_SOURCE - $(2)_SOURCE = $($(3)_SOURCE) - else - $(2)_SOURCE ?= $$($(2)_BASE_NAME).tar.gz - endif +ifdef $(3)_SOURCE + $(2)_SOURCE ?= $($(3)_SOURCE) +else + $(2)_SOURCE ?= $$($(2)_BASE_NAME).tar.gz endif -ifndef $(2)_PATCH - ifdef $(3)_PATCH - $(2)_PATCH = $($(3)_PATCH) - endif -endif +$(2)_PATCH ?= $($(3)_PATCH) -ifndef $(2)_SITE - ifdef $(3)_SITE - $(2)_SITE = $($(3)_SITE) - else - $(2)_SITE ?= \ +ifdef $(3)_SITE + $(2)_SITE ?= $($(3)_SITE) +else + $(2)_SITE ?= \ http://$$(BR2_SOURCEFORGE_MIRROR).dl.sourceforge.net/sourceforge/$(1) - endif endif -ifndef $(2)_SITE_METHOD - ifdef $(3)_SITE_METHOD - $(2)_SITE_METHOD = $($(3)_SITE_METHOD) - else - # Try automatic detection using the scheme part of the URI - $(2)_SITE_METHOD = $(firstword $(subst ://, ,$(call qstrip,$($(2)_SITE)))) - endif +ifdef $(3)_SITE_METHOD + $(2)_SITE_METHOD ?= $($(3)_SITE_METHOD) +else + # Try automatic detection using the scheme part of the URI + $(2)_SITE_METHOD ?= $(firstword $(subst ://, ,$(call qstrip,$($(2)_SITE)))) endif ifeq ($$($(2)_SITE_METHOD),local)
Signature · 1 line
tg: (b6a507c..) t/no-ifdef-in-gentargets (depends on: master)
PATCH 1/1 pkg-infra: use ?= instead of ifdef
Hello,
Le Mon, 19 Mar 2012 00:28:54 +0100,
"Arnout Vandecappelle (Essensium/Mind)" <[email protected]> a ?crit :
Quoted 6 lines
ifdef considers a variable undefined if it is defined as empty. In the GENTARGETS/AUTOTARGETS/CMAKETARGETS infrastructure, the ifdef construct is often used to default to the value from the target-variable if the host-variable is not defined. However, this makes it impossible to override the host-variable as empty. Therefore, use ?= instead to assign the defaults.
Could you share a specific example on why this would be useful? I'm sure there are uses cases, but I'm curious about the one you're having.
Quoted 3 lines
I didn't touch the
_VERSIONinfrastructure because I'm afraid to break it. In fact, I think it already is broken (I don't see how that /-to-_ substitution can work for host-packages).
Not sure I understand what is broken here. What does "/-to-_" means?
Thomas
Signature · 4 lines
Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH 1/1 pkg-infra: use ?= instead of ifdef
Quoted 12 lines · On Monday 19 March 2012 09:57:47 Thomas Petazzoni wrote:
Hello,
Le Mon, 19 Mar 2012 00:28:54 +0100,
"Arnout Vandecappelle (Essensium/Mind)" <[email protected]> a ?crit :Quoted 6 lines
ifdef considers a variable undefined if it is defined as empty. In the GENTARGETS/AUTOTARGETS/CMAKETARGETS infrastructure, the ifdef construct is often used to default to the value from the target-variable if the host-variable is not defined. However, this makes it impossible to override the host-variable as empty. Therefore, use ?= instead to assign the defaults.
Quoted 2 lines
Could you share a specific example on why this would be useful? I'm sure there are uses cases, but I'm curious about the one you're having.
Good question... There was an example on the list a while back, and a
couple of days later a made a patch for it but it never got sent out. Now I'm cleaning up my patch queue and just sent it. But I don't remember the original reason, and looking at the patch doesn't ring a bell either. In fact, all the ones I fixed in this patch don't really make sense if they're defined empty, except for the _SUBDIR and maybe _PATCH.
Quoted 5 lines
Quoted 3 lines
I didn't touch the
_VERSIONinfrastructure because I'm afraid to break it. In fact, I think it already is broken (I don't see how that /-to-_ substitution can work for host-packages).Not sure I understand what is broken here. What does "/-to-_" means?
/-to-_ substitution means the subsitution of / into _
$(2)_VERSION = $(subst /,_,$($(3)_VERSION))
I think it's broken because the $($(3)_VERSION) of a host package
has already been substituted before:
$(2)_DL_VERSION = $($(2)_VERSION)
$(2)_VERSION = $(subst /,_,$($(2)_VERSION))
Actually for PKG_VERSION it's OK, but HOST_PKG_DL_VERSION will be
incorrect because it has _ instead of /. Something we wouldn't notice because we have no packages with / in the version.
Regards,
Arnout
Signature · 9 lines
Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286540 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 -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20120319/ffa2fddf/attachment.html>