diff mbox

[2/9] firefox: valgrind dependency needs --enable-tls for debug build

Message ID 1346855344-15081-3-git-send-email-stefan.froberg@petroprogram.com
State Deferred
Headers show

Commit Message

Stefan Fröberg Sept. 5, 2012, 2:28 p.m. UTC
Signed-off-by: Stefan Fröberg <stefan.froberg@petroprogram.com>
---
 package/valgrind/valgrind.mk |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

Comments

Arnout Vandecappelle Sept. 11, 2012, 10:04 p.m. UTC | #1
On 09/05/12 16:28, Stefan Fröberg wrote:
> +ifeq ($(BR2_TOOLCHAIN_BUILDROOT),y)&&  ($(BR2_GCC_ENABLE_TLS),y)
> +VALGRIND_CONF_OPT = --enable-tls
> +else
>   VALGRIND_CONF_OPT = --disable-tls
> +endif

  I don't like this because it only works for internal toolchains.

  Is it possible to remove the --en/disable-tls and let configure discover it by
itself?  I tried a few configs and it seems to work correctly...  The
--disable-tls was introduced by a version bump 7 years ago, without any
comment why it is needed.  It may have caused runtime problems, but those
may have disappeared by now too.  So I'd risk removing it completely.

  Regards,
  Arnout
Stefan Fröberg Sept. 11, 2012, 11:39 p.m. UTC | #2
12.9.2012 1:04, Arnout Vandecappelle kirjoitti:
> On 09/05/12 16:28, Stefan Fröberg wrote:
>> +ifeq ($(BR2_TOOLCHAIN_BUILDROOT),y)&&  ($(BR2_GCC_ENABLE_TLS),y)
>> +VALGRIND_CONF_OPT = --enable-tls
>> +else
>>   VALGRIND_CONF_OPT = --disable-tls
>> +endif
>
>  I don't like this because it only works for internal toolchains.
>
>  Is it possible to remove the --en/disable-tls and let configure
> discover it by
> itself?  I tried a few configs and it seems to work correctly...  The
> --disable-tls was introduced by a version bump 7 years ago, without any
> comment why it is needed.  It may have caused runtime problems, but those
> may have disappeared by now too.  So I'd risk removing it completely.
>

Firefox  did not complain about missing valgrind.h file ???
Strange...
I have to try to make a debug build of FF again  and valgrind with
--disable-tls and see

Stefan

>  Regards,
>  Arnout
Arnout Vandecappelle Sept. 13, 2012, 5:49 a.m. UTC | #3
On 09/12/12 01:39, Stefan Fröberg wrote:
> 12.9.2012 1:04, Arnout Vandecappelle kirjoitti:
>> On 09/05/12 16:28, Stefan Fröberg wrote:
>>> +ifeq ($(BR2_TOOLCHAIN_BUILDROOT),y)&&   ($(BR2_GCC_ENABLE_TLS),y)
>>> +VALGRIND_CONF_OPT = --enable-tls
>>> +else
>>>    VALGRIND_CONF_OPT = --disable-tls
>>> +endif
>>
>>   I don't like this because it only works for internal toolchains.
>>
>>   Is it possible to remove the --en/disable-tls and let configure
>> discover it by
>> itself?  I tried a few configs and it seems to work correctly...  The
>> --disable-tls was introduced by a version bump 7 years ago, without any
>> comment why it is needed.  It may have caused runtime problems, but those
>> may have disappeared by now too.  So I'd risk removing it completely.
>>
>
> Firefox  did not complain about missing valgrind.h file ???

  No, I meant that valgrind's configure seems to discover by itself if TLS is
available or not, i.e. removing the --disable-tls will default to --enable.

  So my suggestion is: instead of making the --en/disable-tls conditional,
just remove the --disable-tls completely.

  I haven't actually tried building with any of your patches.



  Regards,
  Arnout
diff mbox

Patch

diff --git a/package/valgrind/valgrind.mk b/package/valgrind/valgrind.mk
index 05f402f..1c44df7 100644
--- a/package/valgrind/valgrind.mk
+++ b/package/valgrind/valgrind.mk
@@ -7,7 +7,12 @@ 
 VALGRIND_VERSION = 3.7.0
 VALGRIND_SITE    = http://valgrind.org/downloads/
 VALGRIND_SOURCE  = valgrind-$(VALGRIND_VERSION).tar.bz2
+
+ifeq ($(BR2_TOOLCHAIN_BUILDROOT),y) && ($(BR2_GCC_ENABLE_TLS),y)
+VALGRIND_CONF_OPT = --enable-tls
+else
 VALGRIND_CONF_OPT = --disable-tls
+endif
 
 # On ARM, Valgrind only supports ARMv7, and uses the arch part of the
 # host tuple to determine whether it's being built for ARMv7 or