From bc31d68fe9dbed2fdee98ad7495b3b2ab62a1c1a Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Fri, 29 Jul 2022 20:38:19 +0100 Subject: [PATCH 1/8] Parse out the Baseline-Enable-Preview manifest field --- .../JavaServiceDistributionPlugin.java | 14 -- .../gradle/dist/service/tasks/ModuleArgs.java | 136 +++++++++++++----- 2 files changed, 101 insertions(+), 49 deletions(-) diff --git a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/JavaServiceDistributionPlugin.java b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/JavaServiceDistributionPlugin.java index 2ea336d5c..b2a6c188d 100644 --- a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/JavaServiceDistributionPlugin.java +++ b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/JavaServiceDistributionPlugin.java @@ -34,7 +34,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; -import java.util.Collections; import java.util.stream.Collectors; import org.gradle.api.Action; import org.gradle.api.InvalidUserCodeException; @@ -79,19 +78,6 @@ public void apply(Project project) { JavaServiceDistributionExtension distributionExtension = project.getExtensions().create("distribution", JavaServiceDistributionExtension.class, project); - // In baseline 3.52.0 we added this new plugin as a one-liner to add the --enable-preview flag wherever - // necessary (https://github.com/palantir/gradle-baseline/pull/1549). We're using the extraProperties thing to - // avoid taking a compile dependency on baseline. - project.getPlugins().withId("com.palantir.baseline-enable-preview-flag", _withId -> { - distributionExtension.defaultJvmOpts(project.provider(() -> { - Provider enablePreview = (Provider) project.getExtensions() - .getExtraProperties() - .getProperties() - .get("enablePreview"); - return enablePreview.get() ? Collections.singletonList("--enable-preview") : Collections.emptyList(); - })); - }); - Configuration runtimeClasspath = project.getConfigurations().getByName("runtimeClasspath"); Configuration javaAgentConfiguration = project.getConfigurations().create("javaAgent"); diff --git a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleArgs.java b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleArgs.java index 2a7c39c3a..f1029ec9e 100644 --- a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleArgs.java +++ b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleArgs.java @@ -16,14 +16,25 @@ package com.palantir.gradle.dist.service.tasks; +import com.google.common.base.Preconditions; import com.google.common.base.Splitter; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Iterables; +import com.google.common.collect.Maps; +import com.google.common.collect.MultimapBuilder; +import com.google.common.collect.Multimaps; +import java.io.File; import java.io.IOException; +import java.util.Collection; +import java.util.LinkedHashMap; import java.util.List; -import java.util.Objects; +import java.util.Map; +import java.util.Map.Entry; import java.util.Optional; +import java.util.function.Function; import java.util.jar.JarFile; +import java.util.stream.Collectors; import java.util.stream.Stream; import javax.annotation.Nullable; import org.gradle.api.JavaVersion; @@ -41,12 +52,6 @@ */ final class ModuleArgs { - private static final String ADD_EXPORTS_ATTRIBUTE = "Add-Exports"; - private static final String ADD_OPENS_ATTRIBUTE = "Add-Opens"; - - private static final Splitter ENTRY_SPLITTER = - Splitter.on(' ').trimResults().omitEmptyStrings(); - // Exists for backcompat until infrastructure has rolled out with Add-Exports manifest values. // Support safepoint metrics from the internal sun.management package in production. We prefer not // to use '--illegal-access=permit' so that we can avoid unintentional and unbounded illegal access @@ -60,28 +65,31 @@ static ImmutableList collectClasspathArgs( return ImmutableList.of(); } - ImmutableList classpathInfo = classpath.getFiles().stream() - .map(file -> { + Map parsedJarManifests = classpath.getFiles().stream() + .>map(file -> { try { if (file.getName().endsWith(".jar") && file.isFile()) { - try (JarFile jar = new JarFile(file)) { - java.util.jar.Manifest maybeJarManifest = jar.getManifest(); - Optional parsedModuleInfo = parseModuleInfo(maybeJarManifest); - project.getLogger() - .debug("Jar '{}' produced manifest info: {}", file, parsedModuleInfo); - return parsedModuleInfo.orElse(null); - } + Optional parsedModuleInfo = JarManifestModuleInfo.fromJar(file); + project.getLogger().debug("Jar '{}' produced manifest info: {}", file, parsedModuleInfo); + return Maps.immutableEntry(file, parsedModuleInfo.orElse(null)); } else { project.getLogger().info("File {} wasn't a JAR or file", file); } - return null; } catch (IOException e) { project.getLogger().warn("Failed to check jar {} for manifest attributes", file, e); - return null; } + return Maps.immutableEntry(file, null); }) - .filter(Objects::nonNull) - .collect(ImmutableList.toImmutableList()); + .filter(entry -> entry.getValue() != null) + .collect(Collectors.toMap( + Entry::getKey, + Entry::getValue, + (left, right) -> { + throw new UnsupportedOperationException(); + }, + LinkedHashMap::new)); + + Collection classpathInfo = parsedJarManifests.values(); Stream exports = Stream.concat( DEFAULT_EXPORTS.stream(), classpathInfo.stream().flatMap(info -> info.exports().stream())) .distinct() @@ -92,23 +100,41 @@ static ImmutableList collectClasspathArgs( .distinct() .sorted() .flatMap(ModuleArgs::addOpensArg); - return Stream.concat(exports, opens).collect(ImmutableList.toImmutableList()); - } + Stream enablePreview = enablePreview(javaVersion, parsedJarManifests); - private static Optional parseModuleInfo(@Nullable java.util.jar.Manifest jarManifest) { - return Optional.ofNullable(jarManifest) - .map(manifest -> JarManifestModuleInfo.builder() - .exports(readManifestAttribute(manifest, ADD_EXPORTS_ATTRIBUTE)) - .opens(readManifestAttribute(manifest, ADD_OPENS_ATTRIBUTE)) - .build()) - .filter(JarManifestModuleInfo::isPresent); + return Stream.of(exports, opens, enablePreview) + .flatMap(Function.identity()) + .collect(ImmutableList.toImmutableList()); } - private static List readManifestAttribute(java.util.jar.Manifest jarManifest, String attribute) { - return Optional.ofNullable( - Strings.emptyToNull(jarManifest.getMainAttributes().getValue(attribute))) - .map(ENTRY_SPLITTER::splitToList) - .orElseGet(ImmutableList::of); + private static Stream enablePreview( + JavaVersion javaVersion, Map parsedJarManifests) { + Map> enablePreviewFromJar = parsedJarManifests.entrySet().stream() + .filter(entry -> entry.getValue().enablePreview().isPresent()) + .collect(Multimaps.toMultimap( + entry -> JavaVersion.toVersion( + entry.getValue().enablePreview().get()), + Entry::getKey, + () -> MultimapBuilder.hashKeys().arrayListValues().build())) + .asMap(); + + if (enablePreviewFromJar.size() > 1) { + throw new RuntimeException("Unable to add '--enable-preview' because classpath jars have embedded " + + JarManifestModuleInfo.ENABLE_PREVIEW_ATTRIBUTE + " attribute with different versions:\n" + + enablePreviewFromJar); + } + + if (enablePreviewFromJar.size() == 1) { + JavaVersion enablePreviewVersion = Iterables.getOnlyElement(enablePreviewFromJar.keySet()); + Preconditions.checkState( + enablePreviewVersion.equals(javaVersion), + "Runtime java version (" + javaVersion + ") must match version from embedded " + + JarManifestModuleInfo.ENABLE_PREVIEW_ATTRIBUTE + " attribute (" + enablePreviewVersion + + ") from:\n" + Iterables.getOnlyElement(enablePreviewFromJar.values())); + return Stream.of("--enable-preview"); + } + + return Stream.empty(); } private static Stream addExportArg(String modulePackagePair) { @@ -121,20 +147,60 @@ private static Stream addOpensArg(String modulePackagePair) { private ModuleArgs() {} + /** Values extracted from a jar's manifest - see {@link #fromJar}. */ @Value.Immutable interface JarManifestModuleInfo { + Splitter ENTRY_SPLITTER = Splitter.on(' ').trimResults().omitEmptyStrings(); + String ADD_EXPORTS_ATTRIBUTE = "Add-Exports"; + String ADD_OPENS_ATTRIBUTE = "Add-Opens"; + String ENABLE_PREVIEW_ATTRIBUTE = "Baseline-Enable-Preview"; + ImmutableList exports(); ImmutableList opens(); + /** + * Signifies that {@code --enable-preview} should be added at runtime AND the specific java runtime version + * that must be used. (Code compiled with --enable-preview must run on _exactly_ the same java version). + * */ + Optional enablePreview(); + default boolean isEmpty() { - return exports().isEmpty() && opens().isEmpty(); + return exports().isEmpty() && opens().isEmpty() && enablePreview().isEmpty(); } default boolean isPresent() { return !isEmpty(); } + static Optional fromJar(File file) throws IOException { + try (JarFile jar = new JarFile(file)) { + java.util.jar.Manifest maybeJarManifest = jar.getManifest(); + return JarManifestModuleInfo.fromJarManifest(maybeJarManifest); + } + } + + private static Optional fromJarManifest(@Nullable java.util.jar.Manifest jarManifest) { + return Optional.ofNullable(jarManifest) + .map(manifest -> builder() + .exports(readListAttribute(manifest, ADD_EXPORTS_ATTRIBUTE)) + .opens(readListAttribute(manifest, ADD_OPENS_ATTRIBUTE)) + .enablePreview(readOptionalAttribute(manifest, ENABLE_PREVIEW_ATTRIBUTE)) + .build()) + .filter(JarManifestModuleInfo::isPresent); + } + + private static List readListAttribute(java.util.jar.Manifest jarManifest, String attribute) { + return readOptionalAttribute(jarManifest, attribute) + .map(ENTRY_SPLITTER::splitToList) + .orElseGet(ImmutableList::of); + } + + private static Optional readOptionalAttribute(java.util.jar.Manifest jarManifest, String attribute) { + return Optional.ofNullable( + Strings.emptyToNull(jarManifest.getMainAttributes().getValue(attribute))); + } + static Builder builder() { return new Builder(); } From 339e8d35570894df3abefe59efd036f8aabdf701 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Fri, 29 Jul 2022 20:44:22 +0100 Subject: [PATCH 2/8] Cheeky little test --- .../JavaServiceDistributionPluginTests.groovy | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/gradle-sls-packaging/src/test/groovy/com/palantir/gradle/dist/service/JavaServiceDistributionPluginTests.groovy b/gradle-sls-packaging/src/test/groovy/com/palantir/gradle/dist/service/JavaServiceDistributionPluginTests.groovy index c48d3a7e7..cacf5ebce 100644 --- a/gradle-sls-packaging/src/test/groovy/com/palantir/gradle/dist/service/JavaServiceDistributionPluginTests.groovy +++ b/gradle-sls-packaging/src/test/groovy/com/palantir/gradle/dist/service/JavaServiceDistributionPluginTests.groovy @@ -1225,6 +1225,38 @@ class JavaServiceDistributionPluginTests extends GradleIntegrationSpec { actualOpts.get(compilerPairIndex - 1) == "--add-opens" } + def 'applies --enable-preview based on Baseline-Enable-Preview manifest attribute found in classpath jars'() { + Manifest manifest = new Manifest() + manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0") + manifest.getMainAttributes().putValue('Baseline-Enable-Preview', '17') + File testJar = new File(getProjectDir(),"test.jar"); + testJar.withOutputStream { fos -> + new JarOutputStream(fos, manifest).close() + } + createUntarBuildFile(buildFile) + buildFile << """ + dependencies { + implementation files("test.jar") + } + tasks.jar.archiveBaseName = "internal" + distribution { + javaVersion 17 + }""".stripIndent() + file('src/main/java/test/Test.java') << "package test;\npublic class Test {}" + + when: + runTasks(':build', ':distTar', ':untar') + + then: + def actualOpts = OBJECT_MAPPER.readValue( + file('dist/service-name-0.0.1/service/bin/launcher-static.yml'), + LaunchConfigTask.LaunchConfig) + .jvmOpts() + + actualOpts.contains("--enable-preview") + println actualOpts + } + def 'applies opens based on classpath manifests for manifest classpaths'() { Manifest manifest = new Manifest() manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0") From 416ddac8c174d76e083249913fb57e4d6a754d67 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Fri, 29 Jul 2022 21:05:20 +0100 Subject: [PATCH 3/8] Tests and notional docs --- .../gradle/dist/service/tasks/ModuleArgs.java | 11 +-- .../JavaServiceDistributionPluginTests.groovy | 96 ++++++++++++++----- readme.md | 8 ++ 3 files changed, 87 insertions(+), 28 deletions(-) diff --git a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleArgs.java b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleArgs.java index f1029ec9e..9c8da2f14 100644 --- a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleArgs.java +++ b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleArgs.java @@ -109,12 +109,11 @@ static ImmutableList collectClasspathArgs( private static Stream enablePreview( JavaVersion javaVersion, Map parsedJarManifests) { - Map> enablePreviewFromJar = parsedJarManifests.entrySet().stream() + Map> enablePreviewFromJar = parsedJarManifests.entrySet().stream() .filter(entry -> entry.getValue().enablePreview().isPresent()) .collect(Multimaps.toMultimap( - entry -> JavaVersion.toVersion( - entry.getValue().enablePreview().get()), - Entry::getKey, + entry -> entry.getValue().enablePreview().get(), + entry -> entry.getKey().getName(), () -> MultimapBuilder.hashKeys().arrayListValues().build())) .asMap(); @@ -125,9 +124,9 @@ private static Stream enablePreview( } if (enablePreviewFromJar.size() == 1) { - JavaVersion enablePreviewVersion = Iterables.getOnlyElement(enablePreviewFromJar.keySet()); + String enablePreviewVersion = Iterables.getOnlyElement(enablePreviewFromJar.keySet()); Preconditions.checkState( - enablePreviewVersion.equals(javaVersion), + enablePreviewVersion.equals(javaVersion.toString()), "Runtime java version (" + javaVersion + ") must match version from embedded " + JarManifestModuleInfo.ENABLE_PREVIEW_ATTRIBUTE + " attribute (" + enablePreviewVersion + ") from:\n" + Iterables.getOnlyElement(enablePreviewFromJar.values())); diff --git a/gradle-sls-packaging/src/test/groovy/com/palantir/gradle/dist/service/JavaServiceDistributionPluginTests.groovy b/gradle-sls-packaging/src/test/groovy/com/palantir/gradle/dist/service/JavaServiceDistributionPluginTests.groovy index cacf5ebce..464c82ab6 100644 --- a/gradle-sls-packaging/src/test/groovy/com/palantir/gradle/dist/service/JavaServiceDistributionPluginTests.groovy +++ b/gradle-sls-packaging/src/test/groovy/com/palantir/gradle/dist/service/JavaServiceDistributionPluginTests.groovy @@ -22,18 +22,16 @@ import com.palantir.gradle.dist.GradleIntegrationSpec import com.palantir.gradle.dist.SlsManifest import com.palantir.gradle.dist.Versions import com.palantir.gradle.dist.service.tasks.LaunchConfigTask -import org.gradle.testkit.runner.BuildResult - +import java.util.function.Consumer import java.util.jar.Attributes import java.util.jar.JarOutputStream import java.util.jar.Manifest -import spock.lang.Unroll - import java.util.zip.ZipFile +import java.util.zip.ZipOutputStream +import org.gradle.testkit.runner.BuildResult import org.gradle.testkit.runner.TaskOutcome import org.junit.Assert - -import java.util.zip.ZipOutputStream +import spock.lang.Unroll class JavaServiceDistributionPluginTests extends GradleIntegrationSpec { private static final OBJECT_MAPPER = new ObjectMapper(new YAMLFactory()) @@ -1187,13 +1185,9 @@ class JavaServiceDistributionPluginTests extends GradleIntegrationSpec { } def 'applies opens based on classpath manifests'() { - Manifest manifest = new Manifest() - manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0") - manifest.getMainAttributes().putValue('Add-Opens', 'jdk.compiler/com.sun.tools.javac.file') - File testJar = new File(getProjectDir(),"test.jar"); - testJar.withOutputStream { fos -> - new JarOutputStream(fos, manifest).close() - } + createEmptyJar("test.jar", { manifest -> + manifest.getMainAttributes().putValue('Add-Opens', 'jdk.compiler/com.sun.tools.javac.file') + }) createUntarBuildFile(buildFile) buildFile << """ dependencies { @@ -1226,13 +1220,9 @@ class JavaServiceDistributionPluginTests extends GradleIntegrationSpec { } def 'applies --enable-preview based on Baseline-Enable-Preview manifest attribute found in classpath jars'() { - Manifest manifest = new Manifest() - manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0") - manifest.getMainAttributes().putValue('Baseline-Enable-Preview', '17') - File testJar = new File(getProjectDir(),"test.jar"); - testJar.withOutputStream { fos -> - new JarOutputStream(fos, manifest).close() - } + createEmptyJar("test.jar", { manifest -> + manifest.getMainAttributes().putValue('Baseline-Enable-Preview', '14') + }) createUntarBuildFile(buildFile) buildFile << """ dependencies { @@ -1240,7 +1230,7 @@ class JavaServiceDistributionPluginTests extends GradleIntegrationSpec { } tasks.jar.archiveBaseName = "internal" distribution { - javaVersion 17 + javaVersion 14 }""".stripIndent() file('src/main/java/test/Test.java') << "package test;\npublic class Test {}" @@ -1254,7 +1244,58 @@ class JavaServiceDistributionPluginTests extends GradleIntegrationSpec { .jvmOpts() actualOpts.contains("--enable-preview") - println actualOpts + } + + def 'Gives clear errors if Baseline-Enable-Preview manifest attributes conflict'() { + createEmptyJar("test.jar", { manifest -> + manifest.getMainAttributes().putValue('Baseline-Enable-Preview', '17') + }) + createEmptyJar("other.jar", { manifest -> + manifest.getMainAttributes().putValue('Baseline-Enable-Preview', '19') + }) + createUntarBuildFile(buildFile) + buildFile << """ + dependencies { + implementation files("test.jar") + implementation files("other.jar") + } + tasks.jar.archiveBaseName = "internal" + distribution { + javaVersion 17 + }""".stripIndent() + file('src/main/java/test/Test.java') << "package test;\npublic class Test {}" + + when: + BuildResult result = runTasksAndFail(':createLaunchConfig') + + then: + result.output.contains("Unable to add '--enable-preview' because classpath jars have embedded " + + "Baseline-Enable-Preview attribute with different versions:") + result.output.contains("17=[test.jar]") + result.output.contains("19=[other.jar]") + } + + def 'Gives clear errors if Baseline-Enable-Preview version doesn\'t match runtime java version'() { + createEmptyJar("test.jar", { manifest -> + manifest.getMainAttributes().putValue('Baseline-Enable-Preview', '19') + }) + createUntarBuildFile(buildFile) + buildFile << """ + dependencies { + implementation files("test.jar") + } + tasks.jar.archiveBaseName = "internal" + distribution { + javaVersion 17 + }""".stripIndent() + file('src/main/java/test/Test.java') << "package test;\npublic class Test {}" + + when: + BuildResult result = runTasksAndFail(':createLaunchConfig') + + then: + result.output.contains("Runtime java version (14) must match version from embedded Baseline-Enable-Preview attribute (19) from:\n" + + "[test.jar]") } def 'applies opens based on classpath manifests for manifest classpaths'() { @@ -1364,6 +1405,17 @@ class JavaServiceDistributionPluginTests extends GradleIntegrationSpec { fileExists("dist/service-name-0.0.1/service/bin/go-init-${goJavaLauncherVersion}/service/bin") } + private void createEmptyJar(String filename, Consumer manifestInitializer) { + File testJar = new File(getProjectDir(), filename); + + Manifest manifest = new Manifest() + manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0") + manifestInitializer.accept(manifest); + testJar.withOutputStream {fos -> + new JarOutputStream(fos, manifest).close() + } + } + private static createUntarBuildFile(File buildFile) { buildFile << ''' plugins { diff --git a/readme.md b/readme.md index 1d4aefb65..8f52ad7c6 100644 --- a/readme.md +++ b/readme.md @@ -221,6 +221,14 @@ The `go-java-launcher` and `init.sh` launchers additionally append the list of J options typically override earlier options (although this behavior is undefined and may be JVM-specific); this allows users to override the hard-coded options. +Any jar on the classpath that has the following attributes in its Jar manifest will automatically contribute JVM options: + +- `Add-Exports` produces `--add-exports ...=ALL-UNNAMED` +- `Add-Opens` produces `--add-opens ...=ALL-UNNAMED` +- `Baseline-Enable-Preview` produces `--enable-preview` + +_See the [ModuleArgs](gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleArgs.java) class for specifics._ + #### Runtime environment variables Environment variables can be configured through the `env` blocks of `launcher-static.yml` and `launcher-custom.yml` as From 7f7fe579bdf5f5eaddb479a393d9015bcead0ef4 Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Fri, 29 Jul 2022 20:09:06 +0000 Subject: [PATCH 4/8] Add generated changelog entries --- changelog/@unreleased/pr-1365.v2.yml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/@unreleased/pr-1365.v2.yml diff --git a/changelog/@unreleased/pr-1365.v2.yml b/changelog/@unreleased/pr-1365.v2.yml new file mode 100644 index 000000000..dd5be5493 --- /dev/null +++ b/changelog/@unreleased/pr-1365.v2.yml @@ -0,0 +1,6 @@ +type: feature +feature: + description: '`createLaunchConfig` task automatically adds the `--enable-preview` + flag based on the `Baseline-Enable-Preview` jar manifest attribute' + links: + - https://github.com/palantir/sls-packaging/pull/1365 From 14161936da7ce32ac42aba4ebdea3763e7a4b7fc Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Fri, 29 Jul 2022 21:10:44 +0100 Subject: [PATCH 5/8] ep --- .../java/com/palantir/gradle/dist/service/tasks/ModuleArgs.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleArgs.java b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleArgs.java index 9c8da2f14..f31f4d542 100644 --- a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleArgs.java +++ b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleArgs.java @@ -84,7 +84,7 @@ static ImmutableList collectClasspathArgs( .collect(Collectors.toMap( Entry::getKey, Entry::getValue, - (left, right) -> { + (_left, _right) -> { throw new UnsupportedOperationException(); }, LinkedHashMap::new)); From f0059cf10df613fd5161ae6bdfa8b66dd943210d Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Tue, 1 Nov 2022 23:04:50 +0000 Subject: [PATCH 6/8] Simplify --- .../gradle/dist/service/tasks/ModuleArgs.java | 142 ++++++++---------- 1 file changed, 65 insertions(+), 77 deletions(-) diff --git a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleArgs.java b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleArgs.java index f31f4d542..6f5623e82 100644 --- a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleArgs.java +++ b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleArgs.java @@ -16,25 +16,19 @@ package com.palantir.gradle.dist.service.tasks; -import com.google.common.base.Preconditions; import com.google.common.base.Splitter; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; -import com.google.common.collect.Iterables; -import com.google.common.collect.Maps; -import com.google.common.collect.MultimapBuilder; -import com.google.common.collect.Multimaps; import java.io.File; import java.io.IOException; import java.util.Collection; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; import java.util.jar.JarFile; -import java.util.stream.Collectors; import java.util.stream.Stream; import javax.annotation.Nullable; import org.gradle.api.JavaVersion; @@ -60,88 +54,78 @@ final class ModuleArgs { static ImmutableList collectClasspathArgs( Project project, JavaVersion javaVersion, FileCollection classpath) { + + Map parsedJarManifests = new LinkedHashMap<>(); + for (File file : classpath.getFiles()) { + if (file.getName().endsWith(".jar") && file.isFile()) { + Optional parsedModuleInfo = JarManifestModuleInfo.fromJar(project, file); + project.getLogger().debug("Jar '{}' produced manifest info: {}", file, parsedModuleInfo); + if (parsedModuleInfo.isPresent()) { + parsedJarManifests.put(file, parsedModuleInfo.get()); + } + } else { + project.getLogger().info("File {} wasn't a JAR or file", file); + } + } + + return Stream.of( + exportsArgs(parsedJarManifests.values(), javaVersion), + opensArgs(parsedJarManifests.values()), + enablePreviewArg(parsedJarManifests, javaVersion)) + .flatMap(Function.identity()) + .collect(ImmutableList.toImmutableList()); + } + + private static Stream exportsArgs( + Collection classpathInfo, JavaVersion javaVersion) { // --add-exports is unnecessary prior to java 16 if (javaVersion.compareTo(JavaVersion.toVersion("16")) < 0) { - return ImmutableList.of(); + return Stream.empty(); } - Map parsedJarManifests = classpath.getFiles().stream() - .>map(file -> { - try { - if (file.getName().endsWith(".jar") && file.isFile()) { - Optional parsedModuleInfo = JarManifestModuleInfo.fromJar(file); - project.getLogger().debug("Jar '{}' produced manifest info: {}", file, parsedModuleInfo); - return Maps.immutableEntry(file, parsedModuleInfo.orElse(null)); - } else { - project.getLogger().info("File {} wasn't a JAR or file", file); - } - } catch (IOException e) { - project.getLogger().warn("Failed to check jar {} for manifest attributes", file, e); - } - return Maps.immutableEntry(file, null); - }) - .filter(entry -> entry.getValue() != null) - .collect(Collectors.toMap( - Entry::getKey, - Entry::getValue, - (_left, _right) -> { - throw new UnsupportedOperationException(); - }, - LinkedHashMap::new)); - - Collection classpathInfo = parsedJarManifests.values(); - Stream exports = Stream.concat( - DEFAULT_EXPORTS.stream(), classpathInfo.stream().flatMap(info -> info.exports().stream())) + return Stream.concat(DEFAULT_EXPORTS.stream(), classpathInfo.stream().flatMap(info -> info.exports().stream())) .distinct() .sorted() - .flatMap(ModuleArgs::addExportArg); - Stream opens = classpathInfo.stream() + .flatMap(modulePackagePair -> Stream.of("--add-exports", modulePackagePair + "=ALL-UNNAMED")); + } + + private static Stream opensArgs(Collection classpathInfo) { + return classpathInfo.stream() .flatMap(info -> info.opens().stream()) .distinct() .sorted() - .flatMap(ModuleArgs::addOpensArg); - Stream enablePreview = enablePreview(javaVersion, parsedJarManifests); - - return Stream.of(exports, opens, enablePreview) - .flatMap(Function.identity()) - .collect(ImmutableList.toImmutableList()); + .flatMap(modulePackagePair -> Stream.of("--add-opens", modulePackagePair + "=ALL-UNNAMED")); } - private static Stream enablePreview( - JavaVersion javaVersion, Map parsedJarManifests) { - Map> enablePreviewFromJar = parsedJarManifests.entrySet().stream() - .filter(entry -> entry.getValue().enablePreview().isPresent()) - .collect(Multimaps.toMultimap( - entry -> entry.getValue().enablePreview().get(), - entry -> entry.getKey().getName(), - () -> MultimapBuilder.hashKeys().arrayListValues().build())) - .asMap(); - - if (enablePreviewFromJar.size() > 1) { + private static Stream enablePreviewArg( + Map parsedJarManifests, JavaVersion runtimeJavaVersion) { + + AtomicBoolean enablePreview = new AtomicBoolean(false); + Map problemJars = new LinkedHashMap<>(); + + parsedJarManifests.forEach((jar, info) -> { + if (info.enablePreview().isPresent()) { + String enablePreviewJavaVersion = info.enablePreview().get(); + if (enablePreviewJavaVersion.equals(runtimeJavaVersion.getMajorVersion())) { + enablePreview.set(true); + } else { + problemJars.put(jar, enablePreviewJavaVersion); + } + } + }); + + if (!problemJars.isEmpty()) { throw new RuntimeException("Unable to add '--enable-preview' because classpath jars have embedded " - + JarManifestModuleInfo.ENABLE_PREVIEW_ATTRIBUTE + " attribute with different versions:\n" - + enablePreviewFromJar); + + JarManifestModuleInfo.ENABLE_PREVIEW_ATTRIBUTE + " attribute with different versions compared " + + "to the runtime version " + runtimeJavaVersion.getMajorVersion() + ":\n" + + problemJars); } - if (enablePreviewFromJar.size() == 1) { - String enablePreviewVersion = Iterables.getOnlyElement(enablePreviewFromJar.keySet()); - Preconditions.checkState( - enablePreviewVersion.equals(javaVersion.toString()), - "Runtime java version (" + javaVersion + ") must match version from embedded " - + JarManifestModuleInfo.ENABLE_PREVIEW_ATTRIBUTE + " attribute (" + enablePreviewVersion - + ") from:\n" + Iterables.getOnlyElement(enablePreviewFromJar.values())); + if (enablePreview.get()) { return Stream.of("--enable-preview"); + } else { + return Stream.empty(); } - - return Stream.empty(); - } - - private static Stream addExportArg(String modulePackagePair) { - return Stream.of("--add-exports", modulePackagePair + "=ALL-UNNAMED"); - } - - private static Stream addOpensArg(String modulePackagePair) { - return Stream.of("--add-opens", modulePackagePair + "=ALL-UNNAMED"); } private ModuleArgs() {} @@ -159,9 +143,10 @@ interface JarManifestModuleInfo { ImmutableList opens(); /** - * Signifies that {@code --enable-preview} should be added at runtime AND the specific java runtime version - * that must be used. (Code compiled with --enable-preview must run on _exactly_ the same java version). - * */ + * Signifies that {@code --enable-preview} should be added at runtime AND the specific major java runtime + * version that must be used, e.g. "17". (Code compiled with --enable-preview must run on the same major java + * version). + */ Optional enablePreview(); default boolean isEmpty() { @@ -172,10 +157,13 @@ default boolean isPresent() { return !isEmpty(); } - static Optional fromJar(File file) throws IOException { + static Optional fromJar(Project project, File file) { try (JarFile jar = new JarFile(file)) { java.util.jar.Manifest maybeJarManifest = jar.getManifest(); return JarManifestModuleInfo.fromJarManifest(maybeJarManifest); + } catch (IOException e) { + project.getLogger().warn("Failed to check jar {} for manifest attributes", file, e); + return Optional.empty(); } } From a741240f2a94cd64bd0d7f8eb36d8c6ed1e63b71 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Tue, 1 Nov 2022 23:10:13 +0000 Subject: [PATCH 7/8] Fix up the tests --- .../service/JavaServiceDistributionPluginTests.groovy | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/gradle-sls-packaging/src/test/groovy/com/palantir/gradle/dist/service/JavaServiceDistributionPluginTests.groovy b/gradle-sls-packaging/src/test/groovy/com/palantir/gradle/dist/service/JavaServiceDistributionPluginTests.groovy index 464c82ab6..053f9355b 100644 --- a/gradle-sls-packaging/src/test/groovy/com/palantir/gradle/dist/service/JavaServiceDistributionPluginTests.groovy +++ b/gradle-sls-packaging/src/test/groovy/com/palantir/gradle/dist/service/JavaServiceDistributionPluginTests.groovy @@ -1270,9 +1270,9 @@ class JavaServiceDistributionPluginTests extends GradleIntegrationSpec { then: result.output.contains("Unable to add '--enable-preview' because classpath jars have embedded " + - "Baseline-Enable-Preview attribute with different versions:") - result.output.contains("17=[test.jar]") - result.output.contains("19=[other.jar]") + "Baseline-Enable-Preview attribute with different versions compared to the runtime version 14:") + result.output.contains("/test.jar=17") + result.output.contains("/other.jar=19") } def 'Gives clear errors if Baseline-Enable-Preview version doesn\'t match runtime java version'() { @@ -1294,8 +1294,9 @@ class JavaServiceDistributionPluginTests extends GradleIntegrationSpec { BuildResult result = runTasksAndFail(':createLaunchConfig') then: - result.output.contains("Runtime java version (14) must match version from embedded Baseline-Enable-Preview attribute (19) from:\n" + - "[test.jar]") + result.output.contains("Unable to add '--enable-preview' because classpath jars have embedded " + + "Baseline-Enable-Preview attribute with different versions compared to the runtime version 14:") + result.output.contains("/test.jar=19") } def 'applies opens based on classpath manifests for manifest classpaths'() { From 7b0eb60e054a208519d8f9ae92300ec947c88087 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Tue, 1 Nov 2022 23:14:07 +0000 Subject: [PATCH 8/8] nicer parsing --- .../gradle/dist/service/tasks/ModuleArgs.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleArgs.java b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleArgs.java index 6f5623e82..0c495b7ca 100644 --- a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleArgs.java +++ b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleArgs.java @@ -101,12 +101,12 @@ private static Stream enablePreviewArg( Map parsedJarManifests, JavaVersion runtimeJavaVersion) { AtomicBoolean enablePreview = new AtomicBoolean(false); - Map problemJars = new LinkedHashMap<>(); + Map problemJars = new LinkedHashMap<>(); parsedJarManifests.forEach((jar, info) -> { if (info.enablePreview().isPresent()) { - String enablePreviewJavaVersion = info.enablePreview().get(); - if (enablePreviewJavaVersion.equals(runtimeJavaVersion.getMajorVersion())) { + JavaVersion enablePreviewJavaVersion = info.enablePreview().get(); + if (enablePreviewJavaVersion.equals(runtimeJavaVersion)) { enablePreview.set(true); } else { problemJars.put(jar, enablePreviewJavaVersion); @@ -147,7 +147,7 @@ interface JarManifestModuleInfo { * version that must be used, e.g. "17". (Code compiled with --enable-preview must run on the same major java * version). */ - Optional enablePreview(); + Optional enablePreview(); default boolean isEmpty() { return exports().isEmpty() && opens().isEmpty() && enablePreview().isEmpty(); @@ -172,7 +172,8 @@ private static Optional fromJarManifest(@Nullable java.ut .map(manifest -> builder() .exports(readListAttribute(manifest, ADD_EXPORTS_ATTRIBUTE)) .opens(readListAttribute(manifest, ADD_OPENS_ATTRIBUTE)) - .enablePreview(readOptionalAttribute(manifest, ENABLE_PREVIEW_ATTRIBUTE)) + .enablePreview(readOptionalAttribute(manifest, ENABLE_PREVIEW_ATTRIBUTE) + .map(JavaVersion::toVersion)) .build()) .filter(JarManifestModuleInfo::isPresent); }