From 6bc87170531b02758889928c291248e7281780a4 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 20 Oct 2016 10:05:09 +1100 Subject: [PATCH] Improve logging modules and listing #984 Trim provider list and accept as default if only 1 available --- .../java/org/eclipse/jetty/start/Modules.java | 65 ++++++++++++++++--- .../src/main/config/modules/jcl-slf4j.mod | 1 + .../src/main/config/modules/jul-slf4j.mod | 1 + .../src/main/config/modules/log4j-log4j2.mod | 22 ------- .../src/main/config/modules/slf4j-jcl.mod | 1 + .../src/main/config/modules/slf4j-jul.mod | 1 + 6 files changed, 59 insertions(+), 32 deletions(-) delete mode 100644 jetty-util/src/main/config/modules/log4j-log4j2.mod 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 ddf86cde26f..6633f5f65ae 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 @@ -22,6 +22,7 @@ import java.io.FileInputStream; import java.io.IOException; import java.nio.file.Path; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -308,7 +309,7 @@ public class Modules implements Iterable if (p.isTransitive() && !transitive) p.clearTransitiveEnable(); else - throw new UsageException("%s provides %s, which is already provided by %s enabled in %s",module.getName(),name,p.getName(),p.getEnableSources()); + throw new UsageException("Module %s provides %s, which is already provided by %s enabled in %s",module.getName(),name,p.getName(),p.getEnableSources()); } }; } @@ -337,11 +338,12 @@ public class Modules implements Iterable for(String dependsOn:module.getDepends()) { // Look for modules that provide that dependency - Set providers = _provided.get(dependsOn); - StartLog.debug("%s depends on %s provided by ",module,dependsOn,providers); + Set providers = getAvailableProviders(dependsOn); + + StartLog.debug("Module %s depends on %s provided by ",module,dependsOn,providers); // If there are no known providers of the module - if ((providers==null||providers.isEmpty())) + if (providers.isEmpty()) { // look for a dynamic module if (dependsOn.contains("/")) @@ -359,13 +361,15 @@ public class Modules implements Iterable } // If a provider is already enabled, then add a transitive enable - long enabled=providers.stream().filter(Module::isEnabled).count(); - if (enabled>0) + if (providers.stream().filter(Module::isEnabled).count()!=0) providers.stream().filter(m->m.isEnabled()&&m!=module).forEach(m->enable(newlyEnabled,m,"transitive provider of "+dependsOn+" for "+module.getName(),true)); else { // Is there an obvious default? - Optional dftProvider = providers.stream().filter(m->m.getName().equals(dependsOn)).findFirst(); + Optional dftProvider = (providers.size()==1) + ?providers.stream().findFirst() + :providers.stream().filter(m->m.getName().equals(dependsOn)).findFirst(); + if (dftProvider.isPresent()) enable(newlyEnabled,dftProvider.get(),"transitive provider of "+dependsOn+" for "+module.getName(),true); else if (StartLog.isDebugEnabled()) @@ -374,6 +378,47 @@ public class Modules implements Iterable } } + private Set getAvailableProviders(String name) + { + // Get all available providers + + Set providers = _provided.get(name); + if (providers==null || providers.isEmpty()) + return Collections.emptySet(); + + providers = new HashSet<>(providers); + + // find all currently provided names by other modules + Set provided = new HashSet<>(); + for (Module m : _modules) + { + if (m.isEnabled()) + { + provided.add(m.getName()); + provided.addAll(m.getProvides()); + } + } + + // Remove any that cannot be selected + for (Iterator i = providers.iterator(); i.hasNext();) + { + Module provider = i.next(); + if (!provider.isEnabled()) + { + for (String p : provider.getProvides()) + { + if (provided.contains(p)) + { + i.remove(); + break; + } + } + } + } + + return providers; + } + public Module get(String name) { Module module = _names.get(name); @@ -381,7 +426,7 @@ public class Modules implements Iterable { String reason = _deprecated.getProperty(name); if (reason!=null) - StartLog.warn("Module '%s' is no longer available: %s",name,reason); + StartLog.warn("Module %s is no longer available: %s",name,reason); } return module; } @@ -405,13 +450,13 @@ public class Modules implements Iterable // Check dependencies m.getDepends().forEach(d-> { - Set providers =_provided.get(d); + Set providers = getAvailableProviders(d); if (providers.stream().filter(Module::isEnabled).count()==0) { if (unsatisfied.length()>0) unsatisfied.append(','); unsatisfied.append(m.getName()); - StartLog.error("Module %s requires a `%s` module from one of %s%n",m.getName(),d,providers); + StartLog.error("Module %s requires a module providing %s from one of %s%n",m.getName(),d,providers); } }); }); diff --git a/jetty-util/src/main/config/modules/jcl-slf4j.mod b/jetty-util/src/main/config/modules/jcl-slf4j.mod index 995b457715b..ce109a259e9 100644 --- a/jetty-util/src/main/config/modules/jcl-slf4j.mod +++ b/jetty-util/src/main/config/modules/jcl-slf4j.mod @@ -14,6 +14,7 @@ slf4j-impl [provides] jcl-api jcl-impl +slf4j+jcl [files] maven://org.slf4j/jcl-over-slf4j/${slf4j.version}|lib/slf4j/jcl-over-slf4j-${slf4j.version}.jar diff --git a/jetty-util/src/main/config/modules/jul-slf4j.mod b/jetty-util/src/main/config/modules/jul-slf4j.mod index d74fd4c2e4b..08a4486c0dd 100644 --- a/jetty-util/src/main/config/modules/jul-slf4j.mod +++ b/jetty-util/src/main/config/modules/jul-slf4j.mod @@ -12,6 +12,7 @@ slf4j-impl [provides] jul-impl +slf4j+jul [files] maven://org.slf4j/jul-to-slf4j/${slf4j.version}|lib/slf4j/jul-to-slf4j-${slf4j.version}.jar diff --git a/jetty-util/src/main/config/modules/log4j-log4j2.mod b/jetty-util/src/main/config/modules/log4j-log4j2.mod deleted file mode 100644 index 78801e538e2..00000000000 --- a/jetty-util/src/main/config/modules/log4j-log4j2.mod +++ /dev/null @@ -1,22 +0,0 @@ -[description] -Provides a Log4j v1.2 binding to Log4j v2 logging. - -[tags] -logging -log4j2 -log4j -internal - -[depends] -log4j2-api -log4j2-impl - -[provides] -log4j-api -log4j-impl - -[files] -maven://org.apache.logging.log4j/log4j-1.2-api/${log4j2.version}|lib/log4j/log4j-1.2-api-${log4j2.version}.jar - -[lib] -lib/log4j/log4j-1.2-api-${log4j2.version}.jar diff --git a/jetty-util/src/main/config/modules/slf4j-jcl.mod b/jetty-util/src/main/config/modules/slf4j-jcl.mod index fe7af429b82..43b041e3523 100644 --- a/jetty-util/src/main/config/modules/slf4j-jcl.mod +++ b/jetty-util/src/main/config/modules/slf4j-jcl.mod @@ -13,6 +13,7 @@ jcl-impl [provides] slf4j-impl +slf4j+jcl [files] maven://org.slf4j/slf4j-jcl/${slf4j.version}|lib/slf4j/slf4j-jcl-${slf4j.version}.jar diff --git a/jetty-util/src/main/config/modules/slf4j-jul.mod b/jetty-util/src/main/config/modules/slf4j-jul.mod index fa70f47f116..26bf8b786ae 100644 --- a/jetty-util/src/main/config/modules/slf4j-jul.mod +++ b/jetty-util/src/main/config/modules/slf4j-jul.mod @@ -11,6 +11,7 @@ slf4j-api [provides] slf4j-impl +slf4j+jul [files] maven://org.slf4j/slf4j-jdk14/${slf4j.version}|lib/slf4j/slf4j-jdk14-${slf4j.version}.jar