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

[Gary V. Vaughan]

> Hi Peter,

Ping?

> On 19 Sep 2012, at 21:50, Peter Rosin <address@hidden> wrote:
> 
>> On 2012-09-19 16:20, Gary V. Vaughan wrote:
>>> Hi Peter,
>>> 
>>> My bad, I'm embarrassed to say. I started to write a script to make the
>>> appropriate changes, but ended up doing it manually rather than adding
>>> more and more corner cases to the throwaway script... a poor choice in
>>> hindsight :-(
>> 
>> It's easy to be wise after the fact... If you do redo it, may I suggest
>> breaking up the patch in smaller pieces for bisectability?
> 
> I've pushed a temporary branch called gary/redo-test-operand-order to
> savannah with seven changesets that reverts the manual version of the
> buggy original and redoes it with a painstaking awk script (also checked
> in, for the morbidly curious).
> 
> I'm on the fence about committing in smaller pieces... the policy for
> libtool has always been to make sure the testsuite passes (at least
> on the committer's machine) for every changeset, so that bisecting
> using one of the test cases doesn't fail unexpectedly on another
> commit that was intentionally pushed with know failures.  On the other
> hand, the original was a monster, so I can see the benefits of splitting
> it up a bit too.
> 
> 
>>> I think it will be safer to revert the broken patch, and then
>>> write a script to reapply those changes automatically as I
>>> should have done originally, and only then to merge the result
>>> back to head. If you hold off for a few days, I'll do that as
>>> penance for my sins when I get back to my computer.
>> 
>> I'm not sure a revert of such a big patch is possible after
>> 50-odd more patches? But I was thinking revert too after
>> reading it for a while... Who knows what else hides in
>> there?
> 
> Well, I wrote and applied the script, diffed the results, and
> the testsuite has no regressions for me.  I get a weird failure
> in distcheck which tries to run distclean in _build/tests/cdemo
> aftec removing _build/tests/cdemo/Makefile, that I haven't had
> time to check against current master to see if it is a new regression
> caused by my patch.
> 
>>> But please hold on to your test case to verify that I've made
>>> a better job of things on the do over.
>> 
>> That's easy, on Cygwin:
>>    make check-local TESTSUITEFLAGS="-k Runpath"
>> is supposed to PASS (not FAIL) and
>>    make check-local TESTSUITEFLAGS="-k relpaths"
>> is supposed to XFAIL (not XPASS)
>> 
>> (mentioning it here so that I don't forget myself)
> 
> Cool.  When you have time, please let me know whether the temporary
> branch I made works properly for you.  I'll do some more regression
> testing, and if all goes well, I'll transplant the branch onto
> master.
> 
>> Commit v2.4.2-120-g962aa91
>> syntax-check: fix violations and implement sc_prohibit_test_const_follows_var
>> inadvertenty stomped some comparisons.
>> 
>> * build-aux/ltmain.m4sh (func_mode_compile): Reverse test when checking
>> if non-PIC is attempted in shared libraries.
>> (func_mode_link): Check for dlprefiles, not dlfiles.
>> (func_mode_link): Reverse test so that dependencies are checked when
>> pass_all is not in effect.
>> ---
>> build-aux/ltmain.m4sh |    6 +++---
>> 1 files changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/build-aux/ltmain.m4sh b/build-aux/ltmain.m4sh
>> index 14f3c37..0b8defa 100644
>> --- a/build-aux/ltmain.m4sh
>> +++ b/build-aux/ltmain.m4sh
>> @@ -1350,7 +1350,7 @@ func_mode_compile ()
>>      pic_mode=default
>>      ;;
>>    esac
>> -    if test yes = "$pic_mode" && test pass_all != "$deplibs_check_method"; 
>> then
>> +    if test no = "$pic_mode" && test pass_all != "$deplibs_check_method"; 
>> then
>>      # non-PIC code in shared libraries is not supported
>>      pic_mode=default
>>    fi
>> @@ -5151,7 +5151,7 @@ func_mode_link ()
>>           fi
>> 
>>           # CHECK ME:  I think I busted this.  -Ossama
>> -           if test dlfiles = "$prev"; then
>> +           if test dlprefiles = "$prev"; then
>>             # Preload the old-style object.
>>             func_append dlprefiles " $pic_object"
>>             prev=
>> @@ -6191,7 +6191,7 @@ func_mode_link ()
>>         fi
>>       elif test yes = "$build_libtool_libs"; then
>>         # Not a shared library
>> -         if test pass_all = "$deplibs_check_method"; then
>> +         if test pass_all != "$deplibs_check_method"; then
>>           # We're trying link a shared library against a static one
>>           # but the system doesn't support it.
> 
> 
> My awk script also matches your changes in these hunks, so I'm
> moderately confident that it will have caught any other lurkers too.
> 
> If not, I will branch before 962aa91, run the script, and then apply
> the rest of master to that branch one patch at a time until I arrive
> at a diff that I can apply to master itself, rather than using revert
> as I did on the current temporary branch.
> 
> Cheers,
> -- 
> Gary V. Vaughan (gary AT gnu DOT org)
>