[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] tests: feed -no-undefined when linking libtool libraries
From: |
Gary V. Vaughan |
Subject: |
Re: [PATCH] tests: feed -no-undefined when linking libtool libraries |
Date: |
Fri, 28 Sep 2012 11:08:10 +0700 |
Hi Peter,
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.
>>> 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.
> 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 :(
> You should perhaps add a commit to redo-test-operand-order which silences
> that and makes other more substantial changes stand out more before you
> proceed with the above plan? There are perhaps other harmless changes
> that can be excluded from the "light" revert? Because who needs the
> oscillation?
Agreed. It was all these edge cases that made me abandon the scripted
solution last year. I'm working on improving the script first to catch this and
the other glitches you found.
> 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 check is named sc_prohibit_test_const_follows_var, so one
> would guess that it should prohibit "test x$foo = xbar", no? Or are
> you supposed to be able to dodge the check by needlessly adding x
> in front of LHS $variables?
Not at all. But the long list of substitutions must be interacting
unexpectedly. The idea is for the syntax-check rule to flag any of:
test x"$foo" = "xbar"
test x"$foo" = xbar
test "x$foo" = "xbar"
test "x$foo" = xbar
test x$foo = xbar
where 'x' is one of '.', 'x', or 'X'.
I think we're getting much closer to a working solution though. Many
thanks for taking the time to review.
Cheers,
--
Gary V. Vaughan (gary AT gnu DOT org)
- [PATCH] tests: feed -no-undefined when linking libtool libraries, Peter Rosin, 2012/09/19
- Re: [PATCH] tests: feed -no-undefined when linking libtool libraries, Peter Rosin, 2012/09/19
- Re: [PATCH] tests: feed -no-undefined when linking libtool libraries, Peter Rosin, 2012/09/19
- Re: [PATCH] tests: feed -no-undefined when linking libtool libraries, Gary V. Vaughan, 2012/09/19
- Re: [PATCH] tests: feed -no-undefined when linking libtool libraries, Peter Rosin, 2012/09/19
- Re: [PATCH] tests: feed -no-undefined when linking libtool libraries, Gary V. Vaughan, 2012/09/21
- Re: [PATCH] tests: feed -no-undefined when linking libtool libraries, Peter Rosin, 2012/09/26
- Re: [PATCH] tests: feed -no-undefined when linking libtool libraries, Peter Rosin, 2012/09/26
- Re: [PATCH] tests: feed -no-undefined when linking libtool libraries,
Gary V. Vaughan <=
- Re: [PATCH] tests: feed -no-undefined when linking libtool libraries, Gary V. Vaughan, 2012/09/26
Re: [PATCH] tests: feed -no-undefined when linking libtool libraries, Roumen Petrov, 2012/09/19