[Top][All Lists]

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

Re: [patch 05/19] 288-gary-ltdl-nonrecursive-tests.diff Queue

From: Gary V. Vaughan
Subject: Re: [patch 05/19] 288-gary-ltdl-nonrecursive-tests.diff Queue
Date: Tue, 11 Oct 2005 10:53:17 +0100
User-agent: Mozilla Thunderbird 1.0 (X11/20050305)

Ralf Wildenhues wrote:
Hi Gary,

Hallo Ralf,

Thanks for the review!

* Gary V. Vaughan wrote on Mon, Oct 10, 2005 at 12:26:29PM CEST:
        * tests/ New tests for libltdl as a subdirectory,
        configured and compiled from the toplevel project.
        * tests/ Use it.
        * (TESTSUITE_AT): Depend on it.

See nits below, and this one: all of them FAIL with 2.59/1.9.6 (without
libobjdir fixes):

| $AUTORECONF --force --verbose --install
| stderr:
| autoreconf: Entering directory `.'
| autoreconf: not using Gettext
| autoreconf: running: aclocal --force -I libltdl/m4
| autoreconf: tracing
| autoreconf: not using Libtool
| autoreconf: running: autoconf --force
| autoreconf: running: autoheader --force
| autoreconf: running: automake --add-missing --copy --force-missing
| installing `libltdl/config/install-sh'
| installing `libltdl/config/missing'
| installing `libltdl/config/config.guess'
| installing `libltdl/config/config.sub'
| installing `libltdl/config/compile'
| required file `./lt__dirent.c' not found
| required file `./argz.c' not found
| required file `./lt__strl.c' not found
| installing `libltdl/config/depcomp'
| autoreconf: automake failed with exit status: 1

Can you make them SKIP in this case, so we don't get oodles of bogus bug

Nice catch.  Yes, I'll have a think about this.  I think I also need to
document that nonrecursive mode only works with full LIBOBJDIR support
in Automake and Autoconf...


The naming is unfortunate.  As all the tests share an m4 macro name
space, and you actually use _LTDL_SETUP in several othere tests for a
different purpose, please rename all of them.

No need, it's fine.  The name is quoted in the m4_define exactly so that
we can do this sort of thing.  What is the point of renaming?

Maybe m4_undefine at the
end of the test would be ok as well, but then I believe you wanted to
share some macros.

Okay.  That's less intrusive.  I'll pushdef/popdef the definition then,
which is the cleanest way to stop it leaking out into other tests.

+# -----------
+LT_CONFIG_LTDL_DIR([libltdl], [nonrecursive])

Urgs.  In large packages, these three settings are going to be a major
pain in the long run.  I'm not saying you need to change them here, this
issue isn't a Libtool problem rather than an Autoconf/Automake one, but
it'd be good if people could change all three of these and things would
still work somehow.

(Just thinking out loud; nothing we'll want to worry about before 2.0.)

Yes, the whole LIBOBJ model is, unfortunately, completely broken :-(  (I'm
sure you've seen the threads on gnulib, and the mess in CVS M4) Only toy
packages can cope well with a single directory of LIBOBJ sources.  I have
no idea how to fix this either... although when I transfer my attention
back to M4, inspiration may occur.  Luckily the requirements for the other
build modes aren't nearly so invasive.


Why do you need this?  Is it only for Automake backwards compatibility?

subdir-objects requires it.

If so, then I recommend adding AC_PROG_CC before as well.

Seems like overkill to me... Belt and braces?

$ fgrep -l AM_PROG_CC_C_O /usr/share/aclocal*/*.m4
$ fgrep AC_REQUIRE /usr/share/aclocal*/minuso.m4
$ fgrep -l AC_PROG_CC_C_O /usr/share/autoconf/autoconf/*.m4
$ sed -n '/^# AC_PROG_CC_C_O/,/])# AC_PROG_CC_C_O/p' 
# --------------

+AM_INIT_AUTOMAKE([foreign subdir-objects])

I believe you should have AM_INIT_AUTOMAKE before AM_PROG_CC_C_O.
After all, AM_INIT_AUTOMAKE should be the first automake macro called.

Again, I'm not sure about this reasoning :-)  AM_PROG_CC_C_O is designed
to be a wrapper for AC_PROG_CC, and surely AC_PROG_CC should come before
AM_INIT_AUTOMAKE?  I don't want to change this, but if you are firm, and
it still works then I can be persuaded...

+[[ACLOCAL_AMFLAGS = -I libltdl/m4

If you agree on my suggestions wrt. changes, this needs to
be adjusted as well.

Agreed.  Thanks.

+touch foo.c

Empty source files are not portable.
You can use
  echo 'static int dummy = 0;' > foo.c
instead.  If you want, just fix the same bug in as well,
sorry for not seeing that earlier.

My bad.  I'd meant to change that myself... will do.

Gary V. Vaughan      ())_.  address@hidden,}
Research Scientist   ( '/
GNU Hacker           / )=
Technical Author   `(_~)_

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

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