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

[GR-49347] Check if modules contain packages before attempting to open/export #9157

Merged
merged 1 commit into from
Jun 24, 2024
Merged
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
Check if modules contain packages before attempting to open/export
  • Loading branch information
ivan-ristovic committed Jun 20, 2024
commit e8fdedc0b5ef8adcdb1141b9f1d11f835a73071c
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@

public class NativeImageClassLoaderOptions {
public static final String AddExportsAndOpensFormat = "<module>/<package>=<target-module>(,<target-module>)*";

public static final String AddReadsFormat = "<module>=<target-module>(,<target-module>)*";

@APIOption(name = "add-exports", extra = true, launcherOption = true, valueSeparator = {APIOption.WHITESPACE_SEPARATOR, '='})//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.io.File;
import java.io.IOException;
import java.lang.module.Configuration;
import java.lang.module.FindException;
import java.lang.module.ModuleDescriptor;
import java.lang.module.ModuleFinder;
import java.lang.module.ModuleReader;
Expand Down Expand Up @@ -389,21 +390,29 @@ void processClassLoaderOptions() {
}

processOption(NativeImageClassLoaderOptions.AddExports).forEach(val -> {
if (val.targetModules.isEmpty()) {
Modules.addExportsToAllUnnamed(val.module, val.packageName);
} else {
for (Module targetModule : val.targetModules) {
Modules.addExports(val.module, val.packageName, targetModule);
if (val.module.getPackages().contains(val.packageName)) {
if (val.targetModules.isEmpty()) {
Modules.addExportsToAllUnnamed(val.module, val.packageName);
} else {
for (Module targetModule : val.targetModules) {
Modules.addExports(val.module, val.packageName, targetModule);
}
}
} else {
warn("package " + val.packageName + " not in " + val.module.getName());
}
});
processOption(NativeImageClassLoaderOptions.AddOpens).forEach(val -> {
if (val.targetModules.isEmpty()) {
Modules.addOpensToAllUnnamed(val.module, val.packageName);
} else {
for (Module targetModule : val.targetModules) {
Modules.addOpens(val.module, val.packageName, targetModule);
if (val.module.getPackages().contains(val.packageName)) {
if (val.targetModules.isEmpty()) {
Modules.addOpensToAllUnnamed(val.module, val.packageName);
} else {
for (Module targetModule : val.targetModules) {
Modules.addOpens(val.module, val.packageName, targetModule);
}
}
} else {
warn("package " + val.packageName + " not in " + val.module.getName());
}
});
processOption(NativeImageClassLoaderOptions.AddReads).forEach(val -> {
Expand All @@ -417,6 +426,10 @@ void processClassLoaderOptions() {
});
}

private static void warn(String m) {
LogUtils.warning("WARNING", m, true);
}

private static void processListModulesOption(ModuleLayer layer) {
Class<?> launcherHelperClass = ReflectionUtil.lookupClass(false, "sun.launcher.LauncherHelper");
Method initOutputMethod = ReflectionUtil.lookupMethod(launcherHelperClass, "initOutput", boolean.class);
Expand Down Expand Up @@ -499,33 +512,20 @@ private Stream<AddExportsAndOpensAndReadsFormatValue> processOption(OptionKey<Ac
Stream<AddExportsAndOpensAndReadsFormatValue> parsedOptions = valuesWithOrigins.flatMap(valWithOrig -> {
try {
return Stream.of(asAddExportsAndOpensAndReadsFormatValue(specificOption, valWithOrig));
} catch (UserError.UserException e) {
if (ModuleSupport.modulePathBuild && classpath().isEmpty()) {
throw e;
} else {
/*
* Until we switch to always running the image-builder on module-path we have to
* be tolerant if invalid --add-exports -add-opens or --add-reads options are
* used. GR-30433
*/
LogUtils.warning(e.getMessage());
return Stream.empty();
}
} catch (FindException e) {
/*
* Print a specially-formatted warning to be 100% compatible with the output of
* `java` in this case.
*/
LogUtils.warning("WARNING", e.getMessage(), true);
return Stream.empty();
}
});
return parsedOptions;
}

private static final class AddExportsAndOpensAndReadsFormatValue {
private final Module module;
private final String packageName;
private final List<Module> targetModules;

private AddExportsAndOpensAndReadsFormatValue(Module module, String packageName, List<Module> targetModules) {
this.module = module;
this.packageName = packageName;
this.targetModules = targetModules;
}
private record AddExportsAndOpensAndReadsFormatValue(Module module, String packageName,
List<Module> targetModules) {
}

private AddExportsAndOpensAndReadsFormatValue asAddExportsAndOpensAndReadsFormatValue(OptionKey<?> option, Pair<String, OptionOrigin> valueOrigin) {
Expand Down Expand Up @@ -581,15 +581,15 @@ private AddExportsAndOpensAndReadsFormatValue asAddExportsAndOpensAndReadsFormat
}

Module module = findModule(moduleName).orElseThrow(() -> {
return userErrorAddExportsAndOpensAndReads(option, optionOrigin, optionValue, " Specified module '" + moduleName + "' is unknown.");
throw userWarningModuleNotFound(option, moduleName);
});
List<Module> targetModules;
if (targetModuleNamesList.contains("ALL-UNNAMED")) {
targetModules = Collections.emptyList();
} else {
targetModules = targetModuleNamesList.stream().map(mn -> {
return findModule(mn).orElseThrow(() -> {
throw userErrorAddExportsAndOpensAndReads(option, optionOrigin, optionValue, " Specified target-module '" + mn + "' is unknown.");
throw userWarningModuleNotFound(option, mn);
});
}).collect(Collectors.toList());
}
Expand All @@ -601,6 +601,11 @@ private static UserError.UserException userErrorAddExportsAndOpensAndReads(Optio
return UserError.abort("Invalid option %s provided by %s.%s", SubstrateOptionsParser.commandArgument(option, value), origin, detailMessage);
}

private static FindException userWarningModuleNotFound(OptionKey<?> option, String moduleName) {
String optionName = SubstrateOptionsParser.commandArgument(option, "");
return new FindException("Unknown module: " + moduleName + " specified to " + optionName);
}

Class<?> loadClassFromModule(Module module, String className) {
assert isModuleClassLoader(classLoader, module.getClassLoader()) : "Argument `module` is java.lang.Module from unknown ClassLoader";
return Class.forName(module, className);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,22 @@
import org.graalvm.nativeimage.Platform;
import org.graalvm.nativeimage.Platforms;

import java.io.PrintStream;

// Checkstyle: Allow raw info or warning printing - begin
public class LogUtils {
/**
* Print an info message.
*/
public static void info(String message) {
System.out.println("Info: " + message);
info("Info", message);
}

/**
* Print an info message with the given prefix.
*/
public static void info(String prefix, String message) {
System.out.println(prefix + ": " + message);
}

/**
Expand All @@ -53,7 +62,15 @@ public static void info(String format, Object... args) {
* Print a warning.
*/
public static void warning(String message) {
System.out.println("Warning: " + message);
warning("Warning", message, false);
}

/**
* Print a warning message with the given prefix, optionally to stderr.
*/
public static void warning(String prefix, String message, boolean stderr) {
PrintStream out = stderr ? System.err : System.out;
out.println(prefix + ": " + message);
}

/**
Expand Down