Re: [PATCH 09/25] syntax-check: fix violations and re-enable sc_makefile_at_at_check.

[Gary V. Vaughan]

Hi Chuck, Eric,

Thanks both for the review!

On 15 Nov 2011, at 23:07, Charles Wilson wrote:
> On 11/15/2011 7:53 AM, Gary V. Vaughan wrote:
> tests/mdemo/Makefile.am
>> -## use @LIBLTDL@ because some broken makes do not accept macros in targets
>> +## use $(LIBLTDL) because some broken makes do not accept macros in targets
> 
> This comment now makes zero sense. If you are now forcing the following
> rule:
> 
> +$(LIBLTDL): $(top_distdir)/libtool \
> 
> then (a) remove the comment completely, and (b) document somewhere that
> we now require non-broken make which DOES allow $(macros) in targets.

Yikes, that's what comes of making changes with emacs macros.  Nicely caught!


On 16 Nov 2011, at 00:03, Eric Blake wrote:
> On 11/15/2011 09:49 AM, Charles Wilson wrote:
>> On 11/15/2011 7:53 AM, Gary V. Vaughan wrote:
>> tests/mdemo/Makefile.am
>>> -## use @LIBLTDL@ because some broken makes do not accept macros in targets
>>> +## use $(LIBLTDL) because some broken makes do not accept macros in targets
>> 
>> This comment now makes zero sense. If you are now forcing the following
>> rule:
>> 
>> +$(LIBLTDL): $(top_distdir)/libtool \
>> 
>> then (a) remove the comment completely, and (b) document somewhere that
>> we now require non-broken make which DOES allow $(macros) in targets.
> 
> As an alternative, revert this change, and instead add a setting of
> _makefile_at_at_check_exceptions in cfg.mk that exempts just @address@hidden

Well in the same patch, we also change the @DIST_MAKEFILE_LIST@ target to
$(DIST_MAKEFILE_LIST)...

Anyway, that comment appeared out of the blue in the commit that introduced the
whole of tests/mdemo, in the face of which we already had other Makefile rules
with macro expansions in targets.

I don't remember why I wrote that, or what bizarre or imagined make 
implementation
I was using at the time... but I seriously doubt the veracity of the comment, or
at least that if such a make still exists in the wild, we ever truly supported
it.  Here's a new version of the patch with that comment removed, and a NEWS
item explaining why:


* cfg.mk (local-checks-to-fix): Remove sc_makefile_at_at_check
from list of disabled checks.
* configure.ac (order-only prerequisites): Test with the
order-only pipe symbol in a macro.
* Makefile.am, tests/mdemo/Makefile.am: Convert all @FOO@ to
$(FOO).
* NEWS: Mention that support for make implementations that do
not accept macros in targets has been broken for quite some
time.

Signed-off-by: Gary V. Vaughan <address@hidden>
---
 Makefile.am             |   42 +++++++++++++++++++++---------------------
 NEWS                    |   14 ++++++++++++++
 cfg.mk                  |    1 -
 configure.ac            |    3 ++-
 tests/mdemo/Makefile.am |   12 +++++-------
 5 files changed, 42 insertions(+), 30 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index b559b5e..97de896 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -325,7 +325,7 @@ libtool: $(ltmain_sh) $(config_status) $(dotversion)
 
 .PHONY: configure-subdirs
 configure-subdirs distdir: $(DIST_MAKEFILE_LIST)
address@hidden@:
+$(DIST_MAKEFILE_LIST):
        $(AM_V_at)dir=`echo '$@' |'$(SED)' 's,^[^/]*$$,.,;s,/[^/]*$$,,'`; \
        test -d "$$dir" || mkdir "$$dir" || exit 1; \
        abs_srcdir=`$(lt__cd) '$(srcdir)' && pwd`; \
@@ -417,7 +417,7 @@ EXTRA_DIST  += $(doc_dir)/gendocs_template
 #   info_TEXINFOS      = $(doc_dir)/libtool.texi
 #
 # Producing the following error, even though srcdir is implicitly set:
-# "cannot open < ./@srcdir@/doc/libtool.texi: No such file or directory"
+# "cannot open < ./$(srcdir)/doc/libtool.texi: No such file or directory"
 info_TEXINFOS          = doc/libtool.texi
 doc_libtool_TEXINFOS   = $(doc_dir)/PLATFORMS $(doc_dir)/fdl.texi \
                          $(notes_texi)
@@ -726,12 +726,12 @@ $(testsuite): $(package_m4) $(TESTSUITE_AT) Makefile.am
 $(package_m4): $(dotversion) Makefile.am
        $(AM_V_GEN){ \
          echo '# Signature of the current package.'; \
-         echo 'm4_define([AT_PACKAGE_NAME],      address@hidden@])'; \
-         echo 'm4_define([AT_PACKAGE_TARNAME],   address@hidden@])'; \
-         echo 'm4_define([AT_PACKAGE_VERSION],   address@hidden@])'; \
-         echo 'm4_define([AT_PACKAGE_STRING],    address@hidden@])'; \
-         echo 'm4_define([AT_PACKAGE_BUGREPORT], address@hidden@])'; \
-         echo 'm4_define([AT_PACKAGE_URL],       address@hidden@])'; \
+         echo 'm4_define([AT_PACKAGE_NAME],      [$(PACKAGE_NAME)])'; \
+         echo 'm4_define([AT_PACKAGE_TARNAME],   [$(PACKAGE_TARNAME)])'; \
+         echo 'm4_define([AT_PACKAGE_VERSION],   [$(PACKAGE_VERSION)])'; \
+         echo 'm4_define([AT_PACKAGE_STRING],    [$(PACKAGE_STRING)])'; \
+         echo 'm4_define([AT_PACKAGE_BUGREPORT], [$(PACKAGE_BUGREPORT)])'; \
+         echo 'm4_define([AT_PACKAGE_URL],       [$(PACKAGE_URL)])'; \
        } > '$@'
 
 tests/atconfig: $(config_status)
@@ -966,13 +966,13 @@ INTERACTIVE_TESTS = \
 
 tests/cdemo-undef-exec.log:    tests/cdemo-undef-make.log
 tests/cdemo-undef-make.log:    tests/cdemo-undef.log
-tests/cdemo-undef.log: @ORDER@ tests/cdemo-shared-exec.log
+tests/cdemo-undef.log: $(ORDER)        tests/cdemo-shared-exec.log
 tests/cdemo-shared-exec.log:   tests/cdemo-shared-make.log
 tests/cdemo-shared-make.log:   tests/cdemo-shared.log
-tests/cdemo-shared.log: @ORDER@        tests/cdemo-exec.log
+tests/cdemo-shared.log: $(ORDER) tests/cdemo-exec.log
 tests/cdemo-exec.log:          tests/cdemo-make.log
 tests/cdemo-make.log:          tests/cdemo-conf.log
-tests/cdemo-conf.log: @ORDER@  tests/cdemo-static-exec.log
+tests/cdemo-conf.log: $(ORDER) tests/cdemo-static-exec.log
 tests/cdemo-static-exec.log:   tests/cdemo-static-make.log
 tests/cdemo-static-make.log:   tests/cdemo-static.log
 
@@ -983,24 +983,24 @@ tests/demo-hardcode.log:  tests/demo-shared-inst.log
 tests/demo-shared-inst.log:    tests/demo-shared-exec.log
 tests/demo-shared-exec.log:    tests/demo-shared-make.log
 tests/demo-shared-make.log:    tests/demo-shared.log
-tests/demo-shared.log: @ORDER@ tests/demo-nopic-exec.log
+tests/demo-shared.log: $(ORDER)        tests/demo-nopic-exec.log
 tests/demo-nopic-exec.log:     tests/demo-nopic-make.log
 tests/demo-nopic-make.log:     tests/demo-nopic.log
-tests/demo-nopic.log: @ORDER@  tests/demo-pic-exec.log
+tests/demo-nopic.log: $(ORDER) tests/demo-pic-exec.log
 tests/demo-pic-exec.log:       tests/demo-pic-make.log
 tests/demo-pic-make.log:       tests/demo-pic.log
-tests/demo-pic.log: @ORDER@    tests/demo-nofast-unst.log
+tests/demo-pic.log: $(ORDER)   tests/demo-nofast-unst.log
 tests/demo-nofast-unst.log:    tests/demo-nofast-inst.log
 tests/demo-nofast-inst.log:    tests/demo-nofast-exec.log
 tests/demo-nofast-exec.log:    tests/demo-nofast-make.log
 tests/demo-nofast-make.log:    tests/demo-nofast.log
-tests/demo-nofast.log: @ORDER@ tests/demo-deplibs.log
+tests/demo-nofast.log: $(ORDER)        tests/demo-deplibs.log
 tests/demo-deplibs.log:                tests/demo-unst.log
 tests/demo-unst.log:           tests/demo-inst.log
 tests/demo-inst.log:           tests/demo-exec.log
 tests/demo-exec.log:           tests/demo-make.log
 tests/demo-make.log:           tests/demo-conf.log
-tests/demo-conf.log: @ORDER@   tests/demo-static-unst.log
+tests/demo-conf.log: $(ORDER)  tests/demo-static-unst.log
 tests/demo-static-unst.log:    tests/demo-static-inst.log
 tests/demo-static-inst.log:    tests/demo-static-exec.log
 tests/demo-static-exec.log:    tests/demo-static-make.log
@@ -1011,17 +1011,17 @@ tests/depdemo-relink.log:       
tests/depdemo-shared-inst.log
 tests/depdemo-shared-inst.log: tests/depdemo-shared-exec.log
 tests/depdemo-shared-exec.log: tests/depdemo-shared-make.log
 tests/depdemo-shared-make.log: tests/depdemo-shared.log
-tests/depdemo-shared.log: @ORDER@ tests/depdemo-nofast-unst.log
+tests/depdemo-shared.log: $(ORDER) tests/depdemo-nofast-unst.log
 tests/depdemo-nofast-unst.log: tests/depdemo-nofast-inst.log
 tests/depdemo-nofast-inst.log: tests/depdemo-nofast-exec.log
 tests/depdemo-nofast-exec.log: tests/depdemo-nofast-make.log
 tests/depdemo-nofast-make.log: tests/depdemo-nofast.log
-tests/depdemo-nofast.log: @ORDER@ tests/depdemo-unst.log
+tests/depdemo-nofast.log: $(ORDER) tests/depdemo-unst.log
 tests/depdemo-unst.log:                tests/depdemo-inst.log
 tests/depdemo-inst.log:                tests/depdemo-exec.log
 tests/depdemo-exec.log:                tests/depdemo-make.log
 tests/depdemo-make.log:                tests/depdemo-conf.log
-tests/depdemo-conf.log: @ORDER@        tests/depdemo-static-unst.log
+tests/depdemo-conf.log: $(ORDER) tests/depdemo-static-unst.log
 tests/depdemo-static-unst.log: tests/depdemo-static-inst.log
 tests/depdemo-static-inst.log: tests/depdemo-static-exec.log
 tests/depdemo-static-exec.log: tests/depdemo-static-make.log
@@ -1031,7 +1031,7 @@ tests/mdemo-shared-unst.log:      
tests/mdemo-shared-inst.log
 tests/mdemo-shared-inst.log:   tests/mdemo-shared-exec.log
 tests/mdemo-shared-exec.log:   tests/mdemo-shared-make.log
 tests/mdemo-shared-make.log:   tests/mdemo-shared.log
-tests/mdemo-shared.log: @ORDER@        tests/mdemo-dryrun.log \
+tests/mdemo-shared.log: $(ORDER) tests/mdemo-dryrun.log \
                                tests/mdemo2-exec.log
 
 tests/mdemo-dryrun.log:                tests/mdemo-unst.log
@@ -1039,7 +1039,7 @@ tests/mdemo-unst.log:             tests/mdemo-inst.log
 tests/mdemo-inst.log:          tests/mdemo-exec.log
 tests/mdemo-exec.log:          tests/mdemo-make.log
 tests/mdemo-make.log:          tests/mdemo-conf.log
-tests/mdemo-conf.log: @ORDER@  tests/mdemo-static-unst.log
+tests/mdemo-conf.log: $(ORDER) tests/mdemo-static-unst.log
 tests/mdemo-static-unst.log:   tests/mdemo-static-inst.log
 tests/mdemo-static-inst.log:   tests/mdemo-static-exec.log
 tests/mdemo-static-exec.log:   tests/mdemo-static-make.log
diff --git a/NEWS b/NEWS
index 32e9bb1..10961a9 100644
--- a/NEWS
+++ b/NEWS
@@ -61,6 +61,20 @@ NEWS - list of user-visible changes between releases of GNU 
Libtool
     upgrade to the more standard naming of `ltdl.mk' in keeping with other
     GNU projects.
 
+  - At some point we were supporting some undetermined `broken make', as
+    evidenced by having carried the following code since 2003:
+
+      ## use @LIBLTDL@ because some broken makes do not accept macros in
+      ## targets, we can only do this because our LIBLTDL does not contain
+      ## $(top_builddir)
+      @LIBLTDL@: $(top_distdir)/libtool \
+      ...
+
+    However, we've also had *many* cases of macros in targets for just as
+    long, so most likely we never fully supported makes allegedly broken
+    in this way.  As of this release, we explicitly no longer support
+    make implementations that do not accept macros in targets.
+
 New in 2.4.2 2011-10-17: git version 2.4.1a, Libtool team:
 
 * New features:
diff --git a/cfg.mk b/cfg.mk
index 7d32ebf..ab010c3 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -38,7 +38,6 @@ endif
 VC_LIST_ALWAYS_EXCLUDE_REGEX = ^mail/
 
 local-checks-to-fix =                          \
-       sc_makefile_at_at_check                 \
        sc_prohibit_always-defined_macros       \
        sc_prohibit_always_true_header_tests    \
        sc_prohibit_cvs_keyword                 \
diff --git a/configure.ac b/configure.ac
index 7783e07..5c07723 100644
--- a/configure.ac
+++ b/configure.ac
@@ -186,7 +186,8 @@ AC_CACHE_CHECK([whether ${MAKE-make} supports order-only 
prerequisites],
    touch b
    touch a
 cat >confmk << 'END'
-a: b | c
+ORDER = |
+a: b $(ORDER) c
 a b c:
        touch $[]@
 END
diff --git a/tests/mdemo/Makefile.am b/tests/mdemo/Makefile.am
index a0ab490..7f34be1 100644
--- a/tests/mdemo/Makefile.am
+++ b/tests/mdemo/Makefile.am
@@ -43,18 +43,16 @@ libsub_la_LDFLAGS = -no-undefined
 ## its exported symbols, and we use libltdl as a convenience archive.
 ## Thus, on w32, auto-exporting is turned off.
 libmlib_la_SOURCES = mlib.c
-libmlib_la_LIBADD = @LIBLTDL@ "-dlopen" foo1.la "-dlopen" libfoo2.la
+libmlib_la_LIBADD = $(LIBLTDL) "-dlopen" foo1.la "-dlopen" libfoo2.la
 libmlib_la_LDFLAGS = -no-undefined -export-symbols-regex ".*"
-libmlib_la_DEPENDENCIES = @LIBLTDL@ libsub.la foo1.la libfoo2.la
+libmlib_la_DEPENDENCIES = $(LIBLTDL) libsub.la foo1.la libfoo2.la
 
 noinst_HEADERS = foo.h
 
 bin_PROGRAMS = mdemo mdemo_static
 
-## use @LIBLTDL@ because some broken makes do not accept macros in targets
-## we can only do this because our LIBLTDL does not contain ${top_builddir}
 top_distdir = ../..
address@hidden@: $(top_distdir)/libtool \
+$(LIBLTDL): $(top_distdir)/libtool \
     $(top_distdir)/config.h $(srcdir)/$(top_distdir)/libltdl/ltdl.c \
     $(srcdir)/$(top_distdir)/libltdl/ltdl.h
        (cd $(top_distdir); $(MAKE) `echo $(LIBLTDL) | sed 
's,.*\.\./libltdl/,libltdl/,g'`)
@@ -65,9 +63,9 @@ $(top_distdir)/config.h:
 mdemo_SOURCES = main.c
 mdemo_LDFLAGS = -export-dynamic
 ## The quotes around -dlopen below fool automake into accepting it
-mdemo_LDADD = @LIBLTDL@ libsub.la "-dlopen" self \
+mdemo_LDADD = $(LIBLTDL) libsub.la "-dlopen" self \
                "-dlopen" foo1.la "-dlopen" libfoo2.la
-mdemo_DEPENDENCIES = @LIBLTDL@ libsub.la foo1.la libfoo2.la
+mdemo_DEPENDENCIES = $(LIBLTDL) libsub.la foo1.la libfoo2.la
 
 # Create a statically linked version of mdemo.
 mdemo_static_SOURCES = $(mdemo_SOURCES)
-- 
1.7.7.3