Re: [PATCH] tests: feed -no-undefined when linking libtool libraries

[Gary V. Vaughan]

Hi Peter,

On Oct 3, 2012, at 4:53 PM, Peter Rosin <address@hidden> wrote:
> On 2012-10-03 05:45, Gary V. Vaughan wrote:
>> On Oct 3, 2012, at 12:32 AM, Peter Rosin <address@hidden> wrote:
>>> I just fired up a test suite run...
> 
> Good news! Clean runs on Cygwin, MSYS/MinGW and MSYS/MSVC.

Awesome.

I'll push the merge now, along with some other resurrected patches that
got stuck in the review queue last year.

> Now into nitpicking and other useless ramblings...
> 
> The patch "libtool: unroll nested if into a single case statement" has
> whitespace issues (leading spaces instead of tabs on a few lines), and it
> should perhaps use a simple catch-all "*" (retaining the "fast_install is
> set to needless" comment) as the last case instead of "*,needless" that
> you have put there. But I guess it's harmless as $fast_install is not any
> random string, sans hacks, so no rereredo request from me, just a
> nitpick that I saw when reviewing the "libtool: ..." changes at the
> tip of gary/reredo-test-operand-order. Can always be cleaned up later,
> if someone has the energy.

Agreed on all counts.  Having now split out all the unrelated patches has me
wondering how on earth all that extra crud got in there... especially since 
every
review used to be like a few weeks on the rack.

> I also wonder about the relationship between "libtool: fold if into a
> compound OR statement when more readable" and the next commit "libtool:
> simplify multiple string tests". What I mean is that about half the
> tests from the later commit fit the pattern of the first, why did you
> for example not change
> 
>    if test yes,no = "$avoid_version,$need_version"; then
>      major=
>      versuffix=
>      verstring=
>    fi
> 
> into
> 
>    test yes,no = "$avoid_version,$need_version" && {
>      major=
>      versuffix=
>      verstring=
>    }
> 
> (or some such) when you where at it? (but to me it looks like churn)

Yep :(  But, remember these changes are all to match the conflated original
changeset so that we end up with a useful diff showing errors in the original
that can be applied easily to the current tip of master.  That said, your
comments are more than valid, and a lot of that stuff shouldn't really have
gotten in and/or should have been done consistently.

With my newly self appointed benevolent dictator hat on, and the commit
process being infinitely less arduous as a result, I hope to come back and
undo some of this damage prior to freezing for the next release.

> Anyway, to sum up, I'm ok with merging reredo branch into master.
> Lets put this to rest now, thanks!

And thanks to you too :)

Cheers,
-- 
Gary V. Vaughan (gary AT gnu DOT org)