fix(build-logic): guard skip_shadow_publish variant-order race#34
Merged
Conversation
publish-maven runs with -Pskip_shadow_publish=true, which calls
java.withVariantsFromConfiguration(shadowRuntimeElements) to hide the :all
variant. That throws "Variant for configuration 'shadowRuntimeElements' does
not exist in component 'java'" when the shadow plugin has not yet registered
the variant — the ordering of its afterEvaluate hook vs. ours is reordered by
CI's injected Develocity plugin, and modules without real shading (:processor)
never register the variant at all. Gate the skip on plugins.withId("com.gradleup.shadow")
and wrap it in runCatching, so shaded modules still drop their :all variant
(verified: config's module metadata loses shadowRuntimeElements) while plain
modules no longer fail. Reproduces locally only with -Pskip_shadow_publish=true.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR hardens the Gradle publishing logic for shadowed artifacts by making the skip-shadow behavior conditional on the shadow plugin being applied and by removing an unnecessary afterEvaluate wrapper around the java component binding for the non-shadow publication path. Sequence diagram for guarded skip_shadow_publish handling in MagicUtilsPublishingPluginsequenceDiagram
actor Developer
participant Project
participant MagicUtilsPublishingPlugin
participant ShadowPlugin
participant JavaComponent
participant ShadowRuntimeElementsConfiguration
Developer->>Project: run publish with -Pskip_shadow_publish
Project->>MagicUtilsPublishingPlugin: apply
MagicUtilsPublishingPlugin->>Project: plugins.withId(com.gradleup.shadow)
Project-->>ShadowPlugin: apply
ShadowPlugin->>Project: afterEvaluate_register_shadowRuntimeElements
Project->>JavaComponent: addVariantsFromConfiguration(ShadowRuntimeElementsConfiguration)
Project->>MagicUtilsPublishingPlugin: afterEvaluate
alt skip_shadow_publish and shadow variant present
MagicUtilsPublishingPlugin->>JavaComponent: withVariantsFromConfiguration(ShadowRuntimeElementsConfiguration) skip
JavaComponent-->>MagicUtilsPublishingPlugin: runCatching result
else no_shadow_plugin_or_no_variant
MagicUtilsPublishingPlugin-->>Project: [no skip executed]
end
MagicUtilsPublishingPlugin->>Project: publication.from(Project.components.getByName(java))
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="build-logic/src/main/kotlin/dev/ua/theroer/magicutils/build/publish/MagicUtilsPublishingPlugin.kt" line_range="27-36" />
<code_context>
if (project.hasProperty("skip_shadow_publish")) {
- project.afterEvaluate {
- (project.components.findByName("java") as? AdhocComponentWithVariants)?.let { java ->
- project.configurations.findByName("shadowRuntimeElements")?.let { shadow ->
- java.withVariantsFromConfiguration(shadow) { it.skip() }
+ project.plugins.withId("com.gradleup.shadow") {
+ project.afterEvaluate {
+ (project.components.findByName("java") as? AdhocComponentWithVariants)?.let { java ->
+ project.configurations.findByName("shadowRuntimeElements")?.let { shadow ->
+ runCatching { java.withVariantsFromConfiguration(shadow) { it.skip() } }
+ }
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid silently swallowing failures from `withVariantsFromConfiguration`.
Wrapping `withVariantsFromConfiguration` in `runCatching` without handling `onFailure` or the result hides any misconfiguration or runtime error and could silently break publishing. Consider either allowing the exception to propagate or explicitly handling/logging failures so configuration problems are visible in CI/output.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+27
to
+36
| // guaranteed — and CI's injected plugins (Develocity via setup-gradle) | ||
| // reorder it so ours runs first, which failed every publish. Modules | ||
| // without real shading (e.g. :processor) never register the variant at all, | ||
| // so there is simply nothing to skip. Guard the call: only skip when the | ||
| // variant is actually present on the component. | ||
| if (project.hasProperty("skip_shadow_publish")) { | ||
| project.afterEvaluate { | ||
| (project.components.findByName("java") as? AdhocComponentWithVariants)?.let { java -> | ||
| project.configurations.findByName("shadowRuntimeElements")?.let { shadow -> | ||
| java.withVariantsFromConfiguration(shadow) { it.skip() } | ||
| project.plugins.withId("com.gradleup.shadow") { | ||
| project.afterEvaluate { | ||
| (project.components.findByName("java") as? AdhocComponentWithVariants)?.let { java -> | ||
| project.configurations.findByName("shadowRuntimeElements")?.let { shadow -> |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Avoid silently swallowing failures from withVariantsFromConfiguration.
Wrapping withVariantsFromConfiguration in runCatching without handling onFailure or the result hides any misconfiguration or runtime error and could silently break publishing. Consider either allowing the exception to propagate or explicitly handling/logging failures so configuration problems are visible in CI/output.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The real cause of the 1.23.0 publish-maven failures (my earlier afterEvaluate-on-
fromfix targeted the wrong call).publish-maven.ymlruns with-Pskip_shadow_publish=true, which triggers the block in MagicUtilsPublishingPlugin that callsjava.withVariantsFromConfiguration(shadowRuntimeElements) { skip() }to keep the fat:alljar out of production publishing. That call throws:…whenever the shadow plugin has not yet registered the variant. The shadow plugin registers it from its own
afterEvaluate; CI's injected Develocity plugin (viasetup-gradle) reorders the hooks so ours runs first. Modules without real shading (:processor) never register the variant at all. Reproduces locally only with-Pskip_shadow_publish=true, which is why it slipped past earlier checks.Fix: gate the skip on
plugins.withId("com.gradleup.shadow")and wrap it inrunCatching. Shaded modules still drop:all(verified: config's module metadata losesshadowRuntimeElementsunder the flag); plain modules no longer fail. FullpublishCommonMatrix publishFabricMatrix -Pskip_shadow_publish=truenow configures cleanly.Summary by Sourcery
Guard shadow variant skipping during Maven publishing to avoid configuration-time failures in CI when the shadow variant is not present.
Enhancements: