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 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..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 @@ -19,10 +19,15 @@ import com.google.common.base.Splitter; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; +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.Optional; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Function; import java.util.jar.JarFile; import java.util.stream.Stream; import javax.annotation.Nullable; @@ -41,12 +46,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 @@ -55,86 +54,141 @@ 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(); } - ImmutableList classpathInfo = 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); - } - } 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; - } - }) - .filter(Objects::nonNull) - .collect(ImmutableList.toImmutableList()); - 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); - return Stream.concat(exports, opens).collect(ImmutableList.toImmutableList()); - } - - 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); - } - - 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); + .flatMap(modulePackagePair -> Stream.of("--add-opens", modulePackagePair + "=ALL-UNNAMED")); } - private static Stream addExportArg(String modulePackagePair) { - return Stream.of("--add-exports", modulePackagePair + "=ALL-UNNAMED"); - } + 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()) { + JavaVersion enablePreviewJavaVersion = info.enablePreview().get(); + if (enablePreviewJavaVersion.equals(runtimeJavaVersion)) { + 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 compared " + + "to the runtime version " + runtimeJavaVersion.getMajorVersion() + ":\n" + + problemJars); + } - private static Stream addOpensArg(String modulePackagePair) { - return Stream.of("--add-opens", modulePackagePair + "=ALL-UNNAMED"); + if (enablePreview.get()) { + return Stream.of("--enable-preview"); + } else { + return Stream.empty(); + } } 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 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() { - return exports().isEmpty() && opens().isEmpty(); + return exports().isEmpty() && opens().isEmpty() && enablePreview().isEmpty(); } default boolean isPresent() { return !isEmpty(); } + 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(); + } + } + + 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) + .map(JavaVersion::toVersion)) + .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(); } 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..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 @@ -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 { @@ -1225,6 +1219,86 @@ 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'() { + createEmptyJar("test.jar", { manifest -> + manifest.getMainAttributes().putValue('Baseline-Enable-Preview', '14') + }) + createUntarBuildFile(buildFile) + buildFile << """ + dependencies { + implementation files("test.jar") + } + tasks.jar.archiveBaseName = "internal" + distribution { + javaVersion 14 + }""".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") + } + + 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 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'() { + 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("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'() { Manifest manifest = new Manifest() manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0") @@ -1332,6 +1406,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