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

login
register
mail settings
Submitter Stefan Fröberg
Date Sept. 5, 2012, 2:28 p.m.
Message ID <1346855344-15081-3-git-send-email-stefan.froberg@petroprogram.com>
Download mbox | patch
Permalink /patch/181871/
State Deferred
Headers show

Comments

Stefan Fröberg - Sept. 5, 2012, 2:28 p.m.
Signed-off-by: Stefan Fröberg <stefan.froberg@petroprogram.com>
---
 package/valgrind/valgrind.mk |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
Arnout Vandecappelle - Sept. 11, 2012, 10:04 p.m.
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.
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.
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

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