[RFC,1/1] testcase fail while using cross-test-ssh wrapper

Message ID 6067f3ff4ac4b7e991405f216e928e62f4780c8b.1561620739.git.han_mao@c-sky.com
State New
Headers show
Series
  • [RFC,1/1] testcase fail while using cross-test-ssh wrapper
Related show

Commit Message

Mao Han June 27, 2019, 7:33 a.m.
I did a regression test yesterday, and found tst-locale-locpath.sh and
tst-rtld-preload.sh failed due to some script problem. The ssh wrapper
is invoked twice on these testcase, seems some of the arguments are
not pass correctly for cross test.

tst-locale-locpath.sh test cmd:
/home/vmh/workspace/buildroot2/buildroot/output/build/glibc-02d8b5ab1c89bcef2627d2b621bfb35b573852c2/scripts/cross-test-ssh.sh --timeoutfactor 6000 10.0.2.17 /home/vmh/workspace/buildroot2/buildroot/output/build/glibc-02d8b5ab1c89bcef2627d2b621bfb35b573852c2/scripts/cross-test-ssh.sh --timeoutfactor 6000 10.0.2.17 env LC_ALL=invalid-locale LOCPATH=does-not-exist /home/vmh/workspace/buildroot2/buildroot/output/build/glibc-02d8b5ab1c89bcef2627d2b621bfb35b573852c2/build/elf/ld.so --library-path /home/vmh/workspace/buildroot2/buildroot/output/build/glibc-02d8b5ab1c89bcef2627d2b621bfb35b573852c2/build/ /home/vmh/workspace/buildroot2/buildroot/output/build/glibc-02d8b5ab1c89bcef2627d2b621bfb35b573852c2/build/locale/locale

Cc: Florian Weimer <fweimer@redhat.com>
Cc: David Newall <glibc@davidnewall.com>
Cc: Vincent Chen<vincentc@andestech.com>
---
 elf/tst-rtld-preload.sh      | 2 +-
 locale/tst-locale-locpath.sh | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.7.4

Comments

Florian Weimer July 8, 2019, 12:09 p.m. | #1
* Mao Han:

> I did a regression test yesterday, and found tst-locale-locpath.sh and

> tst-rtld-preload.sh failed due to some script problem. The ssh wrapper

> is invoked twice on these testcase, seems some of the arguments are

> not pass correctly for cross test.

>

> tst-locale-locpath.sh test cmd:

> /home/vmh/workspace/buildroot2/buildroot/output/build/glibc-02d8b5ab1c89bcef2627d2b621bfb35b573852c2/scripts/cross-test-ssh.sh --timeoutfactor 6000 10.0.2.17 /home/vmh/workspace/buildroot2/buildroot/output/build/glibc-02d8b5ab1c89bcef2627d2b621bfb35b573852c2/scripts/cross-test-ssh.sh --timeoutfactor 6000 10.0.2.17 env LC_ALL=invalid-locale LOCPATH=does-not-exist /home/vmh/workspace/buildroot2/buildroot/output/build/glibc-02d8b5ab1c89bcef2627d2b621bfb35b573852c2/build/elf/ld.so --library-path /home/vmh/workspace/buildroot2/buildroot/output/build/glibc-02d8b5ab1c89bcef2627d2b621bfb35b573852c2/build/ /home/vmh/workspace/buildroot2/buildroot/output/build/glibc-02d8b5ab1c89bcef2627d2b621bfb35b573852c2/build/locale/locale

>

> Cc: Florian Weimer <fweimer@redhat.com>

> Cc: David Newall <glibc@davidnewall.com>

> Cc: Vincent Chen<vincentc@andestech.com>

> ---

>  elf/tst-rtld-preload.sh      | 2 +-

>  locale/tst-locale-locpath.sh | 4 ++--

>  2 files changed, 3 insertions(+), 3 deletions(-)

>

> diff --git a/elf/tst-rtld-preload.sh b/elf/tst-rtld-preload.sh

> index f0c0ca1..0fe9e4f 100755

> --- a/elf/tst-rtld-preload.sh

> +++ b/elf/tst-rtld-preload.sh

> @@ -31,7 +31,7 @@ echo "# [${test_wrapper}] [$rtld] [--library-path] [$library_path]" \

>       "[--preload] [$preload] [$test_program]"

>  ${test_wrapper_env} \

>  ${run_program_env} \

> -${test_wrapper} $rtld --library-path "$library_path" \

> +$rtld --library-path "$library_path" \

>    --preload "$preload" $test_program 2>&1 && rc=0 || rc=$?

>  echo "# exit status $rc"

>  

> diff --git a/locale/tst-locale-locpath.sh b/locale/tst-locale-locpath.sh

> index b83de90..1281e9c 100644

> --- a/locale/tst-locale-locpath.sh

> +++ b/locale/tst-locale-locpath.sh

> @@ -20,8 +20,8 @@

>  set -ex

>  

>  common_objpfx=$1

> -test_wrapper_env=$2

> -run_program_env=$3

> +test_wrapper_env=$3

> +run_program_env=

>  

>  LIBPATH="$common_objpfx"


Sorry, this patch doesn't look correct to me.  Why is it appropriate to
ignore evironment variables passed to the test script?

Thanks,
Florian
Mao Han July 9, 2019, 1:30 a.m. | #2
On Mon, Jul 08, 2019 at 02:09:13PM +0200, Florian Weimer wrote:
> * Mao Han:

> >  

> >  common_objpfx=$1

> > -test_wrapper_env=$2

> > -run_program_env=$3

> > +test_wrapper_env=$3

> > +run_program_env=

> >  

> >  LIBPATH="$common_objpfx"

> 

> Sorry, this patch doesn't look correct to me.  Why is it appropriate to

> ignore evironment variables passed to the test script?


I was not intend to ignore the evironment variables. I just posted the problem
I've got and how did I avoid that.
The argument passed by Makefile is:
$(objpfx)tst-locale-locpath.out : tst-locale-locpath.sh $(objpfx)locale
        $(SHELL) $< '$(common-objpfx)' '$(test-wrapper)' '$(test-wrapper-env)'
But the argument taken by the script is:
common_objpfx=$1
test_wrapper_env=$2
run_program_env=$3
The test-wrapper is passed twice and no run_program_env is passed. I don't know
whether the test-wrapper script is unnecessary(last patch) or the argument passed
by Makefile is wrong:

diff --git a/locale/Makefile b/locale/Makefile
index 0ad99ec..d78cf9b 100644
--- a/locale/Makefile
+++ b/locale/Makefile
@@ -113,5 +113,5 @@ lib := locale-programs
 include $(patsubst %,$(..)libof-iterator.mk,$(cpp-srcs-left))
 
 $(objpfx)tst-locale-locpath.out : tst-locale-locpath.sh $(objpfx)locale
-       $(SHELL) $< '$(common-objpfx)' '$(test-wrapper)' '$(test-wrapper-env)' > $@; \
+       $(SHELL) $< '$(common-objpfx)' '$(test-wrapper-env)' '$(run-program-env)' > $@; \
        $(evaluate-test)

Thanks,
Mao Han
Florian Weimer July 9, 2019, 10:16 a.m. | #3
* Mao Han:

> The test-wrapper is passed twice and no run_program_env is passed. I

> don't know whether the test-wrapper script is unnecessary(last patch)

> or the argument passed by Makefile is wrong:

>

> diff --git a/locale/Makefile b/locale/Makefile

> index 0ad99ec..d78cf9b 100644

> --- a/locale/Makefile

> +++ b/locale/Makefile

> @@ -113,5 +113,5 @@ lib := locale-programs

>  include $(patsubst %,$(..)libof-iterator.mk,$(cpp-srcs-left))

>  

>  $(objpfx)tst-locale-locpath.out : tst-locale-locpath.sh $(objpfx)locale

> -       $(SHELL) $< '$(common-objpfx)' '$(test-wrapper)' '$(test-wrapper-env)' > $@; \

> +       $(SHELL) $< '$(common-objpfx)' '$(test-wrapper-env)' '$(run-program-env)' > $@; \

>         $(evaluate-test)


Ah.  Does this patch work for you?  It looks more correct to me, but I
have no way of testing it.

Thanks,
Florian
Mao Han July 9, 2019, 10:50 a.m. | #4
On Tue, Jul 09, 2019 at 12:16:12PM +0200, Florian Weimer wrote:
> * Mao Han:

> 

> > The test-wrapper is passed twice and no run_program_env is passed. I

> > don't know whether the test-wrapper script is unnecessary(last patch)

> > or the argument passed by Makefile is wrong:

> >

> > diff --git a/locale/Makefile b/locale/Makefile

> > index 0ad99ec..d78cf9b 100644

> > --- a/locale/Makefile

> > +++ b/locale/Makefile

> > @@ -113,5 +113,5 @@ lib := locale-programs

> >  include $(patsubst %,$(..)libof-iterator.mk,$(cpp-srcs-left))

> >  

> >  $(objpfx)tst-locale-locpath.out : tst-locale-locpath.sh $(objpfx)locale

> > -       $(SHELL) $< '$(common-objpfx)' '$(test-wrapper)' '$(test-wrapper-env)' > $@; \

> > +       $(SHELL) $< '$(common-objpfx)' '$(test-wrapper-env)' '$(run-program-env)' > $@; \

> >         $(evaluate-test)

> 

> Ah.  Does this patch work for you?  It looks more correct to me, but I

> have no way of testing it.

> 

> Thanks,

> Florian


Yes, I've tested this patch on C-SKY QEMU, it works fine.

Thanks,
Mao Han
Florian Weimer July 9, 2019, 11:59 a.m. | #5
* Mao Han:

> On Tue, Jul 09, 2019 at 12:16:12PM +0200, Florian Weimer wrote:

>> * Mao Han:

>> 

>> > The test-wrapper is passed twice and no run_program_env is passed. I

>> > don't know whether the test-wrapper script is unnecessary(last patch)

>> > or the argument passed by Makefile is wrong:

>> >

>> > diff --git a/locale/Makefile b/locale/Makefile

>> > index 0ad99ec..d78cf9b 100644

>> > --- a/locale/Makefile

>> > +++ b/locale/Makefile

>> > @@ -113,5 +113,5 @@ lib := locale-programs

>> >  include $(patsubst %,$(..)libof-iterator.mk,$(cpp-srcs-left))

>> >  

>> >  $(objpfx)tst-locale-locpath.out : tst-locale-locpath.sh $(objpfx)locale

>> > -       $(SHELL) $< '$(common-objpfx)' '$(test-wrapper)' '$(test-wrapper-env)' > $@; \

>> > +       $(SHELL) $< '$(common-objpfx)' '$(test-wrapper-env)' '$(run-program-env)' > $@; \

>> >         $(evaluate-test)

>> 

>> Ah.  Does this patch work for you?  It looks more correct to me, but I

>> have no way of testing it.

>> 

>> Thanks,

>> Florian

>

> Yes, I've tested this patch on C-SKY QEMU, it works fine.


Great.  Please push this change along with an appropriate ChangeLog
entry.

Thanks,
Florian

Patch

diff --git a/elf/tst-rtld-preload.sh b/elf/tst-rtld-preload.sh
index f0c0ca1..0fe9e4f 100755
--- a/elf/tst-rtld-preload.sh
+++ b/elf/tst-rtld-preload.sh
@@ -31,7 +31,7 @@  echo "# [${test_wrapper}] [$rtld] [--library-path] [$library_path]" \
      "[--preload] [$preload] [$test_program]"
 ${test_wrapper_env} \
 ${run_program_env} \
-${test_wrapper} $rtld --library-path "$library_path" \
+$rtld --library-path "$library_path" \
   --preload "$preload" $test_program 2>&1 && rc=0 || rc=$?
 echo "# exit status $rc"
 
diff --git a/locale/tst-locale-locpath.sh b/locale/tst-locale-locpath.sh
index b83de90..1281e9c 100644
--- a/locale/tst-locale-locpath.sh
+++ b/locale/tst-locale-locpath.sh
@@ -20,8 +20,8 @@ 
 set -ex
 
 common_objpfx=$1
-test_wrapper_env=$2
-run_program_env=$3
+test_wrapper_env=$3
+run_program_env=
 
 LIBPATH="$common_objpfx"