The String check for "groovy.transform.Immutable" would work fine if the 2.4 compiled class was actually loaded with it;s annotations but AnnotationCollector changes any meta-annotation to no longer be an annotation but a class to store the serialized information for the pre-compiled case. I think we need to perhaps adjust what we do there slightly to leave the original annotation intact. I'll try to work on that on the plane trip home - perhaps we should delay 2.5.3 a few days to see if we can address this but any alternative suggestions welcome.
On Thu, Sep 27, 2018 at 8:14 AM Jochen Theodorou <blackd...@gmx.org> wrote: > On 26.09.2018 12:58, Paul King wrote: > > I shouldn't try to respond to emails while rushing between conference > > sessions. Refreshed my memory and yes, the current provisions for 2.4 > > compatibility don't really help. I'll see if Jochen has some ideas on > > how we could improve that. > > I guess we have to compare > > > https://github.com/apache/groovy/blob/e16728b91601573ee18475ec5dee5355817f670c/src/main/org/codehaus/groovy/transform/ImmutableASTTransformation.java#L738 > > and > > > https://github.com/apache/groovy/blob/9914ae4a0b4273a5a4cb8822699a9b2dbb3c3215/src/main/java/org/codehaus/groovy/transform/ImmutableASTTransformation.java#L352 > > which tells me the knownImmutableClasses part is gone from @Immutable > and the 2.5.x version has no way of getting this information anymore, > since this is supposed to be given directly to the method. > > Bad, but let's continue > > https://github.com/ajoberstar/grgit/issues/237 talks about for example > the Commit.groovy, that can be found here: > > https://github.com/ajoberstar/grgit/blob/master/grgit-core/src/main/groovy/org/ajoberstar/grgit/Commit.groovy > > it does have a knownImmutableClasses=[ZonedDateTime], but looking at > > https://github.com/apache/groovy/blob/GROOVY_2_5_X/src/main/java/org/apache/groovy/ast/tools/ImmutablePropertyUtils.java#L101 > it is in the list of builtinImmutables. > > And the error message is of course not about that, it is about Person: > > https://github.com/ajoberstar/grgit/blob/master/grgit-core/src/main/groovy/org/ajoberstar/grgit/Person.groovy > > And that is where I am actually stuck in understanding the issue... > > > if (field == null || field instanceof Enum || > builtinOrMarkedImmutableClass(field.getClass())) { > > return field; > > } > > this code for the 2.4 check should have returned, because > builtinOrMarkedImmutableClass checks if the class of the value for the > field has an annotation named groovy.transform.Immutable, which is the > case for Person (see > > https://github.com/apache/groovy/blob/9914ae4a0b4273a5a4cb8822699a9b2dbb3c3215/src/main/java/org/apache/groovy/ast/tools/ImmutablePropertyUtils.java#L194 > ) > > It is similar for Branch.groovy... > > So what exactly is not working anymore? I am confused (and it is well > too late here for looking into such an issue). Cedric, Paul, can you > explain what exactly goes wrong? > > bye Jochen >
