Issue #1260 expand system properties

jetty-home/src/main/resources/modules/hawtio.mod

@@ -26,6 +26,6 @@ http://www.apache.org/licenses/LICENSE-2.0.html
 
 [ini-template]
 ## Hawt.io configuration
--Dhawtio.authenticationEnabled=false
+-Dhawtio.authenticationEnabled?=false
--Dhawtio.dirname=/dirname
+-Dhawtio.dirname?=/dirname
--Dhawtio.config.dir=${jetty.base}/etc/hawtio
+-Dhawtio.config.dir?=${jetty.base}/etc/hawtio

jetty-start/src/main/java/org/eclipse/jetty/start/Main.java

@@ -332,10 +332,10 @@ public class Main
 
         // ------------------------------------------------------------
         // 5) Lib & XML Expansion / Resolution
+        args.expandSystemProperties();
         args.expandLibs();
         args.expandModules(activeModules);
 
-
         // ------------------------------------------------------------
         // 6) Resolve Extra XMLs
         args.resolveExtraXmls();

jetty-start/src/main/java/org/eclipse/jetty/start/Module.java

@@ -179,7 +179,7 @@ public class Module implements Comparable<Module>
         return _path.equals(other._path);
     }
 
-    public void expandProperties(Props props)
+    public void expandDependencies(Props props)
     {
         Function<String,String> expander = d->{return props.expand(d);};
         

jetty-start/src/main/java/org/eclipse/jetty/start/Modules.java

@@ -322,7 +322,7 @@ public class Modules implements Iterable<Module>
             newlyEnabled.add(module.getName());
             
             // Expand module properties
-            module.expandProperties(_args.getProperties());
+            module.expandDependencies(_args.getProperties());
             
             // Apply default configuration
             if (module.hasDefaultConfig())
@@ -330,7 +330,7 @@ public class Modules implements Iterable<Module>
                 for(String line:module.getDefaultConfig())
                     _args.parse(line,module.getName()+"[ini]");
                 for (Module m:_modules)
-                    m.expandProperties(_args.getProperties());
+                    m.expandDependencies(_args.getProperties());
             }
         }
         
@@ -349,7 +349,7 @@ public class Modules implements Iterable<Module>
                 if (dependsOn.contains("/"))
                 {
                     Path file = _baseHome.getPath("modules/" + dependsOn + ".mod");
-                    registerModule(file).expandProperties(_args.getProperties());
+                    registerModule(file).expandDependencies(_args.getProperties());
                     providers = _provided.get(dependsOn);
                     if (providers==null || providers.isEmpty())
                         throw new UsageException("Module %s does not provide %s",_baseHome.toShortForm(file),dependsOn);

jetty-start/src/main/java/org/eclipse/jetty/start/Props.java

@@ -190,7 +190,7 @@ public final class Props implements Iterable<Prop>
         return expand(str,new Stack<String>());
     }
 
-    public String expand(String str, Stack<String> seenStack)
+    private String expand(String str, Stack<String> seenStack)
     {
         if (str == null)
         {

jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java

@@ -35,6 +35,8 @@ import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
 import java.util.StringTokenizer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
 
 import org.eclipse.jetty.start.Props.Prop;
 import org.eclipse.jetty.start.config.ConfigSource;
@@ -440,6 +442,28 @@ public class StartArgs
      * @throws IOException
      *             if unable to expand the libraries
      */
+    public void expandSystemProperties() throws IOException
+    {
+        StartLog.debug("Expanding Syste Properties");
+        
+        for (String key : systemPropertyKeys)
+        {
+            String value = properties.getString(key);
+            if (value!=null)
+            {
+                String expanded = properties.expand(value);
+                if (!value.equals(expanded))
+                    System.setProperty(key,expanded);
+            }
+        }
+    }
+    
+    /**
+     * Expand any command line added <code>--lib</code> lib references.
+     *
+     * @throws IOException
+     *             if unable to expand the libraries
+     */
     public void expandLibs() throws IOException
     {
         StartLog.debug("Expanding Libs");
@@ -1054,20 +1078,17 @@ public class StartArgs
         if (arg.startsWith("-D"))
         {
             String[] assign = arg.substring(2).split("=",2);
-            systemPropertyKeys.add(assign[0]);
+            
-            switch (assign.length)
+            if (assign.length==0)
-            {
+                return;
-                case 2:
+            String key = assign[0];
-                    System.setProperty(assign[0],assign[1]);
+            String value = assign.length==1?"":assign[1];
-                    setProperty(assign[0],assign[1],source);
+            
-                    break;
+            setProperty(key,value,
-                case 1:
+                k->{return System.getProperty(k);},
-                    System.setProperty(assign[0],"");
+                (k,v)->{System.setProperty(k,v);systemPropertyKeys.add(k);return null;},
-                    setProperty(assign[0],"",source);
+                source);
-                    break;
+                
-                default:
-                    break;
-            }
             return;
         }
 
@@ -1089,35 +1110,10 @@ public class StartArgs
             String key = arg.substring(0,equals);
             String value = arg.substring(equals + 1);
 
-            if (key.endsWith("+"))
+            setProperty(key,value,
-            {
+                k->{return getProperties().getString(k);},
-                key = key.substring(0,key.length() - 1);
+                null,
-                String orig = getProperties().getString(key);
+                source);
-                if (orig == null || orig.isEmpty())
-                {
-                    if (value.startsWith(","))
-                        value = value.substring(1);
-                }
-                else
-                {
-                    value = orig + value;
-                    source = propertySource.get(key) + "," + source;
-                }
-            }
-            if (key.endsWith("?"))
-            {
-                key = key.substring(0,key.length() - 1);
-                if (getProperties().containsKey(key))
-                    return;
-            }
-            else if (propertySource.containsKey(key))
-            {
-                if (!propertySource.get(key).endsWith("[ini]"))
-                    StartLog.warn("Property %s in %s already set in %s",key,source,propertySource.get(key));
-                propertySource.put(key,source);
-            }
-
-            setProperty(key,value,source);
             return;
         }
 
@@ -1146,6 +1142,46 @@ public class StartArgs
         throw new UsageException(UsageException.ERR_BAD_ARG,"Unrecognized argument: \"%s\" in %s",arg,source);
     }
 
+    protected boolean setProperty(String key,String value,Function<String, String> getter, BiFunction<String, String, Void> setter, String source)
+    {
+        if (key.endsWith("+"))
+        {
+            key = key.substring(0,key.length() - 1);
+            String orig = getter.apply(key);
+            if (orig == null || orig.isEmpty())
+            {
+                if (value.startsWith(","))
+                    value = value.substring(1);
+            }
+            else
+            {
+                value = orig + value;
+                source = propertySource.get(key) + "," + source;
+            }
+        }
+        if (key.endsWith("?"))
+        {
+            key = key.substring(0,key.length() - 1);
+            String preset = getter.apply(key);
+            if (preset!=null)
+            {
+                source = source+"?=";
+                value = preset;
+            }
+        }
+        else if (propertySource.containsKey(key))
+        {
+            if (!propertySource.get(key).endsWith("[ini]"))
+                StartLog.warn("Property %s in %s already set in %s",key,source,propertySource.get(key));
+            propertySource.put(key,source);
+        }
+
+        setProperty(key,value,source);
+        if (setter!=null)
+            setter.apply(key,value);
+        return true;
+    }
+    
     private void enableModules(String source, List<String> moduleNames)
     {
         for (String moduleName : moduleNames)

jetty-start/src/main/resources/org/eclipse/jetty/start/usage.txt

@@ -172,8 +172,8 @@ Properties:
     + XML files using the <Property name="pname"/> element 
     + Module files using the ${pname} syntax
 
-  Propertues may be set on the command line, in a ini file or in a [ini]
+  Properties and System Properties may be set on the command line, 
-  section of a module using the following syntax:
+  in a ini file or in a [ini] section of a module using the following syntax:
 
     name=value
       Set a property that can be expanded in XML files with the <Property> element.
@@ -187,7 +187,10 @@ Properties:
     name?=value
       Set a property only if it is not already set.
 
-  Each module may definte it's own properties. Start properties defined include:
+  If any of the previous formats is preceded by -D, then a system property is set 
+  as well as a start property.
+   
+  Each module may define it's own properties. Start properties defined include:
 
     jetty.home=[directory]
       Set the home directory of the jetty distribution.

jetty-start/src/test/java/org/eclipse/jetty/start/ConfigurationAssert.java

@@ -34,6 +34,7 @@ import java.util.List;
 import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import org.eclipse.jetty.start.Props.Prop;
@@ -127,7 +128,7 @@ public class ConfigurationAssert
         Set<String> expectedProperties = new HashSet<>();
         for (String line : textFile)
         {
-            if (line.startsWith("PROP|"))
+            if (line.startsWith("PROP|") || line.startsWith("SYS|"))
             {
                 expectedProperties.add(getValue(line));
             }
@@ -147,6 +148,17 @@ public class ConfigurationAssert
         }
         assertContainsUnordered("Properties", expectedProperties, actualProperties);
         
+        // Validate PROPERTIES (order is not important)
+        for (String line : textFile)
+        {
+            if (line.startsWith("SYS|"))
+            {
+                String[] expected = getValue(line).split("=",2);
+                String actual = System.getProperty(expected[0]);
+                assertThat("System property "+expected[0],actual,Matchers.equalTo(expected[1]));
+            }
+        }
+        
         // Validate Downloads
         List<String> expectedDownloads = new ArrayList<>();
         for (String line : textFile)
@@ -215,8 +227,8 @@ public class ConfigurationAssert
         }
         catch (AssertionError e)
         {
-            System.err.println("Expected: " + expectedSet);
+            System.err.println("Expected: " + expectedSet.stream().sorted().collect(Collectors.toList()));
-            System.err.println("Actual  : " + actualSet);
+            System.err.println("Actual  : " + actualSet.stream().sorted().collect(Collectors.toList()));
             throw e;
         }
         

jetty-start/src/test/resources/usecases/basic-properties.assert.txt

@@ -15,3 +15,8 @@ PROP|jetty.http.port=9090
 PROP|add=beginningmiddleend
 PROP|list=one,two,three
 PROP|name=value
+PROP|name0=/
+PROP|name1=/foo
+PROP|name2=/foo/bar
+SYS|SYSTEM=value
+SYS|PRESET=value

jetty-start/src/test/resources/usecases/basic-properties.cmdline.txt

@@ -8,3 +8,9 @@ list+=,two
 list+=,three
 name?=value
 name?=enoughAlready
+name0=/
+name1=${name0}foo
+name2=${name1}/bar
+-DSYSTEM=${name}
+-DSYSTEM?=IGNORED
+-DPRESET?=${SYSTEM}

jetty-util/src/main/config/modules/jul-impl.mod

@@ -14,5 +14,5 @@ jul-impl
 basehome:modules/jul-impl
 
 [exec]
--Djava.util.logging.config.file=etc/java-util-logging.properties
+-Djava.util.logging.config.file?=${jetty.base}/etc/java-util-logging.properties
 

jetty-util/src/main/config/modules/jul-slf4j.mod

@@ -23,5 +23,4 @@ basehome:modules/jul-slf4j
 lib/slf4j/jul-to-slf4j-${slf4j.version}.jar
 
 [exec]
--Djava.util.logging.config.file=etc/java-util-logging.properties
+-Djava.util.logging.config.file?=${jetty.base}/etc/java-util-logging.properties
-

jetty-util/src/main/config/modules/logging-jul.mod

@@ -13,7 +13,7 @@ jul-impl
 logging
 
 [exec]
--Dorg.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.Slf4jLog
+-Dorg.eclipse.jetty.util.log.class?=org.eclipse.jetty.util.log.Slf4jLog
 
 [ini]
 jetty.webapp.addServerClasses+=,file:${jetty.base}/lib/slf4j/

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

@@ -13,7 +13,7 @@ log4j-impl
 logging
 
 [exec]
--Dorg.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.Slf4jLog
+-Dorg.eclipse.jetty.util.log.class?=org.eclipse.jetty.util.log.Slf4jLog
 
 [ini]
 jetty.webapp.addServerClasses+=,file:${jetty.base}/lib/slf4j/,file:${jetty.base}/lib/log4j/

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

@@ -13,7 +13,7 @@ log4j2-impl
 logging
 
 [exec]
--Dorg.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.Slf4jLog
+-Dorg.eclipse.jetty.util.log.class?=org.eclipse.jetty.util.log.Slf4jLog
 
 [ini]
 jetty.webapp.addServerClasses+=,file:${jetty.base}/lib/slf4j/,file:${jetty.base}/lib/log4j2/

jetty-util/src/main/config/modules/logging-logback.mod

@@ -13,7 +13,7 @@ logback-impl
 logging
 
 [exec]
--Dorg.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.Slf4jLog
+-Dorg.eclipse.jetty.util.log.class?=org.eclipse.jetty.util.log.Slf4jLog
 
 [ini]
 jetty.webapp.addServerClasses+=,file:${jetty.base}/lib/slf4j/,file:${jetty.base}/lib/logback/

jetty-util/src/main/config/modules/logging-slf4j.mod

@@ -13,7 +13,7 @@ slf4j-impl
 logging
 
 [exec]
--Dorg.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.Slf4jLog
+-Dorg.eclipse.jetty.util.log.class?=org.eclipse.jetty.util.log.Slf4jLog
 
 [ini]
 jetty.webapp.addServerClasses+=,file:${jetty.base}/lib/slf4j/