> Hi Peter, Ping?
> On 19 Sep 2012, at 21:50, Peter Rosin <p...@lysator.liu.se> 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) >