Skip to content

Commit

Permalink
8218685: jlink --list-plugins needs to be readable and tidy
Browse files Browse the repository at this point in the history 
Reviewed-by: mchung, alanb
  • Loading branch information
Ian Graves authored and Mandy Chung committed Sep 29, 2020
1 parent 2fe0a5d commit 8df3e72
Show file tree
Hide file tree
Showing 32 changed files with 412 additions and 420 deletions.
61 changes: 18 additions & 43 deletions ...k.jlink/linux/classes/jdk/tools/jlink/internal/plugins/StripNativeDebugSymbolsPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import java.util.Optional;
import java.util.ResourceBundle;

import jdk.tools.jlink.plugin.Plugin;
import jdk.tools.jlink.plugin.PluginException;
import jdk.tools.jlink.plugin.ResourcePool;
import jdk.tools.jlink.plugin.ResourcePoolBuilder;
Expand All @@ -56,7 +55,7 @@
* libraries and binaries.
*
*/
public final class StripNativeDebugSymbolsPlugin implements Plugin {
public final class StripNativeDebugSymbolsPlugin extends AbstractPlugin {

public static final String NAME = "strip-native-debug-symbols";
private static final boolean DEBUG = Boolean.getBoolean("jlink.debug");
Expand Down Expand Up @@ -92,14 +91,10 @@ public StripNativeDebugSymbolsPlugin() {
}

public StripNativeDebugSymbolsPlugin(ObjCopyCmdBuilder cmdBuilder) {
super(NAME, resourceBundle);
this.cmdBuilder = cmdBuilder;
}

@Override
public String getName() {
return NAME;
}

@Override
public ResourcePool transform(ResourcePool in, ResourcePoolBuilder out) {
StrippedDebugInfoBinaryBuilder builder = new StrippedDebugInfoBinaryBuilder(
Expand Down Expand Up @@ -137,10 +132,9 @@ public ResourcePool transform(ResourcePool in, ResourcePoolBuilder out) {
}

private void logError(ResourcePoolEntry resource, String msgKey) {
String msg = PluginsResourceBundle.getMessage(resourceBundle,
msgKey,
NAME,
resource.path());
String msg = getMessage(msgKey,
NAME,
resource.path());
System.err.println(msg);
}

Expand All @@ -149,23 +143,11 @@ public Category getType() {
return Category.TRANSFORMER;
}

@Override
public String getDescription() {
String key = NAME + ".description";
return PluginsResourceBundle.getMessage(resourceBundle, key);
}

@Override
public boolean hasArguments() {
return true;
}

@Override
public String getArgumentsDescription() {
String key = NAME + ".argument";
return PluginsResourceBundle.getMessage(resourceBundle, key);
}

@Override
public void configure(Map<String, String> config) {
doConfigure(true, config);
Expand Down Expand Up @@ -196,8 +178,7 @@ public void doConfigure(boolean withChecks, Map<String, String> orig) {
String[] tokens = arg.split("=");
if (tokens.length != 2 || !KEEP_DEBUG_INFO_ARG.equals(tokens[0])) {
throw new IllegalArgumentException(
PluginsResourceBundle.getMessage(resourceBundle,
NAME + ".iae", NAME, arg));
getMessage(NAME + ".iae", NAME, arg));
}
hasKeepDebugInfo = true;
debuginfoExt = tokens[1];
Expand All @@ -211,8 +192,7 @@ public void doConfigure(boolean withChecks, Map<String, String> orig) {
String[] tokens = arg.split("=");
if (tokens.length != 2 || !STRIP_CMD_ARG.equals(tokens[0])) {
throw new IllegalArgumentException(
PluginsResourceBundle.getMessage(resourceBundle,
NAME + ".iae", NAME, arg));
getMessage(NAME + ".iae", NAME, arg));
}
if (withChecks) {
validateStripArg(tokens[1]);
Expand Down Expand Up @@ -246,26 +226,23 @@ public void doConfigure(boolean withChecks, Map<String, String> orig) {
// on the same plugin instance multiple times. Plugin option can
// repeat.
throw new IllegalArgumentException(
PluginsResourceBundle.getMessage(resourceBundle,
NAME + ".iae.conflict",
NAME,
EXCLUDE_DEBUG_INFO_ARG,
KEEP_DEBUG_INFO_ARG));
getMessage(NAME + ".iae.conflict",
NAME,
EXCLUDE_DEBUG_INFO_ARG,
KEEP_DEBUG_INFO_ARG));
}
if (!arg.startsWith(STRIP_CMD_ARG) &&
!arg.startsWith(KEEP_DEBUG_INFO_ARG) &&
!arg.startsWith(EXCLUDE_DEBUG_INFO_ARG)) {
// unknown arg value; case --strip-native-debug-symbols foobar
throw new IllegalArgumentException(
PluginsResourceBundle.getMessage(resourceBundle,
NAME + ".iae", NAME, arg));
getMessage(NAME + ".iae", NAME, arg));
}
if (!config.isEmpty()) {
// extraneous values; --strip-native-debug-symbols keep-debuginfo-files:foo=bar
throw new IllegalArgumentException(
PluginsResourceBundle.getMessage(resourceBundle,
NAME + ".iae", NAME,
config.toString()));
getMessage(NAME + ".iae", NAME,
config.toString()));
}
includeDebugSymbols = hasKeepDebugInfo;
}
Expand All @@ -275,15 +252,13 @@ private void validateStripArg(String stripArg) throws IllegalArgumentException {
Path strip = Paths.get(stripArg); // verify it's a resonable path
if (!Files.isExecutable(strip)) {
throw new IllegalArgumentException(
PluginsResourceBundle.getMessage(resourceBundle,
NAME + ".invalidstrip",
stripArg));
getMessage(NAME + ".invalidstrip",
stripArg));
}
} catch (InvalidPathException e) {
throw new IllegalArgumentException(
PluginsResourceBundle.getMessage(resourceBundle,
NAME + ".invalidstrip",
e.getInput()));
getMessage(NAME + ".invalidstrip",
e.getInput()));
}
}

Expand Down
18 changes: 18 additions & 0 deletions ...link/linux/classes/jdk/tools/jlink/resources/strip_native_debug_symbols_plugin.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,24 @@ Strip debug symbols from native libraries (if any). \n\
strip-native-debug-symbols.argument=\
<exclude-debuginfo-files|keep-debuginfo-files|objcopy=/path/to/objcopy>


strip-native-debug-symbols.usage=\
\ --strip-native-debug-symbols \
\ <exclude-debuginfo-files|keep-debuginfo-files|objcopy=PATH_TO_OBJ>\n\
\ Strip debug symbols from native libraries (if any). \n\
\ This plugin requires at least one option:\n\
\ objcopy: The path to the 'objcopy' binary.\n\
\ Defaults to 'objcopy' in PATH.\n\
\ exclude-debuginfo-files: Exclude debug info \n\
\ files. Defaults to true.\n\
\ keep-debuginfo-files[=<ext>]: Keep debug info\n\
\ files in <file>.<ext>.\n\
\ Defaults to <file>.debuginfo \n\
\ Examples: --strip-native-debug-symbols \n\
\ keep-debuginfo-files:objcopy=OBJPATH\n\
\ --strip-native-debug-symbols\n\
\ exclude-debuginfo-files

strip-native-debug-symbols.invalidstrip=Invalid objcopy command: {0}

strip-native-debug-symbols.iae={0}: Unrecognized argument ''{1}''
Expand Down
94 changes: 52 additions & 42 deletions src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,22 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.HashMap;
import java.util.Map.Entry;
import java.util.MissingResourceException;
import java.util.ResourceBundle;
import java.util.Set;
import java.util.HashSet;
import java.util.List;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import java.util.Collections;
import java.util.Locale;
import java.util.ResourceBundle;
import java.util.MissingResourceException;
import java.util.Comparator;


import jdk.tools.jlink.builder.DefaultImageBuilder;
import jdk.tools.jlink.builder.ImageBuilder;
Expand Down Expand Up @@ -373,7 +375,7 @@ private void addOrderedPluginOptions(Plugin plugin,
= new PluginOption(false,
(task, opt, arg) -> {
Map<String, String> m = addArgumentMap(plugin);
m.put(DefaultCompressPlugin.NAME, DefaultCompressPlugin.LEVEL_2);
m.put(plugin.getName(), DefaultCompressPlugin.LEVEL_2);
}, false, "--compress", "-c");
mainOptions.add(plugOption);
} else if (plugin instanceof DefaultStripDebugPlugin) {
Expand All @@ -386,14 +388,14 @@ private void addOrderedPluginOptions(Plugin plugin,
} else if (plugin instanceof ExcludeJmodSectionPlugin) {
plugOption = new PluginOption(false, (task, opt, arg) -> {
Map<String, String> m = addArgumentMap(plugin);
m.put(ExcludeJmodSectionPlugin.NAME,
m.put(plugin.getName(),
ExcludeJmodSectionPlugin.MAN_PAGES);
}, false, "--no-man-pages");
mainOptions.add(plugOption);

plugOption = new PluginOption(false, (task, opt, arg) -> {
Map<String, String> m = addArgumentMap(plugin);
m.put(ExcludeJmodSectionPlugin.NAME,
m.put(plugin.getName(),
ExcludeJmodSectionPlugin.INCLUDE_HEADER_FILES);
}, false, "--no-header-files");
mainOptions.add(plugOption);
Expand Down Expand Up @@ -450,8 +452,8 @@ private PluginsConfiguration getPluginsConfig(Path output, Map<String, String> l
// aren't being used at the same time. --strip-debug invokes --strip-native-debug-symbols on
// platforms that support it, so it makes little sense to allow both at the same time.
if ((plugin instanceof DefaultStripDebugPlugin && seenPlugins.contains(STRIP_NATIVE_DEBUG_SYMBOLS_NAME)) ||
(STRIP_NATIVE_DEBUG_SYMBOLS_NAME.equals(plugin.getName()) && seenPlugins.contains(DefaultStripDebugPlugin.NAME))) {
throw new BadArgs("err.plugin.conflicts", "--" + DefaultStripDebugPlugin.NAME,
(STRIP_NATIVE_DEBUG_SYMBOLS_NAME.equals(plugin.getName()) && seenPlugins.contains(plugin.getName()))) {
throw new BadArgs("err.plugin.conflicts", "--" + plugin.getName(),
"-G",
"--" + STRIP_NATIVE_DEBUG_SYMBOLS_NAME);
}
Expand Down Expand Up @@ -606,42 +608,50 @@ public void listPlugins() {
List<Plugin> pluginList = PluginRepository.
getPlugins(pluginOptions.pluginsLayer);

for (Plugin plugin : Utils.getSortedPlugins(pluginList)) {
showPlugin(plugin, log);
}
pluginList.stream()
.sorted(Comparator.comparing((Plugin plugin) -> plugin.getUsage().isEmpty(),
(Boolean res1, Boolean res2) -> Boolean.compare(res2,res1))
.thenComparing(Plugin::getName)
)
.forEach((plugin) -> showPlugin(plugin, log));

log.println("\n" + bundleHelper.getMessage("main.extended.help.footer"));
}

private void showPlugin(Plugin plugin, PrintWriter log) {
if (showsPlugin(plugin)) {
log.println("\n" + bundleHelper.getMessage("main.plugin.name")
+ ": " + plugin.getName());

// print verbose details for non-builtin plugins
if (!Utils.isBuiltin(plugin)) {
log.println(bundleHelper.getMessage("main.plugin.class")
+ ": " + plugin.getClass().getName());
log.println(bundleHelper.getMessage("main.plugin.module")
+ ": " + plugin.getClass().getModule().getName());
Category category = plugin.getType();
log.println(bundleHelper.getMessage("main.plugin.category")
+ ": " + category.getName());
log.println(bundleHelper.getMessage("main.plugin.state")
+ ": " + plugin.getStateDescription());
}
if(!plugin.getUsage().isEmpty()) {
log.println(plugin.getUsage());
} else {
log.println("\n" + bundleHelper.getMessage("main.plugin.name")
+ ": " + plugin.getName());

// print verbose details for non-builtin plugins
if (!Utils.isBuiltin(plugin)) {
log.println(bundleHelper.getMessage("main.plugin.class")
+ ": " + plugin.getClass().getName());
log.println(bundleHelper.getMessage("main.plugin.module")
+ ": " + plugin.getClass().getModule().getName());
Category category = plugin.getType();
log.println(bundleHelper.getMessage("main.plugin.category")
+ ": " + category.getName());
log.println(bundleHelper.getMessage("main.plugin.state")
+ ": " + plugin.getStateDescription());
}

String option = plugin.getOption();
if (option != null) {
log.println(bundleHelper.getMessage("main.plugin.option")
+ ": --" + plugin.getOption()
+ (plugin.hasArguments()? ("=" + plugin.getArgumentsDescription()) : ""));
}

String option = plugin.getOption();
if (option != null) {
log.println(bundleHelper.getMessage("main.plugin.option")
+ ": --" + plugin.getOption()
+ (plugin.hasArguments()? ("=" + plugin.getArgumentsDescription()) : ""));
// description can be long spanning more than one line and so
// print a newline after description label.
log.println(bundleHelper.getMessage("main.plugin.description")
+ ": " + plugin.getDescription());
}

// description can be long spanning more than one line and so
// print a newline after description label.
log.println(bundleHelper.getMessage("main.plugin.description")
+ ": " + plugin.getDescription());
}
}

Expand Down Expand Up @@ -725,6 +735,6 @@ public String version(String key) {

// Display all plugins
private static boolean showsPlugin(Plugin plugin) {
return (!Utils.isDisabled(plugin) && plugin.getOption() != null);
return (!Utils.isDisabled(plugin) && (plugin.getOption() != null) || !(plugin.getUsage().isEmpty()));
}
}
Loading

1 comment on commit 8df3e72

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented on 8df3e72 Sep 29, 2020

Choose a reason for hiding this comment

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

Review

Issues

Please sign in to comment.