Tweak manifest attribute tests (#1786)

* Replace `isGreaterThan` with `isEqualTo`

* Replace `manifest.attribute` calls

* Revert "Replace `manifest.attribute` calls"

This reverts commit 727403d.

* Replace `manifest.from` calls

* Unit test `inheritManifestAttrsFromJars`

* Unit test `inheritManifestMainClassFromJar`

* Simplify `DefaultInheritManifest`

* Document more about `Configuring the JAR Manifest`

* Update docs/configuration/README.md

---------

Co-authored-by: Copilot <[email protected]>

Author: Goooler

docs/configuration/README.md

@@ -91,17 +91,15 @@ When deploying a shadowed JAR as an execution JAR, it is important to note that
 
 ## Configuring the JAR Manifest
 
-Beyond the automatic configuration of the `Class-Path` entry, the [`ShadowJar`][ShadowJar] manifest is configured in a
-number of ways. First, the manifest for the [`ShadowJar`][ShadowJar] task is configured to __inherit__ from the
-manifest of the standard [`Jar`][Jar] task. This means that any configuration performed on the [`Jar`][Jar] task
-will propagate to the [`ShadowJar`][ShadowJar] tasks.
+The [`ShadowJar`][ShadowJar] manifest is configured in a number of ways. First, the manifest for the `shadowJar` task
+is configured to __inherit__ from the manifest of the standard `jar` task.
 
 === "Kotlin"
 
     ```kotlin
     tasks.jar {
       manifest {
-        attributes["Class-Path"] = "/libs/foo.jar"
+        attributes["Main-Class"] = "my.Main"
       }
     }
     ```
@@ -111,19 +109,19 @@ will propagate to the [`ShadowJar`][ShadowJar] tasks.
     ```groovy
     tasks.named('jar', Jar) {
       manifest {
-        attributes 'Class-Path': '/libs/foo.jar'
+        attributes 'Main-Class': 'my.Main'
       }
     }
     ```
 
-Inspecting the `META-INF/MANIFEST.MF` entry in the JAR file will reveal the following attribute:
+Inspecting the `META-INF/MANIFEST.MF` entry in the JAR files will reveal the following attribute:
 
 ```property
-Class-Path: /libs/foo.jar
+Main-Class: my.Main
 ```
 
-If it is desired to inherit a manifest from a JAR task other than the standard [`Jar`][Jar] task, the `from`
-methods on the `shadowJar.manifest` object can be used to configure the upstream.
+If it is desired to merge a manifest from another [`Jar`][Jar] task, the `manifest.from` methods can be used to
+configure the upstream.
 
 === "Kotlin"
 
@@ -149,7 +147,7 @@ methods on the `shadowJar.manifest` object can be used to configure the upstream
     }
 
     tasks.named('shadowJar', com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar) {
-      manifest.from(testJar.get().manifest)
+      manifest.from testJar.get().manifest
     }
     ```
 

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

@@ -6,7 +6,6 @@ import assertk.assertions.contains
 import assertk.assertions.containsMatch
 import assertk.assertions.doesNotContain
 import assertk.assertions.isEqualTo
-import assertk.assertions.isGreaterThan
 import assertk.assertions.isNotEmpty
 import assertk.assertions.isNotEqualTo
 import assertk.assertions.isNotNull
@@ -512,7 +511,7 @@ class JavaPluginsTest : BasePluginTest() {
     assertThat(outputShadowedJar).useAll {
       transform { actual -> actual.entries().toList().map { it.name }.filter { it.endsWith(".class") } }
         .single().isEqualTo("my/plugin/MyPlugin.class")
-      transform { it.mainAttrSize }.isGreaterThan(0)
+      transform { it.mainAttrSize }.isEqualTo(1)
       // Doesn't contain Gradle classes.
       getMainAttr(classPathAttributeKey).isNull()
 
@@ -687,61 +686,6 @@ class JavaPluginsTest : BasePluginTest() {
     )
   }
 
-  @Test
-  fun inheritManifestAttrsFromJars() {
-    projectScript.appendText(
-      """
-        $jarTask {
-          manifest {
-            attributes 'Foo-Attr': 'Foo-Value'
-          }
-        }
-        def testJar = tasks.register('testJar', Jar) {
-          manifest {
-            attributes 'Bar-Attr': 'Bar-Value'
-          }
-        }
-        $shadowJarTask {
-          manifest.from(testJar.get().manifest)
-        }
-      """.trimIndent(),
-    )
-
-    run(shadowJarPath)
-
-    assertThat(outputShadowedJar).useAll {
-      transform { it.mainAttrSize }.isGreaterThan(2)
-      getMainAttr("Foo-Attr").isEqualTo("Foo-Value")
-      getMainAttr("Bar-Attr").isEqualTo("Bar-Value")
-    }
-  }
-
-  @Test
-  fun inheritManifestMainClassFromJar() {
-    projectScript.appendText(
-      """
-        $jarTask {
-          manifest {
-            attributes '$mainClassAttributeKey': 'my.Main'
-          }
-        }
-        $shadowJarTask {
-          mainClass = 'my.Main2' // This should not override the inherited one.
-        }
-      """.trimIndent(),
-    )
-
-    val result = run(shadowJarPath, infoArgument)
-
-    assertThat(result.output).contains(
-      "Skipping adding $mainClassAttributeKey attribute to the manifest as it is already set.",
-    )
-    assertThat(outputShadowedJar).useAll {
-      transform { it.mainAttrSize }.isGreaterThan(1)
-      getMainAttr("Main-Class").isEqualTo("my.Main")
-    }
-  }
-
   @Test
   fun addExtraFilesViaFrom() {
     val mainClassEntry = writeClass()

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

@@ -10,7 +10,6 @@ import org.gradle.api.internal.project.DefaultProject
 import org.gradle.api.java.archives.Manifest
 import org.gradle.api.java.archives.ManifestMergeSpec
 import org.gradle.api.java.archives.internal.DefaultManifest
-import org.gradle.api.java.archives.internal.DefaultManifestMergeSpec
 
 internal class DefaultInheritManifest(
   project: Project,
@@ -21,28 +20,13 @@ internal class DefaultInheritManifest(
   private val internalManifest: Manifest = manifest ?: DefaultManifest(fileResolver),
 ) : InheritManifest,
   Manifest by internalManifest {
-  private val inheritMergeSpecs = mutableListOf<DefaultManifestMergeSpec>()
 
   override fun inheritFrom(
     vararg inheritPaths: Any,
     action: Action<ManifestMergeSpec>,
   ) {
-    val mergeSpec = DefaultManifestMergeSpec()
-    mergeSpec.from(*inheritPaths)
-    inheritMergeSpecs.add(mergeSpec)
-    action.execute(mergeSpec)
-  }
-
-  override fun getEffectiveManifest(): DefaultManifest {
-    var base = DefaultManifest(fileResolver)
-    inheritMergeSpecs.forEach {
-      base = it.merge(base, fileResolver)
+    inheritPaths.forEach {
+      from(it, action)
     }
-    base.from(internalManifest)
-    return base.effectiveManifest
-  }
-
-  override fun writeTo(path: Any): Manifest = apply {
-    effectiveManifest.writeTo(path)
   }
 }

src/test/kotlin/com/github/jengelman/gradle/plugins/shadow/ShadowPropertiesTest.kt

@@ -18,6 +18,7 @@ import com.github.jengelman.gradle.plugins.shadow.ShadowBasePlugin.Companion.sha
 import com.github.jengelman.gradle.plugins.shadow.internal.applicationExtension
 import com.github.jengelman.gradle.plugins.shadow.internal.javaPluginExtension
 import com.github.jengelman.gradle.plugins.shadow.internal.javaToolchainService
+import com.github.jengelman.gradle.plugins.shadow.internal.mainClassAttributeKey
 import com.github.jengelman.gradle.plugins.shadow.internal.runtimeConfiguration
 import com.github.jengelman.gradle.plugins.shadow.legacy.LegacyShadowPlugin
 import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar.Companion.SHADOW_JAR_TASK_NAME
@@ -31,6 +32,8 @@ import org.gradle.api.plugins.ApplicationPlugin
 import org.gradle.api.plugins.JavaPlugin
 import org.gradle.api.plugins.JavaPlugin.API_CONFIGURATION_NAME
 import org.gradle.api.plugins.JavaPlugin.COMPILE_ONLY_CONFIGURATION_NAME
+import org.gradle.api.tasks.TaskContainer
+import org.gradle.api.tasks.TaskProvider
 import org.gradle.api.tasks.bundling.AbstractArchiveTask
 import org.gradle.api.tasks.bundling.Jar
 import org.gradle.language.base.plugins.LifecycleBasePlugin
@@ -64,6 +67,49 @@ class ShadowPropertiesTest {
     }
   }
 
+  @Test
+  fun inheritManifestAttrsFromJars() = with(project) {
+    plugins.apply(JavaPlugin::class.java)
+    tasks.jar.configure {
+      it.manifest.attributes["jar"] = "fromJar"
+    }
+    val jar1 = tasks.register("jar1", Jar::class.java) {
+      it.manifest.attributes["jar1"] = "fromJar1"
+    }
+    val jar2 = tasks.register("jar2", Jar::class.java) {
+      it.manifest.attributes["jar2"] = "fromJar2"
+    }
+    tasks.shadowJar.configure {
+      it.manifest.attributes["shadowJar"] = "fromShadowJar"
+      it.manifest.from(jar1.get().manifest)
+      @Suppress("DEPRECATION") // TODO: remove this once InheritManifest is removed.
+      it.manifest.inheritFrom(jar2.get().manifest)
+    }
+    // Call effectiveManifest as a way to force merging to happen like writing the jar would.
+    assertThat(tasks.shadowJar.get().manifest.effectiveManifest.attributes).containsOnly(
+      "Manifest-Version" to "1.0",
+      "jar" to "fromJar",
+      "jar1" to "fromJar1",
+      "jar2" to "fromJar2",
+      "shadowJar" to "fromShadowJar",
+    )
+  }
+
+  @Test
+  fun inheritManifestMainClassFromJar() = with(project) {
+    plugins.apply(JavaPlugin::class.java)
+    tasks.jar.configure {
+      it.manifest.attributes[mainClassAttributeKey] = "Main"
+    }
+    tasks.shadowJar.configure {
+      it.mainClass.set("Main2") // This should not override the inherited one from jar.
+    }
+    assertThat(tasks.shadowJar.get().manifest.attributes).containsOnly(
+      "Manifest-Version" to "1.0",
+      mainClassAttributeKey to "Main",
+    )
+  }
+
   @Test
   fun applyJavaPlugin() = with(project) {
     plugins.apply(JavaPlugin::class.java)
@@ -207,5 +253,7 @@ class ShadowPropertiesTest {
     const val VERSION = "1.0.0"
 
     val Task.dependsOnTaskNames: List<String> get() = dependsOn.filterIsInstance<Named>().map(Named::getName)
+
+    val TaskContainer.jar: TaskProvider<Jar> get() = named("jar", Jar::class.java)
   }
 }