Message ID | 1346855344-15081-3-git-send-email-stefan.froberg@petroprogram.com |
---|---|
State | Deferred |
Headers | show |
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
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
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 --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
Signed-off-by: Stefan Fröberg <stefan.froberg@petroprogram.com> --- package/valgrind/valgrind.mk | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)