8192945: Need stable sort for MODULES entry in the release file

Reviewed-by: alanb, redestad
This commit is contained in:
Mandy Chung 2017-12-07 09:23:15 -08:00
parent 4bff43558f
commit f296f0cfbc
2 changed files with 87 additions and 36 deletions

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -30,13 +30,16 @@ import jdk.tools.jlink.plugin.ResourcePoolModule;
import jdk.tools.jlink.plugin.ResourcePoolModuleView;
import java.lang.module.ModuleDescriptor;
import java.lang.module.ModuleDescriptor.Requires;
import java.lang.module.ModuleDescriptor.Requires.Modifier;
import java.nio.ByteBuffer;
import java.util.Deque;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Stream;
@ -45,9 +48,8 @@ import java.util.stream.Stream;
* Helper class to sort modules in topological order
*/
public final class ModuleSorter {
private final Deque<ResourcePoolModule> nodes = new LinkedList<>();
private final Map<String, Set<ResourcePoolModule>> edges = new HashMap<>();
private final Deque<ResourcePoolModule> result = new LinkedList<>();
private final Map<ResourcePoolModule, Set<ResourcePoolModule>> graph = new HashMap<>();
private final List<ResourcePoolModule> result = new ArrayList<>();
private final ResourcePoolModuleView moduleView;
@ -69,11 +71,17 @@ public final class ModuleSorter {
private ModuleSorter addModule(ResourcePoolModule module) {
addNode(module);
readModuleDescriptor(module).requires().forEach(req -> {
// the module graph will be traversed in a stable order for
// the topological sort. So add the dependences in the module name order
readModuleDescriptor(module).requires()
.stream()
.sorted(Comparator.comparing(Requires::name))
.forEach(req ->
{
ResourcePoolModule dep = moduleView.findModule(req.name()).orElse(null);
if (dep != null) {
addNode(dep);
edges.get(module.name()).add(dep);
graph.get(module).add(dep);
} else if (!req.modifiers().contains(Modifier.STATIC)) {
throw new PluginException(req.name() + " not found");
}
@ -82,22 +90,23 @@ public final class ModuleSorter {
}
private void addNode(ResourcePoolModule module) {
nodes.add(module);
edges.computeIfAbsent(module.name(), _n -> new HashSet<>());
graph.computeIfAbsent(module, _n -> new LinkedHashSet<>());
}
/*
* The module graph will be traversed in a stable order
* (traversing the modules and their dependences in alphabetical order)
* so that it will produce the same result of a given module graph.
*/
private synchronized void build() {
if (!result.isEmpty() || nodes.isEmpty())
if (!result.isEmpty() || graph.isEmpty())
return;
Deque<ResourcePoolModule> visited = new LinkedList<>();
Deque<ResourcePoolModule> done = new LinkedList<>();
ResourcePoolModule node;
while ((node = nodes.poll()) != null) {
if (!visited.contains(node)) {
visit(node, visited, done);
}
}
Set<ResourcePoolModule> visited = new HashSet<>();
Set<ResourcePoolModule> done = new HashSet<>();
graph.keySet().stream()
.sorted(Comparator.comparing(ResourcePoolModule::name))
.forEach(node -> visit(node, visited, done));
}
public Stream<ResourcePoolModule> sorted() {
@ -106,19 +115,21 @@ public final class ModuleSorter {
}
private void visit(ResourcePoolModule node,
Deque<ResourcePoolModule> visited,
Deque<ResourcePoolModule> done) {
Set<ResourcePoolModule> visited,
Set<ResourcePoolModule> done) {
if (visited.contains(node)) {
if (!done.contains(node)) {
throw new IllegalArgumentException("Cyclic detected: " +
node + " " + edges.get(node.name()));
node + " " + graph.get(node));
}
return;
}
// traverse the dependences of the given module which are
// also sorted in alphabetical order
visited.add(node);
edges.get(node.name())
.forEach(x -> visit(x, visited, done));
graph.get(node).forEach(x -> visit(x, visited, done));
done.add(node);
result.addLast(node);
result.add(node);
}
}

View File

@ -24,14 +24,17 @@
import java.io.File;
import java.io.FileReader;
import java.io.IOException;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
import java.util.Properties;
import java.util.spi.ToolProvider;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import tests.Helper;
import tests.JImageGenerator;
import tests.JImageGenerator.JLinkTask;
/*
* @test
@ -44,6 +47,9 @@ import tests.JImageGenerator;
* jdk.jlink/jdk.tools.jmod
* jdk.jlink/jdk.tools.jimage
* jdk.compiler
* jdk.scripting.nashorn
* jdk.scripting.nashorn.shell
*
* @build tests.*
* @run main ModuleNamesOrderTest
*/
@ -60,12 +66,18 @@ public class ModuleNamesOrderTest {
return;
}
Path outputDir = helper.createNewImageDir("image8168925");
JImageGenerator.getJLinkTask()
.modulePath(helper.defaultModulePath())
.output(outputDir)
.addMods("jdk.scripting.nashorn")
.call().assertSuccess();
testDependences(helper);
testModulesOrder(helper);
}
private static List<String> modulesProperty(Path outputDir, String modulePath, String... roots)
throws IOException
{
JLinkTask jlinkTask = JImageGenerator.getJLinkTask()
.modulePath(modulePath)
.output(outputDir);
Stream.of(roots).forEach(jlinkTask::addMods);
jlinkTask.call().assertSuccess();
File release = new File(outputDir.toString(), "release");
if (!release.exists()) {
@ -81,9 +93,21 @@ public class ModuleNamesOrderTest {
if (!modules.startsWith("\"java.base ")) {
throw new AssertionError("MODULES should start with 'java.base'");
}
if (modules.charAt(0) != '"' || modules.charAt(modules.length()-1) != '"') {
throw new AssertionError("MODULES value should be double quoted");
}
if (!modules.endsWith(" jdk.scripting.nashorn\"")) {
throw new AssertionError("MODULES end with 'jdk.scripting.nashorn'");
return Stream.of(modules.substring(1, modules.length()-1).split("\\s+"))
.collect(Collectors.toList());
}
private static void testDependences(Helper helper) throws IOException {
Path outputDir = helper.createNewImageDir("test");
List<String> modules = modulesProperty(outputDir, helper.defaultModulePath(),
"jdk.scripting.nashorn");
String last = modules.get(modules.size()-1);
if (!last.equals("jdk.scripting.nashorn")) {
throw new AssertionError("Unexpected MODULES value: " + modules);
}
checkDependency(modules, "java.logging", "java.base");
@ -94,7 +118,23 @@ public class ModuleNamesOrderTest {
checkDependency(modules, "jdk.scripting.nashorn", "java.scripting");
}
private static void checkDependency(String modules, String fromMod, String toMod) {
/*
* Verify the MODULES list must be the same for the same module graph
*/
private static void testModulesOrder(Helper helper) throws IOException {
Path image1 = helper.createNewImageDir("test1");
List<String> modules1 = modulesProperty(image1, helper.defaultModulePath(),
"jdk.scripting.nashorn", "jdk.scripting.nashorn.shell");
Path image2 = helper.createNewImageDir("test2");
List<String> modules2 = modulesProperty(image2, helper.defaultModulePath(),
"jdk.scripting.nashorn.shell", "jdk.scripting.nashorn");
if (!modules1.equals(modules2)) {
throw new AssertionError("MODULES should be a stable order: " +
modules1 + " vs " + modules2);
}
}
private static void checkDependency(List<String> modules, String fromMod, String toMod) {
int fromModIdx = modules.indexOf(fromMod);
if (fromModIdx == -1) {
throw new AssertionError(fromMod + " is missing in MODULES");