Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 87DCF200C3A for ; Fri, 31 Mar 2017 12:23:44 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 8621D160B8C; Fri, 31 Mar 2017 10:23:44 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id A0A6B160B80 for ; Fri, 31 Mar 2017 12:23:43 +0200 (CEST) Received: (qmail 60714 invoked by uid 500); 31 Mar 2017 10:23:42 -0000 Mailing-List: contact dev-help@groovy.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@groovy.apache.org Delivered-To: mailing list dev@groovy.apache.org Received: (qmail 60704 invoked by uid 99); 31 Mar 2017 10:23:42 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd4-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 31 Mar 2017 10:23:42 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd4-us-west.apache.org (ASF Mail Server at spamd4-us-west.apache.org) with ESMTP id 1F773C05E9 for ; Fri, 31 Mar 2017 10:23:42 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd4-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: -0.147 X-Spam-Level: X-Spam-Status: No, score=-0.147 tagged_above=-999 required=6.31 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, HTML_MESSAGE=2, KAM_LOTSOFHASH=0.25, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-2.796, RCVD_IN_SORBS_SPAM=0.5, SPF_PASS=-0.001] autolearn=disabled Authentication-Results: spamd4-us-west.apache.org (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com Received: from mx1-lw-eu.apache.org ([10.40.0.8]) by localhost (spamd4-us-west.apache.org [10.40.0.11]) (amavisd-new, port 10024) with ESMTP id 0KsYjggLYSVr for ; Fri, 31 Mar 2017 10:23:40 +0000 (UTC) Received: from mail-vk0-f43.google.com (mail-vk0-f43.google.com [209.85.213.43]) by mx1-lw-eu.apache.org (ASF Mail Server at mx1-lw-eu.apache.org) with ESMTPS id 721855FB9D for ; Fri, 31 Mar 2017 10:23:39 +0000 (UTC) Received: by mail-vk0-f43.google.com with SMTP id s68so85601936vke.3 for ; Fri, 31 Mar 2017 03:23:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=Jn354kvc8W6UKF7HsyjSVcfyRtfbatCG52WdYatZ2QE=; b=i7FysNLK8j7Mry17N3QpeGa5Sa008+wL7AdvzO/WztBEqA7k2o0VXep9IvfmcvM9tX 5RqQiZVBncAGtu8+zb2gqK5mNtuFGt48qUqSqOYJP/6sxK66Ei8mkzGkjU2+fChPEWjA +0IzoOB3DmJYMoy4wgyb1PHJ9PZW2umavEdvReaLo8EdugsZbFmXEPB4ewOh/PVMOPV3 on8F/vqOhbZdIBWVZqt6EBUEuYrZsYJGuqAJ8A8l3OQzVh+jrRIcSS/qFtuwLHu+QHNp nm1LBZ0XNmGhxiEmJkzo5LMPKuxwFlE12hQiml4Tkoj3s1u3Ki94Uh/5aEOWiDKSJ9rU Tp4g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=Jn354kvc8W6UKF7HsyjSVcfyRtfbatCG52WdYatZ2QE=; b=C50yOy9M17gnb33Jw0eyGJMbFNpR8ZKQ5dEXlkbwY8dKLKKt6lKiUsqIbx0JZR+Gik ukSiTYL69lCexZK9YIVaTPdZZlQr2V3K2PoOzh5uIIJxJp+F32UVCr6IlOmBzgD5pXLM YLLVDjDaxd0IinNoXl5+cZHyEEguURYZa/cidQQw/JWofIPdLdNyC5fhlQxoqJfGbMBP GUTsO3U2VYinI4Zgp9eVXxytl4WtPLY48tt3mOSt//k2ZTjDZr5Z0APoxo33s36pfBFu wp7ycFKvkvbldenbk41CxpGusO/EhjTqZ1Nj6URDTm+OOXO8RNfIDe5fKqq8Iu80ggQe MsYw== X-Gm-Message-State: AFeK/H0YtstXkPPYQ8WcTo3kEJ59gJ5NtIQBqyrOAPrgLnMBFDDf5I6vbzRwznOLl4qYVDSbqDmXsLa/45SSTw== X-Received: by 10.159.48.146 with SMTP id j18mr1246167uab.156.1490955818021; Fri, 31 Mar 2017 03:23:38 -0700 (PDT) MIME-Version: 1.0 References: <58DE0FA7.5090206@gmx.org> In-Reply-To: <58DE0FA7.5090206@gmx.org> From: Andrew Bayer Date: Fri, 31 Mar 2017 10:23:27 +0000 Message-ID: Subject: Re: Trying to determine if a change in 2.4.8 was deliberate... To: dev@groovy.apache.org Content-Type: multipart/alternative; boundary=f403045dc5eae8b6e1054c0433c7 archived-at: Fri, 31 Mar 2017 10:23:44 -0000 --f403045dc5eae8b6e1054c0433c7 Content-Type: text/plain; charset=UTF-8 I'll open a JIRA in a bit. Thanks! A. On Fri, Mar 31, 2017 at 1:13 AM Jochen Theodorou wrote: > 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 > > --f403045dc5eae8b6e1054c0433c7 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
I'll open a JIRA in a bit. Thanks!

A.

On Fri, Mar 31, 2017 at 1:13 AM = Jochen Theodorou <blackdrag@gmx.org= > wrote:
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 on= e
> of them easily enough (GlobalClassSet switched to a new collection cla= ss
> for its items field), but the other is hairier. The change in question=
> is
> https://github.com/apache/groovy/commit/0a6789d06cc6451fcfee174ba638c0= 494f2827ef,
> which went into 2.4.8.=C2=A0 What I'm seeing is that
> https://github.com/cloudbees= /groovy-cps/blob/master/src/main/java/com/cloudbees/groovy/cps/sandbox/Defa= ultInvoker.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 ou= r
> transformed code in the inheritor was invoked here. In our CPS context= s,
> that CpsCallableInvocation is the right result, but not here.
>
> Note that this isn't relevant in any other Groovy use case that I&= #39;m
> aware of - it's particular to this usage. Or rather, this is the o= nly
> case I can find where the end result is different. I eventually found<= br class=3D"gmail_msg"> > 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 an= d
> 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/gr= oovy-cps/commit/340ca45ab95af63302bfa7648a9c79aa3f874fa4#diff-31c385af32965= 7115ee24713b9ba928c
> 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<= br class=3D"gmail_msg"> > effect, or a bug? Any thoughts would be much appreciated. =3D)

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

--f403045dc5eae8b6e1054c0433c7--