Skip to content

Make schema ordering safe for the Gradle build cache - #979

Merged
iuliiasobolevska merged 1 commit into
masterfrom
codex/brief-21-schema-content-cache-key
Oct 6, 2026
Merged

iuliiasobolevska merged 1 commit into
masterfrom
codex/brief-21-schema-content-cache-key

Conversation

@iuliiasobolevska

Copy link
Copy Markdown
Collaborator

Summary

generateJava can restore stale output from the build cache because schema parse order depends on absolute filesystem paths and ZIP entry order, while its existing file and classpath fingerprints do not capture those ordering differences. The affected output includes generated field and constructor parameter order.

This change gives Gradle an ordered, relocatable schema input manifest with content hashes, then uses that sequence for generation. Filesystem schemas follow configured root order and root-relative path order. Dependency-jar schemas follow entry-name order and a content-derived jar order. A separate ordered manifest keeps dependency type-mapping precedence in the cache key; the mappings themselves still merge in their original input order. The unreleased changelog calls out fresh-output ordering changes.

Validation

  • Focused SchemaMergingTest (6/6) and GenerateJavaTaskBuildCacheTest (9/9) passed on JDK 17 with the in-process Kotlin compiler. They cover swapped same-name schema contents, configured root reversal, workspace relocation, jar entry and name order, mapping precedence, and --configuration-cache --configuration-cache-problems=fail reuse.
  • :graphql-dgs-codegen-core:formatKotlin, :graphql-dgs-codegen-gradle:formatKotlin, :graphql-dgs-codegen-gradle:validatePlugins, and git diff --check passed.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@iuliiasobolevska
iuliiasobolevska force-pushed the codex/brief-21-schema-content-cache-key branch from 6de6e82 to f328ffd Compare September 30, 2026 18:09
// archive: Gradle's classpath fingerprint ignores entry order, so the zip order must not affect the output.
dependencySchemaSources
.sortedWith(compareBy<DependencySchemaSource> { it.file }.thenBy { it.entryIndex })
.sortedWith(compareBy<DependencySchemaSource> { it.file }.thenBy { it.entryName })

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change makes sense for Gradle caching but with ordering by name it may break existing usages, i.e.

# a-base.graphqls
type Query { foo: Foo }
type Foo { id: String }

# z-legacy.graphqls
extend type Foo { legacy: String }

# b-current.graphqls
extend type Foo { current: String }

where if legacy came before current previously, a constructor taking positional args may switch the positions since b-current.graphqls is written first. Since this is intentional maybe worth a docs update or finding consumers that would break

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I’ve restored the 8.7.0 behavior: codegen reads schemas in ZIP entry order, so existing field and positional constructor order stays the same. Gradle now includes that schema entry order in generateJava’s cache key, which prevents a stale cache hit when a jar is repacked in a different order. The task remains cacheable and can reuse output when the order matches. Checking the key requires scanning entry names, but a cache hit still skips code generation.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed that force-push failed and accidentally merged the PR without the fix - correcting it all in a follow-up #980

@iuliiasobolevska
iuliiasobolevska merged commit c13d054 into master Oct 6, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants