diff mbox series

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

Message ID 20e8e8749d8119823ce18d23dffd2e47f9ad5791.1712422921.git.aburgess@redhat.com
State New
Headers
Series gcore and config.status related Makefile changes |

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

Andrew Burgess April 6, 2024, 5:03 p.m. UTC 
  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(-)

Comments

Simon Marchi April 8, 2024, 3:09 a.m. UTC | #1 
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
diff mbox series

Patch

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