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

[Gary V. Vaughan]

Hi Peter,

Sorry for the delay, it was more complicated than I had imagined :-o

On 28 Sep 2012, at 11:08, "Gary V. Vaughan" <address@hidden> wrote:
> On 27 ก.ย. 2012, at 5:03, Peter Rosin <address@hidden> wrote:
>> On 2012-09-26 14:57, Peter Rosin wrote:
>>> On 2012-09-22 05:31, Gary V. Vaughan wrote:
>> 
>> [Heavily snipped]
>> 
>>> Why not commit the sc_prohibit_test_const_follows_var improvement
>>> last, after fixing all violations?
>> 
>> That doesn't make sense, sorry. But the idea would have worked
>> initially, before the check first existed. I.e., write the check,
>> fixup violations and commit in smaller digestible chunks, then
>> finally commit the actual check preventing any new violations from
>> creeping in.
> 
> Even then, I'm on the fence, since that doesn't leave a revision to
> demonstrate that the syntax-check is finding the violations... on the
> other hand, we also don't want to push "broken" commits since that
> obviously screws up git bisection.

On reflection, I think I prefer to continue to commit a new syntax check
rule along with all the fixes necessary to make it pass.  Although, I'm
horrified by how much unrelated crap crept into 962aa91, and will not be
tackling any patches even close to this size by hand in future :(

>>>> 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.
>>> 
>>> I have to say, given all these difficulties, is it really worth it?
>>> Besides, I think
>> 
>> Hmmm, I said that in the wrong context. Your plan above seems like the
>> best path forward. I guess I was too "excited" about the bugs and didn't
>> read properly, sorry again about my poor communication skills.
> 
> No apologies necessary.  I'm working on the revised redo, which will
> result in a much smaller patch for master later today or tomorrow.

It took me longer than anticipated due to being a lot more work after
untangling all those unrelated parts that I've split out on the branch.

The new branch gary/reredo-test-operand-order (notice the double re) has
everything broken down into digestible chunks.  All the differences between
that and master now look like improvements upon the original hand rolled
version made by my recent scripted revisions, or else outright errors in
the original corrected by the script.  All the errors you flagged on the
list are corrected, as well as several others.

Assuming my running 'make dist' doesn't have any regressions compared to
master, and unless you have additional problems with Windows using the
gary/reredo-test-operand-order branch, I plan to merge the differences
back into master in a day or two.

>> Some of the noise between master and redo-test-operand-order are a bunch
>> of hunks with this pattern:
>> -   if test bar $op $foo; then
>> +   if test bar $op $foo ; then
> 
> Nice catch.  I already tweaked the script regexs to fix that once, but must
> have missed one of the ways for that to happen :(

The script was correctly keeping semi-colons with the same spacing as
was present prior to running the script.  The differences are because
I fixed the errant spacing manually in the original patch.

I'll write another syntax-check, and fix any remaining offenders after
master is fully up to date again.

>> BTW, here is another strange-looking hunk from
>> "git diff master redo-test-operand-order"
>> 
>> diff --git a/m4/libtool.m4 b/m4/libtool.m4
>> index 4413a6d..8ec9beb 100644
>> --- a/m4/libtool.m4
>> +++ b/m4/libtool.m4
>> @@ -5563,7 +5563,7 @@ _LT_EOF
>>      ;;
>>    esac
>> 
>> -    if test sni = "$host_vendor"; then
>> +    if test x$host_vendor = xsni; then
>>      case $host in
>>      sysv4 | sysv4.2uw2* | sysv4.3* | sysv5*)
>>   _LT_TAGVAR(export_dynamic_flag_spec, $1)='$wl-Blargedynsym'
>> 
>> What has caused the difference in this hunk? Why hasn't the script
>> caught this instance? And why isn't the syntax-check triggered?
>> Are the "missing" quotes the key here?
> 
> Not sure... investigating now.

The lack of quotes on both sides was not picked up by my script
nor by the syntax check, although I made the manual changes in the
original patch.  Once the branches are back in sync, I'll beef
up the syntax check and fix any new violations it finds.

Thanks again for keeping me honest with this one.  Please let me
know if you'd like me to wait more than a day or two to give you
time to review.

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