Fix typo in Makefile.am to make it agree with Makefile.in.

Message ID 20180504002648.9054-1-jimw@sifive.com
State New
Headers show
Series
  • Fix typo in Makefile.am to make it agree with Makefile.in.
Related show

Commit Message

Jim Wilson May 4, 2018, 12:26 a.m.
I noticed this today while trying to modify Makefile.am and regenerate
Makefile.in.  I ended up with a change to Makefile.in that I didn't make.  The
problem is a minor bug in a recent patch from Christophe Lyon
    https://sourceware.org/ml/binutils/2018-04/msg00216.html

This has in Makefile.am

earmelfb_linux_fdpiceabi.c: $(srcdir)/emulparams/armelfb_linux_fdpiceabi.sh \
  $(srcdir)/emulparams/armelfb_linux_fdpiceabi.sh \
  ...

which mentions the same file twice, but in Makefile.in it has

earmelfb_linux_fdpiceabi.c: $(srcdir)/emulparams/armelfb_linux_fdpiceabi.sh \
  $(srcdir)/emulparams/armelf_linux_fdpiceabi.sh \
  ...

The version in Makefile.in is correct.  I checked in this patch to fix it.

	ld/
	* Makefile.am (earmelfb_linux_fdpiceabi.c): Fix typo in dependencies.
---
 ld/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.14.1

Comments

John Darrington May 6, 2018, 12:44 p.m. | #1
On Thu, May 03, 2018 at 05:26:48PM -0700, Jim Wilson wrote:

     I noticed this today while trying to modify Makefile.am and regenerate
     Makefile.in.  I ended up with a change to Makefile.in that I didn't make.  The
     problem is a minor bug in a recent patch from Christophe Lyon
         https://sourceware.org/ml/binutils/2018-04/msg00216.html
     

This highlights an issue which I've regarded as strange ever since I've been 
looking at the binutils repostory:   

What on earth are files such as Makefile.in,  configure, and many other 
generated files doing in the repository in the first place?

As I see it, the purpose of the repository is to hold the source code.  That
is to say files which have been typed by someone.  If a file has been generated
(either by autoconf, automake, gettext, a script or whatever), then it has
no business being in the the repository.   Firstly it is redundant, secondly it
forces unnecessary merge conflicts, thirdly it causes confusion in cases such
as this, and fourthly if a something breaks, it might go unnoticed for some
time.

I suggest that all generated files are removed from the repo, (and a bootstrap 
script, or auxiallary Makefile be introduced to regenerate them).

J'

-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.
Jim Wilson May 6, 2018, 3:21 p.m. | #2
On Sun, May 6, 2018 at 5:44 AM, John Darrington
<john@darrington.wattle.id.au> wrote:
> What on earth are files such as Makefile.in,  configure, and many other

> generated files doing in the repository in the first place?


To make it easier for end users to build binutils sources.  To
regenerate Makefile.in, I had to build and install the specific
version of automake that is the same as the one used to build it in
the first place.  This is an unreasonable requirement for end users
who just want to build the sources.

> I suggest that all generated files are removed from the repo, (and a bootstrap

> script, or auxiallary Makefile be introduced to regenerate them).


There are already build rules for this.  Use --enable-maintainer-mode
when configuring binutils.

I can't support removing the generated files from the repo.  I think
that will cause more trouble than it will fix.  I also think that it
won't solve the problem.  The problem arose because Makefile.in was
accidentally edited instead of Makefile.am.  If Makefile.in is created
at build time, then you still have it in your tree, and you can still
accidentally edit it.  It only Makefile.am is checked in, then we
would not have noticed the inconsistency, and it would likely have
taken far longer to notice and fix the problem.  So having both
Makefile.am and Makefile.in helps to catch these kinds of errors.

Jim
John Darrington May 6, 2018, 3:33 p.m. | #3
On Sun, May 06, 2018 at 08:21:17AM -0700, Jim Wilson wrote:
     On Sun, May 6, 2018 at 5:44 AM, John Darrington
     <john@darrington.wattle.id.au> wrote:
     > What on earth are files such as Makefile.in,  configure, and many other

     > generated files doing in the repository in the first place?

     
     To make it easier for end users to build binutils sources.  To
     regenerate Makefile.in, I had to build and install the specific
     version of automake that is the same as the one used to build it in
     the first place.  This is an unreasonable requirement for end users
     who just want to build the sources.

I'm not for one moment suggesting those files be removed from the tarball! - just
from the repository.   End users don't build from the repo - or at least
that's not what it's indended for - so it won't make any difference for them.
     
     > I suggest that all generated files are removed from the repo, (and a bootstrap

     > script, or auxiallary Makefile be introduced to regenerate them).

     
     There are already build rules for this.  Use --enable-maintainer-mode
     when configuring binutils.

That is good to know.  I will try it.
     
     I can't support removing the generated files from the repo.  I think
     that will cause more trouble than it will fix.  I also think that it
     won't solve the problem.  The problem arose because Makefile.in was
     accidentally edited instead of Makefile.am.  If Makefile.in is created
     at build time, then you still have it in your tree, and you can still
     accidentally edit it.  It only Makefile.am is checked in, then we
     would not have noticed the inconsistency, and it would likely have
     taken far longer to notice and fix the problem.  So having both
     Makefile.am and Makefile.in helps to catch these kinds of errors.
     
I'm not sure that I follow the logic here.  It is true that if a non-source file
is manually changed, then bad things will happen.  But we accept that for object files,
output binaries, lex and yacc generated C files, gperf output etc, etc, etc.
Why apply a different rule for autotools?

It's also true that particular automake versions are required, but again,
since it's not intended for end use consumption, I don't see this as a
big price to pay.

Just my $0.02


-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.

Patch

diff --git a/ld/Makefile.am b/ld/Makefile.am
index 6464e10917..2d758b1ee3 100644
--- a/ld/Makefile.am
+++ b/ld/Makefile.am
@@ -757,7 +757,7 @@  earmelfb_linux_eabi.c: $(srcdir)/emulparams/armelfb_linux_eabi.sh \
   $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}
 
 earmelfb_linux_fdpiceabi.c: $(srcdir)/emulparams/armelfb_linux_fdpiceabi.sh \
-  $(srcdir)/emulparams/armelfb_linux_fdpiceabi.sh \
+  $(srcdir)/emulparams/armelf_linux_fdpiceabi.sh \
   $(srcdir)/emulparams/armelf_linux.sh \
   $(ELF_DEPS) $(srcdir)/emultempl/armelf.em \
   $(srcdir)/scripttempl/elf.sc ${GEN_DEPENDS}