[Buildroot] [PATCH] pkg-infra: use ?= instead of ifdef

[Buildroot] [PATCH] pkg-infra: use ?= instead of ifdef

[Arnout Vandecappelle]

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.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
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(-)

diff --git a/package/Makefile.autotools.in b/package/Makefile.autotools.in
index fd118dc..8d0d7f2 100644
--- a/package/Makefile.autotools.in
+++ b/package/Makefile.autotools.in
@@ -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			?=
diff --git a/package/Makefile.cmake.in b/package/Makefile.cmake.in
index 1cd65e4..58d3889 100644
--- a/package/Makefile.cmake.in
+++ b/package/Makefile.cmake.in
@@ -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			?=
diff --git a/package/Makefile.package.in b/package/Makefile.package.in
index f7b6566..6e6f2d6 100644
--- a/package/Makefile.package.in
+++ b/package/Makefile.package.in
@@ -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)
-- 
tg: (b6a507c..) t/no-ifdef-in-gentargets (depends on: master)

[Buildroot] [PATCH] pkg-infra: use ?= instead of ifdef

[Thomas Petazzoni]

Hello,

Le Mon, 19 Mar 2012 00:28:54 +0100,
"Arnout Vandecappelle (Essensium/Mind)" <arnout@mind.be> a ?crit :

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

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

Not sure I understand what is broken here. What does "/-to-_" means?

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH] pkg-infra: use ?= instead of ifdef

[Arnout Vandecappelle]

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)" <arnout@mind.be> a ?crit :
> 
> > 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.

 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.


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


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