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 >> > >>> >> > >> >> > >> >