Re: [SCM] GNU Libtool branch, master, updated. v2.4.2-141-g4099c12
Hi Eric.
On 12/19/2011 02:44 PM, Eric Blake wrote:
On 12/17/2011 10:22 PM, Gary V. Vaughan wrote:
We should try to minimise forks, especially on Windows where they are
+# unreasonably slow, so skip the feature probes when bash is being used:
+if test set = ${BASH_VERSION+set}; then
+: ${lt_HAVE_ARITH_OP=yes}
+: ${lt_HAVE_XSI_OPS=yes}
+# The += operator was introduced in bash 3.1
+test -z $lt_HAVE_PLUSEQ_OP \
+ test 3000 -lt $((${BASH_VERSINFO[0]}*1000 + ${BASH_VERSINFO[1]})) \
This MUST be hidden behind an eval. Otherwise, shells like Solaris
/bin/sh will choke on trying to parse this line:
$ /bin/sh -c 'echo $((${BASH_VERSINFO[0]}*1000 + \
${BASH_VERSINFO[1]}))'
/bin/sh: bad substitution
In truth, for Solaris /bin/sh, protecting the problematic construct with
an if
clause is enough:
$ /bin/sh -c 'if false; then \
echo $((${BASH_VERSINFO[0]}*1000 + ${BASH_VERSINFO[1]})); \
fi; \
echo OK'
OK
But, as I've shown, that is not enough for dash and NetBSD /bin/sh:
$ dash -c 'if false; then \
echo $((${BASH_VERSINFO[0]}*1000 + ${BASH_VERSINFO[1]})); \
fi; \
echo OK'
dash: Syntax error: Bad substitution
So you're still right about the need to use an eval (or to rework the
code so that
it doesn't try to to reference array variables).
Regards,
Stefano
Re: FYI [PATCH] libtool: minimise forks per invocation under bash.
On 12/18/2011 06:15 AM, Gary V. Vaughan wrote:
+# We should try to minimise forks, especially on Windows where they are
+# unreasonably slow, so skip the feature probes when bash is being used:
+if test set = ${BASH_VERSION+set}; then
+: ${lt_HAVE_ARITH_OP=yes}
+: ${lt_HAVE_XSI_OPS=yes}
+# The += operator was introduced in bash 3.1
+test -z $lt_HAVE_PLUSEQ_OP \
+ test 3000 -lt $((${BASH_VERSINFO[0]}*1000 + ${BASH_VERSINFO[1]})) \
+ lt_HAVE_PLUSEQ_OP=yes
+fi
This will likely break with dash 0.5.2:
$ cat foo.sh
#!/bin/sh
if test set = ${BASH_VERSION+set}; then
: ${lt_HAVE_ARITH_OP=yes}
: ${lt_HAVE_XSI_OPS=yes}
# The += operator was introduced in bash 3.1
test -z $lt_HAVE_PLUSEQ_OP \
test 3000 -lt $((${BASH_VERSINFO[0]}*1000 + ${BASH_VERSINFO[1]})) \
lt_HAVE_PLUSEQ_OP=yes
fi
$ dash foo.sh
foo.sh: 7: Syntax error: Bad substitution
Regards,
Stefano
Re: FYI [PATCH] libtool: minimise forks per invocation under bash.
On 12/18/2011 10:52 AM, Stefano Lattarini wrote:
On 12/18/2011 06:15 AM, Gary V. Vaughan wrote:
+# We should try to minimise forks, especially on Windows where they are
+# unreasonably slow, so skip the feature probes when bash is being
used:
+if test set = ${BASH_VERSION+set}; then
+: ${lt_HAVE_ARITH_OP=yes}
+: ${lt_HAVE_XSI_OPS=yes}
+# The += operator was introduced in bash 3.1
+test -z $lt_HAVE_PLUSEQ_OP \
+ test 3000 -lt $((${BASH_VERSINFO[0]}*1000 +
${BASH_VERSINFO[1]})) \
+ lt_HAVE_PLUSEQ_OP=yes
+fi
This will likely break with dash 0.5.2:
$ cat foo.sh
#!/bin/sh
if test set = ${BASH_VERSION+set}; then
: ${lt_HAVE_ARITH_OP=yes}
: ${lt_HAVE_XSI_OPS=yes}
# The += operator was introduced in bash 3.1
test -z $lt_HAVE_PLUSEQ_OP \
test 3000 -lt $((${BASH_VERSINFO[0]}*1000 + ${BASH_VERSINFO[1]})) \
lt_HAVE_PLUSEQ_OP=yes
fi
$ dash foo.sh
foo.sh: 7: Syntax error: Bad substitution
Update: the same happens with NetBSD 5.1 /bin/sh (which is probably
an ash-derivative).
Regards,
Stefano
Re: FYI [PATCH] libtool: minimise forks per invocation under bash.
On 12/18/2011 11:07 AM, Gary V. Vaughan wrote:
Hi Stefano,
On 18 Dec 2011, at 17:02, Stefano Lattarini wrote:
On 12/18/2011 10:52 AM, Stefano Lattarini wrote:
On 12/18/2011 06:15 AM, Gary V. Vaughan wrote:
+# We should try to minimise forks, especially on Windows where they are
+# unreasonably slow, so skip the feature probes when bash is being used:
+if test set = ${BASH_VERSION+set}; then
+: ${lt_HAVE_ARITH_OP=yes}
+: ${lt_HAVE_XSI_OPS=yes}
+# The += operator was introduced in bash 3.1
+test -z $lt_HAVE_PLUSEQ_OP \
+ test 3000 -lt $((${BASH_VERSINFO[0]}*1000 + ${BASH_VERSINFO[1]})) \
+ lt_HAVE_PLUSEQ_OP=yes
+fi
This will likely break with dash 0.5.2:
$ cat foo.sh
#!/bin/sh
if test set = ${BASH_VERSION+set}; then
: ${lt_HAVE_ARITH_OP=yes}
: ${lt_HAVE_XSI_OPS=yes}
# The += operator was introduced in bash 3.1
test -z $lt_HAVE_PLUSEQ_OP \
test 3000 -lt $((${BASH_VERSINFO[0]}*1000 + ${BASH_VERSINFO[1]})) \
lt_HAVE_PLUSEQ_OP=yes
fi
$ dash foo.sh
foo.sh: 7: Syntax error: Bad substitution
Update: the same happens with NetBSD 5.1 /bin/sh (which is probably
an ash-derivative).
Thanks for the report.
Unfortunately, I'm out of ideas on how to portably detect the bash version
without
spending a fork, in which case it seems easiest to spend that fork actually
testing
for += support rather than poking at the bash version.
Can anyone think of something better than just removing the whole
lt_HAVE_PLUSEQ_OP
clause from the patch quoted above, and letting the shell figure it by itself
later
on?
Adding an extra eval seems to do the trick:
eval 'test 3000 -lt $((${BASH_VERSINFO[0]}*1000 +
${BASH_VERSINFO[1]}))'
Of course, a comment about why this eval is needed would be definitely
warranted.
Or, to be even safer, you could directly poke at $BASH_VERSION instead:
case $BASH_VERSION in
[12].*|3.0.*) ;;
*) lt_HAVE_PLUSEQ_OP=yes;;
esac
Regards,
Stefano
Re: [PATCH 02/10] tests: migrate legacy tests/demo tests to Autotest.
Only a quick incomplete review ...
On Friday 25 November 2011, Gary V wrote:
+{
+test -n $objdir || exit 1
+$lt_INSTALL -d $objdir/temp/libs
+cp -f libhello.la $objdir/temp
+cp -f $objdir/libhello.* $objdir/lt-hell$EXEEXT $objdir/temp/libs
+trap func_restore_files 0 1 2 13 15
Quoting autoconf manual:
With AIX sh, a trap on 0 installed in a shell function triggers at function
exit rather than at script.
(I see this is code has only been moved by your patch, so you might want to
fix it in a follow-up?)
+
+/* At some point, cygwin will stop defining __CYGWIN32__, but b19 and
+ * earlier do not define __CYGWIN__. This snippit allows us to check
s/snippit/snippet/ (I see this is code has only been moved by your patch,
so you might want to fix it in a follow-up?)
+ * for __CYGWIN__ reliably for both current, old, and (probable) future
+ * releases.
+ */
Regards,
Stefano
Re: [PATCH 01/10] tests: migrate legacy tests/cdemo tests to Autotest.
Hi Gary. On Friday 25 November 2011, Gary V wrote: The best reason I can find for keeping the various demo directories around (despite the fact we already make use of the much better test harness of Autotest for all our new test cases) from the last time I wanted to migrate everything out of the legacy testsuite, was that it exercises the distribution manager's autotools combination on the testers machine, where the Autotests always use the users autotools. That's no argument as far as I can see: we want tests to fail as early as possible on a users machine to help him know things are not going to work properly if he keeps going - and having the legacy suite pass is only encouraging users to fight with broken installs. This series of patches migrates all 9 of the demo directory test groups into Autotest, and allows us to remove most of the legacy testsuite cruft at the end. Just my 2 cents: I'd like to see the test scripts converted one at the time, rather than one group at the time (and assuming that this is feasible), as I found your huge patches basically un-reviewable in their present state. There's still a few legacy tests left at the end, which I'll tackle later, but at least maintenance is a whole lot easier now that we don't need to wait for 9 additional directories to autoreconf every time we run bootstrap, or distcheck, or roll up a distribution tarball to test on the local network. This is all in keeping with the theme of most of the patches I've posted this year, to make libtool easier and more fun to maintain and contribute to, in the hope of getting more people involved. As usual, subject to feedback, I'll push this whole series in 72 hours or so. Make distcheck passes for me on my Mac 10.7 and my Arch Linux x86_64 machines, but it would be great if folks with access to other machines could give it a spin to see whether I broke any of the tests during migration... if you'd like a pre- rolled distro with my pending patches applied to do that, then please do ask. If you want to send me such a tarball, I might run the testsuite on Solaris 10, Cygwin 1.5.25, NetBSD 5.1 and OpenBSD 4.6 at least. But then you should allow for more than three days for sending feedback -- at least a week. Regards, Stefano
Re: [PATCH 01/10] tests: migrate legacy tests/cdemo tests to Autotest.
On Friday 25 November 2011, Gary V wrote: On 25 Nov 2011, at 18:59, Stefano Lattarini wrote: On Friday 25 November 2011, Gary V wrote: The best reason I can find for keeping the various demo directories around (despite the fact we already make use of the much better test harness of Autotest for all our new test cases) from the last time I wanted to migrate everything out of the legacy testsuite, was that it exercises the distribution manager's autotools combination on the testers machine, where the Autotests always use the users autotools. That's no argument as far as I can see: we want tests to fail as early as possible on a users machine to help him know things are not going to work properly if he keeps going - and having the legacy suite pass is only encouraging users to fight with broken installs. This series of patches migrates all 9 of the demo directory test groups into Autotest, and allows us to remove most of the legacy testsuite cruft at the end. Just my 2 cents: I'd like to see the test scripts converted one at the time, rather than one group at the time (and assuming that this is feasible), as I found your huge patches basically un-reviewable in their present state. The legacy tests are in sets that can't be broken apart without leaving the tree in an inconsistent state part way through that set. Oh. I thought you could convert them one at the time instead, but they are inter-dependent, this could become more cumbersome, if not almost impossible. I could break them up a little more tho', if you think that would help, so instead of converting one demo directory all at once, then a final patch to clean out the configury cruft... there'd be 9 sets of patches each containing almost everything in the current patch, except the deletion of the the files in the given test/demo directory, followed by a series of tiny patches each adding a dozen lines like this: +## --- ## +## Mdemo conf. ## +## --- ## + +AT_SETUP([ltdl load shared and static modules]) + +_LT_SETUP + +_LT_CHECK_CONFIG([], [^build_old_libs=yes], [^build_libtool_libs=yes]) +_LT_CHECK_EXECUTE +_LT_CHECK_INSTALL +_LT_CHECK_UNINSTALL + +AT_CLEANUP plus removing the equivalent legacy set of 3 *.test files, and then a final patch to delete the given test/demo tree, and relevant lines from Makefile.am. It'll take me a while to go through and do that, and we'll end up with 10 large patches each setting up a new tests/demo.at file, about 25 tiny patches per the above, then 10 small patches each removing one of the tests/demo trees, and then the final cruft cleanout patch unchanged. If that will make a big difference, let me know and I'll retract these patches and post 50 or so to replace them in a week or two when I've gone through and broken out the tiny per-test-group sets per the above. Nah, let's forget about this. I thought that breaking up your patches further could be done easily and naturally, but if it is not the case, I see no real gain in the extra churn. There's still a few legacy tests left at the end, which I'll tackle later, but at least maintenance is a whole lot easier now that we don't need to wait for 9 additional directories to autoreconf every time we run bootstrap, or distcheck, or roll up a distribution tarball to test on the local network. This is all in keeping with the theme of most of the patches I've posted this year, to make libtool easier and more fun to maintain and contribute to, in the hope of getting more people involved. As usual, subject to feedback, I'll push this whole series in 72 hours or so. Make distcheck passes for me on my Mac 10.7 and my Arch Linux x86_64 machines, but it would be great if folks with access to other machines could give it a spin to see whether I broke any of the tests during migration... if you'd like a pre- rolled distro with my pending patches applied to do that, then please do ask. If you want to send me such a tarball, I might run the testsuite on Solaris 10, Cygwin 1.5.25, NetBSD 5.1 and OpenBSD 4.6 at least. Much appreciated. Tarballs here: http://vaughan.pe/libtool/libtool-2.4.2.135-aa59c.tar.gz http://vaughan.pe/libtool/libtool-2.4.2.135-aa59c.tar.xz But then you should allow for more than three days for sending feedback -- at least a week. That's fine, and running on those arches would be a great help in giving me confidence the migration didn't break anything. It'll take me at least a week to redo everything into smaller pieces anyway... (unless you think the time spent here will not make much difference to your review). But do let me know either way :) Done above :-) Don't bother in breaking up the series. And thanks also for offering to make the time to look them over. If you meant testing them, that won't require much time from me -- the machines will do basically all the work
Re: [PATCH 6/7] syntax-check: fix violations and implement sc_prohibit_test_const_follows_var.
[adding bug-autoconf in CC] On Tuesday 22 November 2011, Gary V wrote: Hi Eric, On 22 Nov 2011, at 03:07, Eric Blake wrote: On 11/21/2011 07:47 AM, Gary V. Vaughan wrote: To safely use a non-literal first argument to `test', you must always prepend a literal non-`-' character, but often the second operand is a constant that doesn't begin with a `-' already, so always use `test a = $b' instead of noisy `test X$b = Xa'. Not true. test a = $b is just as likely to trigger improper evaluation in buggy test(1) implementations as: test $b = a :-o For real? On non-museum pieces? On Solaris 10 I see this: $ /bin/sh -c 'b=(; test $b = a'; echo status = $? /bin/sh: test: argument expected status = 1 $ /bin/sh -c 'b=!; test $b = a'; echo status = $? /bin/sh: test: argument expected status = 1 And on NetBSD 5.1 I see this: $ /bin/sh -c 'b=(; test $b = a'; echo status = $? test: closing paren expected status = 2 $ /bin/sh -c 'b=!; test $b = a'; echo status = $? test: a: unexpected operator status = 2 And in fact the autconf manual says: Similarly, Posix says that both `test string1 = string2' and `test string1 != string2' work for any pairs of strings, but in practice this is not true for troublesome strings that look like operators or parentheses, or that begin with `-'. (Text that should be probably be expandend to show some examples, *and* to report the exact names and versions of the affected shells). I tested a bunch of /bin/sh, bash, ksh and zsh versions that I have easy access to, and for all of them, the only way to upset test with leading hypens the first argument. I couldn't do this either (for leading hypens, that is); but see the slighlty more elaborated issues presented above. If you cannot guarantee the contents of $b, then you MUST prefix both sides of the comparison with x or X. Conversely, if you CAN guarantee the contents of $b (for example, if you did b=$?, then you KNOW that b is a numeric tring with no problematic characters), then you might as well use the more idiomatic comparison of variable to constant. I don't suppose you can point me at a shell that chokes or fails on: test a != -b || echo bug ? Or at least some reliable documentation that shows we're not dealing with outdated dogma here? I'll postpone pushing this patch until we get to the bottom of the issue though. Cheers, -- Gary V. Vaughan (gary AT gnu DOT org) Regards, Stefano
Re: [PATCH 3/7] tests: migrate tests/sh.test checks to syntax-checks.
On Tuesday 22 November 2011, Gary V wrote: Hi Stefano, On 22 Nov 2011, at 02:52, Stefano Lattarini wrote: Hi Gary. Just a quick nit (I haven't looked at the whole series, and not even at the whole patch in fact; sorry). No apologies necessary, every little helps! Thank you. On Monday 21 November 2011, Gary V wrote: for file do - test -f $file || touch $file + test -f $file || touch $file What's the point of quoting file after `test -f' it it remains unquoted after `touch'? Even though we know there is no whitespace in $file because of the for loop, there still might be other shell meta-characters in there. All uses of $file (including a bunch in the following lines) should be quoted correctly, OK, good. but that is another patch. That's perfectly fine, but IMHO it should me mentioned in the commit message. Regards, Stefano
Re: [PATCH 6/7] syntax-check: fix violations and implement sc_prohibit_test_const_follows_var.
On Tuesday 22 November 2011, Eric Blake wrote: touch =; test -f =; echo $? This is problematic also with pdksh 5.2.14 on Debian: $ pdksh -c 'touch ./=; test -f =; echo $?' pdksh: test: =: missing second argument 2 and with /bin/sh on OpenBSD 4.6 as well: $ /bin/sh -c 'touch ./=; test -f =; echo $?' /bin/sh: test: =: missing second argument 2 Regards, Stefano
Re: [PATCH 3/7] tests: migrate tests/sh.test checks to syntax-checks.
Hi Gary. Just a quick nit (I haven't looked at the whole series, and not even at the whole patch in fact; sorry). On Monday 21 November 2011, Gary V wrote: for file do - test -f $file || touch $file + test -f $file || touch $file What's the point of quoting file after `test -f' it it remains unquoted after `touch'? Regards, Stefano
Re: [PATCH 4/7] syntax-check: fix violations and implement sc_prohibit_bare_basename.
Hi Gary. Again, few quick nits here, probably incomplete.
On Monday 21 November 2011, Gary V wrote:
* cfg.mk (sc_prohibit_bare_basename, sc_prohibit_basename_with_sed):
Make sure not to go back to using occasional `|$basename' or
`|$dirname' syntax.
* build-aux/general.m4sh, build-aux/git-hooks/commit-msg,
build-aux/ltmain.m4sh, build-aux/options-parser,
tests/fcdemo-conf.test, tests/fcdemo-shared.test,
tests/fcdemo-static.test, tests/libtoolize.at: Fix violations.
Signed-off-by: Gary V. Vaughan g...@gnu.org
diff --git a/build-aux/general.m4sh b/build-aux/general.m4sh
index 1f44535..790f4e0 100644
--- a/build-aux/general.m4sh
+++ b/build-aux/general.m4sh
@@ -70,15 +70,15 @@ lt_nl='
'
IFS= $lt_nl
-dirname='s,/[^/]*$,,'
-basename='s,^.*/,,'
+dirname='s|/[^/]*$||'
+basename='s|^.*/||'
What's the point of this change? If it's only stylistic, shouldn't it
be done in a separate patch?
# func_dirname file append nondir_replacement
# Compute the dirname of FILE. If nonempty, add APPEND to the result,
# otherwise set result to NONDIR_REPLACEMENT.
func_dirname ()
{
-func_dirname_result=`$ECHO ${1} | $SED $dirname`
+func_dirname_result=`$ECHO ${1} |$SED $dirname`
Ditto, and for other similar changes as well.
diff --git a/build-aux/ltmain.m4sh b/build-aux/ltmain.m4sh
index 02ff034..b367ddd 100644
--- a/build-aux/ltmain.m4sh
+++ b/build-aux/ltmain.m4sh
@@ -3042,7 +3042,7 @@ func_extract_archives ()
$RM
unfat-$$/${darwin_base_archive}-${darwin_arch}/${darwin_base_archive}
done # $darwin_arches
## Okay now we've a bunch of thin objects, gotta fatten them up
:)
- darwin_filelist=`find unfat-$$ -type f ... -print | $SED -e
$basename | sort -u`
+ darwin_filelist=`find unfat-$$ -type f ... -print | $SED $basename
| sort -u`
Why this quote removal? It seems gratuitous -- even dangerous,
since `$basename' contains shell wildcards (`*' at least).
Regards,
Stefano
Re: [PATCH 6/7] syntax-check: fix violations and implement sc_prohibit_test_const_follows_var.
Hi Gary. Few more random nits... On Monday 21 November 2011, Gary V wrote: To safely use a non-literal first argument to `test', you must always prepend a literal non-`-' character, but often the second operand is a constant that doesn't begin with a `-' already, so always use `test a = $b' instead of noisy `test X$b = Xa'. This seems back-bending to me, and slightly unclear to read. Also, it goes against the (unofficial) conventions of autoconf, which is to use either `test x$b = xa' or `test x$b = Xa'. Also ... # Bootstrap this package from checked-out sources. # Written by Gary V. Vaughan, 2010 @@ -1760,7 +1760,7 @@ func_ifcontains () ;; esac -test $_G_status -eq 0 || exit $_G_status +test 0 -eq $_G_status || exit $_G_status } ... changes like this seems overly paranoid, in case $_G_status is expected (as I surmise it is) to be a non-negative integer. And if this assumption stps to hold dur to a bug in your code, you are going to be bitten by much worse problem anyway: # $shell is either Solaris 1 0or ATT ksh, Solaris 10 XPG4 sh, or # zsh 4.3.12. $ $shell -c 'exit t; echo foo'; echo status = $? status = 0 Regards, Stefano
Re: [PATCH 23/25] syntax-check: enable sc_program_name.
Hi Gary. On Wednesday 16 November 2011, Gary V wrote: Hi Stefano, Thanks for the review. # GPL_version: checks for GPLv3, which we don't use -# program_name: libtool has no programs! But then, since libtool doesn't offer any real program, what is the point of enabling the `sc_program_name' check? Quite arguably, there is no point at the moment. So at least I haven't misunderstood the scope/meaning of your change ... Good for me, I was suspecting to be missing something obvious :-) But at some point we may grow a real program (ltmain.c has been on the horizon for many years), and letting syntax-check run everything it has if at all possible means we won't have to remember to go back and enable those additional NOP tests if that day ever arrives. If you object strongly, I can be persuaded to change my mind however. I don't object *strongly*; it just seems confusing to me to add checks for a feature/setup that is not yet implemented, nor is planned to be implemented in the near future. I leave the decision of whether this is worth worrying about to you. Regards, Stefano
Re: [PATCH 09/25] syntax-check: fix violations and re-enable sc_makefile_at_at_check.
[adding automake list in CC] Hi Eric. On Wednesday 16 November 2011, Eric Blake wrote: + - 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 \ + ... By the way, such make implementations (that don't work with $(macros) in targets) exist: https://lists.gnu.org/archive/html/automake-patches/2008-12/msg00027.html This links says nothing about make not accepting macros in targets; it says that using macros on the left side of a *macro assignment* is not portable: # Bad, not portable. foo$(var) = bar # But this should be OK. foo$(var): @echo crating $@. At least IRIX and HP-UX vendor make fail to handle macros in the target. Are you sure? I'm seeing this in the master branch of automake: $ grep -C1 '^$(.*) *:' lib/am/*.am lib/am/check.am- lib/am/check.am:$(TEST_SUITE_LOG): $(TEST_LOGS) lib/am/check.am-@$(am__sh_e_setup); \ -- lib/am/configure.am-if %?REGEN-ACLOCAL-M4% lib/am/configure.am:$(ACLOCAL_M4): %MAINTAINER-MODE% $(am__aclocal_m4_deps) lib/am/configure.am-?TOPDIR_P? $(am__cd) $(srcdir) $(ACLOCAL) $(ACLOCAL_AMFLAGS) -- lib/am/configure.am-## Avoid the deleted header file problem for the dependencies. lib/am/configure.am:$(am__aclocal_m4_deps): lib/am/configure.am-endif %?REGEN-ACLOCAL-M4% -- lib/am/lisp.am-## an empty target. lib/am/lisp.am:$(am__ELCFILES): elc-stamp lib/am/lisp.am-## Recover from the removal of $@. -- lib/am/subdirs.am- lib/am/subdirs.am:$(RECURSIVE_TARGETS): lib/am/subdirs.am-## Using $failcom allows -k to keep its natural meaning when running a -- lib/am/subdirs.am-## bombs. lib/am/subdirs.am:$(RECURSIVE_CLEAN_TARGETS): lib/am/subdirs.am-## Using $failcom allows -k to keep its natural meaning when running a ... but Ralf Wildnehues used to test automake somewhat regularly on both IRIX and HP-UX, and to my knowledge never reported an error related to the contructs above. + +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. I suppose we can resuscitate make portability if anyone complains loudly enough. We may want to also ask on the automake list if this is still a real limitation, or whether automake has given up on worrying about this as well. See above. Regards, Stefano
Re: [PATCH 22/25] syntax-check: enable sc_bindtextdomain.
On Tuesday 15 November 2011, Gary V wrote: * cfg.mk (local-checks-to-skip): Remove sc_bindtextdomain list of disabled checks. (exclude_file_name_regexp--sc_program_name): Don't check demo s|set_program_name|bindtextdomain| here? programs for use of set_program_name. And here as well? Regards, Stefano
Re: [PATCH 22/25] syntax-check: enable sc_bindtextdomain.
On Tuesday 15 November 2011, Gary V wrote: * cfg.mk (local-checks-to-skip): Remove sc_bindtextdomain list of disabled checks. (exclude_file_name_regexp--sc_program_name): Don't check demo programs for use of set_program_name. Signed-off-by: Gary V. Vaughan g...@gnu.org --- cfg.mk |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/cfg.mk b/cfg.mk index f92039a..1d6ca3c 100644 --- a/cfg.mk +++ b/cfg.mk @@ -46,13 +46,11 @@ local-checks-to-fix = \ local-checks-to-skip = \ $(local-checks-to-fix) \ sc_GPL_version \ - sc_bindtextdomain \ sc_program_name \ sc_trailing_blank \ sc_unmarked_diagnostics # GPL_version: checks for GPLv3, which we don't use -# bindtextdomain: libtool isn't internationalized But then again, if libtool isn't internationalized, what is the point of enabling the `sc_bindtextdomain' check? Regards, Stefano
Re: [PATCH 2/3] maint: rename `libltdl/m4' directory to standard `m4'.
Hi Gary. On Wednesday 02 November 2011, Gary V wrote: package_revision=`$SHELL $ac_aux_dir/git-version-gen .tarball-version` diff --git a/libltdl/.gitignore b/libltdl/.gitignore index 2f39096..5795dbc 100644 --- a/libltdl/.gitignore +++ b/libltdl/.gitignore @@ -2,5 +2,4 @@ /Makefile.am /argz.h /build-aux +/m4 -/dummy.c -/gnulib.mk Shouldn't these last two edits be done in a separate patch? No, because I don't want to introduce broken revisions that cannot build into git history. I think there's a misunderstanding here. What I meant is: since (as far as I can see) dummy.c and gnulib.mk are not touched/moved by this patch, shouldn't any edit to `.gitignore' that involves them better be done in a separate patch? Or am I missing something? Also, shouldn't you report the changes to file `libltdl/.gitignore' in the ChangeLog entry? That's interesting actually. Historically, we have only reported changes to distributed files in ChangeLog, and have always omitted at least VCS control files from the log entries. I'm still leaning slightly towards not introducing ChangeLog noise to describe things that are only important when you have the full git checkout available, and hence access to git log and friends if you want to dig this sort of information out -- BUT I could easily be persuaded to change my mind if you have a good argument for the advantage of putting this stuff into the git log entry, which would then eventually be put into the generated ChangeLog file... I have no strong opinion on the matter (even if I personally prefer, when writing a ChangeLog entry, to mention any non-generated, version-controlled file that gets modified, whther distributed or not). I just wanted to make sure the lack of `.gitignore' mention in the ChangeLog wasn't the result of an overlloking. Regards, Stefano
Re: [PATCH 1/3] maint: rename `libltdl/config' directory to standard `build-aux'.
On Wednesday 02 November 2011, Gary V wrote: Hi Stefano, On 1 Nov 2011, at 21:57, Stefano Lattarini wrote: Hi Gary, hope you won't mind few nits from an outsider ... Absolutely not, any and all feedback is always extremely welcome. Thank you. On Tuesday 01 November 2011, Gary V wrote: # The timestamps on these files must be preserved carefully so we install, # uninstall and set executable with custom rules here. -auxfiles = $(pkgaux_scripts) config/ltmain.sh +auxdest = build-aux +auxfiles = $(pkgaux_scripts) $(auxdest)/ltmain.sh # Everything that gets picked up by aclocal is automatically distributed, # this is the list of macro files we install on the user's system. -aclocalfiles = m4/argz.m4 m4/libtool.m4 m4/ltdl.m4 m4/ltoptions.m4 \ -m4/ltsugar.m4 m4/ltversion.m4 m4/lt~obsolete.m4 +macrodest = m4 +aclocalfiles = $(macrodest)/argz.m4 \ +$(macrodest)/libtool.m4 \ +$(macrodest)/ltdl.m4 \ +$(macrodest)/ltoptions.m4 \ +$(macrodest)/ltsugar.m4 \ +$(macrodest)/ltversion.m4 \ +$(macrodest)/lt~obsolete.m4 Shouldn't this better be done in either a preparatory or follow-up patch? Ditto for other similar changes. I like to make sure the tree is capable of passing `make distcheck' after every patch, And this is a very good policy, with which I fully agree. What I meant was: wouldn't be better to introduce the $(auxdest), $(macrodest) and $(ltdldest) in a preparatory patch (with no semantic change), so that the edit to `Makefile.am' done in the present patch patch can be brought down to a minimal change like auxdest = config = auxdest = build-aux? and splitting this one into move directories around and fixup the aftermath would leave the tree broken in the middle... This is not what I proposing, not at all! What I was proposing is: 1. in a first, preparatory patch, factor out the names/locations of the directories in proper variables and/or makefile macros; 2. in the real patch, move the directories around, leveraging on the previous refactoring to ensure this moving can be done with smaller and more self-contained changes. Sorry if I failed to express myself correctly; I hope I've made my point clearer this time. Regards, Stefano
Re: [PATCH 2/3] maint: rename `libltdl/m4' directory to standard `m4'.
On Tuesday 01 November 2011, Gary V wrote: Similarly to 1/3, full compressed patch attached. 72 hours etc etc. * bootstrap.conf (libtool_link_libltdl_subdirs): Add `m4'. * cfg.mk (exclude_file_name_regexp--sc_prohibit_test_minus_ao): Adjust path. * configure.ac (AC_CONFIG_MACRO_DIR): Adjust. * libltdl/m4: Moved to it's parent directory. s/it's/its/. * tests/cdemo/Makefile.am, tests/demo/Makefile.am, tests/depdemo/Makefile.am, tests/f77demo/Makefile.am, tests/fcdemo/Makefile.am, tests/mdemo/Makefile.am, tests/mdemo2/Makefile.am, tests/pdemo/Makefile.am, tests/tagdemo/Makefile.am (ACLOCAL_AMFLAGS): Use new location of macro_dir. * libtoolize.m4sh: Ditto. diff --git a/bootstrap.conf b/bootstrap.conf index 2909d30..ebdb293 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -353,6 +353,7 @@ libtool_link_libltdl_subdirs () $debug_cmd $require_build_aux +$require_macro_dir $require_pkgaux_scripts my_pkgaux_files=$pkgaux_scripts $build_aux/ltmain.sh @@ -372,6 +373,11 @@ libtool_link_libltdl_subdirs () for my_file in $my_pkgaux_files; do ln -s ../../$my_file libltdl/$my_file done + +# Macro directory is identical, so link the directory. +func_verbose creating libltdl/$macro_dir +rm -f libltdl/$macro_dir || exit 1 +ln -s ../$macro_dir libltdl/$macro_dir } Same as before: is `ln -s' truly portable to e.g. MinGW? Should we care if the boostrapping process does not work there? package_revision=`$SHELL $ac_aux_dir/git-version-gen .tarball-version` diff --git a/libltdl/.gitignore b/libltdl/.gitignore index 2f39096..5795dbc 100644 --- a/libltdl/.gitignore +++ b/libltdl/.gitignore @@ -2,5 +2,4 @@ /Makefile.am /argz.h /build-aux +/m4 -/dummy.c -/gnulib.mk Shouldn't these last two edits be done in a separate patch? Also, shouldn't you report the changes to file `libltdl/.gitignore' in the ChangeLog entry? diff --git a/libltdl/m4/.gitignore b/libltdl/m4/.gitignore deleted file mode 100644 index 1facea7..000 --- a/libltdl/m4/.gitignore +++ /dev/null @@ -1,6 +0,0 @@ -/00gnulib.m4 -/gnulib-cache.m4 -/gnulib-common.m4 -/gnulib-comp.m4 -/gnulib-tool.m4 -/ltversion.m4 Ditto for the ChangeLog entry. Regards, Stefano
