| Message ID | 20221207082309.9966-1-quic_mthiyaga@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | configure: Fix check-tcg not executing any tests | expand |
Hi Mukilan, On 7/12/22 09:23, Mukilan Thiyagarajan wrote: > After configuring with --target-list=hexagon-linux-user > running `make check-tcg` just prints the following: > > ``` > make: Nothing to be done for 'check-tcg' > ``` > > In the probe_target_compiler function, the 'break' > command is used incorrectly. There are no lexically > enclosing loops associated with that break command which > is an unspecfied behaviour in the POSIX standard. > > The dash shell implementation aborts the currently executing > loop, in this case, causing the rest of the logic for the loop > in line 2490 to be skipped, which means no Makefiles are > generated for the tcg target tests. > > Fixes: c3b570b5a9a24d25 (configure: don't enable > cross compilers unless in target_list) When posting a patch fixing an issue introduced by another one, you'll get more feedback if Cc'ing the author/reviewers of such patch. Also Cc'ing the maintainers also help in having your patch picked up :) See: https://www.qemu.org/docs/master/devel/submitting-a-patch.html#cc-the-relevant-maintainer I've Cc'ed the corresponding developers for you. Regards, Phil. > Signed-off-by: Mukilan Thiyagarajan <quic_mthiyaga@quicinc.com> > --- > configure | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/configure b/configure > index 26c7bc5154..7a804fb657 100755 > --- a/configure > +++ b/configure > @@ -1881,9 +1881,7 @@ probe_target_compiler() { > # We shall skip configuring the target compiler if the user didn't > # bother enabling an appropriate guest. This avoids building > # extraneous firmware images and tests. > - if test "${target_list#*$1}" != "$1"; then > - break; > - else > + if test "${target_list#*$1}" = "$1"; then > return 1 > fi >
Thank you for the pointers, Philippe 😊 Will keep them in mind for the future patches. Regards, Mukilan -----Original Message----- From: Philippe Mathieu-Daudé <philmd@linaro.org> Sent: Wednesday, December 7, 2022 2:37 PM To: Mukilan Thiyagarajan (QUIC) <quic_mthiyaga@quicinc.com>; qemu-devel@nongnu.org; Brian Cain <bcain@quicinc.com>; Matheus Bernardino (QUIC) <quic_mathbern@quicinc.com> Cc: Alex Bennée <alex.bennee@linaro.org>; Richard Henderson <richard.henderson@linaro.org>; Paolo Bonzini <pbonzini@redhat.com>; Thomas Huth <thuth@redhat.com> Subject: Re: [PATCH] configure: Fix check-tcg not executing any tests Hi Mukilan, On 7/12/22 09:23, Mukilan Thiyagarajan wrote: > After configuring with --target-list=hexagon-linux-user > running `make check-tcg` just prints the following: > > ``` > make: Nothing to be done for 'check-tcg' > ``` > > In the probe_target_compiler function, the 'break' > command is used incorrectly. There are no lexically > enclosing loops associated with that break command which > is an unspecfied behaviour in the POSIX standard. > > The dash shell implementation aborts the currently executing > loop, in this case, causing the rest of the logic for the loop > in line 2490 to be skipped, which means no Makefiles are > generated for the tcg target tests. > > Fixes: c3b570b5a9a24d25 (configure: don't enable > cross compilers unless in target_list) When posting a patch fixing an issue introduced by another one, you'll get more feedback if Cc'ing the author/reviewers of such patch. Also Cc'ing the maintainers also help in having your patch picked up :) See: https://www.qemu.org/docs/master/devel/submitting-a-patch.html#cc-the-relevant-maintainer I've Cc'ed the corresponding developers for you. Regards, Phil. > Signed-off-by: Mukilan Thiyagarajan <quic_mthiyaga@quicinc.com> > --- > configure | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/configure b/configure > index 26c7bc5154..7a804fb657 100755 > --- a/configure > +++ b/configure > @@ -1881,9 +1881,7 @@ probe_target_compiler() { > # We shall skip configuring the target compiler if the user didn't > # bother enabling an appropriate guest. This avoids building > # extraneous firmware images and tests. > - if test "${target_list#*$1}" != "$1"; then > - break; > - else > + if test "${target_list#*$1}" = "$1"; then > return 1 > fi >
Ping. This patch still needs review Link: https://patchew.org/QEMU/20221207082309.9966-1-quic._5Fmthiyaga@quicinc.com/ Based on my testing, zsh (all versions) and dash shell (till v5.8, used in Ubuntu 18.04) have the behavior of aborting the current loop across function calls when the break doesn't have any lexically enclosing loops. dash v5.9 fixed [1] this behavior to simply ignore the incorrect break silently, which would explain why the CI builds are not facing this issue. Bash shell also ignores the break, in addition to printing a warning. [1]: https://git.kernel.org/pub/scm/utils/dash/dash.git/commit/?id=ebfdd97a10e34a5e70eadfc21ebfc033ef93a563 Regards, Mukilan -----Original Message----- From: Philippe Mathieu-Daudé <philmd@linaro.org> Sent: Wednesday, December 7, 2022 2:37 PM To: Mukilan Thiyagarajan (QUIC) <quic_mthiyaga@quicinc.com>; qemu-devel@nongnu.org; Brian Cain <bcain@quicinc.com>; Matheus Bernardino (QUIC) <quic_mathbern@quicinc.com> Cc: Alex Bennée <alex.bennee@linaro.org>; Richard Henderson <richard.henderson@linaro.org>; Paolo Bonzini <pbonzini@redhat.com>; Thomas Huth <thuth@redhat.com> Subject: Re: [PATCH] configure: Fix check-tcg not executing any tests WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros. Hi Mukilan, On 7/12/22 09:23, Mukilan Thiyagarajan wrote: > After configuring with --target-list=hexagon-linux-user running `make > check-tcg` just prints the following: > > ``` > make: Nothing to be done for 'check-tcg' > ``` > > In the probe_target_compiler function, the 'break' > command is used incorrectly. There are no lexically enclosing loops > associated with that break command which is an unspecfied behaviour in > the POSIX standard. > > The dash shell implementation aborts the currently executing loop, in > this case, causing the rest of the logic for the loop in line 2490 to > be skipped, which means no Makefiles are generated for the tcg target > tests. > > Fixes: c3b570b5a9a24d25 (configure: don't enable cross compilers > unless in target_list) When posting a patch fixing an issue introduced by another one, you'll get more feedback if Cc'ing the author/reviewers of such patch. Also Cc'ing the maintainers also help in having your patch picked up :) See: https://www.qemu.org/docs/master/devel/submitting-a-patch.html#cc-the-relevant-maintainer I've Cc'ed the corresponding developers for you. Regards, Phil. > Signed-off-by: Mukilan Thiyagarajan <quic_mthiyaga@quicinc.com> > --- > configure | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/configure b/configure > index 26c7bc5154..7a804fb657 100755 > --- a/configure > +++ b/configure > @@ -1881,9 +1881,7 @@ probe_target_compiler() { > # We shall skip configuring the target compiler if the user didn't > # bother enabling an appropriate guest. This avoids building > # extraneous firmware images and tests. > - if test "${target_list#*$1}" != "$1"; then > - break; > - else > + if test "${target_list#*$1}" = "$1"; then > return 1 > fi >
On 12/7/22 02:23, Mukilan Thiyagarajan wrote: > After configuring with --target-list=hexagon-linux-user > running `make check-tcg` just prints the following: > > ``` > make: Nothing to be done for 'check-tcg' > ``` > > In the probe_target_compiler function, the 'break' > command is used incorrectly. There are no lexically > enclosing loops associated with that break command which > is an unspecfied behaviour in the POSIX standard. > > The dash shell implementation aborts the currently executing > loop, in this case, causing the rest of the logic for the loop > in line 2490 to be skipped, which means no Makefiles are > generated for the tcg target tests. > > Fixes: c3b570b5a9a24d25 (configure: don't enable > cross compilers unless in target_list) > > Signed-off-by: Mukilan Thiyagarajan <quic_mthiyaga@quicinc.com> > --- > configure | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ > > diff --git a/configure b/configure > index 26c7bc5154..7a804fb657 100755 > --- a/configure > +++ b/configure > @@ -1881,9 +1881,7 @@ probe_target_compiler() { > # We shall skip configuring the target compiler if the user didn't > # bother enabling an appropriate guest. This avoids building > # extraneous firmware images and tests. > - if test "${target_list#*$1}" != "$1"; then > - break; > - else > + if test "${target_list#*$1}" = "$1"; then > return 1 > fi >
Mukilan Thiyagarajan <quic_mthiyaga@quicinc.com> writes: > After configuring with --target-list=hexagon-linux-user > running `make check-tcg` just prints the following: > > ``` > make: Nothing to be done for 'check-tcg' > ``` > > In the probe_target_compiler function, the 'break' > command is used incorrectly. There are no lexically > enclosing loops associated with that break command which > is an unspecfied behaviour in the POSIX standard. > > The dash shell implementation aborts the currently executing > loop, in this case, causing the rest of the logic for the loop > in line 2490 to be skipped, which means no Makefiles are > generated for the tcg target tests. > > Fixes: c3b570b5a9a24d25 (configure: don't enable > cross compilers unless in target_list) > > Signed-off-by: Mukilan Thiyagarajan <quic_mthiyaga@quicinc.com> Queued to testing/next, thanks.
diff --git a/configure b/configure index 26c7bc5154..7a804fb657 100755 --- a/configure +++ b/configure @@ -1881,9 +1881,7 @@ probe_target_compiler() { # We shall skip configuring the target compiler if the user didn't # bother enabling an appropriate guest. This avoids building # extraneous firmware images and tests. - if test "${target_list#*$1}" != "$1"; then - break; - else + if test "${target_list#*$1}" = "$1"; then return 1 fi
After configuring with --target-list=hexagon-linux-user running `make check-tcg` just prints the following: ``` make: Nothing to be done for 'check-tcg' ``` In the probe_target_compiler function, the 'break' command is used incorrectly. There are no lexically enclosing loops associated with that break command which is an unspecfied behaviour in the POSIX standard. The dash shell implementation aborts the currently executing loop, in this case, causing the rest of the logic for the loop in line 2490 to be skipped, which means no Makefiles are generated for the tcg target tests. Fixes: c3b570b5a9a24d25 (configure: don't enable cross compilers unless in target_list) Signed-off-by: Mukilan Thiyagarajan <quic_mthiyaga@quicinc.com> --- configure | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)