Re: [PATCH 1/6] Add --gnulib-version and --news options to announce-gen.
On 1 Sep 2010, at 12:25, Ralf Wildenhues wrote: > Hello Gary, Hallo Ralf, > * Gary V. Vaughan wrote on Wed, Sep 01, 2010 at 03:59:19AM CEST: >> On 1 Sep 2010, at 00:41, Ralf Wildenhues wrote: >>> I personally find the M4SH_GETOPTS rather unreadable; it's a nice table, >>> but I cannot make sense of the entries, or review them, except by >>> looking at the generated code, at which point I'd rather just read the >>> generated code directly; after all, that's how we found several bugs in >>> the ltmain incarnation of this code. >> >> The documentation is in getopts.m4sh, just above the M4SH_GETOPTS >> definition. > > That didn't help to make it more readable, or cause less buggy code > though. Well that's because the shared code in getopts.m4sh is still getting exposure. I think that M4SH_GETOPTS is infinitely more readable and maintainable, and especially worth the effort of using and debugging if I am to contribute a variation to Autoconf's m4sugar libraries. Why maintain several copies of the same code if we can do it just once? >> Surely you're not suggesting that we continue to hand code, maintain, >> synchronize the option parsing loop in each of our scripts? > > Well, bootstrap didn't need one so far, did it? How much maintenance > does an option parsing loop need, once it is written? I didn't have the > feeling that that was a biggie on our list before that. Certainly not a biggie. But after using the M4SH_GETOPTS generated bootstrap script on my gnulib branch for less than a week, going back to the under-featured master branch version is already painful. >>> The --rcfile handling code mistreats quoting in the rcfile, and things >>> like multiple adjacent whitespace. >> >> Examples please. I haven't had any issues using this idiom in any of >> the --rcfile parsing in any of the shell scripts I've used this code >> with since I wrote it for cvs-utils (if memory serves). > > Put this in the rcfile, observe how the two spaces are flattened to one: > --message 'One sentence. Another sentence.' Oh yuck. Nice catch though. I've added a note to investigate that in my copy of announce-gen.m4sh incase I ever publish an --rcfile using shell script that pastes code from here in future. Cheers, -- Gary V. Vaughan (g...@gnu.org) PGP.sig Description: This is a digitally signed message part
Re: [PATCH 6/6] Simplify and improve safety of bootstrap process.
On 1 Sep 2010, at 12:32, Ralf Wildenhues wrote: > func_show_eval > is still buggy in that it causes more filename expansion than we'd like > to have (leading to commits like v2.2.10-48-g3ab9879), but yes, that > shouldn't be an issue here. Ah... I had wondered what that was all about. It makes sense now, thanks. Cheers, -- Gary V. Vaughan (g...@gnu.org) PGP.sig Description: This is a digitally signed message part
Re: [PATCH] Remove announce-gen.m4sh and mailnotify.m4sh.
* Gary V. Vaughan wrote on Wed, Sep 01, 2010 at 07:18:12AM CEST: > From earlier discussion, there is no need to clutter the > Libtool tree with these scripts, and we can amend our release > process to use the gnulib announce-gen script in due course. > > * libltdl/config/announce-gen.m4sh: Removed. This script has > no apparent connection to libtool functionality, and mostly > duplicates the better maintained gnulib announce-gen script. > * libltdl/config/mailnotify.sh: Removed. This script was used > by only clcommit.m4sh and announce-gen.m4sh, both of which are > now removed too. > * Makefile.maint (announce-gen): Target removed. > * bootstrap: Remove the mailnotify regeneration warning. > * HACKING (Release Procedure): Remove references to > announce-gen. > (Alpha release note template, Full release note template): > Reinstated from before announce-gen was introduced. > > Okay to push? Fine with me. Cheers, Ralf
Re: [PATCH v2] Remove clcommit.m4sh.
Hi Gary, * Gary V. Vaughan wrote on Wed, Sep 01, 2010 at 06:50:51AM CEST: > * clcommit.m4sh: Removed. This script was written to help keep > ChangeLog and commit messages in sync when committing to CVS, > and is an anachronism now that Libtool uses git. > * Makefile.maint (commit): Target removed. > * bootstrap: Don't generate commit. > * HACKING (Release Procedure): Adjusted. > > Okay to push? OK thanks! Ralf
Re: [PATCH 6/6] Simplify and improve safety of bootstrap process.
* Gary V. Vaughan wrote on Wed, Sep 01, 2010 at 04:42:52AM CEST: > On 1 Sep 2010, at 01:16, Ralf Wildenhues wrote: > > * Gary V. Vaughan wrote on Tue, Aug 31, 2010 at 08:43:20AM CEST: > >> -test -f clcommit.m4sh && $MAKE -f Makefile.maint ./commit \ > >> -srcdir=. top_srcdir=. PACKAGE="$package" VERSION="$version" \ > >> -M4SH="$AUTOM4TE -l m4sh" \ > >> -SED="$SED" GREP="$GREP" FGREP="$FGREP" EGREP="$EGREP" LN_S="$LN_S" > >> +func_show_eval "$MAKE bootstrap-deps \ > >> +M4SH='$AUTOM4TE --language=m4sh' PACKAGE='$package' \ > >> +PACKAGE_BUGREPORT='$package_bugreport' PACKAGE_NAME='$package_name' \ > >> +PACKAGE_URL='$package_url' SED='$SED' srcdir=. VERSION='$version'" > > > > For bootstrap, the func_show_eval from the rejected patch is probably > > gotten just as easily by just 'set -x'; besides the fact that using eval > > on this string is wrong. > > I was under the impression that the following were equivalent (with the > former removing the opportunity for two near copies of the same complex > invocation to get out of sync): > > func_show_eval "$MAKE bootstrap-deps M4SH='$AUTOM4TE --language=m4sh' " > > and: > > echo "$MAKE bootstrap-deps M4SH='$AUTOM4TE --language=m4sh' ..." > $MAKE bootstrap-deps M4SH='$AUTOM4TE --language=m4sh' ..." > > What am I missing? You are right on that one; sorry about that. Of course, func_show_eval is still buggy in that it causes more filename expansion than we'd like to have (leading to commits like v2.2.10-48-g3ab9879), but yes, that shouldn't be an issue here. Cheers, Ralf
Re: [PATCH 1/6] Add --gnulib-version and --news options to announce-gen.
Hello Gary, * Gary V. Vaughan wrote on Wed, Sep 01, 2010 at 03:59:19AM CEST: > On 1 Sep 2010, at 00:41, Ralf Wildenhues wrote: > > I personally find the M4SH_GETOPTS rather unreadable; it's a nice table, > > but I cannot make sense of the entries, or review them, except by > > looking at the generated code, at which point I'd rather just read the > > generated code directly; after all, that's how we found several bugs in > > the ltmain incarnation of this code. > > The documentation is in getopts.m4sh, just above the M4SH_GETOPTS > definition. That didn't help to make it more readable, or cause less buggy code though. > Surely you're not suggesting that we continue to hand code, maintain, > synchronize the option parsing loop in each of our scripts? Well, bootstrap didn't need one so far, did it? How much maintenance does an option parsing loop need, once it is written? I didn't have the feeling that that was a biggie on our list before that. > > The --rcfile handling code mistreats quoting in the rcfile, and things > > like multiple adjacent whitespace. > > Examples please. I haven't had any issues using this idiom in any of > the --rcfile parsing in any of the shell scripts I've used this code > with since I wrote it for cvs-utils (if memory serves). Put this in the rcfile, observe how the two spaces are flattened to one: --message 'One sentence. Another sentence.' Cheers, Ralf
[PATCH] Remove announce-gen.m4sh and mailnotify.m4sh.
>From earlier discussion, there is no need to clutter the
Libtool tree with these scripts, and we can amend our release
process to use the gnulib announce-gen script in due course.
* libltdl/config/announce-gen.m4sh: Removed. This script has
no apparent connection to libtool functionality, and mostly
duplicates the better maintained gnulib announce-gen script.
* libltdl/config/mailnotify.sh: Removed. This script was used
by only clcommit.m4sh and announce-gen.m4sh, both of which are
now removed too.
* Makefile.maint (announce-gen): Target removed.
* bootstrap: Remove the mailnotify regeneration warning.
* HACKING (Release Procedure): Remove references to
announce-gen.
(Alpha release note template, Full release note template):
Reinstated from before announce-gen was introduced.
Okay to push?
Signed-off-by: Gary V. Vaughan
---
ChangeLog| 14 ++
HACKING | 181 +--
Makefile.maint | 25 --
bootstrap| 10 -
libltdl/config/.cvsignore|1 -
libltdl/config/.gitignore|1 -
libltdl/config/announce-gen.m4sh | 464 --
libltdl/config/mailnotify.m4sh | 403 -
8 files changed, 171 insertions(+), 928 deletions(-)
delete mode 100644 libltdl/config/announce-gen.m4sh
delete mode 100644 libltdl/config/mailnotify.m4sh
diff --git a/ChangeLog b/ChangeLog
index d678169..9fcb522 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,19 @@
2010-09-01 Gary V. Vaughan
+ Remove announce-gen.m4sh and mailnotify.m4sh.
+ * libltdl/config/announce-gen.m4sh: Removed. This script has
+ no apparent connection to libtool functionality, and mostly
+ duplicates the better maintained gnulib announce-gen script.
+ * libltdl/config/mailnotify.sh: Removed. This script was used
+ by only clcommit.m4sh and announce-gen.m4sh, both of which are
+ now removed too.
+ * Makefile.maint (announce-gen): Target removed.
+ * bootstrap: Remove the mailnotify regeneration warning.
+ * HACKING (Release Procedure): Remove references to
+ announce-gen.
+ (Alpha release note template, Full release note template):
+ Reinstated from before announce-gen was introduced.
+
Remove clcommit.m4sh.
* clcommit.m4sh: Removed. This script was written to help keep
ChangeLog and commit messages in sync when committing to CVS,
diff --git a/HACKING b/HACKING
index e079e4b..b462c98 100644
--- a/HACKING
+++ b/HACKING
@@ -687,38 +687,171 @@ or obtained by writing to the Free Software Foundation,
Inc.,
doc/manual using cvs to here:
@cvs.savannah.gnu.org:/webcvs/libtool
-* In the build directory, run `Make -f[../]Makefile.maint announce-gen'.
+* Write a release announcement, and post it to `libt...@gnu.org' and
+ `autotools-annou...@gnu.org' with the Reply-To header set to
+ `bug-libt...@gnu.org'. Stable releases should also be announced
+ on `info-...@gnu.org'.
-* Make sure the source directory has an appropriate .announcerc, along
- the lines of:
+* Post a copy of your release announcement to savannah news:
+ https://savannah.gnu.org/news/submit.php?group=libtool
+ which will automatically propogate to http://planet.gnu.org.
---bootstrap-tools='autoconf,automake'
---gpg-key-id={the id of the key you signed the tarballs with}
---header 'From: {your name} <{your email address}>
---header 'To: Libtool List '
---header 'Cc: Autotools Announce List '
---header 'Reply-To: Libtool Bugs '
---message 'GNU Libtool hides the complexity of using shared'
---message 'libraries behind a consistent, portable interface.'
---message 'GNU Libtool ships with GNU libltdl, which hides the'
---message 'complexity of loading dynamic runtime libraries'
---message '(modules) behind a consistent, portable interface.'
---signature
-* Check the release announcement with:
+13. Alpha release note template
+===
-./announce-gen -r -o --prev=LASTRELEASE alpha
+To: libt...@gnu.org, autotools-annou...@gnu.org
+Reply-To: bug-libt...@gnu.org
+Subject: GNU Libtool @VERSION@ released (alpha release).
- or for a stable release:
+The Libtool Team is pleased to announce alpha release @VERSION@ of GNU
+Libtool.
-./announce-gen -r -o -h 'Cc: info-...@gnu.org' --prev=LASTRELEASE stable
+GNU Libtool hides the complexity of using shared libraries behind a
+consistent, portable interface. GNU Libtool ships with GNU libltdl,
+which hides the complexity of loading dynamic runtime libraries
+(modules) behind a consistent, portable interface.
- When you're happy with the announcement text, post it by rerunning the
- command with `-p' instead of `-o'.
+Here are the compressed sources:
-* Post a copy of the release announcement to savannah news:
- https://savannah.gnu.org/news/submit.php?group=libtool
- which
[PATCH v2] Remove clcommit.m4sh.
Total Annihilation: Removed insidious references from the rest of the tree. * clcommit.m4sh: Removed. This script was written to help keep ChangeLog and commit messages in sync when committing to CVS, and is an anachronism now that Libtool uses git. * Makefile.maint (commit): Target removed. * bootstrap: Don't generate commit. * HACKING (Release Procedure): Adjusted. Okay to push? --- ChangeLog | 10 ++ HACKING|6 +- Makefile.maint | 17 +--- bootstrap |7 +- clcommit.m4sh | 375 5 files changed, 16 insertions(+), 399 deletions(-) delete mode 100644 clcommit.m4sh diff --git a/ChangeLog b/ChangeLog index a4c91cb..d678169 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2010-09-01 Gary V. Vaughan + + Remove clcommit.m4sh. + * clcommit.m4sh: Removed. This script was written to help keep + ChangeLog and commit messages in sync when committing to CVS, + and is an anachronism now that Libtool uses git. + * Makefile.maint (commit): Target removed. + * bootstrap: Don't generate commit. + * HACKING (Release Procedure): Adjusted. + 2010-08-31 Gary V. Vaughan Remove double `Generated from foo.m4sh' lines. diff --git a/HACKING b/HACKING index 19b4b66..e079e4b 100644 --- a/HACKING +++ b/HACKING @@ -647,7 +647,7 @@ or obtained by writing to the Free Software Foundation, Inc., and `make distcheck CC=g++' If there are any problems, fix them and start again. -* Run `./commit -p' from the source tree. +* Run `git commit' from the source tree. * Run `make -fMakefile.maint git-dist' (or `make -f../Makefile.maint git-dist' if you are running from a VPATH build directory, where `../' @@ -676,7 +676,9 @@ or obtained by writing to the Free Software Foundation, Inc., * Update NEWS, ChangeLog. -* Run `./commit -p --tags', to push the new changes and tags to origin. +* Run `git commit'. + +* Run `git push --tags', to push the new changes and tags to origin. * Update the webpages, libtool.html will need to indicate the latest release number. diff --git a/Makefile.maint b/Makefile.maint index 2e655ae..15d5a14 100644 --- a/Makefile.maint +++ b/Makefile.maint @@ -36,17 +36,6 @@ Makefile: TEXI2HTML = texi2html -$(srcdir)/commit: $(srcdir)/$(auxdir)/mailnotify clcommit.m4sh - $(timestamp); \ - cd $(srcdir); \ - rm -f commit commit.in commit.tmp; \ - $(M4SH) -B $(auxdir) clcommit.m4sh > commit.in; \ - $(edit) -e "s,@TIMESTAMP\@,$$TIMESTAMP,g" commit.in > commit.tmp; \ - chmod a+x commit.tmp; \ - chmod a-w commit.tmp; \ - mv -f commit.tmp commit; \ - rm -f commit.in - announce-gen: $(auxdir)/announce-gen.m4sh $(auxdir)/getopt.m4sh $(timestamp); \ rm -f announce-gen announce-gen.in announce-gen.tmp; \ @@ -73,7 +62,7 @@ $(srcdir)/$(auxdir)/mailnotify: $(auxdir)/mailnotify.m4sh rm -f mailnotify.in .PHONY: git-release -git-release: version-check prev-tarball check-news fetch git-commit git-dist diffs web-manual +git-release: version-check prev-tarball check-news fetch git-dist diffs web-manual @tarname="$(PACKAGE)-$(VERSION).tar.gz"; \ lzmaname="$(PACKAGE)-$(VERSION).tar.lzma"; \ diffname="$(PACKAGE)-$(LASTRELEASE)-$(VERSION).diff.gz"; \ @@ -160,10 +149,6 @@ fetch: GPG = gpg # set it to `:' to avoid gpg operations -.PHONY: git-commit -git-commit: check-news - cd $(srcdir) && $(SHELL) ./commit - .PHONY: git-dist git-dist: check-news check-commit ## Build the distribution: diff --git a/bootstrap b/bootstrap index 7ad0979..82828db 100755 --- a/bootstrap +++ b/bootstrap @@ -140,11 +140,6 @@ $MAKE ./$auxdir/ltmain.sh ./$m4dir/ltversion.m4 \ AUTOTEST="$AUTOM4TE --language=autotest" SED="$SED" MAKEINFO="$MAKEINFO" \ GREP="$GREP" FGREP="$FGREP" EGREP="$EGREP" LN_S="$LN_S" -test -f clcommit.m4sh && $MAKE -f Makefile.maint ./commit \ -srcdir=. top_srcdir=. PACKAGE="$PACKAGE" VERSION="$VERSION" \ -M4SH="$AUTOM4TE -l m4sh" \ -SED="$SED" GREP="$GREP" FGREP="$FGREP" EGREP="$EGREP" LN_S="$LN_S" - rm -f Makefile # Make a dummy libtoolize script for autoreconf: @@ -193,7 +188,7 @@ done # Commit script caveat: cat < -# and Alexandre Oliva - -# Copyright (C) 1999, 2000, 2004, 2006, 2008, 2009, 2010 Free Software -# Foundation, Inc. -# This is free software; see the source for copying conditions. There is NO -# warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. - -# Clcommit is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 2 of the License, or -# (at your option) any later version. -# -# Clcommit is distributed in the hope that it will be useful, but -# WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
Re: [PATCH] Remove clcommit.m4sh.
Hi Chuck, On 1 Sep 2010, at 10:50, Charles Wilson wrote: > On 8/31/2010 10:53 PM, Gary V. Vaughan wrote: >> Does anyone else use the commit script? >> >> * clcommit.m4sh: Removed. This script was written to help keep >> ChangeLog and commit messages in sync when committing to CVS, >> and is an anachronism now that Libtool uses git. >> >> Okay to push? >> >> --- >> ChangeLog |7 + >> clcommit.m4sh | 375 >> - >> 2 files changed, 7 insertions(+), 375 deletions(-) >> delete mode 100644 clcommit.m4sh > > Don't you also need to patch Makefile.maint to remove references to this > file? (There's a comment referring to it in mailnotify.m4sh too, but > that's not important). Yes, I was a bit too keen to remove it. Thanks. I've also removed the command from bootstrap that generates the commit script, and I'm in the process of checking that the amended `./commit' free release process still works, and editing HACKING to match. The release process takes a few hours though, so it'll be a short while before I post an updated patch. Cheers, -- Gary V. Vaughan (g...@gnu.org) PGP.sig Description: This is a digitally signed message part
Re: [PATCH] Remove clcommit.m4sh.
On 8/31/2010 10:53 PM, Gary V. Vaughan wrote: > Does anyone else use the commit script? > > * clcommit.m4sh: Removed. This script was written to help keep > ChangeLog and commit messages in sync when committing to CVS, > and is an anachronism now that Libtool uses git. > > Okay to push? > > --- > ChangeLog |7 + > clcommit.m4sh | 375 > - > 2 files changed, 7 insertions(+), 375 deletions(-) > delete mode 100644 clcommit.m4sh Don't you also need to patch Makefile.maint to remove references to this file? (There's a comment referring to it in mailnotify.m4sh too, but that's not important). -- Chuck
[PATCH] Remove clcommit.m4sh.
Does anyone else use the commit script?
* clcommit.m4sh: Removed. This script was written to help keep
ChangeLog and commit messages in sync when committing to CVS,
and is an anachronism now that Libtool uses git.
Okay to push?
---
ChangeLog |7 +
clcommit.m4sh | 375 -
2 files changed, 7 insertions(+), 375 deletions(-)
delete mode 100644 clcommit.m4sh
diff --git a/ChangeLog b/ChangeLog
index a4c91cb..aba79b5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2010-09-01 Gary V. Vaughan
+
+ Remove clcommit.m4sh.
+ * clcommit.m4sh: Removed. This script was written to help keep
+ ChangeLog and commit messages in sync when committing to CVS,
+ and is an anachronism now that Libtool uses git.
+
2010-08-31 Gary V. Vaughan
Remove double `Generated from foo.m4sh' lines.
diff --git a/clcommit.m4sh b/clcommit.m4sh
deleted file mode 100644
index c189cf3..000
--- a/clcommit.m4sh
+++ /dev/null
@@ -1,375 +0,0 @@
-AS_INIT[]m4_divert_push([HEADER-COPYRIGHT])
-# clcommit (GNU @PACKAGE@) version 2.1
-# Written by Gary V. Vaughan
-# and Alexandre Oliva
-
-# Copyright (C) 1999, 2000, 2004, 2006, 2008, 2009, 2010 Free Software
-# Foundation, Inc.
-# This is free software; see the source for copying conditions. There is NO
-# warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
-
-# Clcommit is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 2 of the License, or
-# (at your option) any later version.
-#
-# Clcommit is distributed in the hope that it will be useful, but
-# WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with clcommit; see the file COPYING. If not, a copy
-# can be downloaded from http://www.gnu.org/licenses/gpl.html,
-# or obtained by writing to the Free Software Foundation, Inc.,
-# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
-
-# Usage: $progname [OPTION]... [--] [filepattern ...]
-
-# -a AUTHOR --author=AUTHORoverride changeset author name with AUTHOR
-# -C FILE --changelog=FILE extract commit message from specified FILE
-# --debugenable verbose shell tracing
-# -n--dry-run don't commit anything
-# --fast same as --force --first
-# -F FILE --file=FILEread commit message from FILE
-# -1--firstextract first entry from ChangeLog, no git diff
-# -f--forcedon't check (unless *followed* by -n), and just
-# display commit message instead of running $PAGER
-# --from=ADDRESS override default from ADDRESS in commit email
-# -m STRING --message=STRING set commit message to STRING
-# --msg=STRING same as -m
-# -p--push push the changes back to origin
-# -r [FILE] --rcfile[=FILE]read default option from FILE [./.clcommitrc]
-# -s ADDR --sendmail=ADDRsend commit email of the differences to ADDRESS
-# --signature[=FILE] add FILE to the end of the email [~/.signature]
-# --signoff add a Signed-off-by attribution at the end
-# -S TEXT --summary=TEXT specify a TEXT subject line for the commit email
-# --tags in conjunction with -p, also push tags
-# -v--verbose run in verbose mode
-# --version print version information
-# -h,-? --help print short or long help message
-
-# This script eases checking in changes to git-maintained projects
-# with ChangeLog files. It will check that there have been no
-# conflicting commits in the git repository and print which files it
-# is going to commit to stderr. A list of files to compare and to
-# check in can be given in the command line. If it is not given, all
-# files in the current working directory are considered for check in.
-
-# The commit message will be extracted from the differences between a
-# file named ChangeLog* in the commit list, or named after -C, and the
-# one in the repository (unless a message was specified with `-m' or
-# `-F'). An empty message is not accepted (but a blank line is). If
-# the message is acceptable, it will be presented for verification
-# (and possible edition) using the $PAGER environment variable (or
-# `more', if it is not set, or `cat', if the `-f' switch is given).
-# If $PAGER exits successfully, the modified files (at that moment)
-# are checked in, unless `-n' was specified, in which case nothing is
-# checked in.
-
-# Report bugs to
-
-: ${GIT="git"}
-: ${MAILNOTIFY="mailnotify"}
-: ${MKSTAMP="mkstamp"}
-
-test -f "libltdl/config/
Re: [PATCH 6/6] Simplify and improve safety of bootstrap process.
On 1 Sep 2010, at 01:16, Ralf Wildenhues wrote: > This is a fairly nice cleanup that I think is valuable to redo without > the other rejected patches. Please resubmit with nit below addressed, > and I will approve it then after looking over once more. Well, since it has zero impact on user-facing code, and because I was far too optimistic already about cherry-picking and reordering patches from another branch with significant reorganization without redoing the time consuming testing from scratch... I'll wait until after the release and resubmit then along with the rest of that series where it has all been tested properly. > * Gary V. Vaughan wrote on Tue, Aug 31, 2010 at 08:43:20AM CEST: >> --- a/Makefile.am >> +++ b/Makefile.am > >> +## Document the make macros that are needed to build bootstrap-deps >> +## dependencies when called from `bootstrap' (developer's machine), >> +## where the Makefile.am is fed to make in its raw format before >> +## `configure' has found the correct values (on the build machine). >> +bootstrap_files = \ >> +$(auxdir)/ltmain.sh \ >> +$(m4dir)/ltversion.m4 \ >> +libltdl/Makefile.am >> + >> +.PHONY: bootstrap-deps bootstrap-deps-prep >> +bootstrap-deps: bootstrap-deps-prep $(bootstrap_files) >> +bootstrap-deps-prep: >> +## The following variables are substituted by `bootstrap-dep-preps' >> +@test -n "$(srcdir)" \ >> + || echo "ERROR: don't call $(MAKE) with srcdir unset." >> +@test -n "$(M4SH)" \ >> +|| echo "ERROR: don't call $(MAKE) with M4SH unset." >> +@test -n "$(PACKAGE)" \ >> +|| echo "ERROR: don't call $(MAKE) with PACKAGE unset." >> +@test -n "$(PACKAGE_BUGREPORT)" \ >> +|| echo "ERROR: don't call $(MAKE) with PACKAGE_BUGREPORT unset." >> +@test -n "$(PACKAGE_NAME)" \ >> +|| echo "ERROR: don't call $(MAKE) with PACKAGE_NAME unset." >> +@test -n "$(PACKAGE_URL)" \ >> +|| echo "ERROR: don't call $(MAKE) with PACKAGE_URL unset." >> +@test -n "$(SED)" \ >> +|| echo "ERROR: don't call $(MAKE) with SED unset." >> +@test -n "$(VERSION)" \ >> +|| echo "ERROR: don't call $(MAKE) with VERSION unset." >> +rm -f $(bootstrap_files) > > It would be nice if this rule would actually error out (i.e., have > nonzero exit status) if any of the variables were unset. Agreed. >> -test -f clcommit.m4sh && $MAKE -f Makefile.maint ./commit \ >> -srcdir=. top_srcdir=. PACKAGE="$package" VERSION="$version" \ >> -M4SH="$AUTOM4TE -l m4sh" \ >> -SED="$SED" GREP="$GREP" FGREP="$FGREP" EGREP="$EGREP" LN_S="$LN_S" >> +func_show_eval "$MAKE bootstrap-deps \ >> +M4SH='$AUTOM4TE --language=m4sh' PACKAGE='$package' \ >> +PACKAGE_BUGREPORT='$package_bugreport' PACKAGE_NAME='$package_name' \ >> +PACKAGE_URL='$package_url' SED='$SED' srcdir=. VERSION='$version'" > > For bootstrap, the func_show_eval from the rejected patch is probably > gotten just as easily by just 'set -x'; besides the fact that using eval > on this string is wrong. I was under the impression that the following were equivalent (with the former removing the opportunity for two near copies of the same complex invocation to get out of sync): func_show_eval "$MAKE bootstrap-deps M4SH='$AUTOM4TE --language=m4sh' " and: echo "$MAKE bootstrap-deps M4SH='$AUTOM4TE --language=m4sh' ..." $MAKE bootstrap-deps M4SH='$AUTOM4TE --language=m4sh' ..." What am I missing? Cheers, -- Gary V. Vaughan (g...@gnu.org) PGP.sig Description: This is a digitally signed message part
Re: [PATCH 5/6] Don't leak developer GREP, SED etc into distribution file.
On 1 Sep 2010, at 01:12, Ralf Wildenhues wrote: > * Gary V. Vaughan wrote on Tue, Aug 31, 2010 at 08:43:19AM CEST: >> From: Gary V. Vaughan >> >> * bootstrap (rebuild): Set the shell variable `revision' rather >> than `correctver' for clarity. >> (edit): Split into two parts... >> (bootstrap_edit): ...substitutions that should happen at bootstrap >> time... >> (configure_edit): ...and substitution that should not happen until >> configure time. >> * bootstrap: Move the bootstrap related section up towards the top >> of the file. >> (libltdl/m4/ltversion.m4): Extract `$macro_revision' from this >> file for direct comparison with `$revision', rather than munging >> the file's serial number. Use `bootstrap_edit' for substitutions >> when generating from `ltversion.in'. >> (libltdl/config/ltmain.sh): Similarly, extract `$package_revision' >> for comparison with rebuild's `$revision' setting, and make >> bootstrap time substitutions with `bootstrap_edit'. >> (libtool): Likewise. >> (libtoolize): Use `configure_edit' for substitutions at configure >> time. >> (tests/package.m4, tests/defs): Likewise. >> * HACKING (Release Procedure): Remove the note to workaround the >> bug fixed by this changeset. >> * NEWS (Bug fixes): Mention that this bug is now fixed. > > Please credit Joerg Sonnenberger for finding the bug. Thanks. Will do. > I usually do VPATH builds exclusively, except when trying out patches > where I think in-tree builds could be broken. In my VPATH tree, when I > make -f ../libtool/Makefile.main announce-gen SHELL='/bin/sh -x' > > that contains the following output: > > + -e 's,@TIMESTAMP\@, 1.3257 2010-08-30,g' -e 's,@LASTRELEASE\@,2.2.11,g' -e > 's,@top_srcdir\@,../libtool,g' announce-gen.in > /bin/sh: line 4: -e: command not found As I mentioned already, it seems that I was overly optimistic about being able to cherry pick the non-gnulib related parts of my use-gnulib topic brach onto master and expect them to work :( The error above comes from Makefile.maint, which doesn't exist in use-gnulib, superceded by maint.mk/cfg.mk. Sorry for the bad merge. > Please, once and for all, whenever you rename some identifier, > use some method to verify that you've renamed all instances; > git grep '\' > > is quite a good approximation to the perfect method, and costs maybe a > couple of seconds on your part. Then submit that as separate patch. As I did when writing, testing and committing within the series I developed in. > When you do pure code move-arounds, that also is much easier to review > when done as a separate patch, without any actual changes to the text > that was moved. I remember that we took more than dozen iterations on > the makefile rules to get them halfway correct, I have no intentions of > going there again. Okay. > I have stopped reviewing at this point. I would gladly re-review a > minimal patch based on minimal prerequisites that has been properly > tested. The Makefile.am changes should be tested with both GNU and > non-GNU make, and both VPATH and in-tree. As I did in the context of the use-gnulib branch. On balance, there is an inordinate amount of time and effort involved from each of us to fully test and review these patches out of context (i.e. on master) when half the work has already been done once on another branch (i.e use-gnulib). Let's just forget this patch series for now. Thanks for the review, and sorry for not making the time to retest everything all over again as I cherry-picked. I'll take the pertinent parts into consideration before resubmitting the use-gnulib branch series in its entirety after the release. Cheers, -- Gary V. Vaughan (g...@gnu.org) PGP.sig Description: This is a digitally signed message part
Re: [PATCH 4/6] Rewrite bootstrap script for consistency with our other shell code.
On 1 Sep 2010, at 01:03, Ralf Wildenhues wrote: > * Gary V. Vaughan wrote on Tue, Aug 31, 2010 at 08:43:18AM CEST: >> * bootstrap: Add a proper option parsing loop, along with copies >> of supporting functions from `libltd/m4/getopt.m4sh' and >> `libltdl/m4/general.m4sh'. >> Reformat header comments to work with `func_help' and >> `func_version'. >> (my_sed_traces): Expanded to extract all parameters from >> configure.ac, without additional shell munging. > > This patch does not add new functionality to Libtool, nor does it fix > existing bugs. It is unknown whether this patch introduces regressions, > but experience shows that a patch of this size has a nonzero chance of > doing so. On these grounds, I reject the patch. > > Specifically, I think that bootstrap needs to be only about a screenful > long, excluding comments. We should be making it shorter, not longer. I posted a v2 patch which hasn't made it to the archives yet which address all of these by generating from getopts.m4sh like our other scripts. Cheers, -- Gary V. Vaughan (g...@gnu.org) PGP.sig Description: This is a digitally signed message part
Re: [PATCH 3/6] Support missing detached signatures, like gnulib implementation.
On 1 Sep 2010, at 00:49, Ralf Wildenhues wrote: > * Gary V. Vaughan wrote on Tue, Aug 31, 2010 at 08:43:17AM CEST: >> * libltdl/config/announce-gen.m4sh (func_print_locations): Don't >> bail out or print garbage when trying to find the size of a >> non-existent file. > > OK thanks. While you're at it, TAB before space is a little less likely > to be garbage-collected by editors. Gah! That is a habit I really need to break :( Anyway, I'll add an sc_avoid_space_tab check to save me from myself on the use-gnulib branch for submission after the release. > The summary lines of the commit entry and the ChangeLog entry of this > patch do not match the detailed explanation nor the actual patch though. I guess. The intent of the patch is to cope with the gnulib release rules, which use gnupload to generate detached upload signatures on demand so that they are not available to announce-gen when it tries to display their size with func_print_locations(). I'll amend and resubmit after the release. Cheers, -- Gary V. Vaughan (g...@gnu.org) PGP.sig Description: This is a digitally signed message part
Re: [PATCH 2/6] Fix partial commit support.
Hallo Ralf, On 1 Sep 2010, at 00:46, Ralf Wildenhues wrote: > * Gary V. Vaughan wrote on Tue, Aug 31, 2010 at 08:43:16AM CEST: >> * clcommit.m4sh (func_commit): Commit only staged files instead >> of passing `-a' when no file list was given on the command line. > > FWIW, I've never used clcommit since we moved to git; I just use > plain git. Actually, I was using it as a crutch not to get to grips with git, which is indeed very powerful... but also complex and difficult to understand fully. Now that I've had the opportunity to use git properly, I concur that our commit script is obsolete... especially if we eventually move towards generating our ChangeLog from git commit messages, sidestepping the main point of our commit script entirely (to keep ChangeLog and git log messages in sync). When I next have time after the release, I'll submit a patch to remove it. > Come to think, why do we have this script in the Libtool tree at all? For historical reasons. > There is no apparent connection to the Libtool functionality. Maybe > this, as well as announce-gen, could live in their own project? announce-gen is referenced by cfk.mk (nee Makefile.maint) in the release rules, which makes it useful for automating the release process. Maybe it makes sense to remove announce-gen and the rules that reference it none-the-less, and free the person making each release to type up their release announcement as they see fit. Cheers, -- Gary V. Vaughan (g...@gnu.org) PGP.sig Description: This is a digitally signed message part
Re: [PATCH 1/6] Add --gnulib-version and --news options to announce-gen.
On 1 Sep 2010, at 00:41, Ralf Wildenhues wrote: > this is a review, not an approval. No problem; thanks for the review. [[Review comments I agree with elided...]] > * Gary V. Vaughan wrote on Tue, Aug 31, 2010 at 08:43:15AM CEST: >> * libltdl/config/announce-gen.m4sh: Add support for gnulib >> announce-gen options, previously missing from our m4sh >> implementation, and enforce specifying --gnulib-version when >> `gnulib' is listed in --bootstrap-tools. > > I had avoided looking much at announce-gen so far; I've taken another > look now. I found a few issues: > > Using --output to direct output to stdout seems unusual and inconsistent > with both the GNU Coding Standards recommendations (nodes 'Command-Line > Interfaces' and 'Option Table') and other tools like git. Good point. The intention is to have --output vs --post. I'll look at this again after the release. > I personally find the M4SH_GETOPTS rather unreadable; it's a nice table, > but I cannot make sense of the entries, or review them, except by > looking at the generated code, at which point I'd rather just read the > generated code directly; after all, that's how we found several bugs in > the ltmain incarnation of this code. The documentation is in getopts.m4sh, just above the M4SH_GETOPTS definition. Surely you're not suggesting that we continue to hand code, maintain, synchronize the option parsing loop in each of our scripts? > The --rcfile handling code mistreats quoting in the rcfile, and things > like multiple adjacent whitespace. Examples please. I haven't had any issues using this idiom in any of the --rcfile parsing in any of the shell scripts I've used this code with since I wrote it for cvs-utils (if memory serves). > The help text claims that ./.announcerc is the default place for the > rcfile, but the code looks like $top_srcdir/.announcerc is being used. > > The announce-gen script claims: > # This is the .announcerc for GNU Libtool. Announce-gen is pretty > # smart about picking defaults for package-name and current-version > # and often guesses previous-version correctly too. > > and yet the expanded script contains code like: > top_srcdir=`cd "../libtool" && pwd` > test -d "$top_srcdir" || top_srcdir='../libtool' > > that, when used for another package, will luckily cause stderr noise > that will alert me that just maybe things won't go as smooth as > advertised. Or did you really mean for the script to be generated > anew as part of each package? Well, I mostly intended for it to generate Libtool release announcements for me when I roll up a release. But also to be somewhat similar to the gnulib version, with an eye towards contributing it later. I originally tried to fix the gnulib version to work with libtool's release process, but frankly I'd rather stick forks in my eyes than develop in Perl. Cheers, -- Gary V. Vaughan (g...@gnu.org) PGP.sig Description: This is a digitally signed message part
Re: Backport non-gnulib dependent parts of my use-gnulib topic branch
Hallo Ralf, On 1 Sep 2010, at 00:38, Ralf Wildenhues wrote: > * Gary V. Vaughan wrote on Tue, Aug 31, 2010 at 08:43:14AM CEST: >> Okay to push? Please ignore this patch series. Cherry picking back to master from a long divergent branch that has fairly intrusive reorganisation to move Libtool to use gnulib doesn't work as well as I'd hoped. I'll repost the entire series for consideration after the release. > Please state if and how you tested your patches, each by itself or only > all in conjunction? On the original branch, each one had to pass `make distcheck' before commit. > On what system(s)? For this series, on my development machine only, since the changees are not machine specific. > Did you use GNU tools in $PATH > or maybe ensure they are not found while testing? No. > Anything unusual in your environment setup? No. > Do you try different versions of relevant tools (autoconf, git)? git: no. Autoconf, Automake: minimum supported versions and latest stable versions. > Whenever making things more complex rather than > simpler, do you have a rationale for doing so (besides "that looked like > a cool thing to try out")? For this series, moving towards gnulib is the majority of the rationale, and standardization of our use of scripts is the rest. I assume you are asking why I rewrote the bootstrap script here - that was because the existing script is out on a limb by itself, and I thought it sensible to reuse the format and code from our other scripts. > Why do you post FYI commit emails without patches in them, and without > non-patch content over what gets auto-posted to the libtool-commit list? Sorry, ./commit and ./libltdl/config/mailnotify run amok. I haven't really updated the latter to work properly with git. Cheers, -- Gary V. Vaughan (g...@gnu.org) PGP.sig Description: This is a digitally signed message part
Re: Extract the archive name from the .la file and use $AR (not ar).
Den 2010-08-31 19:30 skrev Ralf Wildenhues: > * Peter Rosin wrote on Tue, Aug 31, 2010 at 07:21:34PM CEST: >> Den 2010-08-31 19:11 skrev Ralf Wildenhues: >>> libtool --mode=link $CC ... -o libfoo.a baz.o libbar.a >>> >>> should be adding baz.o and all objects in libbar.a to libfoo.a, i.e., it >>> should extract all objects from libbar.a Instead, libfoo.a is added *as >>> single member* into libfoo.a. That's the bug. >> >> (assuming typo and that you meant that libbar.a is added *as single member*) > > Yes, sorry. > >> In that case, MS lib does probably not see baz.o when it creates libfoo.a, >> and the test could be made stricter by checking that baz.o is also part of >> libfoo.a, since MS lib does the part that is currently tested for >> (extracting libbar.a and putting the contents in libfoo.a). > > Cool. Yes, please extend the test. If MS lib still passes, you can > adjust the xfail line, something like > > # This test passes with MS lib. > AT_XFAIL_IF([case $AR in ar-lib\ *) false;; *) :;; esac]) It appears that the *as single member* part isn't true. So, after tightening the test up, it still passes with MS lib. Pushing as attached... Cheers, Peter >From 26dacf3464210910602ce0f679f9770241d7a16e Mon Sep 17 00:00:00 2001 From: Peter Rosin Date: Tue, 31 Aug 2010 20:59:11 +0200 Subject: [PATCH] archive-in-archive.at passes with Microsoft lib. * tests/archive-in-archive.at: Tighten test to check that the desired object files are indeed part of the archive. Microsoft lib still passes, so remove the expected failure for that case. Signed-off-by: Peter Rosin --- ChangeLog |5 + tests/archive-in-archive.at |5 - 2 files changed, 9 insertions(+), 1 deletions(-) diff --git a/ChangeLog b/ChangeLog index 355b485..585356c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,10 @@ 2010-08-31 Peter Rosin + archive-in-archive.at passes with Microsoft lib. + * tests/archive-in-archive.at: Tighten test to check that the desired + object files are indeed part of the archive. Microsoft lib still + passes, so remove the expected failure for that case. + Dump archiver output to the log when testing @file support. * libltdl/m4/libtool.m4 (_LT_PROG_AR): Redirect stdout to config.log when testing for @file support. diff --git a/tests/archive-in-archive.at b/tests/archive-in-archive.at index 32e3543..fd67c7d 100644 --- a/tests/archive-in-archive.at +++ b/tests/archive-in-archive.at @@ -26,7 +26,8 @@ AT_SETUP([static library contains static library]) AT_KEYWORDS([libtool]) -AT_XFAIL_IF([:]) dnl This is currently broken +# This test passes with MS lib. +AT_XFAIL_IF([case $AR in ar-lib\ * | *[[/\\]]ar-lib\ *) false;; *) :;; esac]) AT_DATA([foo.c], [ @@ -53,4 +54,6 @@ AT_CHECK([$LIBTOOL --mode=install cp libbar.la $thisdir], [], [ignore], [ignore] eval `$EGREP '^(old_library)=' < libbar.la` libbar=$old_library AT_CHECK([$AR -t $libbar | grep $libfoo],[1],[ignore],[ignore]) +AT_CHECK([$AR -t $libbar | grep foo.$OBJEXT],[],[ignore],[ignore]) +AT_CHECK([$AR -t $libbar | grep bar.$OBJEXT],[],[ignore],[ignore]) AT_CLEANUP -- 1.7.1
Re: [PATCH 6/6] Simplify and improve safety of bootstrap process.
* Gary V. Vaughan wrote on Tue, Aug 31, 2010 at 08:43:20AM CEST: > * Makefile.am (bootstrap_files): List files that need to be > generated at bootstrap time before `./configure && make' can > work. It turns out that this is considerably fewer files than we > had thought necessary previously. > (bootstrap-deps-prep): Ensure minimum set of required substitution > variables are non-empty. > (bootstrap-deps): Depend on `bootstrap' files. > * bootstrap (Generate bootstrap dependencies): Now that > `Makefile.am' is entirely responsible for rebuilding files at > bootstrap time, we need only specify the new `bootstrap-deps' > target, and supply values for the substitutions checked by > `bootstrap-deps-prep'. This is a fairly nice cleanup that I think is valuable to redo without the other rejected patches. Please resubmit with nit below addressed, and I will approve it then after looking over once more. Thanks, Ralf > --- a/Makefile.am > +++ b/Makefile.am > +## Document the make macros that are needed to build bootstrap-deps > +## dependencies when called from `bootstrap' (developer's machine), > +## where the Makefile.am is fed to make in its raw format before > +## `configure' has found the correct values (on the build machine). > +bootstrap_files = \ > +$(auxdir)/ltmain.sh \ > +$(m4dir)/ltversion.m4 \ > +libltdl/Makefile.am > + > +.PHONY: bootstrap-deps bootstrap-deps-prep > +bootstrap-deps: bootstrap-deps-prep $(bootstrap_files) > +bootstrap-deps-prep: > +## The following variables are substituted by `bootstrap-dep-preps' > + @test -n "$(srcdir)" \ > +|| echo "ERROR: don't call $(MAKE) with srcdir unset." > + @test -n "$(M4SH)" \ > + || echo "ERROR: don't call $(MAKE) with M4SH unset." > + @test -n "$(PACKAGE)" \ > + || echo "ERROR: don't call $(MAKE) with PACKAGE unset." > + @test -n "$(PACKAGE_BUGREPORT)" \ > + || echo "ERROR: don't call $(MAKE) with PACKAGE_BUGREPORT unset." > + @test -n "$(PACKAGE_NAME)" \ > + || echo "ERROR: don't call $(MAKE) with PACKAGE_NAME unset." > + @test -n "$(PACKAGE_URL)" \ > + || echo "ERROR: don't call $(MAKE) with PACKAGE_URL unset." > + @test -n "$(SED)" \ > + || echo "ERROR: don't call $(MAKE) with SED unset." > + @test -n "$(VERSION)" \ > + || echo "ERROR: don't call $(MAKE) with VERSION unset." > + rm -f $(bootstrap_files) It would be nice if this rule would actually error out (i.e., have nonzero exit status) if any of the variables were unset. > -test -f clcommit.m4sh && $MAKE -f Makefile.maint ./commit \ > -srcdir=. top_srcdir=. PACKAGE="$package" VERSION="$version" \ > -M4SH="$AUTOM4TE -l m4sh" \ > -SED="$SED" GREP="$GREP" FGREP="$FGREP" EGREP="$EGREP" LN_S="$LN_S" > +func_show_eval "$MAKE bootstrap-deps \ > +M4SH='$AUTOM4TE --language=m4sh' PACKAGE='$package' \ > +PACKAGE_BUGREPORT='$package_bugreport' PACKAGE_NAME='$package_name' \ > +PACKAGE_URL='$package_url' SED='$SED' srcdir=. VERSION='$version'" For bootstrap, the func_show_eval from the rejected patch is probably gotten just as easily by just 'set -x'; besides the fact that using eval on this string is wrong.
Re: [PATCH 5/6] Don't leak developer GREP, SED etc into distribution file.
* Gary V. Vaughan wrote on Tue, Aug 31, 2010 at 08:43:19AM CEST: > From: Gary V. Vaughan > > * bootstrap (rebuild): Set the shell variable `revision' rather > than `correctver' for clarity. > (edit): Split into two parts... > (bootstrap_edit): ...substitutions that should happen at bootstrap > time... > (configure_edit): ...and substitution that should not happen until > configure time. > * bootstrap: Move the bootstrap related section up towards the top > of the file. > (libltdl/m4/ltversion.m4): Extract `$macro_revision' from this > file for direct comparison with `$revision', rather than munging > the file's serial number. Use `bootstrap_edit' for substitutions > when generating from `ltversion.in'. > (libltdl/config/ltmain.sh): Similarly, extract `$package_revision' > for comparison with rebuild's `$revision' setting, and make > bootstrap time substitutions with `bootstrap_edit'. > (libtool): Likewise. > (libtoolize): Use `configure_edit' for substitutions at configure > time. > (tests/package.m4, tests/defs): Likewise. > * HACKING (Release Procedure): Remove the note to workaround the > bug fixed by this changeset. > * NEWS (Bug fixes): Mention that this bug is now fixed. Please credit Joerg Sonnenberger for finding the bug. Thanks. I usually do VPATH builds exclusively, except when trying out patches where I think in-tree builds could be broken. In my VPATH tree, when I make -f ../libtool/Makefile.main announce-gen SHELL='/bin/sh -x' that contains the following output: + -e 's,@TIMESTAMP\@, 1.3257 2010-08-30,g' -e 's,@LASTRELEASE\@,2.2.11,g' -e 's,@top_srcdir\@,../libtool,g' announce-gen.in /bin/sh: line 4: -e: command not found Please, once and for all, whenever you rename some identifier, use some method to verify that you've renamed all instances; git grep '\' is quite a good approximation to the perfect method, and costs maybe a couple of seconds on your part. Then submit that as separate patch. When you do pure code move-arounds, that also is much easier to review when done as a separate patch, without any actual changes to the text that was moved. I remember that we took more than dozen iterations on the makefile rules to get them halfway correct, I have no intentions of going there again. I have stopped reviewing at this point. I would gladly re-review a minimal patch based on minimal prerequisites that has been properly tested. The Makefile.am changes should be tested with both GNU and non-GNU make, and both VPATH and in-tree. > --- a/HACKING > +++ b/HACKING > @@ -620,15 +620,6 @@ or obtained by writing to the Free Software Foundation, > Inc., > > * Update NEWS, ChangeLog. > > -* Until the bug that leaks developer tool paths into the release tarballs > - from ./bootstrap is fixed, make sure the right tools are first in your > - PATH and then: > - export EGREP=egrep > - export FGREP=fgrep > - export GREP=grep > - export MAKE=make > - export SED=sed > - > * Run ./bootstrap. > > * Run ./configure (or create a build directory first and run configure > diff --git a/Makefile.am b/Makefile.am > index de3eafe..f6c0a9a 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -57,89 +57,13 @@ timestamp = set dummy `$(MKSTAMP) $(srcdir)`; shift; \ > *) TIMESTAMP="" ;; \ > esac > > -rebuild = rebuild=:; $(timestamp); correctver=$$1 > - > - > -## ## > -## Libtool scripts. ## > -## ## > - > -# The libtool distributor and the standalone libtool script. > -bin_SCRIPTS = libtoolize libtool > - > -libtoolize: $(srcdir)/libtoolize.in $(top_builddir)/config.status > - rm -f libtoolize.tmp libtoolize > - $(timestamp); \ > - input="libtoolize.m4sh"; \ > - $(edit) -e "s,@TIMESTAMP\@,$$TIMESTAMP,g" \ > - -e 's,@aclocal_DATA\@,$(aclocalfiles),g' \ > - -e "s,@pkgltdl_files\@,$(ltdldatafiles),g" \ > - -e "s,@pkgconfig_files\@,$(auxfiles),g" \ > - $(srcdir)/libtoolize.in > libtoolize.tmp > - chmod a+x libtoolize.tmp > - chmod a-w libtoolize.tmp > - mv -f libtoolize.tmp libtoolize > - > -# Use `$(srcdir)' for the benefit of non-GNU makes: this is > -# how libtoolize.in appears in our dependencies. > -EXTRA_DIST += libtoolize.m4sh > -$(srcdir)/libtoolize.in: $(sh_files) libtoolize.m4sh Makefile.am > - cd $(srcdir); \ > - rm -f libtoolize.in; \ > - $(M4SH) -B $(auxdir) libtoolize.m4sh > libtoolize.in > - > -# We used to do this with a 'stamp-vcl' file, but non-gmake builds > -# would rerun configure on every invocation, so now we manually > -# check the version numbers from the build rule when necessary. > -libtool: $(top_builddir)/config.status $(srcdir)/$(auxdir)/ltmain.sh > ChangeLog > - @target=libtool; $(rebuild); \ > - if test -f "$$target"; then \ > - set dummy `./$$target --version | sed 1q`; actualver="$$5"; \ > - test "$$actualver" = "$$correctver" && rebuild=false; \ > - fi; \ > - for prereq in $?;
Re: [PATCH 4/6] Rewrite bootstrap script for consistency with our other shell code.
* Gary V. Vaughan wrote on Tue, Aug 31, 2010 at 08:43:18AM CEST:
> * bootstrap: Add a proper option parsing loop, along with copies
> of supporting functions from `libltd/m4/getopt.m4sh' and
> `libltdl/m4/general.m4sh'.
> Reformat header comments to work with `func_help' and
> `func_version'.
> (my_sed_traces): Expanded to extract all parameters from
> configure.ac, without additional shell munging.
This patch does not add new functionality to Libtool, nor does it fix
existing bugs. It is unknown whether this patch introduces regressions,
but experience shows that a patch of this size has a nonzero chance of
doing so. On these grounds, I reject the patch.
Specifically, I think that bootstrap needs to be only about a screenful
long, excluding comments. We should be making it shorter, not longer.
I've left in the inline comments as far as I got with reviewing.
> --- a/bootstrap
> +++ b/bootstrap
> @@ -1,16 +1,16 @@
> #! /bin/sh
> -# bootstrap -- Helps bootstrapping libtool, when checked out from repository.
> -#
> -# Copyright (C) 2003, 2004, 2005, 2006, 2009, 2010 Free Software
> -# Foundation, Inc,
> -# Mritten by Gary V. Vaughan, 2003
Whoops!
> -#
> -# This file is part of GNU Libtool.
> -#
> -# GNU Libtool is free software; you can redistribute it and/or
> -# modify it under the terms of the GNU General Public License as
> -# published by the Free Software Foundation; either version 2 of
> -# the License, or (at your option) any later version.
> +
> +# bootstrap (GNU Libtool) version 2010-08-30
> +# Written by Gary V. Vaughan, 2010
> +
> +# Copyright (C) 2010 Free Software Foundation, Inc.
> +# This is free software; see the source for copying conditions. There is NO
> +# warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> +
> +# GNU Libtool is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> #
> # GNU Libtool is distributed in the hope that it will be useful,
> # but WITHOUT ANY WARRANTY; without even the implied warranty of
This change might be nice. It also makes HACKING be inconsistent.
> @@ -22,10 +22,26 @@
> # can be downloaded from http://www.gnu.org/licenses/gpl.html,
> # or obtained by writing to the Free Software Foundation, Inc.,
> # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
> -
>
> -# Exit upon failure.
> -set -e
> +# Usage: $progname [OPTION]...
> +
> +# --debug enable verbose shell tracing
> +# -n, --dry-run print commands rather than running them
> +# -f, --force bootstrap even when sources are not from git
> +# -r, --reconf-dirs=DIR1,DIR2,...
> +# limit the directories to be bootstrapped to the
> +# comma delimited list of DIR1,DIR2,
extra dot
> +# -v, --verbose verbosely report processing
> +# --version print version information and exit
> +# -h, --help print short or long help message
> +
> +# You can also set the following variables to help $progname
> +# locate the right tools:
> +# AUTORECONF, AUTOCONF, AUTOMAKE, AUTOM4TE, CONFIG_SHELL,
> +# EGREP, FGREP, GREP, LN_S, M4, MAKE, MAKEINFO, RM, SED
Since the parallel-tests merge, we don't really use $AUTOCONF and
$AUTOMAKE in the bootstrap script any more. In that way, they could be
garbage-collected. OTOH the statement is not wrong (due to autoreconf
honoring the respective environment variables) but one wonders why
AUTOHEADER and ACLOCAL are not listed here then.
> +# This script bootstraps a git checkout of GNU Libtool by correctly calling
> +# out to parts of the GNU Build Platform.
>
> # It is okay for the bootstrap process to require unreleased autoconf
> # or automake, as long as any released libtool will work with at least
> @@ -33,8 +49,11 @@ set -e
> # better features, and configure.ac documents oldest version of each
> # required for bootstrap (AC_PREREQ, and AM_INIT_AUTOMAKE).
>
> -SHELL=${CONFIG_SHELL-/bin/sh}
> -export SHELL
> +# Report bugs to
> +
> +# Exit upon failure.
> +set -e
> +
> : ${AUTORECONF=autoreconf}
> : ${AUTOCONF=autoconf}
> : ${AUTOMAKE=automake}
> @@ -43,127 +62,440 @@ export SHELL
> : ${GREP=grep}
> : ${EGREP=egrep}
> : ${FGREP=fgrep}
> +: ${RM='rm -f'}
I know you don't read bug reports on the Libtool lists, but maybe you've
come across the one where $RM is used by other packages, or even set in
the user's environment, as, who'd guess, RM=rm, and it causes spurious
output during configure. Well, due to set -e, in this script with your
change it will cause spurious failure for them.
> : ${SED=sed}
> : ${LN_S='ln -s'}
> : ${MAKEINFO=makeinfo}
>
> -case $1 in
> ---help|-h*)
> - cat < -`echo $0 | sed 's,^.*/,,g'`: This script is designed to bootstrap a fresh
> repository checkout
> -of Libtool.
Re: [PATCH 3/6] Support missing detached signatures, like gnulib implementation.
* Gary V. Vaughan wrote on Tue, Aug 31, 2010 at 08:43:17AM CEST:
> * libltdl/config/announce-gen.m4sh (func_print_locations): Don't
> bail out or print garbage when trying to find the size of a
> non-existent file.
OK thanks. While you're at it, TAB before space is a little less likely
to be garbage-collected by editors.
The summary lines of the commit entry and the ChangeLog entry of this
patch do not match the detailed explanation nor the actual patch though.
Cheers,
Ralf
> --- a/libltdl/config/announce-gen.m4sh
> +++ b/libltdl/config/announce-gen.m4sh
> @@ -267,8 +267,12 @@ func_print_locations ()
> echo "Here are the $1:"
> echo
> for my_file in $3; do
> - my_size=`$DU -h $my_file|sed 's,[bB]*[ ].*$,,'`
> - echo " $2/$my_file (${my_size}B)"
> + if test -f "$my_file"; then
> + my_size=" (`$DU -h $my_file|sed 's,[bB]*[ ].*$,,'`B)"
> + else
> + my_size=""
> + fi
> + echo " $2/$my_file$my_size"
> done
> }
>
Re: [PATCH 2/6] Fix partial commit support.
Hello Gary,
* Gary V. Vaughan wrote on Tue, Aug 31, 2010 at 08:43:16AM CEST:
> * clcommit.m4sh (func_commit): Commit only staged files instead
> of passing `-a' when no file list was given on the command line.
FWIW, I've never used clcommit since we moved to git; I just use
plain git. Here's a couple of comments though:
> --- a/clcommit.m4sh
> +++ b/clcommit.m4sh
> @@ -268,11 +268,18 @@ func_commit ()
>
> sleep 1 # give the user some time for a ^C
>
> -subject=`git status 2>/dev/null | $SED -n 's/^#.*[mad][ode][dl].*ed:
> *//p'`
> -test $# -gt 0 && subject="$@"
> +if test $# -gt 0; then
> + # Given a list of files to commit from the command line: unstage all
> + # files, and add back those in the list...
> + for staged in `$GIT status -s --porcelain 2>/dev/null | $SED -n 's/^M.
> //p'`
> + do
> +$GIT reset -- $staged
> + done
I wouldn't use a commit script that messed with my working tree or index
in uncontrollable ways. The 'git status -s' format is described in
detail in the git-status(1) man page, and your extraction is not safe in
the presence of arbitrary changes, for example renames, file deletions,
file additions, unmerged files.
More generally, I would consider this script dangerous in trying to hide
the complexity of git in possibly unexpected ways.
Come to think, why do we have this script in the Libtool tree at all?
There is no apparent connection to the Libtool functionality. Maybe
this, as well as announce-gen, could live in their own project?
> + $GIT add ${1+"$@"}
This ${1+"$@"} doesn't make sense here: either you are guaranteed here
to have at least one positional parameter, in which case you can just
use "$@", or you don't, then you have an error here because git add
requires arguments.
> +fi
> +# ...otherwise, by default, only staged files are committed.
>
> -test $# -gt 0 || { set dummy -a; shift; }
> -func_show_eval "$GIT commit $git_flags -F $log_file ${...@}" "exit
> $EXIT_FAILURE"
> +func_show_eval "$GIT commit $git_flags -F $log_file" "exit $EXIT_FAILURE"
>
> $opt_push && {
>func_show_eval "$GIT push"
Cheers,
Ralf
Re: [PATCH 1/6] Add --gnulib-version and --news options to announce-gen.
Hello Gary,
this is a review, not an approval.
* Gary V. Vaughan wrote on Tue, Aug 31, 2010 at 08:43:15AM CEST:
> * libltdl/config/announce-gen.m4sh: Add support for gnulib
> announce-gen options, previously missing from our m4sh
> implementation, and enforce specifying --gnulib-version when
> `gnulib' is listed in --bootstrap-tools.
I had avoided looking much at announce-gen so far; I've taken another
look now. I found a few issues:
Using --output to direct output to stdout seems unusual and inconsistent
with both the GNU Coding Standards recommendations (nodes 'Command-Line
Interfaces' and 'Option Table') and other tools like git.
I personally find the M4SH_GETOPTS rather unreadable; it's a nice table,
but I cannot make sense of the entries, or review them, except by
looking at the generated code, at which point I'd rather just read the
generated code directly; after all, that's how we found several bugs in
the ltmain incarnation of this code.
The help text has typos: extra "any" in "to any every", missing "the" in
"often guesses previous-version", s/for/with/ in "but for `-p'", and
missing final period.
The --rcfile handling code mistreats quoting in the rcfile, and things
like multiple adjacent whitespace.
The help text claims that ./.announcerc is the default place for the
rcfile, but the code looks like $top_srcdir/.announcerc is being used.
The announce-gen script claims:
# This is the .announcerc for GNU Libtool. Announce-gen is pretty
# smart about picking defaults for package-name and current-version
# and often guesses previous-version correctly too.
and yet the expanded script contains code like:
top_srcdir=`cd "../libtool" && pwd`
test -d "$top_srcdir" || top_srcdir='../libtool'
that, when used for another package, will luckily cause stderr noise
that will alert me that just maybe things won't go as smooth as
advertised. Or did you really mean for the script to be generated
anew as part of each package?
In another package, I might not have any bootstrap tools; in that case,
announce-gen will refuse to work.
Passing '--signature -foo' seems to not complain about a missing
argument to --signature, but also not treat -foo as signature file.
Hmm, the help text could be interpreted as to indicate that plain
'--signature' is needed in order to enable signature appending at all,
but the code sees no state change when --signature -foo is passed, and
the option parsing handling will bail out if --signature is the last
command line argument, contradicting the help text of allowing an
optional argument. Actually, in the code below the command-line
handling, a signature does not seem to ever be added in any case.
> --- a/libltdl/config/announce-gen.m4sh
> +++ b/libltdl/config/announce-gen.m4sh
> @@ -32,9 +32,12 @@ AS_INIT[]m4_divert_push([HEADER-COPYRIGHT])dnl
> # autoconf,automake,bison,gnulib
> # --current-version=VER override current release version number
> # --debug enable verbose shell tracing
> +# --gnulib-version=VERset the version string if gnulib is named
> +# in the --bootstrap-tools list
> # --gpg-key-id=ID gnupg ID of signature key for release
> files
> # -h STRING --header=STRING insert mail header into announcement
> # -m STRING --message=STRINGinsert into message blurb for
> announcement
> +# --news=NEWS_FILEpath to the NEWS file [../NEWS]
The default path for the NEWS file is $top_srcdir/NEWS rather than
../NEWS as stated in the help text. Even more, the --news switch is
ineffective in that the actual code will parse $top_srcdir/NEWS even
if I specify --news=/oops.
> # -c--no-print-checksumsdo not emit MD5 and SHA1 checksums
> # -o--outputoutput to stdout
> # --package-name=NAME override default package name
> @@ -128,13 +131,18 @@ M4SH_GETOPTS(
> mailnotify_flags="${mailnotify_flags+$mailnotify_flags }$opt"],
>[!], [--bootstrap-tools|--bootstrap|--boots], [],
> [
> for tool in `echo "$optarg"|$SED 's|,| |g'`; do
> - ($tool --version) >/dev/null 2>&1 || {
> - func_error "$opt \`$optarg' does not respond to --version
> option."
> - cmd_exit=exit
> - }
> - done],
> + test gnulib = "$tool" ||
> + ($tool --version) >/dev/null 2>&1 || {
> + func_error "$opt \`$optarg' does not respond to --version
> option."
> + cmd_exit=exit
> + }
> + done
> + # squash spaces so that delimiter is just `,' and nothing else:
> + opt_bootstrap_tools=`echo "$optarg" |$SED 's|, *|,|g'`],
>[!], [--current-version|--current|--curr], [...@version@],
> [],
> + [!], [--gnulib-version|--gnulib],[],
> [],
>[!], [--gpg-key-id|--gpg
Re: Backport non-gnulib dependent parts of my use-gnulib topic branch
Hello Gary, * Gary V. Vaughan wrote on Tue, Aug 31, 2010 at 08:43:14AM CEST: > Okay to push? For your information: I'm getting more reluctant to approve your patches, based on how much I expect we have to fix regressions stemming from them later on, when you are not present. Please state if and how you tested your patches, each by itself or only all in conjunction? On what system(s)? Did you use GNU tools in $PATH or maybe ensure they are not found while testing? Anything unusual in your environment setup? Do you try different versions of relevant tools (autoconf, git)? Whenever making things more complex rather than simpler, do you have a rationale for doing so (besides "that looked like a cool thing to try out")? Why do you post FYI commit emails without patches in them, and without non-patch content over what gets auto-posted to the libtool-commit list? Thanks, Ralf
Re: Extract the archive name from the .la file and use $AR (not ar).
* Peter Rosin wrote on Tue, Aug 31, 2010 at 07:21:34PM CEST: > Den 2010-08-31 19:11 skrev Ralf Wildenhues: > > libtool --mode=link $CC ... -o libfoo.a baz.o libbar.a > > > > should be adding baz.o and all objects in libbar.a to libfoo.a, i.e., it > > should extract all objects from libbar.a Instead, libfoo.a is added *as > > single member* into libfoo.a. That's the bug. > > (assuming typo and that you meant that libbar.a is added *as single member*) Yes, sorry. > In that case, MS lib does probably not see baz.o when it creates libfoo.a, > and the test could be made stricter by checking that baz.o is also part of > libfoo.a, since MS lib does the part that is currently tested for > (extracting libbar.a and putting the contents in libfoo.a). Cool. Yes, please extend the test. If MS lib still passes, you can adjust the xfail line, something like # This test passes with MS lib. AT_XFAIL_IF([case $AR in ar-lib\ *) false;; *) :;; esac]) Cheers, Ralf
Re: Fix archiver @FILE support test
Den 2010-08-31 18:57 skrev Ralf Wildenhues: > * Peter Rosin wrote on Tue, Aug 31, 2010 at 06:54:28PM CEST: >> Den 2010-08-31 18:49 skrev Ralf Wildenhues: >>> AC_TRY_EVAL already redirects stderr in config.log, and that was quite >>> helpful when debugging the macro recently. Why not, instead of your >>> patch, redirect stdout also >&AS_MESSAGE_LOG_FD? >> >> So much for obvious. Your suggestion sounds much better... >> >> Should I or will you? > > You have the system to test on, right? ;-) Right :-) Pushing the attached... Cheers, Peter >From 185a2f71d0d1b1727fcc45c3840a5d4f7ebfa7ef Mon Sep 17 00:00:00 2001 From: Peter Rosin Date: Tue, 31 Aug 2010 19:04:22 +0200 Subject: [PATCH] Dump archiver output to the log when testing @file support. * libltdl/m4/libtool.m4 (_LT_PROG_AR): Redirect stdout to config.log when testing for @file support. Suggested by Ralf Wildenhues. Signed-off-by: Peter Rosin --- ChangeLog |5 + libltdl/m4/libtool.m4 |2 +- 2 files changed, 6 insertions(+), 1 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1ee96b1..355b485 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,10 @@ 2010-08-31 Peter Rosin + Dump archiver output to the log when testing @file support. + * libltdl/m4/libtool.m4 (_LT_PROG_AR): Redirect stdout to config.log + when testing for @file support. + Suggested by Ralf Wildenhues. + Silence archiver output when testing @file support. * libltdl/m4/libtool.m4 (_LT_PROG_AR): Redirect stdout and stderr to the bit bucket when testing for @file support. diff --git a/libltdl/m4/libtool.m4 b/libltdl/m4/libtool.m4 index 530c971..666130d 100644 --- a/libltdl/m4/libtool.m4 +++ b/libltdl/m4/libtool.m4 @@ -1360,7 +1360,7 @@ AC_CACHE_CHECK([for archiver @FILE support], [lt_cv_ar_at_file], [lt_cv_ar_at_file=no AC_COMPILE_IFELSE([AC_LANG_PROGRAM], [echo conftest.$ac_objext > conftest.lst - lt_ar_try='$AR $AR_FLAGS libconftest.a @conftest.lst >/dev/null 2>&1' + lt_ar_try='$AR $AR_FLAGS libconftest.a @conftest.lst >&AS_MESSAGE_LOG_FD' AC_TRY_EVAL([lt_ar_try]) if test "$ac_status" -eq 0; then # Ensure the archiver fails upon bogus file names. -- 1.7.1
Re: Extract the archive name from the .la file and use $AR (not ar).
Den 2010-08-31 19:11 skrev Ralf Wildenhues: > * Peter Rosin wrote on Tue, Aug 31, 2010 at 07:09:30PM CEST: >> Den 2010-08-31 19:04 skrev Ralf Wildenhues: >>> * Peter Rosin wrote on Tue, Aug 31, 2010 at 11:49:18AM CEST: Pushing as attached, expected fail on Cygwin/ar and unexpected pass on MSYS/lib just as before. >>> >>> Why does it pass with MSYS/lib? Does lib expand the individual objects >>> of the passed library-to-add? If no, then can we make the test stricter >>> so that it correctly fails here as well? >> >> I don't think there's a way to store an archive in an archive using lib. >> It will just take the contents of the archive and put that in the new >> archive instead, doing exactly what seems to be desired by the test. > > Naa. The test is just exposing a long-term bug in libtool itself: > > libtool --mode=link $CC ... -o libfoo.a baz.o libbar.a > > should be adding baz.o and all objects in libbar.a to libfoo.a, i.e., it > should extract all objects from libbar.a Instead, libfoo.a is added *as > single member* into libfoo.a. That's the bug. (assuming typo and that you meant that libbar.a is added *as single member*) In that case, MS lib does probably not see baz.o when it creates libfoo.a, and the test could be made stricter by checking that baz.o is also part of libfoo.a, since MS lib does the part that is currently tested for (extracting libbar.a and putting the contents in libfoo.a). Cheers, Peter
Re: Extract the archive name from the .la file and use $AR (not ar).
* Peter Rosin wrote on Tue, Aug 31, 2010 at 07:09:30PM CEST: > Den 2010-08-31 19:04 skrev Ralf Wildenhues: > > * Peter Rosin wrote on Tue, Aug 31, 2010 at 11:49:18AM CEST: > >> Pushing as attached, expected fail on Cygwin/ar and unexpected pass on > >> MSYS/lib just as before. > > > > Why does it pass with MSYS/lib? Does lib expand the individual objects > > of the passed library-to-add? If no, then can we make the test stricter > > so that it correctly fails here as well? > > I don't think there's a way to store an archive in an archive using lib. > It will just take the contents of the archive and put that in the new > archive instead, doing exactly what seems to be desired by the test. Naa. The test is just exposing a long-term bug in libtool itself: libtool --mode=link $CC ... -o libfoo.a baz.o libbar.a should be adding baz.o and all objects in libbar.a to libfoo.a, i.e., it should extract all objects from libbar.a Instead, libfoo.a is added *as single member* into libfoo.a. That's the bug. Cheers, Ralf
Re: Extract the archive name from the .la file and use $AR (not ar).
Den 2010-08-31 19:04 skrev Ralf Wildenhues: > * Peter Rosin wrote on Tue, Aug 31, 2010 at 11:49:18AM CEST: >> Pushing as attached, expected fail on Cygwin/ar and unexpected pass on >> MSYS/lib just as before. > > Why does it pass with MSYS/lib? Does lib expand the individual objects > of the passed library-to-add? If no, then can we make the test stricter > so that it correctly fails here as well? I don't think there's a way to store an archive in an archive using lib. It will just take the contents of the archive and put that in the new archive instead, doing exactly what seems to be desired by the test. > Oh, did we discuss this before and I forgot? Don't remember :-) Cheers, Peter
Re: Extract the archive name from the .la file and use $AR (not ar).
* Peter Rosin wrote on Tue, Aug 31, 2010 at 11:49:18AM CEST: > Pushing as attached, expected fail on Cygwin/ar and unexpected pass on > MSYS/lib just as before. Why does it pass with MSYS/lib? Does lib expand the individual objects of the passed library-to-add? If no, then can we make the test stricter so that it correctly fails here as well? Oh, did we discuss this before and I forgot? Thanks, Ralf > Subject: [PATCH] Extract the archive name from the .la file and use $AR (not > ar). > > * Makefile.am: Pass AR through to the testsuite. > * tests/archive-in-archive.at: Bump copyright year. Extract archive > name from the .la file instead of hardcoding the name, and allow > different archivers. Also clarify that the tested functionality is > currently broken.
Re: Fix archiver @FILE support test
* Peter Rosin wrote on Tue, Aug 31, 2010 at 06:54:28PM CEST: > Den 2010-08-31 18:49 skrev Ralf Wildenhues: > > AC_TRY_EVAL already redirects stderr in config.log, and that was quite > > helpful when debugging the macro recently. Why not, instead of your > > patch, redirect stdout also >&AS_MESSAGE_LOG_FD? > > So much for obvious. Your suggestion sounds much better... > > Should I or will you? You have the system to test on, right? ;-)
Re: Fix archiver @FILE support test
Den 2010-08-31 18:49 skrev Ralf Wildenhues: > Hi Peter, > > * Peter Rosin wrote on Tue, Aug 31, 2010 at 01:44:00PM CEST: >> Microsoft lib prints a failure message though. I assume other >> archivers are not unlikely to also do so... >> >> Pushing the attached as obvious. > > AC_TRY_EVAL already redirects stderr in config.log, and that was quite > helpful when debugging the macro recently. Why not, instead of your > patch, redirect stdout also >&AS_MESSAGE_LOG_FD? So much for obvious. Your suggestion sounds much better... Should I or will you? Cheers, Peter
Re: Fix archiver @FILE support test
Hi Peter, * Peter Rosin wrote on Tue, Aug 31, 2010 at 01:44:00PM CEST: > Microsoft lib prints a failure message though. I assume other > archivers are not unlikely to also do so... > > Pushing the attached as obvious. AC_TRY_EVAL already redirects stderr in config.log, and that was quite helpful when debugging the macro recently. Why not, instead of your patch, redirect stdout also >&AS_MESSAGE_LOG_FD? Thanks, Ralf > Subject: [PATCH] Silence archiver output when testing @file support. > > * libltdl/m4/libtool.m4 (_LT_PROG_AR): Redirect stdout and stderr > to the bit bucket when testing for @file support. > --- a/libltdl/m4/libtool.m4 > +++ b/libltdl/m4/libtool.m4 > @@ -1360,7 +1360,7 @@ AC_CACHE_CHECK([for archiver @FILE support], > [lt_cv_ar_at_file], >[lt_cv_ar_at_file=no > AC_COMPILE_IFELSE([AC_LANG_PROGRAM], > [echo conftest.$ac_objext > conftest.lst > - lt_ar_try='$AR $AR_FLAGS libconftest.a @conftest.lst' > + lt_ar_try='$AR $AR_FLAGS libconftest.a @conftest.lst >/dev/null 2>&1' >AC_TRY_EVAL([lt_ar_try]) >if test "$ac_status" -eq 0; then > # Ensure the archiver fails upon bogus file names.
Re: [PATCH] [cygwin|mingw]: Add cross-compile support to cwrapper (take 6)
Den 2010-08-29 23:30 skrev Roumen Petrov: > Peter Rosin wrote: >> Den 2010-08-27 00:27 skrev Roumen Petrov: >>> Ralf Wildenhues wrote: Hi Charles, >>> [SNIP] > + func_wine_to_win32_path_result="$1" > + if test -n "$1"; then > +# Unfortunately, winepath does not exit with a non-zero > +# error code, so we are forced to check the contents of > +# stdout. On the other hand, if the command is not > +# found, the shell will set an exit code of 127 and print > +# *an error message* to stdout. So we must check for both > +# error code of zero AND non-empty stdout, which explains > +# the odd construction: >>> >>> Starting from wine 1.3.1 wine path always output paths: >> >> I have been reading the git source of winepath, and it definitely >> has code paths which output an empty result when certain things >> fail (or rather, a single newline for every path/file it fails to >> convert), so saying that it "always output paths" is not entirely >> correct. >> >>> Lets wine is correctly configured (Z: drive is linked to the file system >>> root): >>> $ cd $WINEPREFIX/dosdevices >>> $ winepath -w `pwd` >>> Z:\%WINEPREFIX_CONVERTED_TO_BACKSLASHES%\dosdevices >>> >>> Now lets remove link: >>> $ rm z: >>> $ winepath -w `pwd` >>> \\?\unix\%WINEPREFIX_CONVERTED_TO_BACKSLASHES%\dosdevices >>> >>> So sed should remove leading //?/unix >> >> This failures are probably comping from inside wine (no mention of >> "//?/unix" or "\\?\unix" in the winepath source), so probably won't >> be seen as failure by winepath. But, if you silently remove \\?\unix, >> then you'll end up with a path that is not complete with a drive >> letter. I think any \\?\unix prefix should be filed as a failure... >> Peter, are you reading this? Looks like a TODO item for automake/lib/compile. ;-) >> >> Yes, I'm reading this :-) Patches to compile (and ar-lib) to follow >> when the dust settles... >> >> Cheers, >> Peter >> >> > > FYI http://bugs.winehq.org/show_bug.cgi?id=13265 Ok, I have read that and studied the Wine source a bit more and see where \??\ is transformed into \\?\. So, sorry for the false accusation regarding copy-paste vs. retyping in my other reply... I have now prepared a patch for compile and ar-lib in automake... Cheers, Peter
Re: [PATCH 1/6] Add --gnulib-version and --news options to announce-gen.
Hi Eric, On 31 Aug 2010, at 21:08, Eric Blake wrote: > On 08/31/2010 12:43 AM, Gary V. Vaughan wrote: >> From: Gary V. Vaughan >> >> * libltdl/config/announce-gen.m4sh: Add support for gnulib >> announce-gen options, previously missing from our m4sh >> implementation, and enforce specifying --gnulib-version when >> `gnulib' is listed in --bootstrap-tools. > > Are you planning on swapping over to gnulib's announce-gen once gnulib is > fully integrated? No, it doesn't work quite right for our release procedure. I tried to make it work again while pulling gnulib modules into the post-release patch series, but Perl still makes my eyes bleed. > And in the meantime, what good does it do to have a --gnulib-version option > if we aren't using gnulib yet? None yet, I just picked all the patches from my use-gnulib topic branch that don't *require* gnulib. Although this particular one has zero impact on user facing code, we can leave the push until after the release... although I was hoping to keep the queue short for easier review. Cheers, -- Gary V. Vaughan (g...@gnu.org) PGP.sig Description: This is a digitally signed message part
Re: [PATCH 1/6] Add --gnulib-version and --news options to announce-gen.
On 08/31/2010 12:43 AM, Gary V. Vaughan wrote: From: Gary V. Vaughan * libltdl/config/announce-gen.m4sh: Add support for gnulib announce-gen options, previously missing from our m4sh implementation, and enforce specifying --gnulib-version when `gnulib' is listed in --bootstrap-tools. Are you planning on swapping over to gnulib's announce-gen once gnulib is fully integrated? And in the meantime, what good does it do to have a --gnulib-version option if we aren't using gnulib yet? -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org
Re: Fix archiver @FILE support test
Den 2010-08-29 07:54 skrev Peter Rosin: > Den 2010-08-28 12:24 skrev Ralf Wildenhues: >> Peter, can you check whether ar-lib, lib, and 'link -lib' reliably fail >> when an @file contains a nonexistent file name? > > I haven't run the testsuite, but manual testing shows that the patch > should be ok. If the testsuite reveals unexpected problems I'll report > back... Microsoft lib prints a failure message though. I assume other archivers are not unlikely to also do so... Pushing the attached as obvious. Cheers, Peter >From 34f87a6b07381f6092851a25c9da62b2c40a4d21 Mon Sep 17 00:00:00 2001 From: Peter Rosin Date: Tue, 31 Aug 2010 12:40:01 +0200 Subject: [PATCH] Silence archiver output when testing @file support. * libltdl/m4/libtool.m4 (_LT_PROG_AR): Redirect stdout and stderr to the bit bucket when testing for @file support. Signed-off-by: Peter Rosin --- ChangeLog |6 ++ libltdl/m4/libtool.m4 |2 +- 2 files changed, 7 insertions(+), 1 deletions(-) diff --git a/ChangeLog b/ChangeLog index a4c91cb..1ee96b1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2010-08-31 Peter Rosin + + Silence archiver output when testing @file support. + * libltdl/m4/libtool.m4 (_LT_PROG_AR): Redirect stdout and stderr + to the bit bucket when testing for @file support. + 2010-08-31 Gary V. Vaughan Remove double `Generated from foo.m4sh' lines. diff --git a/libltdl/m4/libtool.m4 b/libltdl/m4/libtool.m4 index e03543b..530c971 100644 --- a/libltdl/m4/libtool.m4 +++ b/libltdl/m4/libtool.m4 @@ -1360,7 +1360,7 @@ AC_CACHE_CHECK([for archiver @FILE support], [lt_cv_ar_at_file], [lt_cv_ar_at_file=no AC_COMPILE_IFELSE([AC_LANG_PROGRAM], [echo conftest.$ac_objext > conftest.lst - lt_ar_try='$AR $AR_FLAGS libconftest.a @conftest.lst' + lt_ar_try='$AR $AR_FLAGS libconftest.a @conftest.lst >/dev/null 2>&1' AC_TRY_EVAL([lt_ar_try]) if test "$ac_status" -eq 0; then # Ensure the archiver fails upon bogus file names. -- 1.7.1
Remove double `Generated from foo.m4sh' lines.
Pushed as obvious. Remove double `Generated from foo.m4sh' lines. We now require a modern Autoconf to bootstrap libtool, which will add the `Generated by ...' boiler-plate automatically, so we can remove the hand-rolled @configure_input@ substitutions we had been doing: * clcommit.m4sh, libtoolize.m4sh, tests/defs.sh, libltdl/config/announce-gen.m4sh, libltdl/config/ltmain.m4sh, libltdl/config/mailnotify.m4sh: Remove @configure_in...@. * Makefile.am (edit): Remove configure_input substitution. (libtoolize, libltdl/config/ltmain.sh, libltdl/m4/ltversion.m4) (tests/defs): Don't set `$input' shell variable. * Makefile.maint (announce_gen, libltdl/config/mailnotify) (commit): Likewise.
Re: Extract the archive name from the .la file and use $AR (not ar).
Den 2010-08-30 20:55 skrev Ralf Wildenhues: > Hi Peter, > > * Peter Rosin wrote on Mon, Aug 30, 2010 at 03:25:30PM CEST: >> The archive-in-archive.at test uses ar "blindly". It also assumes that >> the "old archive" of libfoo.la is named libfoo.a, but both that's not >> portable. See attached patch. > > OK with nits below. Thanks! > I don't like that we don't have a decent abstraction for static > libraries, but that again is something we *should* fix in Automake > not Libtool IMVHO. (I'm not yet quite sure how exactly ...) > > Thanks, > Ralf > >> --- a/tests/archive-in-archive.at >> +++ b/tests/archive-in-archive.at > > missing copyright year update. > >> @@ -42,11 +42,15 @@ $LIBTOOL --mode=compile --tag=CC $CC $CPPFLAGS $CFLAGS >> -c -o bar.lo bar.c >> $LIBTOOL --mode=link --tag=CC --tag=disable-shared $CC $CFLAGS $LDFLAGS \ >> -o libfoo.la foo.lo -version-info 1:0:0 -rpath $thisdir >> $LIBTOOL --mode=install cp libfoo.la $thisdir >> +eval `$EGREP '^(old_library)=' < libfoo.la` >> +libfoo=$old_library >> AT_CHECK([$LIBTOOL --mode=link --tag=CC --tag=disable-shared $CC $CFLAGS >> $LDFLAGS \ >> - -o libbar.la bar.lo ./libfoo.a -version-info 1:0:0 -rpath $thisdir], >> + -o libbar.la bar.lo $libfoo -version-info 1:0:0 -rpath $thisdir], >> [], [ignore], [ignore]) >> AT_CHECK([$LIBTOOL --mode=install cp libbar.la $thisdir], [], [ignore], >> [ignore]) >> -AT_CHECK([ar -t libbar.a | grep libfoo.a],[1],[ignore],[ignore]) >> -archive_contents=`ar -t libbar.a` >> -AT_XFAIL_IF([case "$archive_contents" in *"libfoo.a"*) : ;; esac]) >> +eval `$EGREP '^(old_library)=' < libbar.la` >> +libbar=$old_library >> +AT_CHECK([$AR -t $libbar | grep $libfoo],[1],[ignore],[ignore]) > > How about $FGREP? You' need to extract it from $LIBTOOL though. Then I can't anchor the fixed pattern at the beginning of the line, so I'm sticking with EGREP... >> +archive_contents=`$AR -t $libbar` >> +AT_XFAIL_IF([case "$archive_contents" in *"$libfoo"*) : ;; esac]) > > This AT_XFAIL_IF it completely bogus at this point. The argument to > AT_XFAIL_IF is, yes, surprisingly, evaluated *before* the rest of the > test group is started. So all it does is > > AT_XFAIL_IF([case "$empty_var" in *"$another_empty_var"*) : ;; esac]) > > Please remove these two lines and instead add > AT_XFAIL_IF([:]) dnl This is currently broken > > at the *beginning* of the test group (so readers are not confused) > right after AT_SETUP, and adjust the log entry accordingly. Ok, done. Pushing as attached, expected fail on Cygwin/ar and unexpected pass on MSYS/lib just as before. Cheers, Peter >From 9a99cfa036a1c9ab7e3afa8815ef79b99d80cc3c Mon Sep 17 00:00:00 2001 From: Peter Rosin Date: Tue, 31 Aug 2010 11:48:28 +0200 Subject: [PATCH] Extract the archive name from the .la file and use $AR (not ar). * Makefile.am: Pass AR through to the testsuite. * tests/archive-in-archive.at: Bump copyright year. Extract archive name from the .la file instead of hardcoding the name, and allow different archivers. Also clarify that the tested functionality is currently broken. Signed-off-by: Peter Rosin --- ChangeLog |9 + Makefile.am |1 + tests/archive-in-archive.at | 14 +- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index 5d7a1dc..8a183e2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +2010-08-31 Peter Rosin + + Extract the archive name from the .la file and use $AR (not ar). + * Makefile.am: Pass AR through to the testsuite. + * tests/archive-in-archive.at: Bump copyright year. Extract archive + name from the .la file instead of hardcoding the name, and allow + different archivers. Also clarify that the tested functionality is + currently broken. + 2010-08-30 Ralf Wildenhues tests: skip -Wall -Werror with Tru64 cc in cwrapper test. diff --git a/Makefile.am b/Makefile.am index de3eafe..b5cde00 100644 --- a/Makefile.am +++ b/Makefile.am @@ -513,6 +513,7 @@ EXTRA_DIST += $(srcdir)/$(TESTSUITE) $(TESTSUITE_AT) $(srcdir)/tests/package TESTS_ENVIRONMENT = MAKE="$(MAKE)" CC="$(CC)" CFLAGS="$(CFLAGS)" \ CPP="$(CPP)" CPPFLAGS="$(CPPFLAGS)" LD="$(LD)" LDFLAGS="$(LDFLAGS)" \ LIBS="$(LIBS)" LN_S="$(LN_S)" NM="$(NM)" RANLIB="$(RANLIB)" \ + AR="$(AR)" \ M4SH="$(M4SH)" SED="$(SED)" STRIP="$(STRIP)" lt_INSTALL="$(INSTALL)" \ MANIFEST_TOOL="$(MANIFEST_TOOL)" \ OBJEXT="$(OBJEXT)" EXEEXT="$(EXEEXT)" \ diff --git a/tests/archive-in-archive.at b/tests/archive-in-archive.at index a57dfdd..32e3543 100644 --- a/tests/archive-in-archive.at +++ b/tests/archive-in-archive.at @@ -1,6 +1,6 @@ # dmacks.at -- test for dmacks bug -*- Autotest -*- # -# Copyright (C) 2007, 2008 Free Software Foundation, Inc. +# Copyright (C) 2007, 2008, 2010 Free Software Foundation, Inc. # Written by Peter O'Gorman, 2007 # # This file is part of GNU
