Skip to content

Gradle configuration cache compatibility - #88

Open
ScoreUnder wants to merge 38 commits into
modrinth:masterfrom
ScoreUnder:score-configcache
Open

Gradle configuration cache compatibility#88
ScoreUnder wants to merge 38 commits into
modrinth:masterfrom
ScoreUnder:score-configcache

Conversation

@ScoreUnder

@ScoreUnder ScoreUnder commented Jul 31, 2026

Copy link
Copy Markdown

I've tried to be conservative with the API breakage. I honestly doubt it will break any real users' code but it's still technically breaking in a few places:

  • NamedAdditionalFile is gone, equivalent is TypedFileCollection
  • Consequentially getNamedAdditionalFilesAsList is gone too
  • Constructors in public classes that I really hope nobody is manually constructing have changed
    • NamedDependencyContainer and inner classes (takes a different container type)
    • ModrinthExtension (doesn't take a Project, not because it's especially problematic in and of itself but because I wanted to make sure people avoid accidentally capturing it in a lambda when adding new properties)
    • TemporaryCreateVersion (doesn't take a Project)
  • TaskModrinthSyncBody is newly abstract, so can no longer be instantiated directly with new
  • "Auto-add dependencies" is deprecated and now does nothing, because the properties on the task should cause the dependencies to be added correctly in the first place.

The standard usage of the DSL itself has not changed.

The central idea of this PR is to tear apart ModrinthExtension from its tasks. They live in separate worlds now. Once a task is realised, it should never refer to the project or the ModrinthExtension. Therefore, we need to wire over the properties that each task uses, and we need to move the loader and game version auto-detection to the configuration phase rather than the task execution phase.

Related inclusions, poke me if I need to remove them:

  • Reworked some javadocs, removed a few entirely formulaic ones
  • Deprecated non-lazy versions of methods
  • Made Dependency Serializable. I don't think it's actually needed, but it's more technically correct for how we use it in tasks now
  • Moved validity checks (e.g. file presence, VersionType parsing) as early as possible in tasks so that we don't wait for the Modrinth slug resolution to happen before it throws the error
  • Made GradleException pass through the exception handler untouched in our tasks, so that we keep the custom messages
  • replaceAllreplace for newline conversion as the latter is faster and still replaces all occurrences
  • Logged the files to be uploaded when in debug mode
  • Extracted a lot of task code out to smaller functions
  • Used a fake token when token is unspecified in debug mode
  • Used InvalidUserDataException (subclass of GradleException) for when it's the user's misconfiguration that caused our task to fail
  • Made them both into "untracked" tasks, i.e. telling Gradle there's no way to know if it's "up to date" or not so it should never skip it
  • Replaced javax @Nullable with Jetbrains @Nullable (we had both in the code)

I've only really tried it on the demo projects and on my personal project, so it might need more extensive testing if anyone's able to do that, but otherwise it seems to respect the configuration cache properly.

Fixes #66
Fixes #69
Fixes #75
See also #76
Will conflict with #84, #85

There's no way for gradle to know if it's "stale" or not, so this makes
that explicit, and makes it always run when requested.
- Use lazy accessors and file collections where possible
- Avoid eagerly resolving files at configuration time
- Wire property over from extension to task
This means we don't accidentally make a property which captures it in a
lambda.
- Lazily access them where possible
- Wire them through from extension to task
- Remove now-unused Util file resolution methods
- Change missing file error to display all missing files
replaceAll is the regex one; replace still replaces all occurrences
Expanded out into a form which takes the information it needs without
relying on Project.
Now that it's being used quite explicitly as a task input, probably best
to be on guard for that.
Both encourage practices that are incompatible with configuration
caching.
@ScoreUnder ScoreUnder changed the title Score configcache Gradle configuration cache compatibility Jul 31, 2026
@ScoreUnder

ScoreUnder commented Jul 31, 2026

Copy link
Copy Markdown
Author

Ideas for further work that I thought of while touching this code:

  • If we need a major version bump for this, we should remove deprecated parts of the code and restructure the dependency DSL
    • DependencyDSL shouldn't be a superclass; we should have an action in ModrinthExtension with takes a closure in the right context in both Groovy and Kotlin (I think that might mean two different functions but I haven't looked incredibly deep into that yet just needs a dependencies(Action<? super DependencyDSL> action) on the ModrinthExtension class). Currently the dependencies {} DSL only works in Groovy and only because dependencies was defined by an outer scope, and the actual DSL part only really works due to coincidence and how Groovy looks properties up. The dependencies {} block doesn't work at all in Kotlin.
  • ModrinthExtension should probably be made abstract and use the automatically generated properties rather than fields.
  • If not, at least we can make the fields private? Shouldn't impact anyone unless they wrote a buildSrc in Java referencing it directly, because both Kotlin and Groovy will use the getter if the field is private.
  • Subclasses of NamedDependencyContainer should probably be thrown out, they're just passing a different constant into the super constructor
  • NamedDependency isn't doing anything that Dependency doesn't already do; we could take it out entirely and skip the conversion step.
  • Dependency.toNew should be abstract and implemented in subclasses (and therefore Dependency should be abstract too)
  • Are we doing lombok? If no, delombok the one class. If yes, use it in our existing classes and maybe add the corresponding gradle plugin too (id 'io.freefair.lombok' version '9.5.0')
  • Dependency.toNew should return a CompletableFuture rather than making blocking HTTP calls. (Technically a breaking change; maybe we should keep toNew around as a wrapper which just does return toNewAsync(api).join()). And then we should batch that up when we're resolving project dependencies, so that we don't have to make the requests in serial.
    Though I wish Modrinth provided a bulk ID lookup endpoint :/
    • Actually wait it does. curl 'https://api.modrinth.com/v2/projects?ids=%5B%22shock%22%2C%22shockcraft%22%5D' | jq '.[].id'
    • I kinda want to cache slug->ID conversions too (between projects, between runs, kinda like the maven cache) but I feel like this would way overshoot the effort/reward ratio
  • Might want a BuildService for the Modrinth API client. Maybe overengineering lol
  • We need tests, because without them it feels a bit precarious making larger changes like this. (https://docs.gradle.org/current/userguide/test_kit.html)
  • We might want to change the detection of signature files, because currently they're 3-character strings that could match a lot of false positives. For example if you have a mod you've called "better-signs" or "*ascension" or whatever. Like maybe check .asc/.sig/.gpg instead? Or rely more on endsWith or something.
  • Look into either packaging a new Modrinth4J for this project, or getting upstream to bump a new version, to fold in the changes to CreateVersion.
  • We might actually be able to get away with removing the dependsOn/mustRunAfter entirely, as long as the user isn't hardcoding the path or something. But it is a "sensible default" for those who do things like additionalFiles = [project.file("build/libs/fg-$version-sources.jar")] (to quote the forge example project), so might be worth keeping to defend against that mistake in the common case.
    • the "correct" way of phrasing that file dependency would be additionalFiles = [tasks.named("sourcesJar")] or even better, additionalFiles { sourcesJar tasks.named("sourcesJar") }.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Add Gradle 9 cache support Bump Modrinth4J version Support for Gradle's Configuration cache

1 participant