gradle reliability roadmap

[keturn]

Reliable builds during development with support for combined gradle & IntelliJ IDEA.

I have a sort of roadmap for this. Maybe it's the sort of thing that should be a Milestone or have a Project board, but I'll start by writing here so it's someplace besides my head:

• [x] [regression] #4014: transient modules/*.jar
• [x] docs: remove mentions of gradlew idea and iml files from wiki:Developing Modules
• [ ] determine whether gradlew idea and gradlew cleanIdea do harmful things and if we need to disable them
• [ ] gradle terasology-modules plugin. started by #3993.
  • [ ] extract cacheReflections task. work in progress, #4022. also relates to #2445.
  • [ ] make terasology-module plugin not depend on /config/gradle/publish.gradle. Surprisingly enough, it seems to work, but it is unexpected source code organization and a blocker to the plugin being used outside this repository. https://github.com/MovingBlocks/Terasology/blob/957543938f28d703e3175e0ca6f2c5f64da0dd1d/buildSrc/src/main/kotlin/terasology-module.gradle.kts#L89
• [x] #4021: reflections and other work with resources will be easier if engine gets rid of its dev source set, see https://github.com/MovingBlocks/Terasology/issues/3979#issuecomment-633094908
• [ ] make syncAssets task (or copyResourcesToClasses, as it's called in engine's build) gradle-safe. or make unnecessary by getting Gestalt to allow us to register more than one filesystem-directory to a gestalt-module. see #4555, #3957 & https://github.com/MovingBlocks/Terasology/blob/957543938f28d703e3175e0ca6f2c5f64da0dd1d/engine/build.gradle#L277-L278
• [ ] remove outputDir overrides on IDEA config; colliding gradle and IDEA build directories is an anti-goal.
  • [ ] in modules: https://github.com/MovingBlocks/Terasology/blob/957543938f28d703e3175e0ca6f2c5f64da0dd1d/buildSrc/src/main/kotlin/terasology-module.gradle.kts#L235-L236
  • [ ] in engine: https://github.com/MovingBlocks/Terasology/blob/957543938f28d703e3175e0ca6f2c5f64da0dd1d/engine/build.gradle#L301-L302
  • [x] in facade: https://github.com/MovingBlocks/Terasology/blob/957543938f28d703e3175e0ca6f2c5f64da0dd1d/facades/PC/build.gradle#L426-L427
• [x] decouple facade's configuration from engine and module projects. Goal is to improve confidence that engine and all modules will be up-to-date whenever facade's exec targets are launched. Hope is that following multi-project best practices makes this more reliable. See also Decoupled Projects.
• [x] #4245: streamline facade's exec tasks. There is a lot of duplication between them. They can probably also take advantage of the java-application run task.
• [ ] IntelliJ mystery: no longer able to effectively rebuild individual modules? Right click module -> Rebuild (if no previous build directory) may result in nothing building, yet Build -> Build Project builds it fine (item added and observed by Cerv)

Additional Criteria

• Use task configuration avoidance and lazy configuration to keep big multi-project workspaces fast.
• Retain compatibility with run-time reloading of classes (with IDE) and assets (with gestalt).

Optional Side Quests

• [ ] separate module test-dependencies from their application dependencies.
• [ ] publish terasology gradle plugin(s) for use from other repositories.
• [ ] extractNatives - is there a better place for this to be so IntelliJ will recognize it as a dependency and always make sure the libs exist before it runs the facade? does anything else need them other than facades? (e.g. tests, compile-time dependencies)
  • progress toward this in #4347

What you were trying to do

just, like, be consistently able to build and run stuff

What actually happened

I learned more about gradle than I ever wanted to know.

[keturn]

make syncAssets task (or copyResourcesToClasses, as it's called in engine's build) gradle-safe. or make unnecessary by getting Gestalt to allow us to register more than one filesystem-directory to a gestalt-module.

I still have the feeling that gestalt might be the better place to do this. But as many times as gradle has yanked the football out from under me, I'm still less afraid of working with gradle than I am of working on the old gestalt branch.

https://gfycat.com/yawningsplendidargentinehornedfrog

[Cervator]

Updated https://github.com/MovingBlocks/Terasology/wiki/Developing-Modules and checked off its box above

[keturn]

It's also worth having another look at Facade's distribution tasks, as came up in #4141. Something about the use of task properties is wonky, but only turned up as broken under Windows?

I think that changing that distribution task is mostly independent from the module build/dependency resolution stuff that makes up the bulk of the work mentioned in this ticket, so I think it'd be possible to do some work on that without being blocked by the other issues.

[keturn]

FYI: Gradle 6.7 has entered release candidate stage, which usually indicates it will be stable within a week or two.

I haven't seen anything between 6.4 – 6.6 that makes upgrading a priority for us, but I will be interested in grabbing 6.7 before resuming work on the terasology-module gradle plugin in /buildSrc/. We don't exactly depend on the features released in 6.7 but they did a significant amount of work under the hood to support the new Java Toolchain stuff, and that will be relevant for the parts of the build that get a little too involved with the java tasks's implementation details, i.e. #4022.

I don't see anything I expect to give us trouble in the upgrade guide.

[keturn]

decouple facade's configuration from engine and module projects. Goal is to improve confidence that engine and all modules will be up-to-date whenever facade's exec targets are launched. Hope is that following multi-project best practices makes this more reliable.

After #4454, this is in a much better state with regards to how facade & module are organized according to gradle's recommendations.

IntelliJ mystery: no longer able to effectively rebuild individual modules? Right click module -> Rebuild (if no previous build directory) may result in nothing building, yet Build -> Build Project builds it fine (item added and observed by Cerv)

Time to re-test this after all the recent configuration changes.

make syncAssets task (or copyResourcesToClasses, as it's called in engine's build) gradle-safe. or make unnecessary by getting Gestalt to allow us to register more than one filesystem-directory to a gestalt-module.

The upgrade to v7 for gestalt-module and gestalt-asset is on the near-term roadmap now, after which I'd be interested in taking another look at this from the module-loading side.

extract cacheReflections task.

There are persistent rumors about some work in progress that's going to replace our use of Reflections. Not sure of what forum or issue I can reference for the status of that. It might be https://github.com/MovingBlocks/gestalt/pull/83 ? But there's no description on that so I'm not sure if it's the right thing, or only part of the thing, or what.

#4022 got stickier than expected, and there's been no news on the gradle issues it links. (Which doesn't mean nothing has changed in gradle, but if it has nobody realized the change is relevant for those old issues and updated them.) What I'm saying is I'd be happy to have an excuse to drop #4022, but I don't really know if whatever is replacing it is any easier.

[keturn]

Speaking of Reflections, this tidbit came up a while back that will be useful to keep in mind if we do continue to work with them:

WARN  org.reflections.Reflections - given scan urls are empty. set urls in the configuration

Immortus says:

It is a non-error. Specifically the way (that version) of gestalt works is it creates empty reflections objects and then loads up precalculated cache files into them.

[DarkWeird]

@keturn

There are persistent rumors about some work in progress that's going to replace our use of Reflections. Not sure of what forum or issue I can reference for the status of that. It might be MovingBlocks/gestalt#83 ? But there's no description on that so I'm not sure if it's the right thing, or only part of the thing, or what.

Yeah this is it. Really we use Reflections as part of DI.. to detect classes like @RegistrerWorldGenerator, @RegisterSystem, @RegistreTypeHandler, etc.

MovingBlocks/gestalt#83 introduce full-feature compile-time DI. Reflections will replaces with BeanDefinition compile time generated class + service link(like ServiceProvide) Then any interaction with world generation will looks like List<WorldGenerator> Also it will replace @In annotations to contstructor injection or @Inject(jsr-330) fields.

Basic DI is ready. next step integrate it with gestalt-module, gestalt-assets and gestalt-entity-system also try to integrate it to DS or TS. Also DI will available only for gestaltv7, because:

1. we have future task to reintegrate gestaltv7
2. many places of gestalt changed between v5(TS) and v7(current and DS)

[keturn]

Gradle 7.0 has reached RC phase: https://docs.gradle.org/7.0-rc-1/release-notes.html

There are some things in there I expect will be interesting to us. Version catalogs, some performance and type-safety stuff, being able to use included builds for settings-plugins. But I don't see anything I expect to have a big difference on our to-do list here, so I'm in no rush to upgrade to Gradle 7.

I'm a little worried about how well included builds will work when the projects use different versions of gradle, so I suggest cleaning up all the MovingBlocks libraries so they are free of deprecation warnings in the latest version of 6.x before switching Terasology to 7.

I know I've seen at least one thing that pops up sometimes, the warning about overwritten or duplicate files in a copy task, that I think might turn in to a hard failure in gradle 7.

[keturn]

Declare repositories in the settings file instead of every project! https://docs.gradle.org/6.8.2/userguide/declaring_repositories.html#sub:centralized-repository-declaration

Does it work for included builds too? That might be weird, as included builds have their own settings script. But would be useful in the case of our build-logic included build.

[keturn]

After having to coach someone through recovering from a broken workspace recently, Skaldarnar brought up the question again of whether it is worth trying to support IntelliJ's built-in run/build/test functions at all, versus having it delegate everything to gradle always.

With that on my mind as I saw another question about build setup, I went and looked at what the IntelliJ docs say these days.

It might be helpful to use IntelliJ IDEA for building a pure Java or Kotlin project. It could speed up the building process since IntelliJ IDEA supports the incremental build. Yet, keep in mind that the IntelliJ IDEA compiler does not support some parts of the Gradle project build processing and might cause problems in building your project correctly.

That matches up with my recollection of when I started working on all this: after some minor change gradle would just do so much more work to re-compile a class or re-run a test than I expected.

For Terasology, the "IntelliJ IDEA compiler does not support some parts of the Gradle project build processing" means:

• ~~Extra compile-time products like the reflections cache.~~ — we stopped doing that at compile time!
• Things where we've adopted non-standard conventions, like the location of assets (see #4555), which wouldn't be too hard to change if/when we feel like it wouldn't hurt the module developer experience to do so.
  • On the other hand, I think the gestalt.assets.module stuff supports this structure for a development workflow regardless of what the build system does, so maybe it doesn't matter?
• The separation between module path and normal java class path. We do want all those module classes compiled and to stay up-to-date, but we don't want them to be on the classpath.
  • I tried to work around this by adding some "do before launch" options to the run config, but from Jacob's recent experience it sounds like they aren't working well enough. https://github.com/MovingBlocks/Terasology/blob/4a301c32359dcb87aacb0fe9994effcbf4cfc363/.idea/runConfigurations/TerasologyPC.xml#L23-L24

I'm listing those out to get a sense of what we can expect from different tools.

I think, for the purposes of this roadmap, it mostly doesn't matter. We're not going to get rid of gradle. There are a few things on the list that help with handling mixed gradle/IntelliJ workflows, but I think those end up being the same things that will help us get better performance out of gradle.

[keturn]

One actionable question from all this is: Should we stop shipping the run-with-IntelliJ "TerasologyPC" run configurations and replace them with run-with-gradle run configurations?

That'd be a better default for the current state of things.

We'll see later, after getting our gradle config sorted out and upgraded to the current version of gradle with its bells and whistles turned on, whether it's slower than IntelliJ for engine and/or module-development workflow.

[I do want to get back to finishing all this up! It's actually not so far off anymore! I just got diverted by this TaskManager-to-Reactor migration first…]

[Cervator]

I believe there are multiple options nowadays to generate IntelliJ run configs right out of JavaExec Gradle tasks rather than rely on the XML hax. It was one of the many items on the todo list to look into further. I'd be glad to see our customizations shrink further in favor of a cleaner Gradle setup. IntelliJ will likely always have trouble fully fitting a complex Terasology workspace in its head without quirks.