[PATCH 0/4] gcore and config.status related Makefile changes

[PATCH 0/4] gcore and config.status related Makefile changes

[Andrew Burgess]

This series started with the observation that 'make all' would not
rebuild the gcore script, instead I needed to do 'make gcore'.

While looking at, and fixing, that problem I noticed a bunch of other
small fixes relating to build rules that invoke config.status to
regenerate files.

Thanks,
Andrew

---

Andrew Burgess (4):
  gdb/Makefile: add gcore to the 'all' target dependency list
  gdb/Makefile: rewrite dependencies for config.status target
  gdb/Makefile: add some missing config.status dependencies
  gdb/Makefile: Print 'GEN' message, and pass SILENT_FLAG more

 gdb/Makefile.in | 41 ++++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 15 deletions(-)


base-commit: 16810e455feb26ef826a3ed876d6d7e6d24818b0
-- 
2.25.4

[PATCH 1/4] gdb/Makefile: add gcore to the 'all' target dependency list

[Andrew Burgess]

The gcore script is initially generated by the configure process, just
like gdb-gdb.gdb and gdb-gdb.py.  However if the gdb/gcore.in input
source is modified then 'make all' in the gdb/ directory does not
regenerate the gcore script.

This is different than the gdb-gdb.gdb and gdb-gdb.py files, if their
input is updated then 'make all' will regenerate these files.

The difference is that for gdb-gdb.* there is an explicit dependency
between the 'all' target and the generated file, this dependency is
missing for gcore.

This commit adds the dependency.  Now, if gcore.in is changed, running
'make all' will regenerate the gcore script.

There is no change in _what_ is generated after this commit.
---
 gdb/Makefile.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index a9f641c0659..df044288b5e 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1938,7 +1938,7 @@ generated_files = \
 # Flags needed to compile Python code
 PYTHON_CFLAGS = @PYTHON_CFLAGS@
 
-all: gdb$(EXEEXT) $(CONFIG_ALL) gdb-gdb.py gdb-gdb.gdb
+all: gdb$(EXEEXT) $(CONFIG_ALL) gdb-gdb.py gdb-gdb.gdb gcore
 	@$(MAKE) $(FLAGS_TO_PASS) DO=all "DODIRS=$(SUBDIRS)" subdir_do
 
 # Rule for compiling .c files in the top-level gdb directory.
-- 
2.25.4

[PATCH 2/4] gdb/Makefile: rewrite dependencies for config.status target

[Andrew Burgess]

I noticed something weird, the rule for the config.status target looks
like this:

  config.status: $(srcdir)/configure configure.nat configure.tgt configure.host ../bfd/development.sh
          $(SHELL) config.status --recheck

What bothered me is that 'configure' is specified as being in
$(srcdir), while all of the other files are not, even though those
files are in the same $(srcdir) as the configure script.

However, I tried touching one of those files, and the config.status
rule does trigger!

This is thanks to the VPATH variable, which is set to $(srcdir), so
make looks in $(srcdir) for any dependencies.

However, this inconsistency bothers me.  Better, I think, to add the
$(srcdir) prefix to each of these files.

I also spotted that the configure script also includes the files
../bfd/config.bfd, yet that is missing from the include list, so in
this commit I plan to add this as a dependency.

The configure script also pulls in two TCL and TK related files:

	. ${TCL_BIN_DIR}/tclConfig.sh
	. ${TK_BIN_DIR}/tkConfig.sh

However, I don't think ${TCL_BIN_DIR} and ${TK_BIN_DIR} are currently
visible in GDB's Makefile, so I'm not planning to add these
dependencies at this time.

In this commit I add a new variable config_status_deps which holds the
list of all the dependencies for config.status, with the $(srcdir)
prefix included, and then I use this in the config.status rule.

After this commit config.status will regenerate if config.bfd changes,
which it wouldn't before, but nothing else changes.
---
 gdb/Makefile.in | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index df044288b5e..9340becbdc9 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -2333,7 +2333,18 @@ nm.h: stamp-nmh ; @true
 stamp-nmh: config.status
 	$(SHELL) config.status nm.h
 
-config.status: $(srcdir)/configure configure.nat configure.tgt configure.host ../bfd/development.sh
+# Files included from config.status or the configure script.  When
+# these change the configure script doesn't need regenerating, but its
+# output (and so that of config.status) might change.
+config_status_deps = \
+	$(srcdir)/configure \
+	$(srcdir)/configure.nat \
+	$(srcdir)/configure.tgt \
+	$(srcdir)/configure.host \
+	$(srcdir)/../bfd/development.sh \
+	$(srcdir)/../bfd/config.bfd
+
+config.status: $(config_status_deps)
 	$(SHELL) config.status --recheck
 
 ACLOCAL = aclocal
-- 
2.25.4

[PATCH 3/4] gdb/Makefile: add some missing config.status dependencies

[Andrew Burgess]

I noticed that for the build targets jit-reader.h, gcore, gdb-gdb.py,
and gdb-gdb.gdb the rules all use the config.status script, but don't
have a dependency on the config.status target.  This means we might
fail to regenerate these targets in a case where config.status, or one
of its dependencies changes.

Two other targets that use config.status do correctly have a
dependency on config.status.

Fixed in this commit by adding the missing dependencies.

There should be no changes in _what_ is generated after this commit.
---
 gdb/Makefile.in | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 9340becbdc9..c1b3144b175 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -2313,16 +2313,16 @@ Makefile: Makefile.in config.status
 run: Makefile
 	./gdb$(EXEEXT) --data-directory=`pwd`/data-directory $(GDBFLAGS)
 
-jit-reader.h: $(srcdir)/jit-reader.in
+jit-reader.h: $(srcdir)/jit-reader.in config.status
 	$(SHELL) config.status $@
 
-gcore: $(srcdir)/gcore.in
+gcore: $(srcdir)/gcore.in config.status
 	$(SHELL) config.status $@
 
-gdb-gdb.py: $(srcdir)/gdb-gdb.py.in
+gdb-gdb.py: $(srcdir)/gdb-gdb.py.in config.status
 	$(SHELL) config.status $@
 
-gdb-gdb.gdb: $(srcdir)/gdb-gdb.gdb.in
+gdb-gdb.gdb: $(srcdir)/gdb-gdb.gdb.in config.status
 	$(SHELL) config.status $@
 
 config.h: stamp-h ; @true
-- 
2.25.4

[PATCH 4/4] gdb/Makefile: Print 'GEN' message, and pass SILENT_FLAG more

[Andrew Burgess]

The targets that use config.status to regenerate themselves don't
currently follow the silent rules that the rest of GDB's Makefile
does.  For example, touch the gdb/gcore.in file and then 'make all' in
the gdb/ directory prints:

  /bin/sh config.status gcore
  config.status: creating gcore

In this commit I make use of the silent-rules.mk mechanism for these
targets, now we get:

  GEN    gcore

Which matches the rest of our Makefile.  Obviously, if you pass 'V=1'
to the build then you'll get the old output back.

There's no change in what is generated after this commit.
---
 gdb/Makefile.in | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index c1b3144b175..e9a6247c38d 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -2307,31 +2307,31 @@ subdir_do: force
 	done
 
 Makefile: Makefile.in config.status
-	$(SHELL) config.status $@
+	$(ECHO_GEN) $(SHELL) config.status $(SILENT_FLAG) $@
 
 .PHONY: run
 run: Makefile
 	./gdb$(EXEEXT) --data-directory=`pwd`/data-directory $(GDBFLAGS)
 
 jit-reader.h: $(srcdir)/jit-reader.in config.status
-	$(SHELL) config.status $@
+	$(ECHO_GEN) $(SHELL) config.status $(SILENT_FLAG) $@
 
 gcore: $(srcdir)/gcore.in config.status
-	$(SHELL) config.status $@
+	$(ECHO_GEN) $(SHELL) config.status $(SILENT_FLAG) $@
 
 gdb-gdb.py: $(srcdir)/gdb-gdb.py.in config.status
-	$(SHELL) config.status $@
+	$(ECHO_GEN) $(SHELL) config.status $(SILENT_FLAG) $@
 
 gdb-gdb.gdb: $(srcdir)/gdb-gdb.gdb.in config.status
-	$(SHELL) config.status $@
+	$(ECHO_GEN) $(SHELL) config.status $(SILENT_FLAG) $@
 
 config.h: stamp-h ; @true
 stamp-h: $(srcdir)/config.in config.status
-	$(SHELL) config.status config.h
+	$(ECHO_GEN) $(SHELL) config.status $(SILENT_FLAG) config.h
 
 nm.h: stamp-nmh ; @true
 stamp-nmh: config.status
-	$(SHELL) config.status nm.h
+	$(ECHO_GEN) $(SHELL) config.status $(SILENT_FLAG) nm.h
 
 # Files included from config.status or the configure script.  When
 # these change the configure script doesn't need regenerating, but its
@@ -2345,7 +2345,7 @@ config_status_deps = \
 	$(srcdir)/../bfd/config.bfd
 
 config.status: $(config_status_deps)
-	$(SHELL) config.status --recheck
+	$(ECHO_GEN) $(SHELL) config.status $(SILENT_FLAG) --recheck
 
 ACLOCAL = aclocal
 
-- 
2.25.4

[PATCHv2 0/6] gcore and config.status related Makefile changes

[Andrew Burgess]

Changes in v2:

  - Nothing has changed in patches 1, 2, 3, or 4,

  - Two new patches: 5 and 6.

---

Andrew Burgess (6):
  gdb/Makefile: add gcore to the 'all' target dependency list
  gdb/Makefile: rewrite dependencies for config.status target
  gdb/Makefile: add some missing config.status dependencies
  gdb/Makefile: Print 'GEN' message, and pass SILENT_FLAG more
  gdb/configure: use AC_MSG_NOTICE not a direct echo call
  gdb/build: apply silent-rules.mk to the data-directory Makefile.in

 gdb/Makefile.in                | 41 +++++++++++++++++++++-------------
 gdb/configure                  |  6 +++--
 gdb/configure.ac               |  4 ++--
 gdb/data-directory/Makefile.in | 40 +++++++++++++++++++--------------
 gdb/silent-rules.mk            |  4 ++++
 5 files changed, 59 insertions(+), 36 deletions(-)


base-commit: 16810e455feb26ef826a3ed876d6d7e6d24818b0
-- 
2.25.4

[PATCHv2 1/6] gdb/Makefile: add gcore to the 'all' target dependency list

[Andrew Burgess]

The gcore script is initially generated by the configure process, just
like gdb-gdb.gdb and gdb-gdb.py.  However if the gdb/gcore.in input
source is modified then 'make all' in the gdb/ directory does not
regenerate the gcore script.

This is different than the gdb-gdb.gdb and gdb-gdb.py files, if their
input is updated then 'make all' will regenerate these files.

The difference is that for gdb-gdb.* there is an explicit dependency
between the 'all' target and the generated file, this dependency is
missing for gcore.

This commit adds the dependency.  Now, if gcore.in is changed, running
'make all' will regenerate the gcore script.

There is no change in _what_ is generated after this commit.
---
 gdb/Makefile.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index a9f641c0659..df044288b5e 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1938,7 +1938,7 @@ generated_files = \
 # Flags needed to compile Python code
 PYTHON_CFLAGS = @PYTHON_CFLAGS@
 
-all: gdb$(EXEEXT) $(CONFIG_ALL) gdb-gdb.py gdb-gdb.gdb
+all: gdb$(EXEEXT) $(CONFIG_ALL) gdb-gdb.py gdb-gdb.gdb gcore
 	@$(MAKE) $(FLAGS_TO_PASS) DO=all "DODIRS=$(SUBDIRS)" subdir_do
 
 # Rule for compiling .c files in the top-level gdb directory.
-- 
2.25.4

[PATCHv2 2/6] gdb/Makefile: rewrite dependencies for config.status target

[Andrew Burgess]

I noticed something weird, the rule for the config.status target looks
like this:

  config.status: $(srcdir)/configure configure.nat configure.tgt configure.host ../bfd/development.sh
          $(SHELL) config.status --recheck

What bothered me is that 'configure' is specified as being in
$(srcdir), while all of the other files are not, even though those
files are in the same $(srcdir) as the configure script.

However, I tried touching one of those files, and the config.status
rule does trigger!

This is thanks to the VPATH variable, which is set to $(srcdir), so
make looks in $(srcdir) for any dependencies.

However, this inconsistency bothers me.  Better, I think, to add the
$(srcdir) prefix to each of these files.

I also spotted that the configure script also includes the files
../bfd/config.bfd, yet that is missing from the include list, so in
this commit I plan to add this as a dependency.

The configure script also pulls in two TCL and TK related files:

	. ${TCL_BIN_DIR}/tclConfig.sh
	. ${TK_BIN_DIR}/tkConfig.sh

However, I don't think ${TCL_BIN_DIR} and ${TK_BIN_DIR} are currently
visible in GDB's Makefile, so I'm not planning to add these
dependencies at this time.

In this commit I add a new variable config_status_deps which holds the
list of all the dependencies for config.status, with the $(srcdir)
prefix included, and then I use this in the config.status rule.

After this commit config.status will regenerate if config.bfd changes,
which it wouldn't before, but nothing else changes.
---
 gdb/Makefile.in | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index df044288b5e..9340becbdc9 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -2333,7 +2333,18 @@ nm.h: stamp-nmh ; @true
 stamp-nmh: config.status
 	$(SHELL) config.status nm.h
 
-config.status: $(srcdir)/configure configure.nat configure.tgt configure.host ../bfd/development.sh
+# Files included from config.status or the configure script.  When
+# these change the configure script doesn't need regenerating, but its
+# output (and so that of config.status) might change.
+config_status_deps = \
+	$(srcdir)/configure \
+	$(srcdir)/configure.nat \
+	$(srcdir)/configure.tgt \
+	$(srcdir)/configure.host \
+	$(srcdir)/../bfd/development.sh \
+	$(srcdir)/../bfd/config.bfd
+
+config.status: $(config_status_deps)
 	$(SHELL) config.status --recheck
 
 ACLOCAL = aclocal
-- 
2.25.4

[PATCHv2 3/6] gdb/Makefile: add some missing config.status dependencies

[Andrew Burgess]

I noticed that for the build targets jit-reader.h, gcore, gdb-gdb.py,
and gdb-gdb.gdb the rules all use the config.status script, but don't
have a dependency on the config.status target.  This means we might
fail to regenerate these targets in a case where config.status, or one
of its dependencies changes.

Two other targets that use config.status do correctly have a
dependency on config.status.

Fixed in this commit by adding the missing dependencies.

There should be no changes in _what_ is generated after this commit.
---
 gdb/Makefile.in | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 9340becbdc9..c1b3144b175 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -2313,16 +2313,16 @@ Makefile: Makefile.in config.status
 run: Makefile
 	./gdb$(EXEEXT) --data-directory=`pwd`/data-directory $(GDBFLAGS)
 
-jit-reader.h: $(srcdir)/jit-reader.in
+jit-reader.h: $(srcdir)/jit-reader.in config.status
 	$(SHELL) config.status $@
 
-gcore: $(srcdir)/gcore.in
+gcore: $(srcdir)/gcore.in config.status
 	$(SHELL) config.status $@
 
-gdb-gdb.py: $(srcdir)/gdb-gdb.py.in
+gdb-gdb.py: $(srcdir)/gdb-gdb.py.in config.status
 	$(SHELL) config.status $@
 
-gdb-gdb.gdb: $(srcdir)/gdb-gdb.gdb.in
+gdb-gdb.gdb: $(srcdir)/gdb-gdb.gdb.in config.status
 	$(SHELL) config.status $@
 
 config.h: stamp-h ; @true
-- 
2.25.4

[PATCHv2 4/6] gdb/Makefile: Print 'GEN' message, and pass SILENT_FLAG more

[Andrew Burgess]

The targets that use config.status to regenerate themselves don't
currently follow the silent rules that the rest of GDB's Makefile
does.  For example, touch the gdb/gcore.in file and then 'make all' in
the gdb/ directory prints:

  /bin/sh config.status gcore
  config.status: creating gcore

In this commit I make use of the silent-rules.mk mechanism for these
targets, now we get:

  GEN    gcore

Which matches the rest of our Makefile.  Obviously, if you pass 'V=1'
to the build then you'll get the old output back.

There's no change in what is generated after this commit.
---
 gdb/Makefile.in | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index c1b3144b175..e9a6247c38d 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -2307,31 +2307,31 @@ subdir_do: force
 	done
 
 Makefile: Makefile.in config.status
-	$(SHELL) config.status $@
+	$(ECHO_GEN) $(SHELL) config.status $(SILENT_FLAG) $@
 
 .PHONY: run
 run: Makefile
 	./gdb$(EXEEXT) --data-directory=`pwd`/data-directory $(GDBFLAGS)
 
 jit-reader.h: $(srcdir)/jit-reader.in config.status
-	$(SHELL) config.status $@
+	$(ECHO_GEN) $(SHELL) config.status $(SILENT_FLAG) $@
 
 gcore: $(srcdir)/gcore.in config.status
-	$(SHELL) config.status $@
+	$(ECHO_GEN) $(SHELL) config.status $(SILENT_FLAG) $@
 
 gdb-gdb.py: $(srcdir)/gdb-gdb.py.in config.status
-	$(SHELL) config.status $@
+	$(ECHO_GEN) $(SHELL) config.status $(SILENT_FLAG) $@
 
 gdb-gdb.gdb: $(srcdir)/gdb-gdb.gdb.in config.status
-	$(SHELL) config.status $@
+	$(ECHO_GEN) $(SHELL) config.status $(SILENT_FLAG) $@
 
 config.h: stamp-h ; @true
 stamp-h: $(srcdir)/config.in config.status
-	$(SHELL) config.status config.h
+	$(ECHO_GEN) $(SHELL) config.status $(SILENT_FLAG) config.h
 
 nm.h: stamp-nmh ; @true
 stamp-nmh: config.status
-	$(SHELL) config.status nm.h
+	$(ECHO_GEN) $(SHELL) config.status $(SILENT_FLAG) nm.h
 
 # Files included from config.status or the configure script.  When
 # these change the configure script doesn't need regenerating, but its
@@ -2345,7 +2345,7 @@ config_status_deps = \
 	$(srcdir)/../bfd/config.bfd
 
 config.status: $(config_status_deps)
-	$(SHELL) config.status --recheck
+	$(ECHO_GEN) $(SHELL) config.status $(SILENT_FLAG) --recheck
 
 ACLOCAL = aclocal
 
-- 
2.25.4

[PATCHv2 5/6] gdb/configure: use AC_MSG_NOTICE not a direct echo call

[Andrew Burgess]

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 ;;
-- 
2.25.4

[PATCHv2 6/6] gdb/build: apply silent-rules.mk to the data-directory Makefile.in

[Andrew Burgess]

This commit makes use of gdb/silent-rules.mk in the data-directory
Makefile.in.  I've only updated the rules that actually generate
things, I've not touched the install or uninstall rules, this matches
gdb/Makefile.in.

I've not managed to completely silence all of the recipe output, the
mkinstalldirs command outputs some diagnostic text which looks like
this:

    GEN    stamp-python
  mkdir -p -- ./python/gdb
  mkdir -p -- ./python/gdb/command
  mkdir -p -- ./python/gdb/dap
  mkdir -p -- ./python/gdb/function
  mkdir -p -- ./python/gdb/printer

I have a patch for mkinstalldirs that fixes this (by adding a new
--silent command line flag), but that patch needs to be submitted to
automake, then an updated mkinstalldirs sync'd to the gcc repository,
and then copied into the binutils-gdb repository... so I'm leaving
that for a future project.

Then the guild compiler also emits some diagnostic output, which looks
like this:

    GEN    stamp-guile
  mkdir -p -- ./guile/.
  mkdir -p -- ./guile/gdb
  wrote `./gdb.go'
  wrote `gdb/experimental.go'
  wrote `gdb/iterator.go'
  wrote `gdb/printing.go'
  wrote `gdb/support.go'
  wrote `gdb/types.go'

The 'wrote' lines are from the guild compiler.  The only way to
silence these would be to redirect stdout to /dev/null I think.  I did
prototype this, but wasn't 100% convinced about that part of the
patch, so I've decided to leave that for another day.

I did need to add a new SILENT_ECHO variable to silent-rules.mk, this
is set to a suitable 'echo' command to use within recipes.  When we
are in silent mode then I use the 'true' command, while in verbose
mode we actually use 'echo'.

So, other than the issues outlined above, the output when building the
data-directory is now greatly reduced, and more inline with the output
when building in the gdb/ directory.

There should be no change in what is actually built after this commit.
---
 gdb/data-directory/Makefile.in | 40 +++++++++++++++++++---------------
 gdb/silent-rules.mk            |  4 ++++
 2 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
index edfa52c2217..720b983ef6c 100644
--- a/gdb/data-directory/Makefile.in
+++ b/gdb/data-directory/Makefile.in
@@ -36,6 +36,8 @@ exec_prefix = @exec_prefix@
 datarootdir = @datarootdir@
 datadir = @datadir@
 
+include $(srcdir)/../silent-rules.mk
+
 SHELL = @SHELL@
 
 LN_S = @LN_S@
@@ -202,7 +204,7 @@ FLAGS_TO_PASS = \
 all: stamp-syscalls stamp-python stamp-guile stamp-system-gdbinit
 
 %.xml: @MAINTAINER_MODE_TRUE@ %.xml.in apply-defaults.xsl linux-defaults.xml.in
-	$(XSLTPROC) -o $(SYSCALLS_SRCDIR)/$@ $(SYSCALLS_SRCDIR)/apply-defaults.xsl\
+	$(ECHO_GEN) $(XSLTPROC) -o $(SYSCALLS_SRCDIR)/$@ $(SYSCALLS_SRCDIR)/apply-defaults.xsl \
 		$(SYSCALLS_SRCDIR)/$@.in
 
 .PHONY: syscall-xml
@@ -219,16 +221,17 @@ clean-syscall-xml:
 # For portability's sake, we need to handle systems that don't have
 # symbolic links.
 stamp-syscalls: Makefile $(SYSCALLS_FILES)
-	rm -rf ./$(SYSCALLS_DIR)
-	mkdir ./$(SYSCALLS_DIR)
-	files='$(SYSCALLS_FILES)' ; \
+	$(ECHO_GEN)
+	$(SILENCE) rm -rf ./$(SYSCALLS_DIR)
+	$(SILENCE) mkdir ./$(SYSCALLS_DIR)
+	$(SILENCE) files='$(SYSCALLS_FILES)' ; \
 	for file in $$files ; do \
 	  f=$(SYSCALLS_SRCDIR)/$$file ; \
 	  if test -f $$f ; then \
 	    $(INSTALL_DATA) $$f ./$(SYSCALLS_DIR) ; \
 	  fi ; \
 	done
-	touch $@
+	$(SILENCE) touch $@
 
 .PHONY: clean-syscalls
 clean-syscalls:
@@ -262,8 +265,9 @@ uninstall-syscalls:
 	done
 
 stamp-python: Makefile $(PYTHON_FILES)
-	rm -rf ./$(PYTHON_DIR)
-	files='$(PYTHON_FILES)' ; \
+	$(ECHO_GEN)
+	$(SILENCE) rm -rf ./$(PYTHON_DIR)
+	$(SILENCE) files='$(PYTHON_FILES)' ; \
 	if test "x$$files" != x ; then \
 	  for file in $$files ; do \
 	    dir=`echo "$$file" | sed 's,/[^/]*$$,,'` ; \
@@ -271,7 +275,7 @@ stamp-python: Makefile $(PYTHON_FILES)
 	    $(INSTALL_DATA) $(PYTHON_SRCDIR)/$$file ./$(PYTHON_DIR)/$$dir ; \
 	  done ; \
 	fi
-	touch $@
+	$(SILENCE) touch $@
 
 .PHONY: clean-python
 clean-python:
@@ -305,8 +309,9 @@ uninstall-python:
 	fi
 
 stamp-guile: Makefile $(GUILE_SOURCE_FILES)
-	rm -rf ./$(GUILE_DIR)
-	if test "x$(GUILE_FILES)" != x ; then \
+	$(ECHO_GEN)
+	$(SILENCE) rm -rf ./$(GUILE_DIR)
+	$(SILENCE) if test "x$(GUILE_FILES)" != x ; then \
 	  files='$(GUILE_SOURCE_FILES)' ; \
 	  for file in $$files ; do \
 	    dir=`echo "$$file" | sed 's,/[^/]*$$,,'` ; \
@@ -317,11 +322,11 @@ stamp-guile: Makefile $(GUILE_SOURCE_FILES)
 	  cd ./$(GUILE_DIR) ; \
 	  for go in $$files ; do \
 	    source="`echo $$go | sed 's/\.go$$/.scm/'`" ; \
-	    echo $(GUILD) compile $(GUILD_COMPILE_FLAGS) -o "$$go" "$$source" ; \
+	    $(SILENT_ECHO) $(GUILD) compile $(GUILD_COMPILE_FLAGS) -o "$$go" "$$source" ; \
 	    $(GUILD) compile $(GUILD_COMPILE_FLAGS) -o "$$go" "$$source" || exit 1 ; \
 	  done ; \
 	fi
-	touch $@
+	$(SILENCE) touch $@
 
 .PHONY: clean-guile
 clean-guile:
@@ -355,16 +360,17 @@ uninstall-guile:
 	fi
 
 stamp-system-gdbinit: Makefile $(SYSTEM_GDBINIT_FILES)
-	rm -rf ./$(SYSTEM_GDBINIT_DIR)
-	mkdir ./$(SYSTEM_GDBINIT_DIR)
-	files='$(SYSTEM_GDBINIT_FILES)' ; \
+	$(ECHO_GEN)
+	$(SILENCE) rm -rf ./$(SYSTEM_GDBINIT_DIR)
+	$(SILENCE) mkdir ./$(SYSTEM_GDBINIT_DIR)
+	$(SILENCE) files='$(SYSTEM_GDBINIT_FILES)' ; \
 	for file in $$files ; do \
 	  f=$(SYSTEM_GDBINIT_SRCDIR)/$$file ; \
 	  if test -f $$f ; then \
 	    $(INSTALL_DATA) $$f ./$(SYSTEM_GDBINIT_DIR) ; \
 	  fi ; \
 	done
-	touch $@
+	$(SILENCE) touch $@
 
 .PHONY: clean-system-gdbinit
 clean-system-gdbinit:
@@ -438,7 +444,7 @@ clean-info:
 MAKEOVERRIDES=
 
 Makefile: $(srcdir)/Makefile.in $(top_builddir)/config.status
-	cd .. && $(SHELL) ./config.status data-directory/Makefile
+	$(ECHO_GEN) cd .. && $(SHELL) ./config.status $(SILENT_FLAG) data-directory/Makefile
 
 # Disable implicit make rules.
 include $(srcdir)/../disable-implicit-rules.mk
diff --git a/gdb/silent-rules.mk b/gdb/silent-rules.mk
index 36791f6683f..43dc2bf3aca 100644
--- a/gdb/silent-rules.mk
+++ b/gdb/silent-rules.mk
@@ -20,4 +20,8 @@ ECHO_RANLIB = @echo "  RANLIB $@";
 SILENCE = @
 # Silence libtool.
 SILENT_FLAG = --silent
+# Used in shell snippets instead of 'echo'.
+SILENT_ECHO = true
+else
+SILENT_ECHO = echo
 endif
-- 
2.25.4

Re: [PATCHv2 2/6] gdb/Makefile: rewrite dependencies for config.status target

[Simon Marchi]

On 2024-04-06 13:03, Andrew Burgess wrote:
> I noticed something weird, the rule for the config.status target looks
> like this:
> 
>   config.status: $(srcdir)/configure configure.nat configure.tgt configure.host ../bfd/development.sh
>           $(SHELL) config.status --recheck
> 
> What bothered me is that 'configure' is specified as being in
> $(srcdir), while all of the other files are not, even though those
> files are in the same $(srcdir) as the configure script.
> 
> However, I tried touching one of those files, and the config.status
> rule does trigger!
> 
> This is thanks to the VPATH variable, which is set to $(srcdir), so
> make looks in $(srcdir) for any dependencies.
> 
> However, this inconsistency bothers me.  Better, I think, to add the
> $(srcdir) prefix to each of these files.
> 
> I also spotted that the configure script also includes the files
> ../bfd/config.bfd, yet that is missing from the include list, so in
> this commit I plan to add this as a dependency.
> 
> The configure script also pulls in two TCL and TK related files:
> 
> 	. ${TCL_BIN_DIR}/tclConfig.sh
> 	. ${TK_BIN_DIR}/tkConfig.sh
> 
> However, I don't think ${TCL_BIN_DIR} and ${TK_BIN_DIR} are currently
> visible in GDB's Makefile, so I'm not planning to add these
> dependencies at this time.
> 
> In this commit I add a new variable config_status_deps which holds the
> list of all the dependencies for config.status, with the $(srcdir)
> prefix included, and then I use this in the config.status rule.
> 
> After this commit config.status will regenerate if config.bfd changes,
> which it wouldn't before, but nothing else changes.
> ---
>  gdb/Makefile.in | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index df044288b5e..9340becbdc9 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -2333,7 +2333,18 @@ nm.h: stamp-nmh ; @true
>  stamp-nmh: config.status
>  	$(SHELL) config.status nm.h
>  
> -config.status: $(srcdir)/configure configure.nat configure.tgt configure.host ../bfd/development.sh
> +# Files included from config.status or the configure script.  When
> +# these change the configure script doesn't need regenerating, but its
> +# output (and so that of config.status) might change.
> +config_status_deps = \
> +	$(srcdir)/configure \
> +	$(srcdir)/configure.nat \
> +	$(srcdir)/configure.tgt \
> +	$(srcdir)/configure.host \
> +	$(srcdir)/../bfd/development.sh \
> +	$(srcdir)/../bfd/config.bfd

I agree that lists formatted like this are easier to read than long
lines.  I also agree that using $(srcdir) consistently helps with
readability, even if it works without (because of VPATH).  Although it
would probably be overkill in contexts like the definition of
`COMMON_SFILES`.

Simon

Re: [PATCHv2 5/6] gdb/configure: use AC_MSG_NOTICE not a direct echo call

[Simon Marchi]

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

Re: [PATCHv2 6/6] gdb/build: apply silent-rules.mk to the data-directory Makefile.in

[Simon Marchi]

On 2024-04-06 13:03, Andrew Burgess wrote:
> This commit makes use of gdb/silent-rules.mk in the data-directory
> Makefile.in.  I've only updated the rules that actually generate
> things, I've not touched the install or uninstall rules, this matches
> gdb/Makefile.in.

When I first worked on adding silent rules, I did the first 80% (most
importantly the compilation of C/C++ files).  My goal was to be able to
kinda see the progress of my build, and that was enough (an output like
Ninja does would be awesome, but I don't think that's happening anytime
soon).  Thanks for working on the other 80%.

> I've not managed to completely silence all of the recipe output, the
> mkinstalldirs command outputs some diagnostic text which looks like
> this:
> 
>     GEN    stamp-python
>   mkdir -p -- ./python/gdb
>   mkdir -p -- ./python/gdb/command
>   mkdir -p -- ./python/gdb/dap
>   mkdir -p -- ./python/gdb/function
>   mkdir -p -- ./python/gdb/printer

The output of these targets is a bit funny, "stamp-something".  It's as
if the stamp file was the final artefact we're interested in, when in
fact it's a build system detail.  Not a problem for me though.

Simon

Re: [PATCHv2 0/6] gcore and config.status related Makefile changes

[Simon Marchi]

On 2024-04-06 13:03, Andrew Burgess wrote:
> Changes in v2:
> 
>   - Nothing has changed in patches 1, 2, 3, or 4,
> 
>   - Two new patches: 5 and 6.
> 
> ---
> 
> Andrew Burgess (6):
>   gdb/Makefile: add gcore to the 'all' target dependency list
>   gdb/Makefile: rewrite dependencies for config.status target
>   gdb/Makefile: add some missing config.status dependencies
>   gdb/Makefile: Print 'GEN' message, and pass SILENT_FLAG more
>   gdb/configure: use AC_MSG_NOTICE not a direct echo call
>   gdb/build: apply silent-rules.mk to the data-directory Makefile.in
> 
>  gdb/Makefile.in                | 41 +++++++++++++++++++++-------------
>  gdb/configure                  |  6 +++--
>  gdb/configure.ac               |  4 ++--
>  gdb/data-directory/Makefile.in | 40 +++++++++++++++++++--------------
>  gdb/silent-rules.mk            |  4 ++++
>  5 files changed, 59 insertions(+), 36 deletions(-)
> 
> 
> base-commit: 16810e455feb26ef826a3ed876d6d7e6d24818b0

Thanks all looks reasonable to me, thanks.

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon

Re: [PATCHv2 6/6] gdb/build: apply silent-rules.mk to the data-directory Makefile.in

[Andrew Burgess]

Simon Marchi <simark@simark.ca> writes:

> On 2024-04-06 13:03, Andrew Burgess wrote:
>
>> I've not managed to completely silence all of the recipe output, the
>> mkinstalldirs command outputs some diagnostic text which looks like
>> this:
>> 
>>     GEN    stamp-python
>>   mkdir -p -- ./python/gdb
>>   mkdir -p -- ./python/gdb/command
>>   mkdir -p -- ./python/gdb/dap
>>   mkdir -p -- ./python/gdb/function
>>   mkdir -p -- ./python/gdb/printer
>
> The output of these targets is a bit funny, "stamp-something".  It's as
> if the stamp file was the final artefact we're interested in, when in
> fact it's a build system detail.  Not a problem for me though.

Indeed.  We already have ECHO_GEN_XML_BUILTIN,
ECHO_GEN_XML_BUILTIN_GENERATED, and ECHO_INIT_C which are used as
alternatives to mentioning various 'stamp-*' targets.

I didn't do this because adding more and more new ECHO_GEN_* defines
just seemed unnecessary; seeing the stamp-* doesn't bother me, and this
is just something developers see, it's not for end users, so my
expectation is that folk will be able to ask, or dig into it if they
really care.

Plus, for some of the rules, like 'stamp-python' and 'stamp-guile',
where many things are done, it's not obvious what you'd replace the
'stamp-*' with.

That all said, if someone came along after and did add some custom
ECHO_GEN_* defines, I'm not going to stop them.

Thanks,
Andrew

Re: [PATCHv2 5/6] gdb/configure: use AC_MSG_NOTICE not a direct echo call

[Andrew Burgess]

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

Re: [PATCHv2 0/6] gcore and config.status related Makefile changes

[Andrew Burgess]

Simon Marchi <simark@simark.ca> writes:

> On 2024-04-06 13:03, Andrew Burgess wrote:
>> Changes in v2:
>> 
>>   - Nothing has changed in patches 1, 2, 3, or 4,
>> 
>>   - Two new patches: 5 and 6.
>> 
>> ---
>> 
>> Andrew Burgess (6):
>>   gdb/Makefile: add gcore to the 'all' target dependency list
>>   gdb/Makefile: rewrite dependencies for config.status target
>>   gdb/Makefile: add some missing config.status dependencies
>>   gdb/Makefile: Print 'GEN' message, and pass SILENT_FLAG more
>>   gdb/configure: use AC_MSG_NOTICE not a direct echo call
>>   gdb/build: apply silent-rules.mk to the data-directory Makefile.in
>> 
>>  gdb/Makefile.in                | 41 +++++++++++++++++++++-------------
>>  gdb/configure                  |  6 +++--
>>  gdb/configure.ac               |  4 ++--
>>  gdb/data-directory/Makefile.in | 40 +++++++++++++++++++--------------
>>  gdb/silent-rules.mk            |  4 ++++
>>  5 files changed, 59 insertions(+), 36 deletions(-)
>> 
>> 
>> base-commit: 16810e455feb26ef826a3ed876d6d7e6d24818b0
>
> Thanks all looks reasonable to me, thanks.
>
> Approved-By: Simon Marchi <simon.marchi@efficios.com>

Pushed.

Thanks,
Andrew

Re: [PATCHv2 5/6] gdb/configure: use AC_MSG_NOTICE not a direct echo call

[Tom Tromey]

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