Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions changelog/@unreleased/pr-1365.v2.yml
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -55,86 +54,141 @@ final class ModuleArgs {

static ImmutableList<String> collectClasspathArgs(
Project project, JavaVersion javaVersion, FileCollection classpath) {

Map<File, JarManifestModuleInfo> parsedJarManifests = new LinkedHashMap<>();
for (File file : classpath.getFiles()) {
if (file.getName().endsWith(".jar") && file.isFile()) {
Optional<JarManifestModuleInfo> 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<String> exportsArgs(
Collection<JarManifestModuleInfo> 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<JarManifestModuleInfo> 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<JarManifestModuleInfo> 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<String> 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<String> opens = classpathInfo.stream()
.flatMap(modulePackagePair -> Stream.of("--add-exports", modulePackagePair + "=ALL-UNNAMED"));
}

private static Stream<String> opensArgs(Collection<JarManifestModuleInfo> 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<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);
}

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);
.flatMap(modulePackagePair -> Stream.of("--add-opens", modulePackagePair + "=ALL-UNNAMED"));
}

private static Stream<String> addExportArg(String modulePackagePair) {
return Stream.of("--add-exports", modulePackagePair + "=ALL-UNNAMED");
}
private static Stream<String> enablePreviewArg(
Map<File, JarManifestModuleInfo> parsedJarManifests, JavaVersion runtimeJavaVersion) {

AtomicBoolean enablePreview = new AtomicBoolean(false);
Map<File, JavaVersion> 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<String> addOpensArg(String modulePackagePair) {
return Stream.of("--add-opens", modulePackagePair + "=ALL-UNNAMED");
if (enablePreview.get()) {
return Stream.of("--enable-preview");
} else {
return Stream.empty();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is pretty complicated, I think we want to stream through the jars, fail if we see any with ENABLE_PREVIEW_ATTRIBUTE != javaVersion, and add the flag if we've seen any ENABLE_PREVIEW_ATTRIBUTE attrs?

imo the Map<String, Collection<String>> enablePreviewFromJar makes things difficult to grok

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay, I've updated to use less stream-y stuff... think this should be a bit more readable now!

}

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 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<JavaVersion> 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(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<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)
.map(JavaVersion::toVersion))
.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();
}
Expand Down
Loading