Skip to content

Add a Gradle Plugin for codegen#1235

Open
adwsingh wants to merge 6 commits into
mainfrom
adwsingh/gradle-plugin
Open

Add a Gradle Plugin for codegen#1235
adwsingh wants to merge 6 commits into
mainfrom
adwsingh/gradle-plugin

Conversation

@adwsingh

Copy link
Copy Markdown
Contributor

What behavior changes?

Adds a Gradle plugin (software.amazon.smithy.java.gradle.smithy-java) that replaces manual smithy-base + afterEvaluate boilerplate with a single plugin application that auto-wires source sets, dependencies, and task ordering.

Why is this change needed?

Every customer project currently duplicates ~30 lines of Gradle plumbing to wire generated code. This plugin makes the zero-config path a one-liner and provides an explicit DSL (modes, generatedPluginOutputs) for customization.

How was this validated?

./gradlew :mcp:mcp-schemas:sourcesJar :mcp:mcp-schemas:jar --rerun-tasks --configuration-cache passes with correct service files in all artifacts. Configuration cache reuse confirmed on subsequent runs.

What should reviewers focus on?

SmithyJavaPlugin.java (dependency wiring via ValueSource, service-file merge with eachFile source-path filtering) and SmithyBuildModesValueSource.java (configuration cache tracked file read).

Additional Links

N/A


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@adwsingh adwsingh requested review from sugmanue and yasmewad June 10, 2026 17:31
@adwsingh adwsingh force-pushed the adwsingh/gradle-plugin branch from b19eccd to 7687f8a Compare June 10, 2026 17:31
Comment thread gradle-plugin/build.gradle.kts Outdated

@Override
public void apply(Project project) {
project.getPlugins().apply(JavaLibraryPlugin.class);

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.

In case of a server I believe this should be just java right?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

JavaBasePlugin.class maybe?

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.

In some cases (e.g. for the client) it makes sense to add java-library because it will create api dependency configuration and for clients that are considered to be published adding smithy-java core dependencies should go to api (rather than implementation). But for the server, that you usually don't publish anywhere, I believe usually java is being used (and it doesn't have api configuration). So the plugin should depend on the mode I guess - java-library for the client/types, java for the server. But even here, in some cases you don't want generated client being published, just having it as a separate module in gradle projects and java should work perfectly fine in this case.

So that being said, probably instead of adding java/java-library it might be better to leave this up to a consumer, and here just react on what they have added:

  • if we're building client/types and java plugin was added - then add dependencies to implementation
  • if we're building client/types and java-library plugin was added - then add dependencies to api
  • if we're building server and java/java-library plugins were added - then add dependencies to implementation
  • if we're building for any more but none of the plugins were added - fail the build

In this case the choice is on the consumer, but the plugin makes correct decisions based on the environment it is in imo.

Set<String> resolved = modes.get();
addIfAbsent(deps, project.getDependencies(), "codegen-plugin", version);
if (resolved.contains("client")) {
addIfAbsent(deps, project.getDependencies(), "client-core", version);

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.

does it need to be here? 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

codegen-plugin only has client-core as compileOnly, and client mode validates Client is present at runtime via Class.forName, so it does need to be present on the smithyBuild classpath. The dependency in the other block is for compiling/consuming the generated source.

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.

Yeah this makes sense, but isn't it that for the clients client-core must be an api-level dependency as the codegen result depends on it and exposes some part of it? smithyBuild inherits implementation that inherits api iirc.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that was my first thought too, but I checked the actual configuration with dependencyInsight and smithyBuild isn’t picking this up from api/implementation in this build. It looks like smithyBuild is on a separate path via compileOnly, so this needs to be present both places.

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.

Hm, that's weird. This is how I set it up in our plugin and everything looks/works good (we only add client-core to api for the client, server-core to implementation for the server and codegen-plugin to smithyBuild for both) 🤔 We're on v1.4.0 of smithy gradle plugins.

I checked the actual configuration with dependencyInsight and smithyBuild isn’t picking this up from api/implementation in this build

I'm curious if smithy-base plugin/task actually does some magic internally to make it work. I just tested our plugin it with removing all our specifics and leaving just "client-core to api and codegen-plugin to smithyBuild" and it seems works fine - smithyBuild succeeds for the client codegen and I can confirm that I don't see client-core being added to smithyBuild configuration (neither explicitly nor implicitly), but it doesn't complain ¯_(ツ)_/¯

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.

@adwsingh if you don't add it to the smithyBuild, are you getting errors during the build? If you can share how you reproduce it, I'm happy to check it further from my side.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My bad, the mistake was using dependencyInsight on smithyBuild as proof of the whole task classpath. It only proves what is in the smithyBuild configuration, not everything the smithyBuild task runs with.

@adwsingh adwsingh Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Turns out this is actually required. What made this confusing is that smithy-base wires more than just the smithyBuild configuration into the task. What exposed it is non-main source sets: jmhSmithyBuild does not get deps from main smithyBuild, so the mode-specific deps need to be added to that target source set’s Smithy config.

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.

jmhSmithyBuild does not get deps from main smithyBuild, so the mode-specific deps need to be added to that target source set’s Smithy config.

I'm curios if this will hide potential issues (e.g. the build succeeds and you can publish the result, but consumers won't be able to use it without tweaking because some of the dependencies weren't set correctly). But I guess that's fine for now, it shouldn't cause too much harm anyway.

addIfAbsent(deps, project.getDependencies(), "client-core", version);
}
if (resolved.contains("server")) {
addIfAbsent(deps, project.getDependencies(), "server-api", version);

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.

shouldn't this be part of implementation instead? 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same rationale as the client-core comment

@adwsingh adwsingh force-pushed the adwsingh/gradle-plugin branch from 7687f8a to cc09ab3 Compare June 10, 2026 21:40
@adwsingh adwsingh force-pushed the adwsingh/gradle-plugin branch 3 times, most recently from 4c661c2 to 60f025e Compare June 11, 2026 21:50
@adwsingh adwsingh requested a review from timocov June 11, 2026 21:51
@adwsingh adwsingh force-pushed the adwsingh/gradle-plugin branch from 60f025e to 7e4b3e6 Compare June 12, 2026 03:29
@adwsingh adwsingh force-pushed the adwsingh/gradle-plugin branch from 7e4b3e6 to e556773 Compare June 12, 2026 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants