[PATCHv2,5/6] gdb/configure: use AC_MSG_NOTICE not a direct echo call
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
After the recent commits, I noticed that GDB's configure script would
still emit two lines even when run in silent mode. If you touch
gdb/Makefile.in and then run 'make all' in the gdb/ build directory
you'll see this:
GEN config.status
enable_sim = no
enableval = no
Obviously the 'no' might be 'yes' depending on how you actually
configured GDB.
This is caused by two direct invocations of 'echo' in GDB's
configure.ac script.
In this commit I replace these calls with use of AC_MSG_NOTICE
instead. Now when configure is run with the --silent command line
option these lines will not be printed.
There should be no changes in the built GDB after this commit.
---
gdb/configure | 6 ++++--
gdb/configure.ac | 4 ++--
2 files changed, 6 insertions(+), 4 deletions(-)
Comments
On 2024-04-06 13:03, Andrew Burgess wrote:
> After the recent commits, I noticed that GDB's configure script would
> still emit two lines even when run in silent mode. If you touch
> gdb/Makefile.in and then run 'make all' in the gdb/ build directory
> you'll see this:
>
> GEN config.status
> enable_sim = no
> enableval = no
>
> Obviously the 'no' might be 'yes' depending on how you actually
> configured GDB.
>
> This is caused by two direct invocations of 'echo' in GDB's
> configure.ac script.
>
> In this commit I replace these calls with use of AC_MSG_NOTICE
> instead. Now when configure is run with the --silent command line
> option these lines will not be printed.
>
> There should be no changes in the built GDB after this commit.
> ---
> gdb/configure | 6 ++++--
> gdb/configure.ac | 4 ++--
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/configure b/gdb/configure
> index d0fd1760b88..ffbc14493e2 100755
> --- a/gdb/configure
> +++ b/gdb/configure
> @@ -32848,8 +32848,10 @@ fi
> #
> # Check whether --enable-sim was given.
> if test "${enable_sim+set}" = set; then :
> - enableval=$enable_sim; echo "enable_sim = $enable_sim";
> - echo "enableval = ${enableval}";
> + enableval=$enable_sim; { $as_echo "$as_me:${as_lineno-$LINENO}: enable_sim = $enable_sim" >&5
> +$as_echo "$as_me: enable_sim = $enable_sim" >&6;};
> + { $as_echo "$as_me:${as_lineno-$LINENO}: enableval = ${enableval}" >&5
> +$as_echo "$as_me: enableval = ${enableval}" >&6;};
> case "${enableval}" in
> yes) ignore_sim=false ;;
> no) ignore_sim=true ;;
> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index aa91bfb3a17..28e750b6b43 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -2032,8 +2032,8 @@ AC_PATH_X
> #
> AC_ARG_ENABLE(sim,
> AS_HELP_STRING([--enable-sim], [link gdb with simulator]),
> -[echo "enable_sim = $enable_sim";
> - echo "enableval = ${enableval}";
> +[AC_MSG_NOTICE([enable_sim = $enable_sim]);
> + AC_MSG_NOTICE([enableval = ${enableval}]);
> case "${enableval}" in
> yes) ignore_sim=false ;;
> no) ignore_sim=true ;;
The change looks fine to me, but I can't help but notice that the
indentation makes things hard to read here. Especially the fact that
the AS_HELP_STRING is not indented w.r.t. AC_ARG_ENABLE. If you want to
improve that at the same time I wouldn't be against it.
Simon
Simon Marchi <simark@simark.ca> writes:
> On 2024-04-06 13:03, Andrew Burgess wrote:
>> diff --git a/gdb/configure.ac b/gdb/configure.ac
>> index aa91bfb3a17..28e750b6b43 100644
>> --- a/gdb/configure.ac
>> +++ b/gdb/configure.ac
>> @@ -2032,8 +2032,8 @@ AC_PATH_X
>> #
>> AC_ARG_ENABLE(sim,
>> AS_HELP_STRING([--enable-sim], [link gdb with simulator]),
>> -[echo "enable_sim = $enable_sim";
>> - echo "enableval = ${enableval}";
>> +[AC_MSG_NOTICE([enable_sim = $enable_sim]);
>> + AC_MSG_NOTICE([enableval = ${enableval}]);
>> case "${enableval}" in
>> yes) ignore_sim=false ;;
>> no) ignore_sim=true ;;
>
> The change looks fine to me, but I can't help but notice that the
> indentation makes things hard to read here. Especially the fact that
> the AS_HELP_STRING is not indented w.r.t. AC_ARG_ENABLE. If you want to
> improve that at the same time I wouldn't be against it.
Thanks for the feedback. I pushed the patch below which realigns this
part of the configure.ac script.
Thanks,
Andrew
---
commit 36192c2be137d2af13fbc2d528de05b41d546805
Author: Andrew Burgess <aburgess@redhat.com>
Date: Mon Apr 8 10:56:51 2024 +0100
gdb/configure: realign the AC_ARG_ENABLE(sim, ....) block
Following the suggestion in this review comment:
https://inbox.sourceware.org/gdb-patches/9420bbb0-2614-4847-9157-8562f8a62d03@simark.ca
this commit realigns the AC_ARG_ENABLE(sim, ....) block. I've added
additional [...] quoting in a couple of places, which is inline with
how other AC_ARG_ENABLE blocks are formatted within GDB's configure.ac
file.
There should be no change in how GDB configures or builds after this
commit.
diff --git a/gdb/configure b/gdb/configure
index ffbc14493e2..a77e3e27332 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -32850,13 +32850,13 @@ fi
if test "${enable_sim+set}" = set; then :
enableval=$enable_sim; { $as_echo "$as_me:${as_lineno-$LINENO}: enable_sim = $enable_sim" >&5
$as_echo "$as_me: enable_sim = $enable_sim" >&6;};
- { $as_echo "$as_me:${as_lineno-$LINENO}: enableval = ${enableval}" >&5
+ { $as_echo "$as_me:${as_lineno-$LINENO}: enableval = ${enableval}" >&5
$as_echo "$as_me: enableval = ${enableval}" >&6;};
- case "${enableval}" in
- yes) ignore_sim=false ;;
- no) ignore_sim=true ;;
- *) ignore_sim=false ;;
- esac
+ case "${enableval}" in
+ yes) ignore_sim=false ;;
+ no) ignore_sim=true ;;
+ *) ignore_sim=false ;;
+ esac
else
ignore_sim=false
fi
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 28e750b6b43..62ff09cea20 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -2030,16 +2030,16 @@ AC_PATH_X
# are when --disable-sim is specified, or if the simulator directory is
# not part of the source tree.
#
-AC_ARG_ENABLE(sim,
-AS_HELP_STRING([--enable-sim], [link gdb with simulator]),
-[AC_MSG_NOTICE([enable_sim = $enable_sim]);
- AC_MSG_NOTICE([enableval = ${enableval}]);
- case "${enableval}" in
- yes) ignore_sim=false ;;
- no) ignore_sim=true ;;
- *) ignore_sim=false ;;
- esac],
-[ignore_sim=false])
+AC_ARG_ENABLE([sim],
+ [AS_HELP_STRING([--enable-sim], [link gdb with simulator])],
+ [AC_MSG_NOTICE([enable_sim = $enable_sim]);
+ AC_MSG_NOTICE([enableval = ${enableval}]);
+ case "${enableval}" in
+ yes) ignore_sim=false ;;
+ no) ignore_sim=true ;;
+ *) ignore_sim=false ;;
+ esac],
+ [ignore_sim=false])
if test ! -d "${srcdir}/../sim"; then
ignore_sim=true
>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
Andrew> AS_HELP_STRING([--enable-sim], [link gdb with simulator]),
Andrew> -[echo "enable_sim = $enable_sim";
Andrew> - echo "enableval = ${enableval}";
To me this looks like old debugging echos that slipped in by mistake.
I think it would be just as good to simply drop them.
Tom
@@ -32848,8 +32848,10 @@ fi
#
# Check whether --enable-sim was given.
if test "${enable_sim+set}" = set; then :
- enableval=$enable_sim; echo "enable_sim = $enable_sim";
- echo "enableval = ${enableval}";
+ enableval=$enable_sim; { $as_echo "$as_me:${as_lineno-$LINENO}: enable_sim = $enable_sim" >&5
+$as_echo "$as_me: enable_sim = $enable_sim" >&6;};
+ { $as_echo "$as_me:${as_lineno-$LINENO}: enableval = ${enableval}" >&5
+$as_echo "$as_me: enableval = ${enableval}" >&6;};
case "${enableval}" in
yes) ignore_sim=false ;;
no) ignore_sim=true ;;
@@ -2032,8 +2032,8 @@ AC_PATH_X
#
AC_ARG_ENABLE(sim,
AS_HELP_STRING([--enable-sim], [link gdb with simulator]),
-[echo "enable_sim = $enable_sim";
- echo "enableval = ${enableval}";
+[AC_MSG_NOTICE([enable_sim = $enable_sim]);
+ AC_MSG_NOTICE([enableval = ${enableval}]);
case "${enableval}" in
yes) ignore_sim=false ;;
no) ignore_sim=true ;;