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 @LIBLTDL@. 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 <g...@gnu.org> --- 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) -@DIST_MAKEFILE_LIST@: +$(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], [@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@])'; \ + 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 = ../.. -@LIBLTDL@: $(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