Improve logging modules and listing #984

Trim provider list and accept as default if only 1 available

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<Module>
                         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<Module>
         for(String dependsOn:module.getDepends())
         {
             // Look for modules that provide that dependency
-            Set<Module> providers = _provided.get(dependsOn);
+            Set<Module> providers = getAvailableProviders(dependsOn);
-            StartLog.debug("%s depends on %s provided by ",module,dependsOn,providers);
+                
+            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<Module>
             }
             
             // If a provider is already enabled, then add a transitive enable
-            long enabled=providers.stream().filter(Module::isEnabled).count();
+            if (providers.stream().filter(Module::isEnabled).count()!=0)
-            if (enabled>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<Module> dftProvider = providers.stream().filter(m->m.getName().equals(dependsOn)).findFirst();
+                Optional<Module> 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<Module>
         }
     }
     
+    private Set<Module> getAvailableProviders(String name)
+    {
+        // Get all available providers 
+        
+        Set<Module> 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<String> 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<Module> 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<Module>
         {
             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<Module>
             // Check dependencies
             m.getDepends().forEach(d->
             {
-                Set<Module> providers =_provided.get(d);
+                Set<Module> 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);
                 }
             });
         });

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

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

jetty-util/src/main/config/modules/log4j-log4j2.mod

@@ -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

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

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