libtool-patches
[Top][All Lists]
Advanced

[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

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

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