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:

• [regression] gradle does not provide modules/*.jar for transient dependencies (regression) #4014: transient modules/*.jar
• 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 Convert the module build script to a precompiled plugin #3993.
  • extract cacheReflections task. work in progress, gradle plugin for cacheReflections #4022. also relates to Modules are tied to "org"-namespace #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.

    Terasology/buildSrc/src/main/kotlin/terasology-module.gradle.kts

    Line 89 in 9575439

    // Grab all the common stuff like plugins to use, artifact repositories, code analysis config, Artifactory settings, Git magic

• refactor: move o.t.benchmarks back in to main source set #4021: reflections and other work with resources will be easier if engine gets rid of its dev source set, see gradle: merge engine-tests back in to engine directory #3979 (comment)
• 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 Move all module resources (assets, deltas, overrides) into src/ #4555, letting EngineModule see its assets #3957 &

  Terasology/engine/build.gradle

  Lines 277 to 278 in 9575439

  	//TODO: Remove it when gestalt will can to handle ProtectionDomain without classes (Resources)
  	task copyResourcesToClasses(type:Copy) {

• remove outputDir overrides on IDEA config; colliding gradle and IDEA build directories is an anti-goal.
  • in modules:

    Terasology/buildSrc/src/main/kotlin/terasology-module.gradle.kts

    Lines 235 to 236 in 9575439

    	outputDir = file("build/classes")
    	testOutputDir = file("build/testClasses")

  • in engine:

    Terasology/engine/build.gradle

    Lines 301 to 302 in 9575439

    	outputDir = file('build/classes')
    	testOutputDir = file('build/testClasses')

  • in facade:
    

    Terasology/facades/PC/build.gradle

    Lines 426 to 427 in 9575439

    	outputDir = file('build/classes')
    	testOutputDir = file('build/testClasses')

• 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.
• Extract "run game" task configurations #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 build(facades.PC): tidy distribution tasks #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 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.

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