No objections. Den tirs. 6. maj 2025 kl. 16.54 skrev James Daugherty <jdaughe...@jdresources.net.invalid>:
> I reached out to security inquiring if our work around would be > acceptable. That will help guide us on the priority of the groovy > changes. > > I haven't seen anyone object to merging the snapshot changes and we can > easily reverse them if needed. I intend to merge these later today unless > there are objections. > > -James > > On Mon, May 5, 2025 at 9:08 PM James Daugherty <jdaughe...@jdresources.net > > > wrote: > > > To go back to security fully, I think we'll need to make a script that > > downloads the jars, builds locally, and then compares. Currently the > > script locally just builds twice and compares. > > > > As a starter, I'll ask security if they're ok with diffing via the > > decompile approach we've been taking. > > > > On Mon, May 5, 2025 at 2:24 PM James Fredley <jamesfred...@apache.org> > > wrote: > > > >> I think we should proceed with updating the ASF security team on the > >> progress so we receive updated feedback. > >> > >> Since we are working on 7.0.0-SNAPSHOT, will most likely need Groovy > >> 4.0.27 for Grails 7.0.0-M4 and already put the Apache Snapshot > Repository > >> in the right locations to pull Grails SNAPSHOTS and Groovy SNAPSHOTS, > I'd > >> be OK with Groovy 4.0.27-SNAPSHOT for the entire build for now and also > OK > >> just for documentation. We ran into issues running an earlier Groovy > >> SNAPSHOT last year, but the root causes for those issues are addressed > in > >> grails-core. > >> > >> I also think it would be fine to merge and then start a new PR once > >> addition updates can be made. > >> > >> On 2025/05/05 16:11:22 James Daugherty wrote: > >> > Paul was able to fix the groovydoc differences via groovy > >> 4.0.27-SNAPSHOT. > >> > I've updated the ticket; we're down to these jars differing: > >> > > >> > grails-cache/build/libs/grails-cache-7.0.0-SNAPSHOT.jar > >> > > >> > grails-datamapping-tck/build/libs/grails-datamapping-tck-7.0.0-SNAPSHOT.jar > >> > grails-fields/build/libs/grails-fields-7.0.0-SNAPSHOT.jar > >> > > >> > grails-gsp/grails-sitemesh3/build/libs/grails-sitemesh3-7.0.0-SNAPSHOT.jar > >> > > >> > grails-gsp/grails-web-gsp-taglib/build/libs/grails-web-gsp-taglib-7.0.0-SNAPSHOT.jar > >> > grails-gsp/plugin/build/libs/grails-gsp-7.0.0-SNAPSHOT.jar > >> > > >> > grails-rest-transforms/build/libs/grails-rest-transforms-7.0.0-SNAPSHOT.jar > >> > > >> grails-scaffolding/build/libs/grails-scaffolding-7.0.0-SNAPSHOT.jar > >> > grails-shell-cli/build/libs/grails-shell-cli-7.0.0-SNAPSHOT.jar > >> > > grails-views-gson/build/libs/grails-views-gson-7.0.0-SNAPSHOT.jar > >> > > >> > grails-views-markup/build/libs/grails-views-markup-7.0.0-SNAPSHOT-javadoc.jar > >> > > >> grails-views-markup/build/libs/grails-views-markup-7.0.0-SNAPSHOT.jar > >> > > >> > grails-web-url-mappings/build/libs/grails-web-url-mappings-7.0.0-SNAPSHOT.jar > >> > > >> grails-wrapper-impl/build/libs/grails-wrapper-impl-7.0.0-SNAPSHOT.jar > >> > > >> > My latest PR is here: > >> https://github.com/apache/grails-core/issues/14679 It > >> > uses the 4.0.27 groovy snasphot to fix the javadoc issue. Instead of > >> just > >> > assigning 4.0.27 for documentation generation, I set it for the entire > >> > project default. This PR also fixes the comparison of the jars in the > >> > grails-gradle project & addresses some documentation issues (one > >> example: > >> > grails data docs aren't fully being generated). > >> > > >> > Of these jars, the differences look due to fields reordering from > traits > >> > (fields have annotations like @TraitBridge on them). I'm guessing > >> there's > >> > some issue related to how groovy processes the traits, but this could > be > >> > something we're doing (I haven't figured out the specifics, but > >> > ApplicationTagLib is the easiest case of this since none of our AST > >> > transforms appear to do anything to that file). I am under the > >> impression > >> > that Paul is continuing to look at this issue to see if he can help or > >> > identify a groovy specific issue. > >> > > >> > My current dilemma is if I should merge this PR. I could scale it > back > >> to > >> > use groovy 4.0.26 for compiling & only use the snapshot for > >> documentation > >> > generation. The problem is if this is a groovy issue, we'll need a > >> groovy > >> > 4.0.27 release anyhow. What are people's thoughts here? Should I > just > >> > leave this on a side branch for now? Should we depend entirely on the > >> > snapshot build? Should only documentation depend on the snapshot > build? > >> > > >> > Alternatively, I think we can respond to ASF security that we can > verify > >> > the build artifacts by using the same process we use to test > >> reproducible > >> > builds. Namely, we can diff any artifact that has a different > checksum > >> via > >> > the decompiler in IntelliJ to confirm the code is the same. This > >> solution > >> > isn't a permanent solution, but for the Milestone / RC builds, I think > >> it's > >> > acceptable. > >> > > >> > -James > >> > > >> > On Mon, Apr 28, 2025 at 10:15 PM James Daugherty < > >> jdaughe...@jdresources.net> > >> > wrote: > >> > > >> > > Hi Everyone, > >> > > > >> > > I summarized my progress thus far under this ticket: > >> > > https://github.com/apache/grails-core/issues/14679 > >> > > > >> > > I checked in several scripts (see ticket) that assist in easily > >> testing > >> > > the jar files for differences. > >> > > > >> > > From what I can tell, there is an ordering issue when multiple, > >> inherited > >> > > traits are involved. I've found a simplified scenario in the > >> grails-gsp > >> > > plugin (ApplicationTagLib) that doesn't appear to be transformed by > >> our > >> > > code in a material way, and yet the difference always deals with the > >> copied > >> > > trait methods being in a random order. I'm at a loss for how to > >> further > >> > > debug this issue. From what I can tell, Grails isn't copying those > >> > > methods. I would assume Groovy is. > >> > > > >> > > The steps to reproduce this: > >> > > 1. etc/bin/test-reproducible-builds.sh > >> > > 2. etc/bin/extract-build-artifact.sh > >> > > grails-gsp/plugin/build/libs/grails-gsp-7.0.0-SNAPSHOT.jar > >> > > 3. select `firstArtifact` and `secondArtifact` in IntelliJ under > >> > > `etc/bin/results` and right click to select `Compare Directories` > >> > > > >> > > I intend to send an email to the groovy dev list to see if they can > >> help > >> > > me understand how I can debug this further. > >> > > > >> > > If we can't resolve these issues fully in Grails 7, the good news is > >> the > >> > > scripts I've checked in can be used to "verify" that the jars were > >> built > >> > > correctly - we can build locally and manually diff the artifacts to > >> confirm > >> > > they're correct. This is the work around I plan to suggest to ASF > >> security. > >> > > > >> > > -James > >> > > > >> > > > >> > > > >> > > > >> > > On Mon, Apr 28, 2025 at 11:58 AM James Daugherty < > >> > > jdaughe...@jdresources.net> wrote: > >> > > > >> > >> Thank you Jochen. I believe I've switched to order stable > >> collections > >> > >> throughout the AST code base. Unfortunately, I'm still seeing > >> issues. > >> > >> > >> > >> I've narrowed the differences down to the following: > >> > >> > >> > grails-async/core/build/libs/grails-async-core-7.0.0-SNAPSHOT-javadoc.jar > >> > >> > >> grails-bootstrap/build/libs/grails-bootstrap-7.0.0-SNAPSHOT-javadoc.jar > >> > >> grails-cache/build/libs/grails-cache-7.0.0-SNAPSHOT.jar > >> > >> > >> > grails-converters/build/libs/grails-converters-7.0.0-SNAPSHOT-javadoc.jar > >> > >> grails-core/build/libs/grails-core-7.0.0-SNAPSHOT-javadoc.jar > >> > >> > >> > >> > >> > grails-data-hibernate5/core/build/libs/grails-data-hibernate5-core-7.0.0-SNAPSHOT-javadoc.jar > >> > >> > >> > >> > >> > grails-data-hibernate5/dbmigration/build/libs/grails-data-hibernate5-dbmigration-7.0.0-SNAPSHOT-javadoc.jar > >> > >> > >> > >> > >> > grails-datamapping-core/build/libs/grails-datamapping-core-7.0.0-SNAPSHOT-javadoc.jar > >> > >> > >> > >> > >> > grails-datamapping-tck-domains/build/libs/grails-datamapping-tck-domains-7.0.0-SNAPSHOT.jar > >> > >> > >> > >> > >> > grails-datamapping-tck-tests/build/libs/grails-datamapping-tck-tests-7.0.0-SNAPSHOT.jar > >> > >> > >> > >> > >> > grails-datamapping-validation/build/libs/grails-datamapping-validation-7.0.0-SNAPSHOT-javadoc.jar > >> > >> > >> > >> > >> > grails-datastore-core/build/libs/grails-datastore-core-7.0.0-SNAPSHOT-javadoc.jar > >> > >> grails-fields/build/libs/grails-fields-7.0.0-SNAPSHOT.jar > >> > >> > grails-gsp/core/build/libs/grails-gsp-core-7.0.0-SNAPSHOT-javadoc.jar > >> > >> > >> > grails-gsp/grails-sitemesh3/build/libs/grails-sitemesh3-7.0.0-SNAPSHOT.jar > >> > >> > >> > >> > >> > grails-gsp/grails-taglib/build/libs/grails-taglib-7.0.0-SNAPSHOT-javadoc.jar > >> > >> > >> > >> > >> > grails-gsp/grails-web-gsp-taglib/build/libs/grails-web-gsp-taglib-7.0.0-SNAPSHOT.jar > >> > >> > >> > >> > >> > grails-gsp/grails-web-gsp/build/libs/grails-web-gsp-7.0.0-SNAPSHOT-javadoc.jar > >> > >> grails-gsp/plugin/build/libs/grails-gsp-7.0.0-SNAPSHOT.jar > >> > >> > >> > >> > >> > grails-rest-transforms/build/libs/grails-rest-transforms-7.0.0-SNAPSHOT.jar > >> > >> grails-scaffolding/build/libs/grails-scaffolding-7.0.0-SNAPSHOT.jar > >> > >> > >> grails-shell-cli/build/libs/grails-shell-cli-7.0.0-SNAPSHOT-javadoc.jar > >> > >> grails-shell-cli/build/libs/grails-shell-cli-7.0.0-SNAPSHOT.jar > >> > >> > >> grails-test-core/build/libs/grails-test-core-7.0.0-SNAPSHOT-javadoc.jar > >> > >> > >> > grails-views-core/build/libs/grails-views-core-7.0.0-SNAPSHOT-javadoc.jar > >> > >> grails-views-gson/build/libs/grails-views-gson-7.0.0-SNAPSHOT.jar > >> > >> > >> > >> > >> > grails-views-markup/build/libs/grails-views-markup-7.0.0-SNAPSHOT-javadoc.jar > >> > >> > grails-views-markup/build/libs/grails-views-markup-7.0.0-SNAPSHOT.jar > >> > >> > >> > grails-web-common/build/libs/grails-web-common-7.0.0-SNAPSHOT-javadoc.jar > >> > >> > >> > >> > >> > grails-web-url-mappings/build/libs/grails-web-url-mappings-7.0.0-SNAPSHOT-javadoc.jar > >> > >> > >> > >> > >> > grails-web-url-mappings/build/libs/grails-web-url-mappings-7.0.0-SNAPSHOT.jar > >> > >> > >> > >> Since the javadoc jars now make up 13 of the remaining 26 jar > >> > >> differences, I was hoping someone can help here: > >> > >> > >> > >> The javadoc jars above are interesting because the source files & > >> > >> compiled classes no longer differ for projects like `grails-core`. > >> These > >> > >> jars actually contain the groovydoc (for java + groovy classes). > In > >> the > >> > >> case of `grails-core`, the example difference is the > >> > >> groovy.lang.Script#getBinding() and groovy.lang.Script#setBinding() > >> methods > >> > >> are reordered. > >> > >> Since this is groovydoc generating this, do you know if there are > >> known > >> > >> defined orders in groovydoc? This is from the gradle task, so I > >> assume > >> > >> this is being done with groovy 3's groovydoc. Is this something > >> addressed > >> > >> in groovydoc under groovy 4? > >> > >> > >> > >> The source file in question is rather simple. The methods being > >> > >> reordered are from the parent class (which is in Groovy instead of > >> > >> Grails). Here's the source: > >> > >> > >> > >> @CompileStatic > >> > >> abstract class SettingsFile extends Script { > >> > >> > >> > >> > >> > >> void include(String[] projectPaths) { > >> > >> binding.setVariable("projectPaths", projectPaths) > >> > >> } > >> > >> > >> > >> void includeFlat(String[] projectPaths) { > >> > >> binding.setVariable("projectPaths", projectPaths) > >> > >> } > >> > >> > >> > >> def methodMissing(String name, args) { > >> > >> // ignore > >> > >> } > >> > >> > >> > >> def propertyMissing(String name) { > >> > >> // ignore > >> > >> } > >> > >> } > >> > >> > >> > >> > >> > >> > >> > >> On Sat, Apr 26, 2025 at 5:32 AM Jochen Theodorou > >> > >> <blackd...@gmx.org.invalid> wrote: > >> > >> > >> > >>> On 24.04.25 23:06, James Daugherty wrote: > >> > >>> [...] > >> > >>> > I'm planning to continue researching this, but if anyone has > >> experience > >> > >>> > with the AST transforms or the ordering problem, it would be > >> helpful to > >> > >>> > solve this. This is probably the largest blocker to us > proceeding > >> > >>> with an > >> > >>> > ASF release. > >> > >>> > >> > >>> as for the ordering. Groovy wants in general to use the order of > >> > >>> declaration in the source code. What we are talking about are most > >> > >>> likely methods added by transforms. And if it is one time stable > >> and one > >> > >>> time not, then I guess Maps are involved. ClassNode has several > >> > >>> getDeclaredMethodsXY methods, which are based on a methods maps, > >> while > >> > >>> getMethods or getMethodsList is based on the methods list. > >> > >>> > >> > >>> If you then have something like > >> > >>> > >> > >>> classNode.getAllDeclaredMethods().findAll(it annnotated with > >> XY).each > >> > >>> {doSomethingWith(it)} > >> > >>> > >> > >>> and there is more than one method annotated, then the order might > be > >> > >>> broken between builds. Just to give one example. > >> > >>> > >> > >>> > >> > >>> I would suggest to go through all transforms and check on the > usage > >> of > >> > >>> the getDeclaredMethods methods and then avoid them to use for > >> iteration. > >> > >>> They are ok for lookup of course. > >> > >>> > >> > >>> bye Jochen > >> > >>> > >> > >> > >> > > >> > > > -- Med venlig hilsen, Søren Berg Glasius Hedevej 1, Gl. Rye, 8680 Ry Mobile: +45 40 44 91 88 --- Press ESC once to quit - twice to save the changes.
