Skip to contentSkip to conversation

package/dvb-app: handle static/shared only build

PATCH Superseded Romain Naour 3 participants 4 messages Jan 4, 2015 – Jan 5, 2015

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Romain Naour

author
#0 lore raw

PATCH v2 1/1 package/dvb-app: handle static/shared only build

Also remove tests since they require static libraries.

Signed-off-by Romain Naour <[email protected]>

---
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
package/dvb-apps/0003-handle-static-shared-only-build.patch new file +35 -0
new file mode 100644
index 0000000..7a7d59a
@@ -0,0 +1,35 @@
+From f461e831f8c0ee9a59c9f194c0306eb73298396b Mon Sep 17 00:00:00 2001
+From: Romain Naour <[email protected]>
+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 <[email protected]>
+---
+ 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
+ 
+ .PHONY: library
+ 
+-- 
+1.9.3
+
package/dvb-apps/0003-support-static-only-build.patch deleted +0 -20
deleted file mode 100644
index 236f1a3..0000000
@@ -1,20 +0,0 @@
-Make.rules: don't build .so libraries when static=1
-
-Signed-off-by: Thomas Petazzoni <[email protected]>
-
-Index: b/Make.rules
-===================================================================
---- a/Make.rules
-+++ b/Make.rules
-@@ -9,7 +9,11 @@
- CFLAGS_LIB ?= -fPIC
- CFLAGS += $(CFLAGS_LIB)
- 
-+ifeq ($(static),1)
-+libraries = $(lib_name).a
-+else
- libraries = $(lib_name).so $(lib_name).a
-+endif
- 
- .PHONY: library
-
package/dvb-apps/0004-Makefile-remove-test.patch new file +27 -0
new file mode 100644
index 0000000..45bfad9
@@ -0,0 +1,27 @@
+From c578772d6abc5fdf3ec83f632c371373e5baf9f1 Mon Sep 17 00:00:00 2001
+From: Romain Naour <[email protected]>
+Date: Fri, 26 Dec 2014 01:04:58 +0100
+Subject: [PATCH] Makefile: remove test
+
+Tests needs static libraries, remove them for shared only build.
+
+Signed-off-by: Romain Naour <[email protected]>
+---
+ Makefile | 1 -
+ 1 file changed, 1 deletion(-)
+
+diff --git a/Makefile b/Makefile
+index 65a2273..105c460 100644
+--- a/Makefile
++++ b/Makefile
+@@ -10,7 +10,6 @@ DVB_API_MINOR := $(word 3, $(shell grep -m1 "DVB_API_VERSION_MINOR" $(VERSION_FI
+ 
+ all clean install:
+ 	$(MAKE) -C lib $@
+-	$(MAKE) -C test $@
+ 	$(MAKE) -C util $@
+ 
+ update:
+-- 
+1.9.3
+
package/dvb-apps/dvb-apps.mk +3 -1
index 892af63..a5037af 100644
@@ -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
 
 DVB_APPS_INSTALL_STAGING = YES
Signature · 1 line
1.9.3
#1 lore raw

PATCH v2 1/1 package/dvb-app: handle static/shared only build

Romain, All,

On 2015-01-04 13:45 +0100, Romain Naour spake thusly:

Quoted 41 lines

Also remove tests since they require static libraries.

Signed-off-by Romain Naour <[email protected]>

---
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
package/dvb-apps/0003-handle-static-shared-only-build.patch new file +19 -0
new file mode 100644
index 0000000..7a7d59a
@@ -0,0 +1,35 @@
+From f461e831f8c0ee9a59c9f194c0306eb73298396b Mon Sep 17 00:00:00 2001
+From: Romain Naour <[email protected]>
+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 <[email protected]>
+---
+ 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

package/dvb-apps/0003-handle-static-shared-only-build.patch line 29

Quoted 10 lines
package/dvb-apps/0003-handle-static-shared-only-build.patch new file +10 -0
+ 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

[--SNIP--]

package/dvb-apps/dvb-apps.mk DVB_APPS_LDLIBS += -liconv · line 22

Quoted 13 lines
package/dvb-apps/dvb-apps.mk +3 -1
index 892af63..a5037af 100644
@@ -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.

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?

Regards,
Yann E. MORIN.

Quoted 8 lines
package/dvb-apps/dvb-apps.mk +0 -0
 DVB_APPS_INSTALL_STAGING = YES
Signature · 6 lines
1.9.3

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
author
#2 lore raw

PATCH v2 1/1 package/dvb-app: handle static/shared only build

Hi Yann, all

Le 04/01/2015 16:09, Yann E. MORIN a ?crit :

Quoted 68 lines

Romain, All,

On 2015-01-04 13:45 +0100, Romain Naour spake thusly:

Quoted 41 lines

Also remove tests since they require static libraries.

Signed-off-by Romain Naour <[email protected]>

---
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
package/dvb-apps/0003-handle-static-shared-only-build.patch new file +19 -0
new file mode 100644
index 0000000..7a7d59a
@@ -0,0 +1,35 @@
+From f461e831f8c0ee9a59c9f194c0306eb73298396b Mon Sep 17 00:00:00 2001
+From: Romain Naour <[email protected]>
+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 <[email protected]>
+---
+ 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

package/dvb-apps/0003-handle-static-shared-only-build.patch line 29

Quoted 10 lines
package/dvb-apps/0003-handle-static-shared-only-build.patch new file +10 -0
+ 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
Quoted 5 lines

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.

Quoted 25 lines

[--SNIP--]

package/dvb-apps/dvb-apps.mk DVB_APPS_LDLIBS += -liconv · line 22

Quoted 13 lines
package/dvb-apps/dvb-apps.mk +3 -1
index 892af63..a5037af 100644
@@ -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
Quoted 3 lines

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

Quoted 4 lines

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

Quoted 13 lines

Regards,
Yann E. MORIN.

Quoted 8 lines
package/dvb-apps/dvb-apps.mk +0 -0
 DVB_APPS_INSTALL_STAGING = YES
Signature · 6 lines
1.9.3

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
#3 lore raw

PATCH v2 1/1 package/dvb-app: handle static/shared only build

In reply to Romain Naour

Dear Romain Naour,

Quoted 6 lines · Romain Naour

Also remove tests since they require static libraries.

Signed-off-by Romain Naour <[email protected]>

---
v2: remove tests (ThomasP)
rework static/shared handling logic

Quoted 9 lines
---
 .../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

Patch marked as Changes Requested in patchwork, following the comments made by Yann. Romain, can you submit an updated version?

Thanks,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com