Add the --enable-preview flag based on the Baseline-Enable-Preview jar manifest attribute

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

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<Boolean> enablePreview = (Provider<Boolean>) 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");
 

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<String> collectClasspathArgs(
             return ImmutableList.of();
         }
 
-        ImmutableList<JarManifestModuleInfo> classpathInfo = classpath.getFiles().stream()
-                .map(file -> {
+        Map<File, JarManifestModuleInfo> parsedJarManifests = classpath.getFiles().stream()
+                .<Entry<File, JarManifestModuleInfo>>map(file -> {
                     try {
                         if (file.getName().endsWith(".jar") && file.isFile()) {
-                            try (JarFile jar = new JarFile(file)) {
-                                java.util.jar.Manifest maybeJarManifest = jar.getManifest();
-                                Optional<JarManifestModuleInfo> parsedModuleInfo = parseModuleInfo(maybeJarManifest);
-                                project.getLogger()
-                                        .debug("Jar '{}' produced manifest info: {}", file, parsedModuleInfo);
-                                return parsedModuleInfo.orElse(null);
-                            }
+                            Optional<JarManifestModuleInfo> 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<JarManifestModuleInfo> classpathInfo = parsedJarManifests.values();
         Stream<String> exports = Stream.concat(
                         DEFAULT_EXPORTS.stream(), classpathInfo.stream().flatMap(info -> info.exports().stream()))
                 .distinct()
@@ -92,23 +100,40 @@ static ImmutableList<String> collectClasspathArgs(
                 .distinct()
                 .sorted()
                 .flatMap(ModuleArgs::addOpensArg);
-        return Stream.concat(exports, opens).collect(ImmutableList.toImmutableList());
-    }
+        Stream<String> enablePreview = enablePreview(javaVersion, parsedJarManifests);
 
-    private static Optional<JarManifestModuleInfo> parseModuleInfo(@Nullable java.util.jar.Manifest jarManifest) {
-        return Optional.ofNullable(jarManifest)
-                .<JarManifestModuleInfo>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<String> 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<String> enablePreview(
+            JavaVersion javaVersion, Map<File, JarManifestModuleInfo> parsedJarManifests) {
+        Map<String, Collection<String>> 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) {
+            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) {
+            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()));
+            return Stream.of("--enable-preview");
+        }
+
+        return Stream.empty();
     }
 
     private static Stream<String> addExportArg(String modulePackagePair) {
@@ -121,20 +146,60 @@ private static Stream<String> 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<String> exports();
 
         ImmutableList<String> 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<String> enablePreview();
+
         default boolean isEmpty() {
-            return exports().isEmpty() && opens().isEmpty();
+            return exports().isEmpty() && opens().isEmpty() && enablePreview().isEmpty();
         }
 
         default boolean isPresent() {
             return !isEmpty();
         }
 
+        static Optional<JarManifestModuleInfo> 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<JarManifestModuleInfo> fromJarManifest(@Nullable java.util.jar.Manifest jarManifest) {
+            return Optional.ofNullable(jarManifest)
+                    .<JarManifestModuleInfo>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<String> readListAttribute(java.util.jar.Manifest jarManifest, String attribute) {
+            return readOptionalAttribute(jarManifest, attribute)
+                    .map(ENTRY_SPLITTER::splitToList)
+                    .orElseGet(ImmutableList::of);
+        }
+
+        private static Optional<String> readOptionalAttribute(java.util.jar.Manifest jarManifest, String attribute) {
+            return Optional.ofNullable(
+                    Strings.emptyToNull(jarManifest.getMainAttributes().getValue(attribute)));
+        }
+
         static Builder builder() {
             return new Builder();
         }

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,85 @@ 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:")
+        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'() {
         Manifest manifest = new Manifest()
         manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0")
@@ -1332,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<Manifest> 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 {

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