[PATCH] configure: Fix check-tcg not executing any tests

[PATCH] configure: Fix check-tcg not executing any tests

[Mukilan Thiyagarajan]

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(-)

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
 
-- 
2.17.1

Re: [PATCH] configure: Fix check-tcg not executing any tests

[Philippe Mathieu-Daudé]

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
>   

RE: [PATCH] configure: Fix check-tcg not executing any tests

[Mukilan Thiyagarajan (QUIC)]

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
>   

RE: [PATCH] configure: Fix check-tcg not executing any tests

[Mukilan Thiyagarajan (QUIC)]

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
>

Re: [PATCH] configure: Fix check-tcg not executing any tests

[Richard Henderson]

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
>   

Re: [PATCH] configure: Fix check-tcg not executing any tests

[Alex Bennée]

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.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro