[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [patch 1/2] 307-gary-fix-recursive-ltdl-dist-rules.diff
From: |
Gary V. Vaughan |
Subject: |
Re: [patch 1/2] 307-gary-fix-recursive-ltdl-dist-rules.diff |
Date: |
Thu, 10 Nov 2005 21:09:32 +0000 |
User-agent: |
Mozilla Thunderbird 1.0.6 (Macintosh/20050716) |
Ralf Wildenhues wrote:
> Hi Gary,
Hallo Ralf,
Thanks for the review!
> * Gary V. Vaughan wrote on Thu, Nov 10, 2005 at 11:35:49AM CET:
>
>>Okay to commit?
>
> No. The test suite modifies the source. After a testsuite run,
> libltdl/Makefile.am will contain, for example, 6 lines of the form:
> | EXTRA_DIST += ltdl.c
>
> and quite a bit of other garbage.
Argh... the only fiddly bit of the patch was adding the glob_nolink
parameters so that I could make sure that Makefile.{am,inc} are
always copied to prevent the EXTRA_DIST goo from following the link
back to the source file. It works for me, can you debug it on your
machine? Or send me a trace please?
>>Your wish is my command -- now \(non\)?recursive modes don't blow up the
>>make dist rule of parent projects. This turned out to be more straight
>>forward that I had expected, so I went ahead and did it myself. It also
>>exposes a bug in the testsuite which I'll address in my next patch.
>
>
> Well, for me,
> | Recursive Automake Libltdl.
> | 34: distributing libltdl
>
> fails because it wants to distribute configure.ac in one run,
> and Makefile.inc in another, and
That's because your source Makefile.am is corrupted.
> | Subproject Libltdl.
> | 25: distributing libltdl FAILED
> (subproject.at:109)
>
> searches for Makefile.inc as well. I assume all followup failures.
Likewise.
> Since you do `make distcheck' inside our testsuite now, this patch _has_
> to go in after undoing the `make -e'. So it needs to come after my
> patch, or will fail mysteriously in some environments. :)
Oh yes, good point. Okay, NP.
> Further nits below. Also, I wonder why you can't just add a
> LT_AT_MAKE([distcheck])
Force of habit. Yes, s/all distcheck/distcheck/g is better. Thanks.
> at the end of the existing tests. That would save a bunch of time,
> without really less test coverage?
ACK.
>>Index: libtool--devo--1.0/ChangeLog
>>from Gary V. Vaughan <address@hidden>
>> * libltdl/Makefile.inc (EXTRA_DIST): Move files that are not
>> installed unconditionally to a client from here...
>> * Makefile.am (EXTRA_DIST): ...to here.
>> * libtoolize.m4sh: Append the paths to installed files to
>> EXTRA_DIST in newly copied Makefile.am.
>> * tests/nonrecursive.at, tests/recursive.at, tests/subproject.at:
>> Add new dist tests to prevent a regression.
>>
>>Index: libtool--devo--1.0/Makefile.am
>>===================================================================
>>--- libtool--devo--1.0.orig/Makefile.am
>>+++ libtool--devo--1.0/Makefile.am
>>@@ -214,7 +214,6 @@ $(srcdir)/libltdl/Makefile.am: $(srcdir)
>> $(SED) -n '/^.. DO NOT REMOVE THIS LINE -- /,$$ \
>> { s,libltdl_,,; s,libltdl/,,; s,: libltdl/,: ,; \
>> s,\$$(libltdl_,$$(,; p; }' $$in >> $$out;
>>- chmod a-w $(srcdir)/libltdl/Makefile.am
>
>
> Why do you need this?
We use tar to install Makefile.am into the user's tree so permission
bit are propogated into the user's tree, Without the writable bit the
EXTRA_DIST stanzas can't be concatenated by libtoolize... I should
note that in the ChangeLog, sorry.
>>Index: libtool--devo--1.0/libtoolize.m4sh
>>===================================================================
>>--- libtool--devo--1.0.orig/libtoolize.m4sh
>>+++ libtool--devo--1.0/libtoolize.m4sh
>>@@ -1066,6 +1066,19 @@ func_nonemptydir_p ()
>>
>> func_copy_some_files "$pkgltdl_files" "$pkgltdldir/libltdl" "$ltdldir"
>>
>>+ # Fixup the EXTRA_DIST contents in Makefile.inc if necessary
>>+ my_makefile="$ltdldir/Makefile.am"
>>+ test -f "$ltdldir/Makefile.inc" && my_makefile="$ltdldir/Makefile.inc"
>>+set -x
>
> Remove your debugging traces! ;)
> (They make tests fail, too)
Ah boo... brown paper bag please!
>>+ my_save_IFS="$IFS"
>>+ IFS=:
>>+ for file in $pkgltdl_files; do
>>+ IFS="$my_save_IFS"
>>+ test nonrecursive = "$ltdl_mode" && file="$ltdldir/$file"
>>+ echo "EXTRA_DIST += $file" >> $my_makefile
>>+ done
>>+ IFS="$my_save_IFS"
>>+set +x
>
> Ditto.
Make that two...
Cheers,
Gary.
--
Gary V. Vaughan ())_. address@hidden,gnu.org}
Research Scientist ( '/ http://tkd.kicks-ass.net
GNU Hacker / )= http://www.gnu.org/software/libtool
Technical Author `(_~)_ http://sources.redhat.com/autobook
signature.asc
Description: OpenPGP digital signature