libtool-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: 304-gary-add-libtool-macro-file-serial-tags.diff


From: Ralf Wildenhues
Subject: Re: 304-gary-add-libtool-macro-file-serial-tags.diff
Date: Fri, 28 Oct 2005 16:30:27 +0200
User-agent: Mutt/1.5.11

Hi Gary,

* Gary V. Vaughan wrote on Fri, Oct 28, 2005 at 04:11:31PM CEST:
> Ralf Wildenhues wrote:
> 
> >This patch actually makes several logical changes:
> 
> ACK.  If you'd rather see it as separate commits, I can break it up.
> I figured that since they were there to fix a single bug that putting
> them in a single patch was okay.  I don't mind either way :-D

If you don't mind, then separate patches are better; but I see that they
are more work, too.  They should get reviewed more quickly.  :)

> >- use that to find out versions in hand-maintained aclocal.m4.
> >  (Would that work with `acinclude.m4' as well, by the way?)
> 
> Only if m4_include(acinclude.m4) is added to aclocal.m4.  acinclude.m4
> doesn't make sense for a hand maintained aclocal.m4 -- either aclocal
> manages aclocal.m4 and acinclude.m4 is copied into it, or else aclocal.m4
> is hand maintained, in which case macros will be pasted into aclocal.m4
> directly.
> 
> We could add support for users that hand maintain acinclude.m4 instead
> of letting aclocal m4_include local copies of macro files, but use
> aclocal to maintain aclocal.m4 proper. There are definitely some odd
> corner cases, such as aclocal.m4 not yet generated.  But doing that is
> a feature: hence, post 2.0.

OK.

> >- treat argz.m4 like ltdl.m4.  This is the most troublesome.
> >`aclocal' is actually smart enough not to include argz.m4 in the case
> >that libltdl is a subproject, because *then*, the user doesn't actually
> >need the argz tests in the toplevel project; but she does need other
> >stuff in ltdl.m4.
> 
> But Automake will not distribute argz.m4 for the top-level Makefile.am,
> even if it is in AC_CONFIG_MACRO_DIR, unless aclocal uses it.  Only ltdl.m4
> uses argz.m4, so it seems odd to treat them differently.  However, I'm happy
> to commit just the other parts and discuss this issue separately.

My point is: Automake and aclocal are both smart enough, yet libtoolize
warns.  To reproduce, use OpenMPI again, see below.  :)

> >I get the impression that we are trying to outsmart something which, in
> >the end, may just as well break again with the next change in aclocal.
> >Why not just tell those users that hand-maintain aclocal.m4 to get their
> >act together and take care of keeping it up-to-date themselves.  No need
> >to try to be any smarter than that.
> 
> Apart from the argz.m4 change, the rest of this patch is only about
> asking the user to update some or all of the newly copied libtool m4
> macro files.  Just getting the messages right, and nothing else...

I know.  But the message was bogus:
| [Running] /tmp/install/libtool-2.1/bin/libtoolize --automake --copy 
--ltdl=opal/libltdl
| libtoolize: You should add the contents of `./config/argz.m4' to `aclocal.m4'.

And as such, I don't think your patch is ok.

> >I am somewhat undecided on that matter, and you certainly have more
> >experience.  What do you think?
> >>-    case `echo " "$my_included_files" "` in
> >>+    case `echo " "$my_included_files" " | $NL2SP` in
> >
> >You can simplify this to either
> >       case `echo " $my_included_files " | $NL2SP` in
> 
> ACK.  I've changed to this format.
> 
> >or
> >       case `echo " "$my_included_files" "` in
> >In the first case, word splitting will not occur, but NL2SP will help
> >you; in the second case, word splitting will occur on $my_included_files
> >before `echo' is called.  The fact that the first resulting word there
> >starts with a space is fine.
> 
> Until I added NL2SP, the latter version wasn't matching at all with
> bash-2.05b or bash-3.0 on my darwin machine.

Weird.  What's different at your site?

$ foo='a
b
c'
$ f=b
$ case `echo " "$foo" "` in *" $f "*) echo yes;; esac
yes
$ uname -mrs
Darwin 8.2.0 Power Macintosh
$ echo $BASH_VERSION
2.05b.0(1)-release

Cheers,
Ralf




reply via email to

[Prev in Thread] Current Thread [Next in Thread]