Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] autotools: add with/without and enable/disable helpers
@ 2010-11-19  4:33 Mike Frysinger
  2010-11-19  7:55 ` Thomas Petazzoni
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Mike Frysinger @ 2010-11-19  4:33 UTC (permalink / raw)
  To: buildroot

Rather than have to write ugly logic in every package .mk file to check
a config var and then expand into a --{en,dis}able-foo flag, add helpers
so code can cleanly expand things.

A simple example:
- ifeq ($(BR2_PACKAGE_LIBVORBIS),y)
- SDL_MIXER_CONF_OPT += --enable-music-ogg
- else
- SDL_MIXER_CONF_OPT += --disable-music-ogg
- endif
+ SDL_MIXER_CONF_OPT += $(call USE_ENABLE,PACKAGE_LIBVORBIS,music-ogg)

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 package/Makefile.autotools.in |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/package/Makefile.autotools.in b/package/Makefile.autotools.in
index 7d04e44..08a190b 100644
--- a/package/Makefile.autotools.in
+++ b/package/Makefile.autotools.in
@@ -279,3 +279,21 @@ else
 $(call AUTOTARGETS_INNER,$(2),$(call UPPERCASE,$(2)),$(call UPPERCASE,$(2)),$(1),target)
 endif
 endef
+
+################################################################################
+# AUTOTOOLS HELPERS
+################################################################################
+
+# $(call _USE_CONF,enable,disable,LIB_FFMPEG,video,blah) -> --enable-video=blah if LIB_FFMPEG
+# $(call _USE_CONF,with,without,LIB_FFMPEG,video)        -> --with-video if LIB_FFMPEG
+_USE_CONF = $(shell \
+	opt="$(5)"; test "$${opt:+set}" = "set" && opt="=$${opt}"; \
+	test "$(BR2_$(3))" = "y" \
+		&& echo "--$(1)-$(4)$${opt}" \
+		|| echo "--$(2)-$(4)")
+
+# $(call USE_ENABLE,LIB_FFMPEG,video) => --enable-video if LIB_FFMPEG is set
+USE_ENABLE = $(call _USE_CONF,enable,disable,$(1),$(2),$(3))
+
+# $(call USE_WITH,LIB_FFMPEG,video) => --with-video if LIB_FFMPEG is set
+USE_WITH = $(call _USE_CONF,with,without,$(1),$(2),$(3))
-- 
1.7.3.2

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

* [Buildroot] [PATCH] autotools: add with/without and enable/disable helpers
  2010-11-19  4:33 [Buildroot] [PATCH] autotools: add with/without and enable/disable helpers Mike Frysinger
@ 2010-11-19  7:55 ` Thomas Petazzoni
  2010-11-19  8:20   ` Mike Frysinger
  2010-11-19 10:23 ` [Buildroot] [PATCH v2] " Mike Frysinger
  2010-11-20  2:37 ` [Buildroot] [PATCH v3] " Mike Frysinger
  2 siblings, 1 reply; 13+ messages in thread
From: Thomas Petazzoni @ 2010-11-19  7:55 UTC (permalink / raw)
  To: buildroot

Hello,

On Thu, 18 Nov 2010 23:33:57 -0500
Mike Frysinger <vapier@gentoo.org> wrote:

> Rather than have to write ugly logic in every package .mk file to check
> a config var and then expand into a --{en,dis}able-foo flag, add helpers
> so code can cleanly expand things.

Sounds a very good idea!

> +# $(call _USE_CONF,enable,disable,LIB_FFMPEG,video,blah) -> --enable-video=blah if LIB_FFMPEG
> +# $(call _USE_CONF,with,without,LIB_FFMPEG,video)        -> --with-video if LIB_FFMPEG
> +_USE_CONF = $(shell \
> +	opt="$(5)"; test "$${opt:+set}" = "set" && opt="=$${opt}"; \
> +	test "$(BR2_$(3))" = "y" \
> +		&& echo "--$(1)-$(4)$${opt}" \
> +		|| echo "--$(2)-$(4)")

However, I'm worried that this will start a shell process when
evaluating *each* USE_ENABLE/USE_WITH call, and this will slow down the
start up of Buildroot. In the past, we were forking/execing "tr" once
per-package and it was slowing down Buildroot startup needlessly
(~10-15 seconds doing hundreds of fork/exec).

It'd be nicer if we could use a pure Makefile implementation. What
about a simpler :

USE_WITH = $(if $(BR2_$(1)),--with-$(2),--without-$(2))
USE_ENABLE = $(if $(BR2_$(1),--enable-$(2),--disable-$(2)))

However, their usage might be a little limited: often when the option
is enabled, we need to add a dependency to the package as well.

Regards,

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

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

* [Buildroot] [PATCH] autotools: add with/without and enable/disable helpers
  2010-11-19  7:55 ` Thomas Petazzoni
@ 2010-11-19  8:20   ` Mike Frysinger
  2010-11-19  9:56     ` Thomas Petazzoni
  0 siblings, 1 reply; 13+ messages in thread
From: Mike Frysinger @ 2010-11-19  8:20 UTC (permalink / raw)
  To: buildroot

On Friday, November 19, 2010 02:55:18 Thomas Petazzoni wrote:
> On Thu, 18 Nov 2010 23:33:57 -0500 Mike Frysinger wrote:
> > +# $(call _USE_CONF,enable,disable,LIB_FFMPEG,video,blah) ->
> > --enable-video=blah if LIB_FFMPEG +# $(call
> > _USE_CONF,with,without,LIB_FFMPEG,video)        -> --with-video if
> > LIB_FFMPEG +_USE_CONF = $(shell \
> > +	opt="$(5)"; test "$${opt:+set}" = "set" && opt="=$${opt}"; \
> > +	test "$(BR2_$(3))" = "y" \
> > +		&& echo "--$(1)-$(4)$${opt}" \
> > +		|| echo "--$(2)-$(4)")
> 
> However, I'm worried that this will start a shell process when
> evaluating *each* USE_ENABLE/USE_WITH call, and this will slow down the
> start up of Buildroot.

not really.  this is the whole point of make's lazy evaluation.  so unless 
somewhere the code is wrongly using ":=", these shouldnt be executed unless 
the configure script in question is actually run.  a simple test on my side 
says that it is working as i expect -- lazily.

> It'd be nicer if we could use a pure Makefile implementation. What
> about a simpler :
> 
> USE_WITH = $(if $(BR2_$(1)),--with-$(2),--without-$(2))
> USE_ENABLE = $(if $(BR2_$(1),--enable-$(2),--disable-$(2)))

this isnt functionally equivalent ... there is no support for the optional 
[=val] with the flag

> However, their usage might be a little limited: often when the option
> is enabled, we need to add a dependency to the package as well.

this is due to poor design on the behalf of buildroot.  it really needs to 
adopt more Kconfig style and do stuff like:
foo-y =
foo-$(BR2_xxx) += libpng

and again, please retain cc in replies
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20101119/6ac90925/attachment.pgp>

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

* [Buildroot] [PATCH] autotools: add with/without and enable/disable helpers
  2010-11-19  8:20   ` Mike Frysinger
@ 2010-11-19  9:56     ` Thomas Petazzoni
  2010-11-19 10:07       ` Mike Frysinger
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Petazzoni @ 2010-11-19  9:56 UTC (permalink / raw)
  To: buildroot

Hello Mike,

All your patches are well appreciated. However, I feel some
aggressiveness in your replies, which I don't really understand. Again,
I really do appreciate all those improvements, so can we keep the
discussion nice and friendly ?

On Fri, 19 Nov 2010 03:20:18 -0500
Mike Frysinger <vapier@gentoo.org> wrote:

> not really.  this is the whole point of make's lazy evaluation.  so
> unless somewhere the code is wrongly using ":=", these shouldnt be
> executed unless the configure script in question is actually run.  a
> simple test on my side says that it is working as i expect -- lazily.

Right. The case I was mentionning was the UPPERCASE macro which was use
directly in the $(eval $(call AUTOTARGETS,...)) of each package. So,
this macro was actually evaluated when make was parsing all
packages .mk files. But that's different from the current case, you're
right.

> > It'd be nicer if we could use a pure Makefile implementation. What
> > about a simpler :
> > 
> > USE_WITH = $(if $(BR2_$(1)),--with-$(2),--without-$(2))
> > USE_ENABLE = $(if $(BR2_$(1),--enable-$(2),--disable-$(2)))
> 
> this isnt functionally equivalent ... there is no support for the
> optional [=val] with the flag

Ah, correct, I missed that. So, what about:

USE_WITH = $(if $(BR2_$(1)),       \
              $(if $(3),           \
                 --with-$(2)=$(3), \
                 --with-$(2)       \
              ),                   \
              --without-$(2))

USE_ENABLE = $(if $(BR2_$(1)),        \
                $(if $(3),            \
                  --enable-$(2)=$(3), \
                  --enable-$(2)       \
                ),                    \
                --disable-$(2))

It's not that I'm against using the shell, but for this particular
macro, the make language seems to be quite appropriate.

And it'd be nice to add some words in the documentation about this as
well (but I can do that later on if you wish).

> this is due to poor design on the behalf of buildroot.  it really
> needs to adopt more Kconfig style and do stuff like:
> foo-y =
> foo-$(BR2_xxx) += libpng

I'm for sure open to improvements/changes in the package
infrastructure. For which variables should we use this ? I'm thinking
of all "cumulative" variables. So in the generic infrastructure:

 <pkg>_DEPENDENCIES
 <pkg>_POST_PATCH_HOOKS
 <pkg>_PRE_CONFIGURE_HOOKS
 <pkg>_POST_CONFIGURE_HOOKS
 <pkg>_POST_BUILD_HOOKS
 <pkg>_POST_INSTALL_HOOKS
 <pkg>_POST_INSTALL_STAGING_HOOKS
 <pkg>_POST_INSTALL_TARGET_HOOKS

and for the autotools packages, also:

 <pkg>_CONF_ENV
 <pkg>_CONF_OPT
 <pkg>_MAKE_ENV
 <pkg>_MAKE_OPT
 <pkg>_AUTORECONF_OPT
 <pkg>_INSTALL_STAGING_OPT
 <pkg>_INSTALL_TARGET_OPT
 <pkg>_CLEAN_OPT
 <pkg>_UNINSTALL_STAGING_OPT
 <pkg>_UNINSTALL_TARGET_OPT

For all those variable, we would take into account:

 <pkg>_<varname> (for compatibility reasons)

and

 <pkg>_<varname>-y

Does that sounds like what you're suggesting ?

> and again, please retain cc in replies

Sorry, done this time. But I may well forget again in the future, I'm
not used to retain cc in replies. Will try my best to not forget it.

Thanks again!

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20101119/17a8b208/attachment.pgp>

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

* [Buildroot] [PATCH] autotools: add with/without and enable/disable helpers
  2010-11-19  9:56     ` Thomas Petazzoni
@ 2010-11-19 10:07       ` Mike Frysinger
  0 siblings, 0 replies; 13+ messages in thread
From: Mike Frysinger @ 2010-11-19 10:07 UTC (permalink / raw)
  To: buildroot

On Friday, November 19, 2010 04:56:10 Thomas Petazzoni wrote:
> All your patches are well appreciated. However, I feel some
> aggressiveness in your replies, which I don't really understand. Again,
> I really do appreciate all those improvements, so can we keep the
> discussion nice and friendly ?

i dont recall injecting aggressiveness into my messages.  i have been told 
before that my response can be too curt.  not the same, but easy to perceive 
as the same i guess.  personally i try to minimize word count to focus on 
subject matter and not get bogged down with essays.

> > > It'd be nicer if we could use a pure Makefile implementation. What
> > > about a simpler :
> > > 
> > > USE_WITH = $(if $(BR2_$(1)),--with-$(2),--without-$(2))
> > > USE_ENABLE = $(if $(BR2_$(1),--enable-$(2),--disable-$(2)))
> > 
> > this isnt functionally equivalent ... there is no support for the
> > optional [=val] with the flag
> 
> It's not that I'm against using the shell, but for this particular
> macro, the make language seems to be quite appropriate.

i'm not against doing it in make's native language ... it's just that your 
solution didnt support everything mine did.  as long as they're functionally 
equivalent, avoiding mixing two languages (make and shell) is of course a good 
thing.

i'm importing work that i've done for another distro, so while it's been 
heavily tested elsewhere, i wouldnt be surprised if there will be more room 
for improvements as you've shown here.

> And it'd be nice to add some words in the documentation about this as
> well (but I can do that later on if you wish).

i'm not terribly familiar with current documentation on the backend

> > this is due to poor design on the behalf of buildroot.  it really
> > needs to adopt more Kconfig style and do stuff like:
> > foo-y =
> > foo-$(BR2_xxx) += libpng
> 
> I'm for sure open to improvements/changes in the package
> infrastructure. For which variables should we use this ? I'm thinking
> of all "cumulative" variables.

pretty much everything that uses "+=" that ive seen in buildroot has been a 
candidate for conversion.

> Does that sounds like what you're suggesting ?

yes, but i was focusing on the common ones.  TARGETS in the top level and all 
the dependency related variables.  but it probably makes more sense to just do 
it for all variables so as to stay consistent.  doing some will probably be a 
bit confusing for people, and even aggravating when they want one that wasnt 
converted.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20101119/f308615c/attachment.pgp>

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

* [Buildroot] [PATCH v2] autotools: add with/without and enable/disable helpers
  2010-11-19  4:33 [Buildroot] [PATCH] autotools: add with/without and enable/disable helpers Mike Frysinger
  2010-11-19  7:55 ` Thomas Petazzoni
@ 2010-11-19 10:23 ` Mike Frysinger
  2010-11-19 11:13   ` Thomas Petazzoni
  2010-11-20  2:37 ` [Buildroot] [PATCH v3] " Mike Frysinger
  2 siblings, 1 reply; 13+ messages in thread
From: Mike Frysinger @ 2010-11-19 10:23 UTC (permalink / raw)
  To: buildroot

Rather than have to write ugly logic in every package .mk file to check
a config var and then expand into a --{en,dis}able-foo flag, add helpers
so code can cleanly expand things.

A simple example:
- ifeq ($(BR2_PACKAGE_LIBVORBIS),y)
- SDL_MIXER_CONF_OPT += --enable-music-ogg
- else
- SDL_MIXER_CONF_OPT += --disable-music-ogg
- endif
+ SDL_MIXER_CONF_OPT += $(call USE_ENABLE,PACKAGE_LIBVORBIS,music-ogg)

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v2
	- rewrite _USE_CONF in make

 package/Makefile.autotools.in |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/package/Makefile.autotools.in b/package/Makefile.autotools.in
index 7d04e44..8f76349 100644
--- a/package/Makefile.autotools.in
+++ b/package/Makefile.autotools.in
@@ -279,3 +279,17 @@ else
 $(call AUTOTARGETS_INNER,$(2),$(call UPPERCASE,$(2)),$(call UPPERCASE,$(2)),$(1),target)
 endif
 endef
+
+################################################################################
+# AUTOTOOLS HELPERS
+################################################################################
+
+# $(call _USE_CONF,enable,disable,LIB_FFMPEG,video,blah) -> --enable-video=blah if LIB_FFMPEG
+# $(call _USE_CONF,with,without,LIB_FFMPEG,video)        -> --with-video if LIB_FFMPEG
+_USE_CONF = $(if $(BR2_$(3)), --$(1)-$(4)$(if $(5),=$(5)), --$(2)-$(4))
+
+# $(call USE_ENABLE,LIB_FFMPEG,video) => --enable-video if LIB_FFMPEG is set
+USE_ENABLE = $(call _USE_CONF,enable,disable,$(1),$(2),$(3))
+
+# $(call USE_WITH,LIB_FFMPEG,video) => --with-video if LIB_FFMPEG is set
+USE_WITH = $(call _USE_CONF,with,without,$(1),$(2),$(3))
-- 
1.7.3.2

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

* [Buildroot] [PATCH v2] autotools: add with/without and enable/disable helpers
  2010-11-19 10:23 ` [Buildroot] [PATCH v2] " Mike Frysinger
@ 2010-11-19 11:13   ` Thomas Petazzoni
  2010-11-19 11:24     ` Mike Frysinger
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Petazzoni @ 2010-11-19 11:13 UTC (permalink / raw)
  To: buildroot

Hello,

On Fri, 19 Nov 2010 05:23:36 -0500
Mike Frysinger <vapier@gentoo.org> wrote:

> Rather than have to write ugly logic in every package .mk file to check
> a config var and then expand into a --{en,dis}able-foo flag, add helpers
> so code can cleanly expand things.
> 
> A simple example:
> - ifeq ($(BR2_PACKAGE_LIBVORBIS),y)
> - SDL_MIXER_CONF_OPT += --enable-music-ogg
> - else
> - SDL_MIXER_CONF_OPT += --disable-music-ogg
> - endif
> + SDL_MIXER_CONF_OPT += $(call USE_ENABLE,PACKAGE_LIBVORBIS,music-ogg)

So typically, one would do:

SDL_MIXER_CONF_OPT += $(call USE_ENABLE,PACKAGE_LIBVORBIS,music-ogg)
SDL_MIXER_DEPENDENCIES-$(BR2_PACKAGE_LIBVORBIS) += libvorbis

what about doing something more consistent, like:

SDL_MIXER_CONF_OPT     += $(call USE_ENABLE,PACKAGE_LIBVORBIS,music-ogg)
SDL_MIXER_DEPENDENCIES += $(call USE_DEP,PACKAGE_LIBVORBIS,libvorbis)

I know it's a litte bit more characters that the previous solution, but
it looks more consistent to me.

> +# $(call _USE_CONF,enable,disable,LIB_FFMPEG,video,blah) -> --enable-video=blah if LIB_FFMPEG
> +# $(call _USE_CONF,with,without,LIB_FFMPEG,video)        -> --with-video if LIB_FFMPEG
> +_USE_CONF = $(if $(BR2_$(3)), --$(1)-$(4)$(if $(5),=$(5)), --$(2)-$(4))

What about testing directly BR2_PACKAGE_$(3), since all packages are
prefixed by BR2_PACKAGE_ ?

Thanks!

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

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

* [Buildroot] [PATCH v2] autotools: add with/without and enable/disable helpers
  2010-11-19 11:13   ` Thomas Petazzoni
@ 2010-11-19 11:24     ` Mike Frysinger
  2010-11-19 12:49       ` Paulius Zaleckas
  0 siblings, 1 reply; 13+ messages in thread
From: Mike Frysinger @ 2010-11-19 11:24 UTC (permalink / raw)
  To: buildroot

On Friday, November 19, 2010 06:13:26 Thomas Petazzoni wrote:
> So typically, one would do:
> 
> SDL_MIXER_CONF_OPT += $(call USE_ENABLE,PACKAGE_LIBVORBIS,music-ogg)
> SDL_MIXER_DEPENDENCIES-$(BR2_PACKAGE_LIBVORBIS) += libvorbis
> 
> what about doing something more consistent, like:
> 
> SDL_MIXER_CONF_OPT     += $(call USE_ENABLE,PACKAGE_LIBVORBIS,music-ogg)
> SDL_MIXER_DEPENDENCIES += $(call USE_DEP,PACKAGE_LIBVORBIS,libvorbis)
> 
> I know it's a litte bit more characters that the previous solution, but
> it looks more consistent to me.

personally, i prefer to stick to the kconfig syntax where possible.  the only 
reason for USE_ENABLE is that we need to expand into different text strings.

> > +_USE_CONF = $(if $(BR2_$(3)), --$(1)-$(4)$(if $(5),=$(5)),
> > --$(2)-$(4))
> 
> What about testing directly BR2_PACKAGE_$(3), since all packages are
> prefixed by BR2_PACKAGE_ ?

if that's the case, then there's no reason

in the original implementation, i avoided making the 2nd argument optional.  
this was because i'd have to execute tr or something to do lower->upper.  but 
if it's all in make now, it should be possible to support the syntax:
	$(call USE_ENABLE,libvorbis)
the 1st arg would always be passed through the UPPERCASE helper, and hte 2nd 
arg would default to the 1st.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20101119/b054eeb6/attachment.pgp>

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

* [Buildroot] [PATCH v2] autotools: add with/without and enable/disable helpers
  2010-11-19 11:24     ` Mike Frysinger
@ 2010-11-19 12:49       ` Paulius Zaleckas
  2010-11-19 23:25         ` Mike Frysinger
  0 siblings, 1 reply; 13+ messages in thread
From: Paulius Zaleckas @ 2010-11-19 12:49 UTC (permalink / raw)
  To: buildroot

On Fri, Nov 19, 2010 at 1:24 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Friday, November 19, 2010 06:13:26 Thomas Petazzoni wrote:
>> So typically, one would do:
>>
>> SDL_MIXER_CONF_OPT += $(call USE_ENABLE,PACKAGE_LIBVORBIS,music-ogg)
>> SDL_MIXER_DEPENDENCIES-$(BR2_PACKAGE_LIBVORBIS) += libvorbis
>>
>> what about doing something more consistent, like:
>>
>> SDL_MIXER_CONF_OPT ? ? += $(call USE_ENABLE,PACKAGE_LIBVORBIS,music-ogg)
>> SDL_MIXER_DEPENDENCIES += $(call USE_DEP,PACKAGE_LIBVORBIS,libvorbis)
>>
>> I know it's a litte bit more characters that the previous solution, but
>> it looks more consistent to me.
>
> personally, i prefer to stick to the kconfig syntax where possible. ?the only
> reason for USE_ENABLE is that we need to expand into different text strings.
>
>> > +_USE_CONF = $(if $(BR2_$(3)), --$(1)-$(4)$(if $(5),=$(5)),
>> > --$(2)-$(4))
>>
>> What about testing directly BR2_PACKAGE_$(3), since all packages are
>> prefixed by BR2_PACKAGE_ ?
>
> if that's the case, then there's no reason
>
> in the original implementation, i avoided making the 2nd argument optional.
> this was because i'd have to execute tr or something to do lower->upper. ?but
> if it's all in make now, it should be possible to support the syntax:
> ? ? ? ?$(call USE_ENABLE,libvorbis)
> the 1st arg would always be passed through the UPPERCASE helper, and hte 2nd
> arg would default to the 1st.

I believe he means BR2_PACKAGE_$(3) instead of BR2_$(3), not to get rid of it.

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

* [Buildroot] [PATCH v2] autotools: add with/without and enable/disable helpers
  2010-11-19 12:49       ` Paulius Zaleckas
@ 2010-11-19 23:25         ` Mike Frysinger
  0 siblings, 0 replies; 13+ messages in thread
From: Mike Frysinger @ 2010-11-19 23:25 UTC (permalink / raw)
  To: buildroot

On Friday, November 19, 2010 07:49:54 Paulius Zaleckas wrote:
> On Fri, Nov 19, 2010 at 1:24 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> > On Friday, November 19, 2010 06:13:26 Thomas Petazzoni wrote:
> >> > +_USE_CONF = $(if $(BR2_$(3)), --$(1)-$(4)$(if $(5),=$(5)),
> >> > --$(2)-$(4))
> >> 
> >> What about testing directly BR2_PACKAGE_$(3), since all packages are
> >> prefixed by BR2_PACKAGE_ ?
> > 
> > if that's the case, then there's no reason
> > 
> > in the original implementation, i avoided making the 2nd argument
> > optional. this was because i'd have to execute tr or something to do
> > lower->upper.  but if it's all in make now, it should be possible to
> > support the syntax: $(call USE_ENABLE,libvorbis)
> > the 1st arg would always be passed through the UPPERCASE helper, and hte
> > 2nd arg would default to the 1st.
> 
> I believe he means BR2_PACKAGE_$(3) instead of BR2_$(3), not to get rid of
> it.

i know what he meant.  i think you're confusing unrelated threads of thought.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20101119/6961619e/attachment-0001.pgp>

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

* [Buildroot] [PATCH v3] autotools: add with/without and enable/disable helpers
  2010-11-19  4:33 [Buildroot] [PATCH] autotools: add with/without and enable/disable helpers Mike Frysinger
  2010-11-19  7:55 ` Thomas Petazzoni
  2010-11-19 10:23 ` [Buildroot] [PATCH v2] " Mike Frysinger
@ 2010-11-20  2:37 ` Mike Frysinger
  2010-11-20  3:47   ` Mike Frysinger
  2 siblings, 1 reply; 13+ messages in thread
From: Mike Frysinger @ 2010-11-20  2:37 UTC (permalink / raw)
  To: buildroot

Rather than have to write ugly logic in every package .mk file to check
a config var and then expand into a --{en,dis}able-foo flag, add helpers
so code can cleanly expand things.

A simple example:
- ifeq ($(BR2_PACKAGE_LIBVORBIS),y)
- SDL_MIXER_CONF_OPT += --enable-music-ogg
- else
- SDL_MIXER_CONF_OPT += --disable-music-ogg
- endif
+ SDL_MIXER_CONF_OPT += $(call USE_ENABLE,LIBVORBIS,music-ogg)

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v3
	- absorb the "PACKAGE_" part of config options too

 package/Makefile.autotools.in |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/package/Makefile.autotools.in b/package/Makefile.autotools.in
index 7d04e44..82000c0 100644
--- a/package/Makefile.autotools.in
+++ b/package/Makefile.autotools.in
@@ -279,3 +279,17 @@ else
 $(call AUTOTARGETS_INNER,$(2),$(call UPPERCASE,$(2)),$(call UPPERCASE,$(2)),$(1),target)
 endif
 endef
+
+################################################################################
+# AUTOTOOLS HELPERS
+################################################################################
+
+# $(call _USE_CONF,enable,disable,LIB_FFMPEG,video,blah) -> --enable-video=blah if LIB_FFMPEG
+# $(call _USE_CONF,with,without,LIB_FFMPEG,video)        -> --with-video if LIB_FFMPEG
+_USE_CONF = $(if $(BR2_PACKAGE_$(3)), --$(1)-$(4)$(if $(5),=$(5)), --$(2)-$(4))
+
+# $(call USE_ENABLE,LIB_FFMPEG,video) => --enable-video if LIB_FFMPEG is set
+USE_ENABLE = $(call _USE_CONF,enable,disable,$(1),$(2),$(3))
+
+# $(call USE_WITH,LIB_FFMPEG,video) => --with-video if LIB_FFMPEG is set
+USE_WITH = $(call _USE_CONF,with,without,$(1),$(2),$(3))
-- 
1.7.3.2

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

* [Buildroot] [PATCH v3] autotools: add with/without and enable/disable helpers
  2010-11-20  2:37 ` [Buildroot] [PATCH v3] " Mike Frysinger
@ 2010-11-20  3:47   ` Mike Frysinger
  0 siblings, 0 replies; 13+ messages in thread
From: Mike Frysinger @ 2010-11-20  3:47 UTC (permalink / raw)
  To: buildroot

On Friday, November 19, 2010 21:37:59 Mike Frysinger wrote:
> v3
> 	- absorb the "PACKAGE_" part of config options too

looking some more, this isnt a good idea.  it would prevent using common 
options like BR2_INET_IPV6 to expand into ipv6 configure options.  so ignore 
the v3 patch.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20101119/668e0b98/attachment.pgp>

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

* [Buildroot] [PATCH] autotools: add with/without and enable/disable helpers
@ 2011-03-28  8:35 Mike Frysinger
  0 siblings, 0 replies; 13+ messages in thread
From: Mike Frysinger @ 2011-03-28  8:35 UTC (permalink / raw)
  To: buildroot

Rather than have to write ugly logic in every package .mk file to check
a config var and then expand into a --{en,dis}able-foo flag, add helpers
so code can cleanly expand things.

A simple example:
- ifeq ($(BR2_PACKAGE_LIBVORBIS),y)
- SDL_MIXER_CONF_OPT += --enable-music-ogg
- else
- SDL_MIXER_CONF_OPT += --disable-music-ogg
- endif
+ SDL_MIXER_CONF_OPT += $(call USE_ENABLE,LIBVORBIS,music-ogg)

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 package/Makefile.autotools.in |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/package/Makefile.autotools.in b/package/Makefile.autotools.in
index d445348..e8d1eb0 100644
--- a/package/Makefile.autotools.in
+++ b/package/Makefile.autotools.in
@@ -279,3 +279,17 @@ else
 $(call AUTOTARGETS_INNER,$(2),$(call UPPERCASE,$(2)),$(call UPPERCASE,$(2)),$(1),target)
 endif
 endef
+
+################################################################################
+# AUTOTOOLS HELPERS
+################################################################################
+
+# $(call _USE_CONF,enable,disable,LIB_FFMPEG,video,blah) -> --enable-video=blah if LIB_FFMPEG
+# $(call _USE_CONF,with,without,LIB_FFMPEG,video)        -> --with-video if LIB_FFMPEG
+_USE_CONF = $(if $(BR2_PACKAGE_$(3)), --$(1)-$(4)$(if $(5),=$(5)), --$(2)-$(4))
+
+# $(call USE_ENABLE,LIB_FFMPEG,video) => --enable-video if LIB_FFMPEG is set
+USE_ENABLE = $(call _USE_CONF,enable,disable,$(1),$(2),$(3))
+
+# $(call USE_WITH,LIB_FFMPEG,video) => --with-video if LIB_FFMPEG is set
+USE_WITH = $(call _USE_CONF,with,without,$(1),$(2),$(3))
-- 
1.7.4.1

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

end of thread, other threads:[~2011-03-28  8:35 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-19  4:33 [Buildroot] [PATCH] autotools: add with/without and enable/disable helpers Mike Frysinger
2010-11-19  7:55 ` Thomas Petazzoni
2010-11-19  8:20   ` Mike Frysinger
2010-11-19  9:56     ` Thomas Petazzoni
2010-11-19 10:07       ` Mike Frysinger
2010-11-19 10:23 ` [Buildroot] [PATCH v2] " Mike Frysinger
2010-11-19 11:13   ` Thomas Petazzoni
2010-11-19 11:24     ` Mike Frysinger
2010-11-19 12:49       ` Paulius Zaleckas
2010-11-19 23:25         ` Mike Frysinger
2010-11-20  2:37 ` [Buildroot] [PATCH v3] " Mike Frysinger
2010-11-20  3:47   ` Mike Frysinger
  -- strict thread matches above, loose matches on Subject: below --
2011-03-28  8:35 [Buildroot] [PATCH] " Mike Frysinger

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