diff mbox series

open_posix_testsuite: Fix compilation issues

Message ID 1584430031-47227-1-git-send-email-zou_wei@huawei.com
State Changes Requested
Headers show
Series open_posix_testsuite: Fix compilation issues | expand

Commit Message

Samuel Zou March 17, 2020, 7:27 a.m. UTC 
Fix compilation issues:
1. Add the '-lpthread -lrt -lm' config into LDFLAGS
2. Update makefile and locate-test script

Signed-off-by: Zou Wei <zou_wei@huawei.com>
---
 testcases/open_posix_testsuite/LDFLAGS             | 1 +
 testcases/open_posix_testsuite/functional/Makefile | 2 +-
 testcases/open_posix_testsuite/scripts/locate-test | 2 +-
 testcases/open_posix_testsuite/stress/Makefile     | 2 +-
 4 files changed, 4 insertions(+), 3 deletions(-)

--
2.6.2

Comments

Li Wang March 17, 2020, 8:21 a.m. UTC | #1 
On Tue, Mar 17, 2020 at 3:21 PM Zou Wei <zou_wei@huawei.com> wrote:

> Fix compilation issues:
> 1. Add the '-lpthread -lrt -lm' config into LDFLAGS
> 2. Update makefile and locate-test script
>
> Signed-off-by: Zou Wei <zou_wei@huawei.com>
>
Reviewed-by: Li Wang <liwang@redhat.com>
Yang Xu March 17, 2020, 9:07 a.m. UTC | #2 
Hi Zou


> Fix compilation issues:
> 1. Add the '-lpthread -lrt -lm' config into LDFLAGS
See ltp/openposix/build[1] documention, as below:

----------------------------
If you need to pass in any header locations, preprocessor definitions, etc,
   set the value via CFLAGS.
- If you need to pass in any libraries for linking, set the value via 
LDLIBS.
- If you need to pass any other values to the linker, set the value via 
LDFLAGS.
----------------------------

I guess we should use LDLIBS.

Also, do we must specify three '-lpthread -lrt -lm' configs into this?
Some case only needs one lib, such as functional/timers/timers .  I 
guess we can only add LDLIBS in case directory such as 
functional/timers/timers because generate-makefiles.sh will load libs 
from current directory.

ps: I will test this patch by using travis.

[1]https://github.com/linux-test-project/ltp/blob/master/testcases/open_posix_testsuite/BUILD

Best Regards
Yang Xu
> 2. Update makefile and locate-test script
> 
> Signed-off-by: Zou Wei <zou_wei@huawei.com>
> ---
>   testcases/open_posix_testsuite/LDFLAGS             | 1 +
>   testcases/open_posix_testsuite/functional/Makefile | 2 +-
>   testcases/open_posix_testsuite/scripts/locate-test | 2 +-
>   testcases/open_posix_testsuite/stress/Makefile     | 2 +-
>   4 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/testcases/open_posix_testsuite/LDFLAGS b/testcases/open_posix_testsuite/LDFLAGS
> index e69de29..6c112d3 100644
> --- a/testcases/open_posix_testsuite/LDFLAGS
> +++ b/testcases/open_posix_testsuite/LDFLAGS
> @@ -0,0 +1 @@
> +-lpthread -lrt -lm
> diff --git a/testcases/open_posix_testsuite/functional/Makefile b/testcases/open_posix_testsuite/functional/Makefile
> index 3b22c89..50e03aa 100644
> --- a/testcases/open_posix_testsuite/functional/Makefile
> +++ b/testcases/open_posix_testsuite/functional/Makefile
> @@ -5,7 +5,7 @@
>   #
> 
>   all clean install test:
> -	@for dir in `ls -d */Makefile 2>/dev/null | sed -e 's,/Makefile$$,,g'`; do \
> +	@for dir in `find . -name *Makefile | egrep -v '^./Makefile' | sed -e 's,/Makefile$$,,g'`; do \
>   		$(MAKE) -C $$dir $@;						   \
>   	done
> 
> diff --git a/testcases/open_posix_testsuite/scripts/locate-test b/testcases/open_posix_testsuite/scripts/locate-test
> index 27f0bb5..e4ef994 100755
> --- a/testcases/open_posix_testsuite/scripts/locate-test
> +++ b/testcases/open_posix_testsuite/scripts/locate-test
> @@ -92,7 +92,7 @@ runnable)
>   	# waltz down the tools directory and try and build t0 (which doesn't
>   	# make sense as it's a tool, not a test). Better criterion needs to
>   	# be established for this file.
> -	find "$WHERE/conformance" "$WHERE/stress" -type f -name '*[0-9].c' -o -name '[0-9]*-[0-9]*.sh' | grep -v buildonly | grep -v '^./tools'
> +	find "$WHERE/conformance" "$WHERE/stress" -type f -name '*[0-9a-z].c' -o -name '[0-9]*-[0-9]*.sh' | grep -v buildonly | grep -v '^./tools' | grep -v 'testfrmw.c$'
>   	find "$WHERE/functional" -type f -name '*.c'
>   	;;
>   test-tools)
> diff --git a/testcases/open_posix_testsuite/stress/Makefile b/testcases/open_posix_testsuite/stress/Makefile
> index b09527f..d6872a9 100644
> --- a/testcases/open_posix_testsuite/stress/Makefile
> +++ b/testcases/open_posix_testsuite/stress/Makefile
> @@ -5,7 +5,7 @@
>   #
> 
>   all clean install test:
> -	@for dir in `ls -d */Makefile 2>/dev/null | sed -e 's,/Makefile$$,,g'`; do \
> +	@for dir in `find . -name *Makefile | egrep -v '^./Makefile' | sed -e 's,/Makefile$$,,g'`; do \
>   		$(MAKE) -C $$dir $@;						  \
>   	done
> 
> --
> 2.6.2
> 
>
Li Wang March 17, 2020, 9:32 a.m. UTC | #3 
On Tue, Mar 17, 2020 at 5:08 PM Yang Xu <xuyang2018.jy@cn.fujitsu.com>
wrote:

> Hi Zou
>
>
> > Fix compilation issues:
> > 1. Add the '-lpthread -lrt -lm' config into LDFLAGS
> See ltp/openposix/build[1] documention, as below:
>
> ----------------------------
> If you need to pass in any header locations, preprocessor definitions, etc,
>    set the value via CFLAGS.
> - If you need to pass in any libraries for linking, set the value via
> LDLIBS.
> - If you need to pass any other values to the linker, set the value via
> LDFLAGS.
> ----------------------------
>
> I guess we should use LDLIBS.
>

Xu is right, I misread these two words(LDLIBS vs LDFLAGS) during my
reviewing.
Enji Cooper March 17, 2020, 11:30 a.m. UTC | #4 
NAK.

This should use LDLIBS, as others have pointed out, not LDFLAGS.

This also spams these libraries on all tests, instead of the ones that need it.

I can see there’s a desire to build/make open posix testsuite by default. I’ll look into making it so soon because the current model is a PITA to maintain and doesn’t integrate well into CI/CD.

Cheers,
-Enji

> On Mar 17, 2020, at 00:20, Zou Wei <zou_wei@huawei.com> wrote:
> 
> Fix compilation issues:
> 1. Add the '-lpthread -lrt -lm' config into LDFLAGS
> 2. Update makefile and locate-test script
> 
> Signed-off-by: Zou Wei <zou_wei@huawei.com>
> ---
> testcases/open_posix_testsuite/LDFLAGS             | 1 +
> testcases/open_posix_testsuite/functional/Makefile | 2 +-
> testcases/open_posix_testsuite/scripts/locate-test | 2 +-
> testcases/open_posix_testsuite/stress/Makefile     | 2 +-
> 4 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/testcases/open_posix_testsuite/LDFLAGS b/testcases/open_posix_testsuite/LDFLAGS
> index e69de29..6c112d3 100644
> --- a/testcases/open_posix_testsuite/LDFLAGS
> +++ b/testcases/open_posix_testsuite/LDFLAGS
> @@ -0,0 +1 @@
> +-lpthread -lrt -lm
> diff --git a/testcases/open_posix_testsuite/functional/Makefile b/testcases/open_posix_testsuite/functional/Makefile
> index 3b22c89..50e03aa 100644
> --- a/testcases/open_posix_testsuite/functional/Makefile
> +++ b/testcases/open_posix_testsuite/functional/Makefile
> @@ -5,7 +5,7 @@
> #
> 
> all clean install test:
> -    @for dir in `ls -d */Makefile 2>/dev/null | sed -e 's,/Makefile$$,,g'`; do \
> +    @for dir in `find . -name *Makefile | egrep -v '^./Makefile' | sed -e 's,/Makefile$$,,g'`; do \
>        $(MAKE) -C $$dir $@;                           \
>    done
> 
> diff --git a/testcases/open_posix_testsuite/scripts/locate-test b/testcases/open_posix_testsuite/scripts/locate-test
> index 27f0bb5..e4ef994 100755
> --- a/testcases/open_posix_testsuite/scripts/locate-test
> +++ b/testcases/open_posix_testsuite/scripts/locate-test
> @@ -92,7 +92,7 @@ runnable)
>    # waltz down the tools directory and try and build t0 (which doesn't
>    # make sense as it's a tool, not a test). Better criterion needs to
>    # be established for this file.
> -    find "$WHERE/conformance" "$WHERE/stress" -type f -name '*[0-9].c' -o -name '[0-9]*-[0-9]*.sh' | grep -v buildonly | grep -v '^./tools'
> +    find "$WHERE/conformance" "$WHERE/stress" -type f -name '*[0-9a-z].c' -o -name '[0-9]*-[0-9]*.sh' | grep -v buildonly | grep -v '^./tools' | grep -v 'testfrmw.c$'
>    find "$WHERE/functional" -type f -name '*.c'
>    ;;
> test-tools)
> diff --git a/testcases/open_posix_testsuite/stress/Makefile b/testcases/open_posix_testsuite/stress/Makefile
> index b09527f..d6872a9 100644
> --- a/testcases/open_posix_testsuite/stress/Makefile
> +++ b/testcases/open_posix_testsuite/stress/Makefile
> @@ -5,7 +5,7 @@
> #
> 
> all clean install test:
> -    @for dir in `ls -d */Makefile 2>/dev/null | sed -e 's,/Makefile$$,,g'`; do \
> +    @for dir in `find . -name *Makefile | egrep -v '^./Makefile' | sed -e 's,/Makefile$$,,g'`; do \
>        $(MAKE) -C $$dir $@;                          \
>    done
> 
> --
> 2.6.2
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp
Cyril Hrubis March 17, 2020, 8:52 p.m. UTC | #5 
Hi!
> Fix compilation issues:
> 1. Add the '-lpthread -lrt -lm' config into LDFLAGS

Just linking with -lpthread is not correct way how to compile pthread
programs on Linux, we have to add -pthread to both CLFAGS and LDFLAGS.

I guess that we should also default to the output of getconf for pthread
CFLAGS and LDFLAGS if available[1]. Unfortunately it looks like this is not
implemented on Linux yet.

So if getconf fails we will have to probably fallback to per-OS table of
CFLAGS and LDFLAGS.

[1] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/c99.html#tagtcjh_25
Samuel Zou March 19, 2020, 1:26 a.m. UTC | #6 
Hi Cyril,

So we need to add -pthread to both CFLAGS and LDFLAGS to avoid the compilation and linking problem?

Best Regards
Zou Wei

-----邮件原件-----
发件人: Cyril Hrubis [mailto:chrubis@suse.cz] 
发送时间: 2020年3月18日 4:52
收件人: Zouwei (Samuel) <zou_wei@huawei.com>
抄送: ltp@lists.linux.it
主题: Re: [LTP] [PATCH] open_posix_testsuite: Fix compilation issues

Hi!
> Fix compilation issues:
> 1. Add the '-lpthread -lrt -lm' config into LDFLAGS

Just linking with -lpthread is not correct way how to compile pthread programs on Linux, we have to add -pthread to both CLFAGS and LDFLAGS.

I guess that we should also default to the output of getconf for pthread CFLAGS and LDFLAGS if available[1]. Unfortunately it looks like this is not implemented on Linux yet.

So if getconf fails we will have to probably fallback to per-OS table of CFLAGS and LDFLAGS.

[1] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/c99.html#tagtcjh_25

--
Cyril Hrubis
chrubis@suse.cz
diff mbox series

Patch

diff --git a/testcases/open_posix_testsuite/LDFLAGS b/testcases/open_posix_testsuite/LDFLAGS
index e69de29..6c112d3 100644
--- a/testcases/open_posix_testsuite/LDFLAGS
+++ b/testcases/open_posix_testsuite/LDFLAGS
@@ -0,0 +1 @@ 
+-lpthread -lrt -lm
diff --git a/testcases/open_posix_testsuite/functional/Makefile b/testcases/open_posix_testsuite/functional/Makefile
index 3b22c89..50e03aa 100644
--- a/testcases/open_posix_testsuite/functional/Makefile
+++ b/testcases/open_posix_testsuite/functional/Makefile
@@ -5,7 +5,7 @@ 
 #

 all clean install test:
-	@for dir in `ls -d */Makefile 2>/dev/null | sed -e 's,/Makefile$$,,g'`; do \
+	@for dir in `find . -name *Makefile | egrep -v '^./Makefile' | sed -e 's,/Makefile$$,,g'`; do \
 		$(MAKE) -C $$dir $@;						   \
 	done

diff --git a/testcases/open_posix_testsuite/scripts/locate-test b/testcases/open_posix_testsuite/scripts/locate-test
index 27f0bb5..e4ef994 100755
--- a/testcases/open_posix_testsuite/scripts/locate-test
+++ b/testcases/open_posix_testsuite/scripts/locate-test
@@ -92,7 +92,7 @@  runnable)
 	# waltz down the tools directory and try and build t0 (which doesn't
 	# make sense as it's a tool, not a test). Better criterion needs to
 	# be established for this file.
-	find "$WHERE/conformance" "$WHERE/stress" -type f -name '*[0-9].c' -o -name '[0-9]*-[0-9]*.sh' | grep -v buildonly | grep -v '^./tools'
+	find "$WHERE/conformance" "$WHERE/stress" -type f -name '*[0-9a-z].c' -o -name '[0-9]*-[0-9]*.sh' | grep -v buildonly | grep -v '^./tools' | grep -v 'testfrmw.c$'
 	find "$WHERE/functional" -type f -name '*.c'
 	;;
 test-tools)
diff --git a/testcases/open_posix_testsuite/stress/Makefile b/testcases/open_posix_testsuite/stress/Makefile
index b09527f..d6872a9 100644
--- a/testcases/open_posix_testsuite/stress/Makefile
+++ b/testcases/open_posix_testsuite/stress/Makefile
@@ -5,7 +5,7 @@ 
 #

 all clean install test:
-	@for dir in `ls -d */Makefile 2>/dev/null | sed -e 's,/Makefile$$,,g'`; do \
+	@for dir in `find . -name *Makefile | egrep -v '^./Makefile' | sed -e 's,/Makefile$$,,g'`; do \
 		$(MAKE) -C $$dir $@;						  \
 	done