[Buildroot] [PATCH v2] package/dvb-app: handle static/shared only build
Romain Naour
romain.naour at openwide.fr
Sun Jan 4 23:10:02 UTC 2015
Hi Yann, all
Le 04/01/2015 16:09, Yann E. MORIN a écrit :
> Romain, All,
>
> On 2015-01-04 13:45 +0100, Romain Naour spake thusly:
>> Also remove tests since they require static libraries.
>>
>> Signed-off-by: Romain Naour <romain.naour at openwide.fr>
>> ---
>> v2: remove tests (ThomasP)
>> rework static/shared handling logic
>> ---
>> .../0003-handle-static-shared-only-build.patch | 35 ++++++++++++++++++++++
>> .../dvb-apps/0003-support-static-only-build.patch | 20 -------------
>> package/dvb-apps/0004-Makefile-remove-test.patch | 27 +++++++++++++++++
>> package/dvb-apps/dvb-apps.mk | 4 ++-
>> 4 files changed, 65 insertions(+), 21 deletions(-)
>> create mode 100644 package/dvb-apps/0003-handle-static-shared-only-build.patch
>> delete mode 100644 package/dvb-apps/0003-support-static-only-build.patch
>> create mode 100644 package/dvb-apps/0004-Makefile-remove-test.patch
>>
>> diff --git a/package/dvb-apps/0003-handle-static-shared-only-build.patch b/package/dvb-apps/0003-handle-static-shared-only-build.patch
>> new file mode 100644
>> index 0000000..7a7d59a
>> --- /dev/null
>> +++ b/package/dvb-apps/0003-handle-static-shared-only-build.patch
>> @@ -0,0 +1,35 @@
>> +From f461e831f8c0ee9a59c9f194c0306eb73298396b Mon Sep 17 00:00:00 2001
>> +From: Romain Naour <romain.naour at openwide.fr>
>> +Date: Thu, 25 Dec 2014 19:22:16 +0100
>> +Subject: [PATCH] Make.rules: Handle static/shared only build
>> +
>> +Do not build .a library when disable_static is set
>> +Do not build .so library when disable_shared is set
>> +
>> +Signed-off-by: Romain Naour <romain.naour at openwide.fr>
>> +---
>> + Make.rules | 8 +++++++-
>> + 1 file changed, 7 insertions(+), 1 deletion(-)
>> +
>> +diff --git a/Make.rules b/Make.rules
>> +index 3410d7b..4add272 100644
>> +--- a/Make.rules
>> ++++ b/Make.rules
>> +@@ -9,7 +9,13 @@ ifneq ($(lib_name),)
>> + CFLAGS_LIB ?= -fPIC
>> + CFLAGS += $(CFLAGS_LIB)
>> +
>> +-libraries = $(lib_name).so $(lib_name).a
>> ++ifeq ($(disable_static),)
>> ++libraries = $(lib_name).a
>> ++endif
>> ++
>> ++ifeq ($(disable_shared),)
>> ++libraries += $(lib_name).so
>> ++endif
>
> I find it weird that one would need to _disable_stuff. I really prefer
> we use positive logic whenever possible, it is much easier to understand.
>
> What about:
>
> ifneq ($(enable_static),no)
> libraries += $(lib_name).a
> endif
>
> ifneq ($(enable_shared),no)
> libraries += $(lib_name).so
> endif
>
> Ok, so this is negative logic _in_ the Makefile. It is still positive
> from the caller::
> - by default both are built
> - it is possible to selectively disable each by passing enable_foo=no,
> which is not unlike the traditional way it is handled in autotools
Ok, I'm fine with this change if you prefer.
>
> [--SNIP--]
>> diff --git a/package/dvb-apps/dvb-apps.mk b/package/dvb-apps/dvb-apps.mk
>> index 892af63..a5037af 100644
>> --- a/package/dvb-apps/dvb-apps.mk
>> +++ b/package/dvb-apps/dvb-apps.mk
>> @@ -16,7 +16,9 @@ DVB_APPS_LDLIBS += -liconv
>> endif
>>
>> ifeq ($(BR2_STATIC_LIBS),y)
>> -DVB_APPS_MAKE_OPTS += static=1
>> +DVB_APPS_MAKE_OPTS += disable_shared=1
>> +else ifeq ($(BR2_SHARED_LIBS),y)
>> +DVB_APPS_MAKE_OPTS += disable_static=1
>> endif
>
> So you'd get:
>
> ifeq ($(BR2_STATIC_LIBS),y)
> DVB_APPS_MAKE_OPTS += enable_shared=no
> endif
> ifeq ($(BR2_SHARED_LIBS),y)
> DVB_APPS_MAKE_OPTS += enable_static=no
> endif
>
> Granted, it is very, very similar to what you're doing, but I really
> prefer options to have positive logic, rather than negative. Also, I
> believe this might be more easily upstreamable.
I already sent the series upstream:
http://www.mail-archive.com/linux-media@vger.kernel.org/msg83830.html
But Arnout noticed that my SoB is missing on the two first patches.
I'll resend a new version tomorrow.
>
> Note: I read Thomas' comment on the previous iteration, and it does
> not seem like he said he would want disable_XXX variables instead of
> enable_XXX. Thomas, what's your opinion?
Thanks for the review.
Best regards,
Romain
>
> Regards,
> Yann E. MORIN.
>
>> DVB_APPS_INSTALL_STAGING = YES
>> --
>> 1.9.3
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
>
More information about the buildroot
mailing list