From b82d4ee2fef80145e49ac8281cf75e7dda2553b5 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 19 Oct 2016 15:00:18 +1100 Subject: [PATCH] Improve logging modules and listing #984 Avoid sorting all modules. Only sort enabled modules. This allows for cycles to exist within all modules, but they are only seen as a problem if they actually enabled. --- .../java/org/eclipse/jetty/start/Main.java | 14 ++------- .../java/org/eclipse/jetty/start/Modules.java | 30 +++++++++++-------- .../org/eclipse/jetty/start/usage.txt | 1 - .../jetty/start/ModuleGraphWriterTest.java | 1 - .../org/eclipse/jetty/start/ModulesTest.java | 1 - .../resources/usecases/dynamic-loop/start.ini | 2 +- .../test/resources/usecases/loop.prepare.txt | 3 +- .../test/resources/usecases/loop/start.ini | 2 +- 8 files changed, 24 insertions(+), 30 deletions(-) diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java b/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java index 1193528b78a..09c7692e959 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java @@ -308,20 +308,9 @@ public class Main } } - StartLog.debug("Sorting Modules"); - try - { - modules.sort(); - } - catch (Exception e) - { - throw new UsageException(ERR_BAD_GRAPH,e); - } - args.setAllModules(modules); List activeModules = modules.getEnabled(); - final Version START_VERSION = new Version(StartArgs.VERSION); for(Module enabled: activeModules) @@ -466,8 +455,9 @@ public class Main { invokeMain(cl, args); } - catch (Exception e) + catch (Throwable e) { + e.printStackTrace(); usageExit(e,ERR_INVOKE_MAIN,startupArgs.isTestingModeEnabled()); } } diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/Modules.java b/jetty-start/src/main/java/org/eclipse/jetty/start/Modules.java index 1143b2e809d..6fb3749fed9 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/Modules.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/Modules.java @@ -168,7 +168,8 @@ public class Modules implements Iterable public void dumpEnabled() { int i=0; - for (Module module:getEnabled()) + List enabled = getEnabled(); + for (Module module:enabled) { String name=module.getName(); String index=(i++)+")"; @@ -240,31 +241,31 @@ public class Modules implements Iterable return str.toString(); } - public void sort() + public List getEnabled() { + List enabled = _modules.stream().filter(m->{return m.isEnabled();}).collect(Collectors.toList()); + TopologicalSort sort = new TopologicalSort<>(); - for (Module module: _modules) + for (Module module: enabled) { Consumer add = name -> { Module dependency = _names.get(name); - if (dependency!=null) + if (dependency!=null && dependency.isEnabled()) sort.addDependency(module,dependency); Set provided = _provided.get(name); if (provided!=null) - provided.forEach(p->sort.addDependency(module,p)); + for (Module p : provided) + if (p.isEnabled()) + sort.addDependency(module,p); }; module.getDepends().forEach(add); module.getOptional().forEach(add); } - - sort.sort(_modules); - } - public List getEnabled() - { - return _modules.stream().filter(m->{return m.isEnabled();}).collect(Collectors.toList()); + sort.sort(enabled); + return enabled; } /** Enable a module @@ -287,6 +288,12 @@ public class Modules implements Iterable { StartLog.debug("enable %s from %s transitive=%b",module,enabledFrom,transitive); + if (newlyEnabled.contains(module.getName())) + { + StartLog.debug("Cycle at %s",module); + return; + } + // Check that this is not already provided by another module! for (String name:module.getProvides()) { @@ -341,7 +348,6 @@ public class Modules implements Iterable { Path file = _baseHome.getPath("modules/" + dependsOn + ".mod"); registerModule(file).expandProperties(_args.getProperties()); - sort(); providers = _provided.get(dependsOn); if (providers==null || providers.isEmpty()) throw new UsageException("Module %s does not provide %s",_baseHome.toShortForm(file),dependsOn); diff --git a/jetty-start/src/main/resources/org/eclipse/jetty/start/usage.txt b/jetty-start/src/main/resources/org/eclipse/jetty/start/usage.txt index 2aa02f82958..7e953c9bb98 100644 --- a/jetty-start/src/main/resources/org/eclipse/jetty/start/usage.txt +++ b/jetty-start/src/main/resources/org/eclipse/jetty/start/usage.txt @@ -81,7 +81,6 @@ Module Management: Note: this can also be used in the ${jetty.base}/start.ini or ${jetty.base}/start.d/*.ini files. - --add=(,)* --add-to-start=(,)* Add the modules to the list of modules enabled at start. Transitive dependencies are followed and dependent diff --git a/jetty-start/src/test/java/org/eclipse/jetty/start/ModuleGraphWriterTest.java b/jetty-start/src/test/java/org/eclipse/jetty/start/ModuleGraphWriterTest.java index 62daf868024..c750733e9ec 100644 --- a/jetty-start/src/test/java/org/eclipse/jetty/start/ModuleGraphWriterTest.java +++ b/jetty-start/src/test/java/org/eclipse/jetty/start/ModuleGraphWriterTest.java @@ -62,7 +62,6 @@ public class ModuleGraphWriterTest Modules modules = new Modules(basehome, args); modules.registerAll(); - modules.sort(); Path outputFile = basehome.getBasePath("graph.dot"); diff --git a/jetty-start/src/test/java/org/eclipse/jetty/start/ModulesTest.java b/jetty-start/src/test/java/org/eclipse/jetty/start/ModulesTest.java index 093fbd5d65a..e30c26183d5 100644 --- a/jetty-start/src/test/java/org/eclipse/jetty/start/ModulesTest.java +++ b/jetty-start/src/test/java/org/eclipse/jetty/start/ModulesTest.java @@ -168,7 +168,6 @@ public class ModulesTest // Enable 2 modules modules.enable("base",TEST_SOURCE); modules.enable("optional",TEST_SOURCE); - modules.sort(); // Collect active module list List active = modules.getEnabled(); diff --git a/jetty-start/src/test/resources/usecases/dynamic-loop/start.ini b/jetty-start/src/test/resources/usecases/dynamic-loop/start.ini index 40d0099317b..9d2989cbe56 100644 --- a/jetty-start/src/test/resources/usecases/dynamic-loop/start.ini +++ b/jetty-start/src/test/resources/usecases/dynamic-loop/start.ini @@ -1 +1 @@ --module=root +--module=root diff --git a/jetty-start/src/test/resources/usecases/loop.prepare.txt b/jetty-start/src/test/resources/usecases/loop.prepare.txt index 99dcff0e0ff..535d63e02ec 100644 --- a/jetty-start/src/test/resources/usecases/loop.prepare.txt +++ b/jetty-start/src/test/resources/usecases/loop.prepare.txt @@ -1 +1,2 @@ ---create-startd --add=tom +--create-startd +--add-to-start=tom diff --git a/jetty-start/src/test/resources/usecases/loop/start.ini b/jetty-start/src/test/resources/usecases/loop/start.ini index 40d0099317b..9d2989cbe56 100644 --- a/jetty-start/src/test/resources/usecases/loop/start.ini +++ b/jetty-start/src/test/resources/usecases/loop/start.ini @@ -1 +1 @@ --module=root +--module=root