diff --git a/changelog/@unreleased/pr-1215.v2.yml b/changelog/@unreleased/pr-1215.v2.yml new file mode 100644 index 000000000..f26d9262a --- /dev/null +++ b/changelog/@unreleased/pr-1215.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: Apply `--add-exports` to Java 16+ services based on manifest entries + links: + - https://github.com/palantir/sls-packaging/pull/1215 diff --git a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/LaunchConfigTask.java b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/LaunchConfigTask.java index 7de895962..ff72d9bca 100644 --- a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/LaunchConfigTask.java +++ b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/LaunchConfigTask.java @@ -60,11 +60,6 @@ public abstract class LaunchConfigTask extends DefaultTask { ImmutableList.of("-XX:+ShowCodeDetailsInExceptionMessages"); private static final ImmutableList java15Options = ImmutableList.of("-XX:+UnlockDiagnosticVMOptions", "-XX:+ExpandSubTypeCheckAtParseTime"); - // 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 - // that we aren't aware of. - private static final ImmutableList java16PlusOptions = - ImmutableList.of("--add-exports", "java.management/sun.management=ALL-UNNAMED"); private static final ImmutableList disableBiasedLocking = ImmutableList.of("-XX:-UseBiasedLocking"); private static final ImmutableList alwaysOnJvmOptions = ImmutableList.of( @@ -199,10 +194,7 @@ public final void createConfig() throws IOException { javaVersion.get().compareTo(JavaVersion.toVersion("15")) < 0 ? disableBiasedLocking : ImmutableList.of()) - .addAllJvmOpts( - javaVersion.get().compareTo(JavaVersion.toVersion("16")) >= 0 - ? java16PlusOptions - : ImmutableList.of()) + .addAllJvmOpts(ModuleExports.getExports(getProject(), javaVersion.get(), getClasspath())) .addAllJvmOpts(gcJvmOptions.get()) .addAllJvmOpts(defaultJvmOpts.get()) .putAllEnv(defaultEnvironment) diff --git a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleExports.java b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleExports.java new file mode 100644 index 000000000..5d96b9554 --- /dev/null +++ b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/service/tasks/ModuleExports.java @@ -0,0 +1,89 @@ +/* + * (c) Copyright 2021 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.gradle.dist.service.tasks; + +import com.google.common.base.Splitter; +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableList; +import java.io.IOException; +import java.util.jar.JarFile; +import java.util.stream.Stream; +import org.gradle.api.JavaVersion; +import org.gradle.api.Project; +import org.gradle.api.file.FileCollection; + +/** + * This utility class reuses the {@code Add-Exports} manifest entry defined in + * JEP-261 to collect required exports + * from the runtime classpath so they may be applied to the static configuration. + * + * Note that this mirrors {@code Add-Exports} plumbing from + * gradle-baseline#1944. + */ +final class ModuleExports { + + private static final String ADD_EXPORTS_ATTRIBUTE = "Add-Exports"; + private static final Splitter EXPORT_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 + // that we aren't aware of. + private static final ImmutableList DEFAULT_EXPORTS = ImmutableList.of("java.management/sun.management"); + + static ImmutableList getExports(Project project, JavaVersion javaVersion, FileCollection classpath) { + // --add-exports is unnecessary prior to java 16 + if (javaVersion.compareTo(JavaVersion.toVersion("16")) < 0) { + return ImmutableList.of(); + } + + return Stream.concat( + classpath.getFiles().stream().flatMap(file -> { + try { + if (file.getName().endsWith(".jar") && file.isFile()) { + try (JarFile jar = new JarFile(file)) { + String value = jar.getManifest() + .getMainAttributes() + .getValue(ADD_EXPORTS_ATTRIBUTE); + if (Strings.isNullOrEmpty(value)) { + return Stream.empty(); + } + project.getLogger() + .debug( + "Found manifest entry {}: {} in jar {}", + ADD_EXPORTS_ATTRIBUTE, + value, + file); + return EXPORT_SPLITTER.splitToStream(value); + } + } + return Stream.empty(); + } catch (IOException e) { + project.getLogger().warn("Failed to check jar {} for manifest attributes", file, e); + return Stream.empty(); + } + }), + DEFAULT_EXPORTS.stream()) + .distinct() + .sorted() + .flatMap(modulePackagePair -> Stream.of("--add-exports", modulePackagePair + "=ALL-UNNAMED")) + .collect(ImmutableList.toImmutableList()); + } + + private ModuleExports() {} +} 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 a9ac07d5f..f9027091b 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,6 +22,10 @@ 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 java.util.jar.Attributes +import java.util.jar.JarOutputStream +import java.util.jar.Manifest import java.util.zip.ZipFile import org.gradle.testkit.runner.TaskOutcome import org.junit.Assert @@ -1131,6 +1135,45 @@ class JavaServiceDistributionPluginTests extends GradleIntegrationSpec { ]) } + def 'applies exports based on classpath manifests'() { + Manifest manifest = new Manifest() + manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0") + manifest.getMainAttributes().putValue('Add-Exports', 'jdk.compiler/com.sun.tools.javac.file') + File testJar = new File(getProjectDir(),"test.jar"); + testJar.withOutputStream { fos -> + new JarOutputStream(fos, manifest).close() + } + createUntarBuildFile(buildFile) + buildFile << """ + dependencies { + implementation files("test.jar") + javaAgent "net.bytebuddy:byte-buddy-agent:1.10.21" + } + 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() + + // Quick check + actualOpts.containsAll([ + "--add-exports", + "jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED"]) + + // Verify args are set in the correct order + int compilerPairIndex = actualOpts.indexOf("jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED") + actualOpts.get(compilerPairIndex - 1) == "--add-exports" + } + private static createUntarBuildFile(File buildFile) { buildFile << ''' plugins {