bdwgc: add optional cplusplus support

PATCH Superseded Fabrice Fontaine 2 participants 4 messages Oct 24, 2018 – Oct 25, 2018

lore

Share & tools

lore

Fabrice Fontaine

author

2018-10-24 21:01 UTC #0 lore raw

PATCH v3 1/1 bdwgc: add optional cplusplus support

Use CFLAGS_EXTRA to pass C and C++ flags in a single variable

Signed-off-by Fabrice Fontaine <[email protected]>

---
Changes v2 -> v3 (after review of Baruch Siach):
- Replace BDWGC_CFLAGS by BDWGC_CFLAGS_EXTRA

Changes v1 -> v2 (after review of Arnout Vandecappelle and Baruch Siach):
- Use CFLAGS_EXTRA to pass C and C++ flags in a single variable instead of using CFLAGS and CPPFLAGS

 package/bdwgc/bdwgc.mk | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

package/bdwgc/bdwgc.mk +11 -4

index 5603df8d9d..1c80f47b92 100644
@@ -19,17 +19,24 @@ HOST_BDWGC_DEPENDENCIES = host-libatomic_ops host-pkgconf
 # solution.
 BDWGC_AUTORECONF = YES
 
-BDWGC_CFLAGS = $(TARGET_CFLAGS)
 ifeq ($(BR2_sparc),y)
-BDWGC_CFLAGS += -DAO_NO_SPARC_V9
+BDWGC_CFLAGS_EXTRA += -DAO_NO_SPARC_V9
 endif
 ifeq ($(BR2_STATIC_LIBS),y)
-BDWGC_CFLAGS += -DGC_NO_DLOPEN
+BDWGC_CFLAGS_EXTRA += -DGC_NO_DLOPEN
 endif
 
+BDWGC_CONF_OPTS = CFLAGS_EXTRA="$(BDWGC_CFLAGS_EXTRA)"
+
 # Ensure we use the system libatomic_ops, and not the internal one.
-BDWGC_CONF_OPTS = --with-libatomic-ops=yes CFLAGS="$(BDWGC_CFLAGS)"
+BDWGC_CONF_OPTS += --with-libatomic-ops=yes
 HOST_BDWGC_CONF_OPTS = --with-libatomic-ops=yes
 
+ifeq ($(BR2_INSTALL_LIBSTDCPP),y)
+BDWGC_CONF_OPTS += --enable-cplusplus
+else
+BDWGC_CONF_OPTS += --disable-cplusplus
+endif
+
 $(eval $(autotools-package))
 $(eval $(host-autotools-package))

Signature · 1 line

2.17.1

Baruch Siach

2018-10-25 03:57 UTC #1 lore raw

PATCH v3 1/1 bdwgc: add optional cplusplus support

Hi Fabrice,

Quoted 20 lines · Fabrice Fontaine

Use CFLAGS_EXTRA to pass C and C++ flags in a single variable
Signed-off-by Fabrice Fontaine <[email protected]>
---
Changes v2 -> v3 (after review of Baruch Siach):
- Replace BDWGC_CFLAGS by BDWGC_CFLAGS_EXTRA
Changes v1 -> v2 (after review of Arnout Vandecappelle and Baruch Siach):
- Use CFLAGS_EXTRA to pass C and C++ flags in a single variable instead of using CFLAGS and CPPFLAGS
 package/bdwgc/bdwgc.mk | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)
package/bdwgc/bdwgc.mk +0 -0
index 5603df8d9d..1c80f47b92 100644

package/bdwgc/bdwgc.mk HOST_BDWGC_DEPENDENCIES = host-libatomic_ops host-pkgconf · line 22

Quoted 4 lines

package/bdwgc/bdwgc.mk +0 -1

@@ -19,17 +19,24 @@ HOST_BDWGC_DEPENDENCIES = host-libatomic_ops host-pkgconf
 # solution.
 BDWGC_AUTORECONF = YES
 
-BDWGC_CFLAGS = $(TARGET_CFLAGS)

This removes TARGET_CLAGS ...

package/bdwgc/bdwgc.mk HOST_BDWGC_DEPENDENCIES = host-libatomic_ops host-pkgconf · line 29

Quoted 10 lines

package/bdwgc/bdwgc.mk +3 -2

 ifeq ($(BR2_sparc),y)
-BDWGC_CFLAGS += -DAO_NO_SPARC_V9
+BDWGC_CFLAGS_EXTRA += -DAO_NO_SPARC_V9
 endif
 ifeq ($(BR2_STATIC_LIBS),y)
-BDWGC_CFLAGS += -DGC_NO_DLOPEN
+BDWGC_CFLAGS_EXTRA += -DGC_NO_DLOPEN
 endif
 
+BDWGC_CONF_OPTS = CFLAGS_EXTRA="$(BDWGC_CFLAGS_EXTRA)"

... so you need to add it back here.

Nit: I'd move the BDWGC_CONF_OPTS assignment upper.

baruch

Quoted 14 lines

package/bdwgc/bdwgc.mk +8 -1

+
 # Ensure we use the system libatomic_ops, and not the internal one.
-BDWGC_CONF_OPTS = --with-libatomic-ops=yes CFLAGS="$(BDWGC_CFLAGS)"
+BDWGC_CONF_OPTS += --with-libatomic-ops=yes
 HOST_BDWGC_CONF_OPTS = --with-libatomic-ops=yes
 
+ifeq ($(BR2_INSTALL_LIBSTDCPP),y)
+BDWGC_CONF_OPTS += --enable-cplusplus
+else
+BDWGC_CONF_OPTS += --disable-cplusplus
+endif
+
 $(eval $(autotools-package))
 $(eval $(host-autotools-package))

Signature · 3 lines

     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

Fabrice Fontaine

author

2018-10-25 17:27 UTC #2 lore raw

PATCH v3 1/1 bdwgc: add optional cplusplus support

Dear Baruch,
Le jeu. 25 oct. 2018 ? 05:57, Baruch Siach <[email protected]> a ?crit :

Quoted 27 lines

Hi Fabrice,
Quoted 23 lines · Fabrice Fontaine
Use CFLAGS_EXTRA to pass C and C++ flags in a single variable
Signed-off-by Fabrice Fontaine <[email protected]>
---
Changes v2 -> v3 (after review of Baruch Siach):
- Replace BDWGC_CFLAGS by BDWGC_CFLAGS_EXTRA
Changes v1 -> v2 (after review of Arnout Vandecappelle and Baruch Siach):
- Use CFLAGS_EXTRA to pass C and C++ flags in a single variable instead of using CFLAGS and CPPFLAGS
 package/bdwgc/bdwgc.mk | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)
package/bdwgc/bdwgc.mk +0 -0
index 5603df8d9d..1c80f47b92 100644
@@ -19,17 +19,24 @@ HOST_BDWGC_DEPENDENCIES = host-libatomic_ops host-pkgconf
 # solution.
 BDWGC_AUTORECONF = YES
 

Quoted 3 lines

package/bdwgc/bdwgc.mk HOST_BDWGC_DEPENDENCIES = host-libatomic_ops host-pkgconf · line 22
Quoted 1 line
package/bdwgc/bdwgc.mk +0 -1
-BDWGC_CFLAGS = $(TARGET_CFLAGS)
This removes TARGET_CLAGS ...

Yes, TARGET_CFLAGS is already passed in CFLAGS and CXXFLAGS by the autotools-package infrastructure.
Because we don't override CFLAGS anymore, this line can be removed. Did I miss something?

Quoted 10 lines

package/bdwgc/bdwgc.mk +2 -2

 ifeq ($(BR2_sparc),y)
-BDWGC_CFLAGS += -DAO_NO_SPARC_V9
+BDWGC_CFLAGS_EXTRA += -DAO_NO_SPARC_V9
 endif
 ifeq ($(BR2_STATIC_LIBS),y)
-BDWGC_CFLAGS += -DGC_NO_DLOPEN
+BDWGC_CFLAGS_EXTRA += -DGC_NO_DLOPEN
 endif

Quoted 5 lines

package/bdwgc/bdwgc.mk HOST_BDWGC_DEPENDENCIES = host-libatomic_ops host-pkgconf · line 29
Quoted 1 line
package/bdwgc/bdwgc.mk +1 -0
+BDWGC_CONF_OPTS = CFLAGS_EXTRA="$(BDWGC_CFLAGS_EXTRA)"
... so you need to add it back here.
Nit: I'd move the BDWGC_CONF_OPTS assignment upper.

I can send a v4 with this update if you want. What is the best practice in buildroot concerning variable assignment? Should we set BDWGC_CONF_OPTS = CFLAGS_EXTRA="$(BDWGC_CFLAGS_EXTRA)" before putting any value in BDWGC_CFLAGS_EXTRA?
Or perhaps the issue is that I used = to make the assignment after ifeq statements.
If I replace = by +=, can I keep this line here?

Quoted 9 lines

baruch

Quoted 6 lines

package/bdwgc/bdwgc.mk +2 -1

+
 # Ensure we use the system libatomic_ops, and not the internal one.
-BDWGC_CONF_OPTS = --with-libatomic-ops=yes CFLAGS="$(BDWGC_CFLAGS)"
+BDWGC_CONF_OPTS += --with-libatomic-ops=yes
 HOST_BDWGC_CONF_OPTS = --with-libatomic-ops=yes

Quoted 13 lines

package/bdwgc/bdwgc.mk HOST_BDWGC_DEPENDENCIES = host-libatomic_ops host-pkgconf · line 42
Quoted 8 lines
package/bdwgc/bdwgc.mk +6 -0
+ifeq ($(BR2_INSTALL_LIBSTDCPP),y)
+BDWGC_CONF_OPTS += --enable-cplusplus
+else
+BDWGC_CONF_OPTS += --disable-cplusplus
+endif
+
 $(eval $(autotools-package))
 $(eval $(host-autotools-package))
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

Best Regards,

Fabrice

Baruch Siach

2018-10-25 18:46 UTC #3 lore raw

PATCH v3 1/1 bdwgc: add optional cplusplus support

Hi Fabrice,

Quoted 25 lines · Fabrice Fontaine

Le jeu. 25 oct. 2018 ? 05:57, Baruch Siach <[email protected]> a ?crit :
Quoted 24 lines
Quoted 23 lines · Fabrice Fontaine
Use CFLAGS_EXTRA to pass C and C++ flags in a single variable
Signed-off-by Fabrice Fontaine <[email protected]>
---
Changes v2 -> v3 (after review of Baruch Siach):
- Replace BDWGC_CFLAGS by BDWGC_CFLAGS_EXTRA
Changes v1 -> v2 (after review of Arnout Vandecappelle and Baruch Siach):
- Use CFLAGS_EXTRA to pass C and C++ flags in a single variable instead of using CFLAGS and CPPFLAGS
 package/bdwgc/bdwgc.mk | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)
package/bdwgc/bdwgc.mk +0 -0
index 5603df8d9d..1c80f47b92 100644
@@ -19,17 +19,24 @@ HOST_BDWGC_DEPENDENCIES = host-libatomic_ops host-pkgconf
 # solution.
 BDWGC_AUTORECONF = YES
 

Quoted 7 lines

Quoted 3 lines
package/bdwgc/bdwgc.mk HOST_BDWGC_DEPENDENCIES = host-libatomic_ops host-pkgconf · line 22
Quoted 1 line
package/bdwgc/bdwgc.mk +0 -1
-BDWGC_CFLAGS = $(TARGET_CFLAGS)
This removes TARGET_CLAGS ...
Yes, TARGET_CFLAGS is already passed in CFLAGS and CXXFLAGS by the autotools-package infrastructure.
Because we don't override CFLAGS anymore, this line can be removed. Did I miss something?

No. You are right.

Quoted 9 lines

package/bdwgc/bdwgc.mk +2 -2

 ifeq ($(BR2_sparc),y)
-BDWGC_CFLAGS += -DAO_NO_SPARC_V9
+BDWGC_CFLAGS_EXTRA += -DAO_NO_SPARC_V9
 endif
 ifeq ($(BR2_STATIC_LIBS),y)
-BDWGC_CFLAGS += -DGC_NO_DLOPEN
+BDWGC_CFLAGS_EXTRA += -DGC_NO_DLOPEN
 endif

Quoted 7 lines

Quoted 5 lines
package/bdwgc/bdwgc.mk HOST_BDWGC_DEPENDENCIES = host-libatomic_ops host-pkgconf · line 29
Quoted 1 line
package/bdwgc/bdwgc.mk +1 -0
+BDWGC_CONF_OPTS = CFLAGS_EXTRA="$(BDWGC_CFLAGS_EXTRA)"
... so you need to add it back here.
Nit: I'd move the BDWGC_CONF_OPTS assignment upper.
I can send a v4 with this update if you want. What is the best practice in buildroot concerning variable assignment?

The common practice is the group unconditional assignment at the top of the .mk file, and add conditions below.

Quoted 2 lines

Should we set BDWGC_CONF_OPTS = CFLAGS_EXTRA="$(BDWGC_CFLAGS_EXTRA)" before putting any value in BDWGC_CFLAGS_EXTRA?

Make evaluates this "recursively expanded" variable value only when referenced.

https://www.gnu.org/software/make/manual/html_node/Flavors.html#Flavors

So no problem with BDWGC_CONF_OPTS assignment before BDWGC_CFLAGS_EXTRA is set.

Quoted 3 lines

Or perhaps the issue is that I used = to make the assignment after ifeq statements.
If I replace = by +=, can I keep this line here?

I didn't think about that. That is a another reason to follow the common practice of grouping unconditional assignments at the top.

baruch

Quoted 6 lines

package/bdwgc/bdwgc.mk +2 -1

+
 # Ensure we use the system libatomic_ops, and not the internal one.
-BDWGC_CONF_OPTS = --with-libatomic-ops=yes CFLAGS="$(BDWGC_CFLAGS)"
+BDWGC_CONF_OPTS += --with-libatomic-ops=yes
 HOST_BDWGC_CONF_OPTS = --with-libatomic-ops=yes

Quoted 8 lines

Quoted 8 lines
package/bdwgc/bdwgc.mk HOST_BDWGC_DEPENDENCIES = host-libatomic_ops host-pkgconf · line 42
Quoted 8 lines
package/bdwgc/bdwgc.mk +6 -0
+ifeq ($(BR2_INSTALL_LIBSTDCPP),y)
+BDWGC_CONF_OPTS += --enable-cplusplus
+else
+BDWGC_CONF_OPTS += --disable-cplusplus
+endif
+
 $(eval $(autotools-package))
 $(eval $(host-autotools-package))

--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -