groovy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jochen Theodorou <blackd...@gmx.org>
Subject Re: Trying to determine if a change in 2.4.8 was deliberate...
Date Fri, 31 Mar 2017 08:13:27 GMT
On 30.03.2017 19:16, Andrew Bayer wrote:
> I'm working on getting Jenkins' groovy-cps transforming magic working
> with 2.5.0-alpha-1-SNAPSHOT and hit a few problems. I worked around one
> of them easily enough (GlobalClassSet switched to a new collection class
> for its items field), but the other is hairier. The change in question
> is
> https://github.com/apache/groovy/commit/0a6789d06cc6451fcfee174ba638c0494f2827ef,
> which went into 2.4.8.  What I'm seeing is that
> https://github.com/cloudbees/groovy-cps/blob/master/src/main/java/com/cloudbees/groovy/cps/sandbox/DefaultInvoker.java#L32
> is invoking the super's method through the inheritor. As far as I can
> tell - we end up with a CpsCallableInvocation being thrown, meaning our
> transformed code in the inheritor was invoked here. In our CPS contexts,
> that CpsCallableInvocation is the right result, but not here.
>
> Note that this isn't relevant in any other Groovy use case that I'm
> aware of - it's particular to this usage. Or rather, this is the only
> case I can find where the end result is different. I eventually found
> the specific internal behavior that changed, getting us a different
> method to invoke -
> https://github.com/cloudbees/groovy-cps/pull/24#issuecomment-290250394
> has a test case that passes in 2.4.7 and earlier but fails in 2.4.8 and
> later.

The example can be fixed by using A instead of B. But that looks wrong 
to me. I think we have to restore the old behaviour.

> I do have what *seems* to be a workaround at
> https://github.com/abayer/groovy-cps/commit/340ca45ab95af63302bfa7648a9c79aa3f874fa4#diff-31c385af329657115ee24713b9ba928c
> but that feels dirty to me. Is there a better way to do this? Was this
> an intended result of the change, an unintentional but desirable side
> effect, or a bug? Any thoughts would be much appreciated. =)

I think this was not really intended, no. Maybe the fix means a piece of 
code outlifed itself and we did not notice, reactivating it is not right.

For me this is a critical bug

bye Jochen


Mime
View raw message