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