Skip to content

Commit de8ecf5

Browse files
JonathingGoooler
andauthored
Don't re-add suppressed Gradle API to compileOnly configuration (#1422)
As part of Gradle's experiments with publishing an external Gradle API (gradle/gradle issue 29483), they are beginning to look at suppressing the local Gradle API dependency that is usually used when the `java-gradle-plugin` plugin is applied. Because of this, though, the current strategy that the Shadow plugin uses to move it from `api` to `compileOnly` doesn't check if it exists to begin with. This PR aims to solve that problem. I've added tests as needed. Gradle's Plugin Publish plugin has a similar issue which I've gone into detail in gradle/plugin-portal-requests issue 260. * Add tests for checking presence of Gradle API in dependencies * Clean up tests * Less diff in configureJavaGradlePlugin * Remove afterEvaluate * Apply suggestions from code review * Update changelog * Try `named` * Revert "Try `named`" This reverts commit 0cae38e. --------- Co-authored-by: Goooler <[email protected]>
1 parent a3482b6 commit de8ecf5

File tree

3 files changed

+56
-6
lines changed

3 files changed

+56
-6
lines changed

docs/changes/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
**Changed**
66

7+
- Don't re-add suppressed Gradle API to `compileOnly` configuration. ([#1422](https://github.com/GradleUp/shadow/pull/1422))
78
- Bump the min Gradle requirement to 8.11. ([#1479](https://github.com/GradleUp/shadow/pull/1479))
89
- Expose Ant as `compile` scope. ([#1488](https://github.com/GradleUp/shadow/pull/1488))
910

src/functionalTest/kotlin/com/github/jengelman/gradle/plugins/shadow/JavaPluginTest.kt

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import assertk.assertThat
55
import assertk.assertions.contains
66
import assertk.assertions.containsMatch
77
import assertk.assertions.containsOnly
8+
import assertk.assertions.doesNotContain
89
import assertk.assertions.isEqualTo
910
import assertk.assertions.isFalse
1011
import assertk.assertions.isGreaterThan
@@ -35,6 +36,8 @@ import kotlin.io.path.name
3536
import kotlin.io.path.outputStream
3637
import kotlin.io.path.writeText
3738
import org.gradle.api.plugins.JavaPlugin
39+
import org.gradle.api.plugins.JavaPlugin.API_CONFIGURATION_NAME
40+
import org.gradle.api.plugins.JavaPlugin.COMPILE_ONLY_CONFIGURATION_NAME
3841
import org.gradle.api.tasks.bundling.Jar
3942
import org.gradle.testfixtures.ProjectBuilder
4043
import org.junit.jupiter.api.Test
@@ -489,6 +492,47 @@ class JavaPluginTest : BasePluginTest() {
489492
}
490493
}
491494

495+
@Issue(
496+
"https://github.com/GradleUp/shadow/issues/1422",
497+
)
498+
@Test
499+
fun movesLocalGradleApiToCompileOnly() {
500+
projectScriptPath.writeText(
501+
"""
502+
${getDefaultProjectBuildScript("java-gradle-plugin")}
503+
""".trimIndent() + System.lineSeparator(),
504+
)
505+
506+
val outputCompileOnly = dependencies(COMPILE_ONLY_CONFIGURATION_NAME)
507+
val outputApi = dependencies(API_CONFIGURATION_NAME)
508+
509+
// "unspecified" is the local Gradle API.
510+
assertThat(outputCompileOnly).contains("unspecified")
511+
assertThat(outputApi).doesNotContain("unspecified")
512+
}
513+
514+
@Issue(
515+
"https://github.com/GradleUp/shadow/issues/1422",
516+
)
517+
@ParameterizedTest
518+
@ValueSource(strings = [COMPILE_ONLY_CONFIGURATION_NAME, API_CONFIGURATION_NAME])
519+
fun doesNotReAddSuppressedGradleApi(configuration: String) {
520+
projectScriptPath.writeText(
521+
"""
522+
${getDefaultProjectBuildScript("java-gradle-plugin")}
523+
""".trimIndent() + System.lineSeparator(),
524+
)
525+
526+
val output = dependencies(
527+
configuration = configuration,
528+
// Internal flag added in 8.14 to experiment with suppressing local Gradle API.
529+
"-Dorg.gradle.unsafe.suppress-gradle-api=true",
530+
)
531+
532+
// "unspecified" is the local Gradle API.
533+
assertThat(output).doesNotContain("unspecified")
534+
}
535+
492536
@Issue(
493537
"https://github.com/GradleUp/shadow/issues/1070",
494538
)
@@ -734,4 +778,8 @@ class JavaPluginTest : BasePluginTest() {
734778
containsOnly(*entriesInAB, *manifestEntries)
735779
}
736780
}
781+
782+
private fun dependencies(configuration: String, vararg flags: String): String {
783+
return run("dependencies", "--configuration", configuration, *flags).output
784+
}
737785
}

src/main/kotlin/com/github/jengelman/gradle/plugins/shadow/ShadowJavaPlugin.kt

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -101,15 +101,16 @@ public abstract class ShadowJavaPlugin @Inject constructor(
101101

102102
protected open fun Project.configureJavaGradlePlugin() {
103103
plugins.withType(JavaGradlePluginPlugin::class.java).configureEach {
104+
val gradleApi = dependencies.gradleApi()
104105
// Remove the gradleApi so it isn't merged into the jar file.
105106
// This is required because 'java-gradle-plugin' adds gradleApi() to the 'api' configuration.
106107
// See https://github.com/gradle/gradle/blob/972c3e5c6ef990dd2190769c1ce31998a9402a79/subprojects/plugin-development/src/main/java/org/gradle/plugin/devel/plugins/JavaGradlePluginPlugin.java#L161.
107-
configurations.named(JavaPlugin.API_CONFIGURATION_NAME) {
108-
it.dependencies.remove(dependencies.gradleApi())
109-
}
110-
// Compile only gradleApi() to make sure the plugin can compile against Gradle API.
111-
configurations.named(JavaPlugin.COMPILE_ONLY_CONFIGURATION_NAME) {
112-
it.dependencies.add(dependencies.gradleApi())
108+
configurations.named(JavaPlugin.API_CONFIGURATION_NAME) { api ->
109+
// Only proceed if the removal is successful.
110+
if (!api.dependencies.remove(gradleApi)) return@named
111+
// Compile only gradleApi() to make sure the plugin can compile against Gradle API.
112+
configurations.getByName(JavaPlugin.COMPILE_ONLY_CONFIGURATION_NAME)
113+
.dependencies.add(dependencies.gradleApi())
113114
}
114115
}
115116
}

0 commit comments

Comments
 (0)