All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/1] ltp-testsuite: support building with toolchains without native RPC
@ 2015-07-24 20:08 Thomas De Schampheleire
  2015-07-25 12:19 ` Yann E. MORIN
  2015-07-25 14:32 ` Yann E. MORIN
  0 siblings, 2 replies; 6+ messages in thread
From: Thomas De Schampheleire @ 2015-07-24 20:08 UTC (permalink / raw)
  To: buildroot

From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

ltp-testsuite needs RPC, but this could also be provided by libtirpc.

Since musl toolchains never have RPC support, this change would now allow
building of ltp-testsuite on musl toolchains. Unfortunately, ltp-testsuite
does not build yet with musl, so a specific check on musl is added.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
---
 package/ltp-testsuite/Config.in        |  7 ++++---
 package/ltp-testsuite/ltp-testsuite.mk | 14 ++++++++++++--
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/package/ltp-testsuite/Config.in b/package/ltp-testsuite/Config.in
index 52c02ce..91b09ce 100644
--- a/package/ltp-testsuite/Config.in
+++ b/package/ltp-testsuite/Config.in
@@ -6,7 +6,8 @@ config BR2_PACKAGE_LTP_TESTSUITE
 	bool "ltp-testsuite"
 	depends on BR2_USE_MMU # fork()
 	depends on BR2_TOOLCHAIN_HAS_THREADS
-	depends on BR2_TOOLCHAIN_HAS_NATIVE_RPC
+	depends on !BR2_TOOLCHAIN_USES_MUSL
+	select BR2_PACKAGE_LIBTIRPC if !BR2_TOOLCHAIN_HAS_NATIVE_RPC
 	# does not build, cachectl.h issue
 	depends on !BR2_nios2
 	help
@@ -21,7 +22,7 @@ config BR2_PACKAGE_LTP_TESTSUITE
 
 	  http://ltp.sourceforge.net/
 
-comment "ltp-testsuite needs a toolchain w/ RPC, threads"
+comment "ltp-testsuite needs a non-musl toolchain w/ threads"
 	depends on !BR2_nios2
 	depends on BR2_USE_MMU
-	depends on !BR2_TOOLCHAIN_HAS_THREADS || !BR2_TOOLCHAIN_HAS_NATIVE_RPC
+	depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_TOOLCHAIN_USES_MUSL
diff --git a/package/ltp-testsuite/ltp-testsuite.mk b/package/ltp-testsuite/ltp-testsuite.mk
index 2d4f286..05ffaf1 100644
--- a/package/ltp-testsuite/ltp-testsuite.mk
+++ b/package/ltp-testsuite/ltp-testsuite.mk
@@ -19,8 +19,18 @@ endif
 
 # ltp-testsuite uses <fts.h>, which isn't compatible with largefile
 # support.
+LTP_TESTSUITE_CFLAGS = $(filter-out -D_FILE_OFFSET_BITS=64,$(TARGET_CFLAGS))
+LTP_TESTSUITE_CPPFLAGS = $(filter-out -D_FILE_OFFSET_BITS=64,$(TARGET_CPPFLAGS))
+
+ifeq ($(BR2_PACKAGE_LIBTIRPC),y)
+LTP_TESTSUITE_DEPENDENCIES += libtirpc
+LTP_TESTSUITE_CFLAGS += -I$(STAGING_DIR)/usr/include/tirpc/
+LTP_TESTSUITE_LDFLAGS += -ltirpc
+endif
+
 LTP_TESTSUITE_CONF_ENV += \
-	CFLAGS="$(filter-out -D_FILE_OFFSET_BITS=64,$(TARGET_CFLAGS))" \
-	CPPFLAGS="$(filter-out -D_FILE_OFFSET_BITS=64,$(TARGET_CPPFLAGS))"
+	CFLAGS="$(LTP_TESTSUITE_CFLAGS)" \
+	CPPFLAGS="$(LTP_TESTSUITE_CPPFLAGS)" \
+	LDFLAGS="$(LTP_TESTSUITE_LDFLAGS)"
 
 $(eval $(autotools-package))
-- 
1.8.5.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH 1/1] ltp-testsuite: support building with toolchains without native RPC
  2015-07-24 20:08 [Buildroot] [PATCH 1/1] ltp-testsuite: support building with toolchains without native RPC Thomas De Schampheleire
@ 2015-07-25 12:19 ` Yann E. MORIN
  2015-07-25 13:12   ` Thomas De Schampheleire
  2015-07-25 14:32 ` Yann E. MORIN
  1 sibling, 1 reply; 6+ messages in thread
From: Yann E. MORIN @ 2015-07-25 12:19 UTC (permalink / raw)
  To: buildroot

Thmoas, All,

On 2015-07-24 22:08 +0200, Thomas De Schampheleire spake thusly:
> From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
> 
> ltp-testsuite needs RPC, but this could also be provided by libtirpc.
> 
> Since musl toolchains never have RPC support, this change would now allow
> building of ltp-testsuite on musl toolchains. Unfortunately, ltp-testsuite
> does not build yet with musl, so a specific check on musl is added.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
> ---
>  package/ltp-testsuite/Config.in        |  7 ++++---
>  package/ltp-testsuite/ltp-testsuite.mk | 14 ++++++++++++--
>  2 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/package/ltp-testsuite/Config.in b/package/ltp-testsuite/Config.in
> index 52c02ce..91b09ce 100644
> --- a/package/ltp-testsuite/Config.in
> +++ b/package/ltp-testsuite/Config.in
> @@ -6,7 +6,8 @@ config BR2_PACKAGE_LTP_TESTSUITE
>  	bool "ltp-testsuite"
>  	depends on BR2_USE_MMU # fork()
>  	depends on BR2_TOOLCHAIN_HAS_THREADS
> -	depends on BR2_TOOLCHAIN_HAS_NATIVE_RPC
> +	depends on !BR2_TOOLCHAIN_USES_MUSL
> +	select BR2_PACKAGE_LIBTIRPC if !BR2_TOOLCHAIN_HAS_NATIVE_RPC
>  	# does not build, cachectl.h issue
>  	depends on !BR2_nios2
>  	help
> @@ -21,7 +22,7 @@ config BR2_PACKAGE_LTP_TESTSUITE
>  
>  	  http://ltp.sourceforge.net/
>  
> -comment "ltp-testsuite needs a toolchain w/ RPC, threads"
> +comment "ltp-testsuite needs a non-musl toolchain w/ threads"
>  	depends on !BR2_nios2
>  	depends on BR2_USE_MMU
> -	depends on !BR2_TOOLCHAIN_HAS_THREADS || !BR2_TOOLCHAIN_HAS_NATIVE_RPC
> +	depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_TOOLCHAIN_USES_MUSL
> diff --git a/package/ltp-testsuite/ltp-testsuite.mk b/package/ltp-testsuite/ltp-testsuite.mk
> index 2d4f286..05ffaf1 100644
> --- a/package/ltp-testsuite/ltp-testsuite.mk
> +++ b/package/ltp-testsuite/ltp-testsuite.mk
> @@ -19,8 +19,18 @@ endif
>  
>  # ltp-testsuite uses <fts.h>, which isn't compatible with largefile
>  # support.
> +LTP_TESTSUITE_CFLAGS = $(filter-out -D_FILE_OFFSET_BITS=64,$(TARGET_CFLAGS))
> +LTP_TESTSUITE_CPPFLAGS = $(filter-out -D_FILE_OFFSET_BITS=64,$(TARGET_CPPFLAGS))

This hunk does not seem to have anything to do with the issue at hand,
namely RPC and musl, nor is it explained in the commit log. Care to
explain why filtering it out is needed (in the commit log and a comment
in the code), please?

Regards,
Yann E. MORIN.

> +ifeq ($(BR2_PACKAGE_LIBTIRPC),y)
> +LTP_TESTSUITE_DEPENDENCIES += libtirpc
> +LTP_TESTSUITE_CFLAGS += -I$(STAGING_DIR)/usr/include/tirpc/
> +LTP_TESTSUITE_LDFLAGS += -ltirpc
> +endif
> +
>  LTP_TESTSUITE_CONF_ENV += \
> -	CFLAGS="$(filter-out -D_FILE_OFFSET_BITS=64,$(TARGET_CFLAGS))" \
> -	CPPFLAGS="$(filter-out -D_FILE_OFFSET_BITS=64,$(TARGET_CPPFLAGS))"
> +	CFLAGS="$(LTP_TESTSUITE_CFLAGS)" \
> +	CPPFLAGS="$(LTP_TESTSUITE_CPPFLAGS)" \
> +	LDFLAGS="$(LTP_TESTSUITE_LDFLAGS)"
>  
>  $(eval $(autotools-package))
> -- 
> 1.8.5.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH 1/1] ltp-testsuite: support building with toolchains without native RPC
  2015-07-25 12:19 ` Yann E. MORIN
@ 2015-07-25 13:12   ` Thomas De Schampheleire
  2015-07-25 14:15     ` Yann E. MORIN
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas De Schampheleire @ 2015-07-25 13:12 UTC (permalink / raw)
  To: buildroot

Hi Yann,

On July 25, 2015 2:19:50 PM CEST, "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
>Thmoas, All,
>
>On 2015-07-24 22:08 +0200, Thomas De Schampheleire spake thusly:
>> From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>> 
>> ltp-testsuite needs RPC, but this could also be provided by libtirpc.
>> 
>> Since musl toolchains never have RPC support, this change would now
>allow
>> building of ltp-testsuite on musl toolchains. Unfortunately,
>ltp-testsuite
>> does not build yet with musl, so a specific check on musl is added.
>> 
>> Signed-off-by: Thomas De Schampheleire
><thomas.de.schampheleire@gmail.com>
>> ---
>>  package/ltp-testsuite/Config.in        |  7 ++++---
>>  package/ltp-testsuite/ltp-testsuite.mk | 14 ++++++++++++--
>>  2 files changed, 16 insertions(+), 5 deletions(-)
>> 
>> diff --git a/package/ltp-testsuite/Config.in
>b/package/ltp-testsuite/Config.in
>> index 52c02ce..91b09ce 100644
>> --- a/package/ltp-testsuite/Config.in
>> +++ b/package/ltp-testsuite/Config.in
>> @@ -6,7 +6,8 @@ config BR2_PACKAGE_LTP_TESTSUITE
>>  	bool "ltp-testsuite"
>>  	depends on BR2_USE_MMU # fork()
>>  	depends on BR2_TOOLCHAIN_HAS_THREADS
>> -	depends on BR2_TOOLCHAIN_HAS_NATIVE_RPC
>> +	depends on !BR2_TOOLCHAIN_USES_MUSL
>> +	select BR2_PACKAGE_LIBTIRPC if !BR2_TOOLCHAIN_HAS_NATIVE_RPC
>>  	# does not build, cachectl.h issue
>>  	depends on !BR2_nios2
>>  	help
>> @@ -21,7 +22,7 @@ config BR2_PACKAGE_LTP_TESTSUITE
>>  
>>  	  http://ltp.sourceforge.net/
>>  
>> -comment "ltp-testsuite needs a toolchain w/ RPC, threads"
>> +comment "ltp-testsuite needs a non-musl toolchain w/ threads"
>>  	depends on !BR2_nios2
>>  	depends on BR2_USE_MMU
>> -	depends on !BR2_TOOLCHAIN_HAS_THREADS ||
>!BR2_TOOLCHAIN_HAS_NATIVE_RPC
>> +	depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_TOOLCHAIN_USES_MUSL
>> diff --git a/package/ltp-testsuite/ltp-testsuite.mk
>b/package/ltp-testsuite/ltp-testsuite.mk
>> index 2d4f286..05ffaf1 100644
>> --- a/package/ltp-testsuite/ltp-testsuite.mk
>> +++ b/package/ltp-testsuite/ltp-testsuite.mk
>> @@ -19,8 +19,18 @@ endif
>>  
>>  # ltp-testsuite uses <fts.h>, which isn't compatible with largefile
>>  # support.
>> +LTP_TESTSUITE_CFLAGS = $(filter-out
>-D_FILE_OFFSET_BITS=64,$(TARGET_CFLAGS))
>> +LTP_TESTSUITE_CPPFLAGS = $(filter-out
>-D_FILE_OFFSET_BITS=64,$(TARGET_CPPFLAGS))
>
>This hunk does not seem to have anything to do with the issue at hand,
>namely RPC and musl, nor is it explained in the commit log. Care to
>explain why filtering it out is needed (in the commit log and a comment
>in the code), please?

These lines were already in that file (see removes below); I left the comment clarifying it. 
I just introduced intermediate variables to cleanly handle the tirpc changes.

>
>> +ifeq ($(BR2_PACKAGE_LIBTIRPC),y)
>> +LTP_TESTSUITE_DEPENDENCIES += libtirpc
>> +LTP_TESTSUITE_CFLAGS += -I$(STAGING_DIR)/usr/include/tirpc/
>> +LTP_TESTSUITE_LDFLAGS += -ltirpc
>> +endif
>> +
>>  LTP_TESTSUITE_CONF_ENV += \
>> -	CFLAGS="$(filter-out -D_FILE_OFFSET_BITS=64,$(TARGET_CFLAGS))" \
>> -	CPPFLAGS="$(filter-out -D_FILE_OFFSET_BITS=64,$(TARGET_CPPFLAGS))"

---------^

>> +	CFLAGS="$(LTP_TESTSUITE_CFLAGS)" \
>> +	CPPFLAGS="$(LTP_TESTSUITE_CPPFLAGS)" \
>> +	LDFLAGS="$(LTP_TESTSUITE_LDFLAGS)"
>>  
>>  $(eval $(autotools-package))

/Thomas

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH 1/1] ltp-testsuite: support building with toolchains without native RPC
  2015-07-25 13:12   ` Thomas De Schampheleire
@ 2015-07-25 14:15     ` Yann E. MORIN
  0 siblings, 0 replies; 6+ messages in thread
From: Yann E. MORIN @ 2015-07-25 14:15 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2015-07-25 15:12 +0200, Thomas De Schampheleire spake thusly:
> On July 25, 2015 2:19:50 PM CEST, "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> >On 2015-07-24 22:08 +0200, Thomas De Schampheleire spake thusly:
[--SNIP--]
> >> index 2d4f286..05ffaf1 100644
> >> --- a/package/ltp-testsuite/ltp-testsuite.mk
> >> +++ b/package/ltp-testsuite/ltp-testsuite.mk
> >> @@ -19,8 +19,18 @@ endif
> >>  
> >>  # ltp-testsuite uses <fts.h>, which isn't compatible with largefile
> >>  # support.
> >> +LTP_TESTSUITE_CFLAGS = $(filter-out
> >-D_FILE_OFFSET_BITS=64,$(TARGET_CFLAGS))
> >> +LTP_TESTSUITE_CPPFLAGS = $(filter-out
> >-D_FILE_OFFSET_BITS=64,$(TARGET_CPPFLAGS))
> >
> >This hunk does not seem to have anything to do with the issue at hand,
> >namely RPC and musl, nor is it explained in the commit log. Care to
> >explain why filtering it out is needed (in the commit log and a comment
> >in the code), please?
> 
> These lines were already in that file (see removes below); I left the comment clarifying it. 
> I just introduced intermediate variables to cleanly handle the tirpc changes.

Indeed, my bad.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH 1/1] ltp-testsuite: support building with toolchains without native RPC
  2015-07-24 20:08 [Buildroot] [PATCH 1/1] ltp-testsuite: support building with toolchains without native RPC Thomas De Schampheleire
  2015-07-25 12:19 ` Yann E. MORIN
@ 2015-07-25 14:32 ` Yann E. MORIN
  2015-07-25 19:44   ` Thomas De Schampheleire
  1 sibling, 1 reply; 6+ messages in thread
From: Yann E. MORIN @ 2015-07-25 14:32 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2015-07-24 22:08 +0200, Thomas De Schampheleire spake thusly:
> From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
> 
> ltp-testsuite needs RPC, but this could also be provided by libtirpc.
> 
> Since musl toolchains never have RPC support, this change would now allow
> building of ltp-testsuite on musl toolchains. Unfortunately, ltp-testsuite
> does not build yet with musl, so a specific check on musl is added.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
> ---
>  package/ltp-testsuite/Config.in        |  7 ++++---
>  package/ltp-testsuite/ltp-testsuite.mk | 14 ++++++++++++--
>  2 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/package/ltp-testsuite/Config.in b/package/ltp-testsuite/Config.in
> index 52c02ce..91b09ce 100644
> --- a/package/ltp-testsuite/Config.in
> +++ b/package/ltp-testsuite/Config.in
> @@ -6,7 +6,8 @@ config BR2_PACKAGE_LTP_TESTSUITE
>  	bool "ltp-testsuite"
>  	depends on BR2_USE_MMU # fork()
>  	depends on BR2_TOOLCHAIN_HAS_THREADS
> -	depends on BR2_TOOLCHAIN_HAS_NATIVE_RPC
> +	depends on !BR2_TOOLCHAIN_USES_MUSL
> +	select BR2_PACKAGE_LIBTIRPC if !BR2_TOOLCHAIN_HAS_NATIVE_RPC

libtirpc depends on !BR2_TOOLCHAIN_EXTERNAL_BLACKFIN_UCLINUX_2012R2 so I
wonder why you did not propagate this dependency here.

However, that is not necessary, since that toolchain does provide native
RPC, so we would not be selecting libtirpc with that toolchain.

Maybe worth a comment?

>  	# does not build, cachectl.h issue
>  	depends on !BR2_nios2
>  	help
> @@ -21,7 +22,7 @@ config BR2_PACKAGE_LTP_TESTSUITE
>  
>  	  http://ltp.sourceforge.net/
>  
> -comment "ltp-testsuite needs a toolchain w/ RPC, threads"
> +comment "ltp-testsuite needs a non-musl toolchain w/ threads"

We usually use positive logic, like in samab4, libunwind or vlc:

    comment "ltp-testsuite needs an (e)glibc or uClibc toolchain w/ threads"

> diff --git a/package/ltp-testsuite/ltp-testsuite.mk b/package/ltp-testsuite/ltp-testsuite.mk
> index 2d4f286..05ffaf1 100644
> --- a/package/ltp-testsuite/ltp-testsuite.mk
> +++ b/package/ltp-testsuite/ltp-testsuite.mk
> @@ -19,8 +19,18 @@ endif
>  
>  # ltp-testsuite uses <fts.h>, which isn't compatible with largefile
>  # support.
> +LTP_TESTSUITE_CFLAGS = $(filter-out -D_FILE_OFFSET_BITS=64,$(TARGET_CFLAGS))
> +LTP_TESTSUITE_CPPFLAGS = $(filter-out -D_FILE_OFFSET_BITS=64,$(TARGET_CPPFLAGS))
> +
> +ifeq ($(BR2_PACKAGE_LIBTIRPC),y)
> +LTP_TESTSUITE_DEPENDENCIES += libtirpc
> +LTP_TESTSUITE_CFLAGS += -I$(STAGING_DIR)/usr/include/tirpc/
> +LTP_TESTSUITE_LDFLAGS += -ltirpc

libtirpc install a .pc file, so why not use it, as in:

    LTP_TESTSUITE_DEPENDENCIES += libtirpc host-pkgconf
    LTP_TESTSUITE_CFLAGS += `$(HOST_PKG_CONFIG_BINARY) --cflags libtirpc`
    LTP_TESTSUITE_LDFLAGS += `$(HOST_PKG_COFNIG_BINARY) --libs libtirpc`

Shoehorn a --static when needed, too, for good measure.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH 1/1] ltp-testsuite: support building with toolchains without native RPC
  2015-07-25 14:32 ` Yann E. MORIN
@ 2015-07-25 19:44   ` Thomas De Schampheleire
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas De Schampheleire @ 2015-07-25 19:44 UTC (permalink / raw)
  To: buildroot

Hi Yann,

Thanks for the review, see my feedback below...

On Sat, Jul 25, 2015 at 4:32 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Thomas, All,
>
> On 2015-07-24 22:08 +0200, Thomas De Schampheleire spake thusly:
>> From: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>>
>> ltp-testsuite needs RPC, but this could also be provided by libtirpc.
>>
>> Since musl toolchains never have RPC support, this change would now allow
>> building of ltp-testsuite on musl toolchains. Unfortunately, ltp-testsuite
>> does not build yet with musl, so a specific check on musl is added.
>>
>> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>> ---
>>  package/ltp-testsuite/Config.in        |  7 ++++---
>>  package/ltp-testsuite/ltp-testsuite.mk | 14 ++++++++++++--
>>  2 files changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/package/ltp-testsuite/Config.in b/package/ltp-testsuite/Config.in
>> index 52c02ce..91b09ce 100644
>> --- a/package/ltp-testsuite/Config.in
>> +++ b/package/ltp-testsuite/Config.in
>> @@ -6,7 +6,8 @@ config BR2_PACKAGE_LTP_TESTSUITE
>>       bool "ltp-testsuite"
>>       depends on BR2_USE_MMU # fork()
>>       depends on BR2_TOOLCHAIN_HAS_THREADS
>> -     depends on BR2_TOOLCHAIN_HAS_NATIVE_RPC
>> +     depends on !BR2_TOOLCHAIN_USES_MUSL
>> +     select BR2_PACKAGE_LIBTIRPC if !BR2_TOOLCHAIN_HAS_NATIVE_RPC
>
> libtirpc depends on !BR2_TOOLCHAIN_EXTERNAL_BLACKFIN_UCLINUX_2012R2 so I
> wonder why you did not propagate this dependency here.
>
> However, that is not necessary, since that toolchain does provide native
> RPC, so we would not be selecting libtirpc with that toolchain.
>
> Maybe worth a comment?

In fact there is another reason why this blackfin-specific dependency
does not need propagation: ltp-testsuite depends on MMU support, which
is not available for Blackfin anyway.

I used the other packages that have libtirpc support as example and
used the same methods. All of them depend on MMU and none of them
propagated the blackfin dependency. I did not see any comments about
this so I did neither. I can clarify it in the commit message though.

>
>>       # does not build, cachectl.h issue
>>       depends on !BR2_nios2
>>       help
>> @@ -21,7 +22,7 @@ config BR2_PACKAGE_LTP_TESTSUITE
>>
>>         http://ltp.sourceforge.net/
>>
>> -comment "ltp-testsuite needs a toolchain w/ RPC, threads"
>> +comment "ltp-testsuite needs a non-musl toolchain w/ threads"
>
> We usually use positive logic, like in samab4, libunwind or vlc:
>
>     comment "ltp-testsuite needs an (e)glibc or uClibc toolchain w/ threads"

Here it depends on the viewpoint: the 'depends on' check is on !MUSL,
so I find it logical to make the command the same.
Alternatively, we can check on GLIBC || UCLIBC and make the comment
correspond, but I feel it's not correct. Suppose tomorrow we add
support for another C library: when checking on !MUSL, ltp-testsuite
could be built against this C library, as expected. However, if we
check on GLIBC || UCLIBC, ltp-testsuite would be automatically
excluded from compilation against this C library, without a valid
reason.

>
>> diff --git a/package/ltp-testsuite/ltp-testsuite.mk b/package/ltp-testsuite/ltp-testsuite.mk
>> index 2d4f286..05ffaf1 100644
>> --- a/package/ltp-testsuite/ltp-testsuite.mk
>> +++ b/package/ltp-testsuite/ltp-testsuite.mk
>> @@ -19,8 +19,18 @@ endif
>>
>>  # ltp-testsuite uses <fts.h>, which isn't compatible with largefile
>>  # support.
>> +LTP_TESTSUITE_CFLAGS = $(filter-out -D_FILE_OFFSET_BITS=64,$(TARGET_CFLAGS))
>> +LTP_TESTSUITE_CPPFLAGS = $(filter-out -D_FILE_OFFSET_BITS=64,$(TARGET_CPPFLAGS))
>> +
>> +ifeq ($(BR2_PACKAGE_LIBTIRPC),y)
>> +LTP_TESTSUITE_DEPENDENCIES += libtirpc
>> +LTP_TESTSUITE_CFLAGS += -I$(STAGING_DIR)/usr/include/tirpc/
>> +LTP_TESTSUITE_LDFLAGS += -ltirpc
>
> libtirpc install a .pc file, so why not use it, as in:
>
>     LTP_TESTSUITE_DEPENDENCIES += libtirpc host-pkgconf
>     LTP_TESTSUITE_CFLAGS += `$(HOST_PKG_CONFIG_BINARY) --cflags libtirpc`
>     LTP_TESTSUITE_LDFLAGS += `$(HOST_PKG_COFNIG_BINARY) --libs libtirpc`

This is because I did the same as in other packages that use libtirpc,
but you're absolutely right.
I will fix that and also line up the other packages to use the same approach.

>
> Shoehorn a --static when needed, too, for good measure.

Yes, I will add that.
However, it does look like --static is not used consistently currently:

$ grep -rn '$(PKG_CONFIG_HOST_BINARY)' package/ | grep -v '\--static' | wc -l
37
$ grep -rn '$(PKG_CONFIG_HOST_BINARY)' package/ | grep '\--static' | wc -l
5

So this is another cleanup opportunity...

/Thomas

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-07-25 19:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-24 20:08 [Buildroot] [PATCH 1/1] ltp-testsuite: support building with toolchains without native RPC Thomas De Schampheleire
2015-07-25 12:19 ` Yann E. MORIN
2015-07-25 13:12   ` Thomas De Schampheleire
2015-07-25 14:15     ` Yann E. MORIN
2015-07-25 14:32 ` Yann E. MORIN
2015-07-25 19:44   ` Thomas De Schampheleire

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.