LinuxLists.cc - [PATCH 1/2] Automake AM_CFLAGS, AM

2011-11-12 23:48:49

Subject: [PATCH 1/2] Automake AM_CFLAGS, AM_LDFLAGS fixups

Reference: "Flag Variables Ordering"[1]

In automake, if you modify CFLAGS or LDFLAGS of a target you
actually override the autoconf supplied flags. This is highly
none standard and may lead to undesired results, as some
targets will be compiled with the autoconf supplied flags and
some won't.

This patch adds AM_CFLAGS to any flags overridden.

Some notes:

1. Remove none common flags:
-DBLUETOOTH_PLUGIN_BUILTIN -DPLUGINDIR=\""$(build_plugindir)"\"
Moved to bluetoothd only, as BLUETOOTH_PLUGIN_BUILTIN should
effect only sources compiled within it.
And PLUGINDIR is used only by the daemon.

2. More targets now will be compiled with:
@DBUS_CFLAGS@ @GLIB_CFLAGS@ @CAPNG_CFLAGS@
If this is a problem I will add these to specific
targets.

3. I am not sure there is a reason to set specific
trivial CFLAGS for simple includes in target specific. Simpler
is to put all at AM_CFLAGS.

4. All @XXX_XXFLAGS@ can actually be used as make macros
$(XXX_XXFLAGS),
Since AC_SUBST are automatically generated as make
macros.

5. Why don't you split up the Makefile.am into smaller
by directory/library? It would be easier to control the
setup.

[1] http://www.gnu.org/software/automake/manual/automake.html#Flag-Variables-Ordering
---
Makefile.am | 21 +++++++++++----------
1 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 283af4d..fdc58f6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -87,7 +87,7 @@ sbc_libsbc_la_SOURCES = sbc/sbc.h sbc/sbc.c sbc/sbc_math.h sbc/sbc_tables.h \
sbc/sbc_primitives_neon.h sbc/sbc_primitives_neon.c \
sbc/sbc_primitives_armv6.h sbc/sbc_primitives_armv6.c

-sbc_libsbc_la_CFLAGS = -finline-functions -fgcse-after-reload \
+sbc_libsbc_la_CFLAGS = $(AM_CFLAGS) -finline-functions -fgcse-after-reload \
-funswitch-loops -funroll-loops

noinst_PROGRAMS += sbc/sbcinfo sbc/sbcdec sbc/sbcenc
@@ -102,7 +102,7 @@ if SNDFILE
noinst_PROGRAMS += sbc/sbctester

sbc_sbctester_LDADD = @SNDFILE_LIBS@ -lm
-sbc_sbctest_CFLAGS = @SNDFILE_CFLAGS@
+sbc_sbctest_CFLAGS = $(AM_CFLAGS) @SNDFILE_CFLAGS@
endif
endif

@@ -287,7 +287,7 @@ if MAINTAINER_MODE
plugin_LTLIBRARIES += plugins/external-dummy.la
plugins_external_dummy_la_SOURCES = plugins/external-dummy.c
plugins_external_dummy_la_LDFLAGS = -module -avoid-version -no-undefined
-plugins_external_dummy_la_CFLAGS = -fvisibility=hidden
+plugins_external_dummy_la_CFLAGS = $(AM_CFLAGS) -fvisibility=hidden
endif

sbin_PROGRAMS += src/bluetoothd
@@ -317,6 +317,8 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \
src/oob.h src/oob.c src/eir.h src/eir.c
src_bluetoothd_LDADD = lib/libbluetooth-private.la @GLIB_LIBS@ @DBUS_LIBS@ \
@CAPNG_LIBS@ -ldl -lrt
+src_bluetoothd_CFLAGS = $(AM_CFLAGS) \
+ -DBLUETOOTH_PLUGIN_BUILTIN -DPLUGINDIR=\""$(build_plugindir)"\"
src_bluetoothd_LDFLAGS = -Wl,--export-dynamic \
-Wl,--version-script=$(srcdir)/src/bluetooth.ver

@@ -353,14 +355,14 @@ audio_libasound_module_pcm_bluetooth_la_SOURCES = audio/pcm_bluetooth.c \
audio_libasound_module_pcm_bluetooth_la_LDFLAGS = -module -avoid-version #-export-symbols-regex [_]*snd_pcm_.*
audio_libasound_module_pcm_bluetooth_la_LIBADD = sbc/libsbc.la \
lib/libbluetooth-private.la @ALSA_LIBS@
-audio_libasound_module_pcm_bluetooth_la_CFLAGS = @ALSA_CFLAGS@
+audio_libasound_module_pcm_bluetooth_la_CFLAGS = $(AM_CFLAGS) @ALSA_CFLAGS@

audio_libasound_module_ctl_bluetooth_la_SOURCES = audio/ctl_bluetooth.c \
audio/rtp.h audio/ipc.h audio/ipc.c
audio_libasound_module_ctl_bluetooth_la_LDFLAGS = -module -avoid-version #-export-symbols-regex [_]*snd_ctl_.*
audio_libasound_module_ctl_bluetooth_la_LIBADD = \
lib/libbluetooth-private.la @ALSA_LIBS@
-audio_libasound_module_ctl_bluetooth_la_CFLAGS = @ALSA_CFLAGS@
+audio_libasound_module_ctl_bluetooth_la_CFLAGS = $(AM_CFLAGS) @ALSA_CFLAGS@

if DATAFILES
alsaconfdir = $(datadir)/alsa
@@ -387,8 +389,8 @@ audio_libgstbluetooth_la_SOURCES = audio/gstbluetooth.c audio/gstpragma.h \
audio_libgstbluetooth_la_LDFLAGS = -module -avoid-version
audio_libgstbluetooth_la_LIBADD = sbc/libsbc.la lib/libbluetooth-private.la \
@DBUS_LIBS@ @GSTREAMER_LIBS@ -lgstaudio-0.10 -lgstrtp-0.10
-audio_libgstbluetooth_la_CFLAGS = -fvisibility=hidden -fno-strict-aliasing \
- $(AM_CFLAGS) @DBUS_CFLAGS@ @GSTREAMER_CFLAGS@
+audio_libgstbluetooth_la_CFLAGS = $(AM_CFLAGS) -fvisibility=hidden -fno-strict-aliasing \
+ @DBUS_CFLAGS@ @GSTREAMER_CFLAGS@
endif
endif

@@ -432,8 +434,7 @@ EXTRA_DIST += doc/manager-api.txt \

AM_YFLAGS = -d

-AM_CFLAGS = @DBUS_CFLAGS@ @GLIB_CFLAGS@ @CAPNG_CFLAGS@ \
- -DBLUETOOTH_PLUGIN_BUILTIN -DPLUGINDIR=\""$(build_plugindir)"\"
+AM_CFLAGS = @DBUS_CFLAGS@ @GLIB_CFLAGS@ @CAPNG_CFLAGS@

INCLUDES = -I$(builddir)/lib -I$(builddir)/src -I$(srcdir)/src \
-I$(srcdir)/audio -I$(srcdir)/sbc -I$(srcdir)/gdbus \
@@ -450,7 +451,7 @@ unit_tests = unit/test-eir
noinst_PROGRAMS += $(unit_tests)

unit_test_eir_SOURCES = unit/test-eir.c src/eir.c src/glib-helper.c
-unit_test_eir_CFLAGS = @GLIB_CFLAGS@ @CHECK_CFLAGS@
+unit_test_eir_CFLAGS = $(AM_CFLAGS) @GLIB_CFLAGS@ @CHECK_CFLAGS@
unit_test_eir_LDADD = lib/libbluetooth-private.la @GLIB_LIBS@ @CHECK_LIBS@
unit_objects += $(unit_test_eir_OBJECTS)

--
1.7.3.4

2011-11-16 18:38:47

by Alon Bar-Lev

[permalink] [raw]

Subject: Re: [PATCH] Automake - do not override LDFLAGS

On Sun, Nov 13, 2011 at 9:40 AM, Alon Bar-Lev <[email protected]> wrote:
> Add AM_LDFLAGS to every _LDFLAGS, so that autoconf's
> LDFLAGS won't be overridden.
> ---
> Makefile.am | 12 ++++++------
> 1 files changed, 6 insertions(+), 6 deletions(-)
>

Hello,
Any comment?
If we fixed the CFLAGS, good to have the LDFLAGS synced.
Thanks!

2011-11-13 07:40:50

by Alon Bar-Lev

[permalink] [raw]

Subject: [PATCH] Automake - do not override LDFLAGS

Add AM_LDFLAGS to every _LDFLAGS, so that autoconf's
LDFLAGS won't be overridden.
---
Makefile.am | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 57aa33c..344699e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -70,7 +70,7 @@ lib_LTLIBRARIES += lib/libbluetooth.la

lib_libbluetooth_la_SOURCES = $(lib_headers) \
lib/bluetooth.c lib/hci.c lib/sdp.c lib/uuid.c
-lib_libbluetooth_la_LDFLAGS = -version-info 14:4:11
+lib_libbluetooth_la_LDFLAGS = $(AM_LDFLAGS) -version-info 14:4:11
lib_libbluetooth_la_DEPENDENCIES = $(local_headers)

noinst_LTLIBRARIES += lib/libbluetooth-private.la
@@ -286,7 +286,7 @@ endif
if MAINTAINER_MODE
plugin_LTLIBRARIES += plugins/external-dummy.la
plugins_external_dummy_la_SOURCES = plugins/external-dummy.c
-plugins_external_dummy_la_LDFLAGS = -module -avoid-version -no-undefined
+plugins_external_dummy_la_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version -no-undefined
plugins_external_dummy_la_CFLAGS = $(AM_CFLAGS) -fvisibility=hidden
endif

@@ -317,7 +317,7 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \
src/oob.h src/oob.c src/eir.h src/eir.c
src_bluetoothd_LDADD = lib/libbluetooth-private.la @GLIB_LIBS@ @DBUS_LIBS@ \
@CAPNG_LIBS@ -ldl -lrt
-src_bluetoothd_LDFLAGS = -Wl,--export-dynamic \
+src_bluetoothd_LDFLAGS = $(AM_LDFLAGS) -Wl,--export-dynamic \
-Wl,--version-script=$(srcdir)/src/bluetooth.ver

src_bluetoothd_DEPENDENCIES = lib/libbluetooth-private.la
@@ -354,14 +354,14 @@ alsa_LTLIBRARIES = audio/libasound_module_pcm_bluetooth.la \

audio_libasound_module_pcm_bluetooth_la_SOURCES = audio/pcm_bluetooth.c \
audio/rtp.h audio/ipc.h audio/ipc.c
-audio_libasound_module_pcm_bluetooth_la_LDFLAGS = -module -avoid-version #-export-symbols-regex [_]*snd_pcm_.*
+audio_libasound_module_pcm_bluetooth_la_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version #-export-symbols-regex [_]*snd_pcm_.*
audio_libasound_module_pcm_bluetooth_la_LIBADD = sbc/libsbc.la \
lib/libbluetooth-private.la @ALSA_LIBS@
audio_libasound_module_pcm_bluetooth_la_CFLAGS = $(AM_CFLAGS) @ALSA_CFLAGS@

audio_libasound_module_ctl_bluetooth_la_SOURCES = audio/ctl_bluetooth.c \
audio/rtp.h audio/ipc.h audio/ipc.c
-audio_libasound_module_ctl_bluetooth_la_LDFLAGS = -module -avoid-version #-export-symbols-regex [_]*snd_ctl_.*
+audio_libasound_module_ctl_bluetooth_la_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version #-export-symbols-regex [_]*snd_ctl_.*
audio_libasound_module_ctl_bluetooth_la_LIBADD = \
lib/libbluetooth-private.la @ALSA_LIBS@
audio_libasound_module_ctl_bluetooth_la_CFLAGS = $(AM_CLAGS) @ALSA_CFLAGS@
@@ -388,7 +388,7 @@ audio_libgstbluetooth_la_SOURCES = audio/gstbluetooth.c audio/gstpragma.h \
audio/gstsbcutil.h audio/gstsbcutil.c \
audio/gstrtpsbcpay.h audio/gstrtpsbcpay.c \
audio/rtp.h audio/ipc.h audio/ipc.c
-audio_libgstbluetooth_la_LDFLAGS = -module -avoid-version
+audio_libgstbluetooth_la_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version
audio_libgstbluetooth_la_LIBADD = sbc/libsbc.la lib/libbluetooth-private.la \
@DBUS_LIBS@ @GSTREAMER_LIBS@ \
-lgstaudio-0.10 -lgstrtp-0.10
--
1.7.3.4

2011-11-13 04:50:49

by Marcel Holtmann

[permalink] [raw]

Subject: Re: [PATCH 2/2] Automake AM_CFLAGS, AM_LDFLAGS fixups

2011-11-13 04:49:53

by Marcel Holtmann

[permalink] [raw]

Subject: Re: [PATCH 1/2] Automake AM_CFLAGS, AM_LDFLAGS fixups

Hi Alon,

> Reference: "Flag Variables Ordering"[1]
>
> In automake, if you modify CFLAGS or LDFLAGS of a target you
> actually override the autoconf supplied flags. This is highly
> none standard and may lead to undesired results, as some
> targets will be compiled with the autoconf supplied flags and
> some won't.
>
> This patch adds AM_CFLAGS to any flags overridden.
>
> Some notes:
>
> 1. Remove none common flags:
> -DBLUETOOTH_PLUGIN_BUILTIN -DPLUGINDIR=\""$(build_plugindir)"\"
> Moved to bluetoothd only, as BLUETOOTH_PLUGIN_BUILTIN should
> effect only sources compiled within it.
> And PLUGINDIR is used only by the daemon.
>
> 2. More targets now will be compiled with:
> @DBUS_CFLAGS@ @GLIB_CFLAGS@ @CAPNG_CFLAGS@
> If this is a problem I will add these to specific
> targets.
>
> 3. I am not sure there is a reason to set specific
> trivial CFLAGS for simple includes in target specific. Simpler
> is to put all at AM_CFLAGS.
>
> 4. All @XXX_XXFLAGS@ can actually be used as make macros
> $(XXX_XXFLAGS),
> Since AC_SUBST are automatically generated as make
> macros.

I redid this patch since I needed to add SHORTNAME to not turn the build
output into some ugly mess.

> 5. Why don't you split up the Makefile.am into smaller
> by directory/library? It would be easier to control the
> setup.

We are using non-recursive build on purpose.

Regards

Marcel

2011-11-12 23:48:50

by Alon Bar-Lev

[permalink] [raw]

Subject: [PATCH 2/2] Automake AM_CFLAGS, AM_LDFLAGS fixups

2011-11-12 23:43:33

by Anderson Lizardo

[permalink] [raw]

Subject: Re: [PATCH 1/2] Automake AM_CFLAGS, AM_LDFLAGS fixups

Hi Alon,

On Sat, Nov 12, 2011 at 7:24 PM, Alon Bar-Lev <[email protected]> wrote:
>> And also please send patches inline and via attachments.
>
> This would be difficult, as gmail does not allow no wrapping.
> git accepts this correctly, I use:
>
> $ curl article.gmane.org/gmane.linux.bluez.kernel/18337/raw | git am
>
> If this is really important I will try to find some other mail client.

Complementing Marcel's suggestion, you can have this in your ~/.gitconfig:

[sendemail]
smtpserver = smtp.gmail.com
smtpuser = [email protected]
smtpssl = 1

And to send e-mail:

git send-email --to [email protected] patch1.patch ...

It will then ask for your password after confirmation.

More information: man git-send-email

Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

2011-11-12 23:28:16

by Marcel Holtmann

[permalink] [raw]

Subject: Re: [PATCH 1/2] Automake AM_CFLAGS, AM_LDFLAGS fixups

Hi Alon,

> >> Signed-off-by: Alon Bar-Lev <[email protected]>
> >
> > please no Signed-off-by lines, we are not using them for the userspace
> > parts.
>
> I did not think it damage anything...

our policy is to NOT use Signed-off-by. That simple.

> > And also please send patches inline and via attachments.
>
> This would be difficult, as gmail does not allow no wrapping.
> git accepts this correctly, I use:
>
> $ curl article.gmane.org/gmane.linux.bluez.kernel/18337/raw | git am
>
> If this is really important I will try to find some other mail client.

This is not about being able to apply it, this is about being able to
review it properly and give inline comments.

Also just use git send-email. Works perfectly fine.

Regards

Marcel

2011-11-12 23:24:09

by Alon Bar-Lev

[permalink] [raw]

Subject: Re: [PATCH 1/2] Automake AM_CFLAGS, AM_LDFLAGS fixups

On Sun, Nov 13, 2011 at 1:15 AM, Marcel Holtmann <[email protected]> wrot=
e:
> Hi Alon,
>
>> Reference: "Flag Variables Ordering"[1]
>>
>> In automake, if you modify CFLAGS or LDFLAGS of a target you
>> actually override the autoconf supplied flags. This is highly
>> none standard and may lead to undesired results, as some
>> targets will be compiled with the autoconf supplied flags and
>> some won't.
>>
>> This patch adds AM_CFLAGS, AM_LDFLAGS to any flags override.
>>
>> Some notes:
>>
>> 1. Remove none common flags:
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 -DBLUETOOTH_PLUGIN_BUILTIN -DPLUGINDIR=3D\""=
$(build_plugindir)"\"
>> Moved to bluetoothd only, as BLUETOOTH_PLUGIN_BUILTIN should effect
>> only sources compiled within it.
>> And PLUGINDIR is used only by the daemon.
>>
>> 2. More targets now will be compiled with:
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 @DBUS_CFLAGS@ @GLIB_CFLAGS@ @CAPNG_CFLAGS@
>> If this is a problem I will add these to specific targets.
>>
>> 3. I am not sure there is a reason to set specific trivial CFLAGS for
>> simple includes in target specific. Simpler is to put all at AM_CFLAGS.
>>
>> 4. All @XXX_XXFLAGS@ can actually be used as make macros $(XXX_XXFLAGS),
>> Since AC_SUBST are automatically generated as make macros.
>>
>> 5. Why don't you split up the Makefile.am into smaller by
>> directory/library? It would be easier to control the setup.
>>
>> [1] http://www.gnu.org/software/automake/manual/automake.html#Flag-Varia=
bles-Ordering
>>
>> Signed-off-by: Alon Bar-Lev <[email protected]>
>
> please no Signed-off-by lines, we are not using them for the userspace
> parts.

I did not think it damage anything...

> And also please send patches inline and via attachments.

This would be difficult, as gmail does not allow no wrapping.
git accepts this correctly, I use:

$ curl article.gmane.org/gmane.linux.bluez.kernel/18337/raw | git am

If this is really important I will try to find some other mail client.

> I also prefer that you split this into two patches, one for CFLAGS and
> one for LDFLAGS.

In a few moments.

Alon.

2011-11-12 23:15:12

by Marcel Holtmann

[permalink] [raw]

Subject: Re: [PATCH 1/2] Automake AM_CFLAGS, AM_LDFLAGS fixups

Hi Alon,

> Reference: "Flag Variables Ordering"[1]
>
> In automake, if you modify CFLAGS or LDFLAGS of a target you
> actually override the autoconf supplied flags. This is highly
> none standard and may lead to undesired results, as some
> targets will be compiled with the autoconf supplied flags and
> some won't.
>
> This patch adds AM_CFLAGS, AM_LDFLAGS to any flags override.
>
> Some notes:
>
> 1. Remove none common flags:
> -DBLUETOOTH_PLUGIN_BUILTIN -DPLUGINDIR=\""$(build_plugindir)"\"
> Moved to bluetoothd only, as BLUETOOTH_PLUGIN_BUILTIN should effect
> only sources compiled within it.
> And PLUGINDIR is used only by the daemon.
>
> 2. More targets now will be compiled with:
> @DBUS_CFLAGS@ @GLIB_CFLAGS@ @CAPNG_CFLAGS@
> If this is a problem I will add these to specific targets.
>
> 3. I am not sure there is a reason to set specific trivial CFLAGS for
> simple includes in target specific. Simpler is to put all at AM_CFLAGS.
>
> 4. All @XXX_XXFLAGS@ can actually be used as make macros $(XXX_XXFLAGS),
> Since AC_SUBST are automatically generated as make macros.
>
> 5. Why don't you split up the Makefile.am into smaller by
> directory/library? It would be easier to control the setup.
>
> [1] http://www.gnu.org/software/automake/manual/automake.html#Flag-Variables-Ordering
>
> Signed-off-by: Alon Bar-Lev <[email protected]>

please no Signed-off-by lines, we are not using them for the userspace
parts. And also please send patches inline and via attachments.

I also prefer that you split this into two patches, one for CFLAGS and
one for LDFLAGS.

Regards

Marcel