jdaugherty commented on PR #16337: URL: https://github.com/apache/grails-core/pull/16337#issuecomment-6002747095
@jamesfredley I'm in favor of the goal of this PR: Forge should be a Grails application. I have two asks: target 8.1.x, and keep Forge as its own build. The conversion doesn't depend on folding Forge into the root build, and I think the fold works against it. ### Target 8.1.x As I asked above, this belongs on 8.1.x. The 8.x lines should ship the Grails version of Forge. If it only lands on 9.0.x, we maintain two Forge implementations: Micronaut on 8.0.x/8.1.x and Grails on 9.0.x. Every Forge fix on 8.x would have to be written twice, and every merge-up from 8.1.x into 9.0.x would have to reconcile two different implementations of the same code. The deploy workflow serves each release line from its own slot (`latest`, `next`, `prev`, and their snapshots), so the hosted generator would also run both implementations side by side. ### Keep Forge as its own build This is the build structure Gradle recommends for this situation, and it's the one we already have: - "Composite builds let you split a large codebase into independent builds that can each be opened and worked on in isolation (for example, in the IDE), while still being buildable together as a whole. This is a common approach in monorepo setups." Gradle describes a "monorepo layout, where an uber-root build knits together a set of independent builds that can also be worked on in isolation." ([Composite Builds](https://docs.gradle.org/current/userguide/composite_builds.html)) - Earlier versions of the same page describe the purpose as letting you "decompose a large multi-project build into smaller, more isolated chunks that can be worked in independently or together as needed" ([Gradle 7.3](https://docs.gradle.org/7.3.3/userguide/composite_builds.html)). - "Each included build is configured in isolation — included builds do not share repositories, plugins, or properties with one another or with the root build" ([Composite Builds](https://docs.gradle.org/current/userguide/composite_builds.html)). - "Included builds are complete Gradle builds and can be opened, worked on, and built independently as standalone projects" and "are treated just like external dependencies, which is a simpler mental model" ([Best Practices for Structuring Builds](https://docs.gradle.org/current/userguide/best_practices_structuring_builds.html)). - Gradle's [Structuring Software Projects sample](https://docs.gradle.org/9.0.0/samples/sample_structuring_software_projects.html) structures "a software product that consists of multiple components as a set of connected Gradle builds". Each application is its own build in the same repository and can be built directly, for example `cd server-application` and `../gradlew :app:bootRun`. - Stefan Oehme's [announcement of composite builds](https://blog.gradle.org/introducing-composite-builds): "With composite builds, you can break your monorepo up into several independent builds within the same repository. Developers can work with the individual builds to get fast turnarounds or work with the whole composite when they want to ensure that everything still plays well together." Gradle draws the line at whether the builds [should remain independent](https://discuss.gradle.org/t/about-composite-builds/22180). Forge should, for four reasons. **1. Forge should consume Grails the way our users do.** If the point is to use Grails ourselves, consuming it the way users do is how we know the Grails we build is the Grails they get. On 9.0.x, Forge declares `platform("org.apache.grails:grails-bom:$projectVersion")`, `org.apache.grails:grails-shell-cli` and `org.apache.grails:grails-core`. Gradle maps them to local projects only because `grails-forge/settings.gradle` has `includeBuild('..')`, so Forge's build can only declare what a user's build can declare. This PR rewrites them as `platform(project(':grails-bom'))`, `project(':grails-core')` and `project(':grails-dependencies-starter-web')`, which no application can declare. Gradle's documentation is clear that local project resolution and published artifacts are not the same thing. "Any time the artifacts and dependencies specified for the default configuration of a project don't match what is published to a repository, the composite build may exhibit different behavior" ([Composite Builds](https://docs.gradle.org/current/userguide/composite_builds.html)). Its [module metadata comparison](https://docs.gradle.org/current/userguide/publishing_gradle_module_metadata.html) lists component capabilities as "Not published" to a POM and notes that "Gradle dependency constraints are transitive, while Maven's dependency management block isn't". The Grails CLI resolves profiles with Maven Resolver, which reads POMs. With coordinates, Forge keeps a way to check the published side: Gradle documents `useGlobalDependencySubstitutionRules = false` to "resolve a published version of a module that is also available as part of an included build", for example "to compar e published and locally built JAR files". With `project(':...')` dependencies, that check means rewriting every dependency. **2. Work on core doesn't have to deal with Forge until it's ready.** Forge will stay on the same Grails version, and a core change that breaks Forge still has to fix it before it merges. What changes is when you have to deal with it. Today the root build doesn't include Forge, so while you're working on a core change locally, `./gradlew build`, the IDE import and every core task leave Forge alone. When the change is ready, you build Forge, fix whatever broke, and open the PR. Folded in, Forge is configured in every core build and imported into every core IDE session. A large core change in progress breaks Forge's compile, and with it `./gradlew build`, from the first commit unless you remember to pass `-PonlyCoreTests`. That is what Gradle means by builds that "can each be opened and worked on in isolation (for example, in the IDE), while still being buildable together as a whole", and what Oehme means by working "with the individual builds to get fast turnarounds" and with "the whole composite when they want to ensure that everything still plays well together". The same announcement names the cost of the alternative: "importing large monorepo projects into an IDE often results in an unresponsive and overwhelming experience." **3. Forge needs to be able to change on its own.** The hosted generator is deployed on demand through its own workflow, separately from framework releases. When Forge needs a build change (a plugin, a repository, a dependency pin or exclusion, a resolution workaround, the deploy packaging), a separate build lets us make and test it in Forge's own settings and build files without changing the framework build. A change to the framework build also can't change Forge without anyone noticing. That is the isolation Gradle describes above: included builds "do not share repositories, plugins, or properties". Folded in, everything Forge needs at the settings level is a change to the core build, and every change to the core build's settings and root rules is a change to Forge. **4. Disabling tasks is not isolation.** In this PR, the root `subprojects {}` block now applies to Forge (for example `ext.grailsCliAutoProvision = false` and the resolution caching rules), and Forge's plugin versions (shadow, sdkvendors, spotless, nohttp) move into the root `settings.gradle`. `-PonlyCoreTests` is another `subprojects {}` block that reaches into each Forge project's `tasks`, and its own comment says: "Disabled tasks still keep the project in the build". Gradle's guidance is against all of this: - "Cross-project configuration can also introduce configuration-time coupling between projects" ([Sharing Build Logic](https://docs.gradle.org/current/userguide/sharing_build_logic_between_subprojects.html)). - "Don't rely on blocks like allprojects {}, subprojects {}, or afterEvaluate {} that are highly dependent on project structure and file layout" ([General Best Practices](https://docs.gradle.org/current/userguide/best_practices_general.html)). - Under [Isolated Projects](https://docs.gradle.org/current/userguide/isolated_projects.html), Gradle's direction for large builds, "build logic, such as build scripts or plugins, applied to a project cannot directly access the mutable state of other projects", and a project's `tasks` are listed as mutable state. ### We have repeatedly been wrong about how our own build resolves Each of these is a case where we believed resolution inside our build matched what users get, and it didn't. In every case, we found out from something that consumed Grails by coordinates, not from the integrated build. - **The shell.** `grails-shell-cli` depends on `project(':grails-bootstrap')` with `requireCapability('org.apache.grails.bootstrap:grails-bootstrap-cli')`. That resolves inside our build. Once published, external consumers couldn't resolve the same capability request (fixed in 0055d02ae9). #15948 was the same kind of break in the published module metadata. This is the capability limitation Gradle documents above. The fixes went into code we ship: `CliPublishingSupport` and the `grails-cli-library` plugin rewrite the published metadata, and `GrailsCliGradlePlugin` has a separate code path (`findProjectCliCompanion`) just for projects in the same build. - **Spring dependency management.** We believed the Grails BOM resolved the same way in our build as in users' builds. The plugin imports BOMs in its own detached configurations, outside Gradle's dependency substitution, and writes its own `<dependencyManagement>` into the POMs it generates ([reference](https://docs.spring.io/dependency-management-plugin/docs/current/reference/html/)), so it didn't. The plugin is gone, but the lesson isn't. Our in-build example of it had to be excluded on unpublished branch versions and in release builds (57883c41e4, d7b393ca72) until it moved to the end-to-end build. While it lived in the core build, it "silently fell back to whatever the Apache snapshot repository happened to hold rather than the working tree" (`end-to-end/README.md`). - **`GRAILS_REPO_URL`.** The CLI, profiles and Forge's generated apps resolve Grails by coordinates. Until we added `GRAILS_REPO_URL`, they pulled whatever snapshot was published instead of the code we had just built (15f56cc4b1, aea7854745, d17aa4961d). - **The functional test apps.** These are the only projects in the root build that consume Grails the way an application does, and they only do it through a substitution layer we wrote by hand in `gradle/functional-test-config.gradle`. That layer handles the BOM, test-fixture variants, CLI companion capabilities, and `evaluationDependsOn` on every subproject. It keeps needing fixes: the CLI split had to teach it about companion capabilities (36ad682bdf), and test-fixture libraries weren't handled until 954e60cec9. - **The end-to-end build** exists because of all this: "It resolves Grails from the artifacts the core build **publishes**, not by project substitution" (`end-to-end/README.md`). Given that record, I'd rather make the architectural decision that guards against the next case than rely on catching it after we publish. That is what Gradle recommends: independent products in independent builds that consume each other by coordinates. ### On the other points - **Forge already configured the root build.** Agreed. That is the cost of the substitution, and I'm fine paying it. What matters is that the edge is a coordinate substitution we control, not a hard project dependency. - **Build files.** Forge already applies the same convention plugins from `build-logic` and the shared scripts under `gradle/`. Moving Forge's `buildSrc` code into `build-logic` is a good change, and it doesn't require the fold. Most of that code is the Rocker plugin. Now that Forge is a Grails app, I'd argue its templates should be GSPs, which would remove the Rocker plugin altogether. That's more cleanup, and the repo shape doesn't decide it either way. The Forge wrapper can go entirely: `./gradlew -p grails-forge <task>` runs Forge's build with the root wrapper, the same way Gradle's sample uses `../gradlew`. That leaves a `settings.gradle` and a `gradle.properties`. - **Test publishing.** The full-publish `dependsOn` lives in the `allprojects` block in `grails-forge/build.gradle`. Scoping it to the two TestKit modules works the same way in a separate build. - **Version alignment.** Forge already takes `projectVersion` and the BOM by coordinate, so it tracks the release it belongs to. Keeping it a separate build doesn't change that. - **Publish job.** By the numbers above, this saves runner time, not total workflow time. That matters less to me than the boundary. So: retarget to 8.1.x and keep Forge behind `grails-forge/settings.gradle` with coordinate dependencies on Grails. Everything else in this PR still works that way: the Micronaut-to-Spring move, the Grails web app, moving Forge's build code into `build-logic`, scoping the TestKit publish, and dropping the Forge wrapper. Kept that way, Forge-as-a-Grails-app consumes Grails the way our users do, which is the point of dogfooding. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
