Fix #627 Use start.d or start.ini not both

Updated start module to look for start.d and use it if it exists
Error or warning of both start.d and start.ini exist
updated jetty-distribution to use one or the other

jetty-distribution/pom.xml

@@ -217,6 +217,28 @@
             </configuration>
           </execution>
 
+          <execution>
+            <id>unpack-test-webapp-config</id>
+            <phase>process-resources</phase>
+            <goals>
+              <goal>unpack</goal>
+            </goals>
+            <configuration>
+              <artifactItems>
+                <artifactItem>
+                  <groupId>org.eclipse.jetty</groupId>
+                  <artifactId>test-jetty-webapp</artifactId>
+                  <version>${project.version}</version>
+                  <classifier>config</classifier>
+                  <type>jar</type>
+                  <overWrite>true</overWrite>
+                  <outputDirectory>${assembly-directory}</outputDirectory>
+                </artifactItem>
+              </artifactItems>
+              <excludes>META-INF/**</excludes>
+            </configuration>
+          </execution>
+
           <execution>
             <id>unpack-test-jaas-config</id>
             <phase>process-resources</phase>
@@ -524,21 +546,6 @@
               <goal>java</goal>
             </goals>
           </execution>
-          <execution>
-            <id>setup demo-base-ini</id>
-            <phase>process-classes</phase>
-            <configuration>
-              <mainClass>org.eclipse.jetty.start.Main</mainClass>
-              <arguments>
-                <argument>jetty.home=${assembly-directory}</argument>
-                <argument>jetty.base=${assembly-directory}/demo-base</argument>
-                <argument>--add-to-start=continuation,deploy,websocket,ext,resources,client,annotations,jndi,servlets</argument>
-              </arguments>
-            </configuration>
-            <goals>
-              <goal>java</goal>
-            </goals>
-          </execution>
           <execution>
             <id>setup demo-base-startd</id>
             <phase>process-classes</phase>
@@ -547,7 +554,7 @@
               <arguments>
                 <argument>jetty.home=${assembly-directory}</argument>
                 <argument>jetty.base=${assembly-directory}/demo-base</argument>
-                <argument>--add-to-startd=jsp,jstl,http,https</argument>
+                <argument>--add-to-startd=continuation,deploy,websocket,ext,resources,client,annotations,jndi,servlets,jsp,jstl,http,https</argument>
               </arguments>
             </configuration>
             <goals>

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

@@ -107,31 +107,18 @@ public class BaseBuilder
         Modules modules = startArgs.getAllModules();
 
         // Select all the added modules to determine which ones are newly enabled
+        // TODO this does not look correct????
         Set<String> enabled = new HashSet<>();
-        Set<String> startDModules = new HashSet<>();
         Set<String> startModules = new HashSet<>();
-        if (!startArgs.getAddToStartdIni().isEmpty() || !startArgs.getAddToStartIni().isEmpty())
+        if (!startArgs.getStartModules().isEmpty())
         {
-            if (startArgs.isAddToStartdFirst())
-            {
-                for (String name:startArgs.getAddToStartdIni())
-                    startDModules.addAll(modules.select(name,"--add-to-startd"));
-                for (String name:startArgs.getAddToStartIni())
-                    startModules.addAll(modules.select(name,"--add-to-start"));
-            }
-            else
-            {
-                for (String name:startArgs.getAddToStartIni())
-                    startModules.addAll(modules.select(name,"--add-to-start"));
-                for (String name:startArgs.getAddToStartdIni())
-                    startDModules.addAll(modules.select(name,"--add-to-startd"));
-            }
-            enabled.addAll(startDModules);
+            for (String name:startArgs.getStartModules())
+                startModules.addAll(modules.select(name,"--add-to-start[d]"));
             enabled.addAll(startModules);
         }
 
         if (StartLog.isDebugEnabled())
-            StartLog.debug("startD=%s start=%s",startDModules,startModules);
+            StartLog.debug("start[d]=%s",startModules);
         
         // Check the licenses
         if (startArgs.isLicenseCheckRequired())
@@ -158,38 +145,43 @@ public class BaseBuilder
         List<FileArg> files = new ArrayList<FileArg>();
         AtomicReference<BaseBuilder.Config> builder = new AtomicReference<>();
         AtomicBoolean modified = new AtomicBoolean();
-        Consumer<Module> do_build_add = module ->
-        {
-            try
-            {
-                if (module.isSkipFilesValidation())
-                {
-                    StartLog.debug("Skipping [files] validation on %s",module.getName());
-                } 
-                else 
-                {
-                    if (builder.get().addModule(module))
-                        modified.set(true);
-                    for (String file : module.getFiles())
-                        files.add(new FileArg(module,startArgs.getProperties().expand(file)));
-                }
-            }
-            catch(Exception e)
-            {
-                throw new RuntimeException(e);
-            }
-        };
-        
-        if (!startDModules.isEmpty())
-        {
-            builder.set(new StartDirBuilder(this));
-            startDModules.stream().map(n->modules.get(n)).forEach(do_build_add);
-        }
 
         if (!startModules.isEmpty())
         {
-            builder.set(new StartIniBuilder(this));
-            startModules.stream().map(n->modules.get(n)).forEach(do_build_add);
+            Path startd = getBaseHome().getBasePath("start.d");
+            Path startini = getBaseHome().getBasePath("start.ini");
+            
+            if (Files.exists(startini)) 
+            {
+                if (Files.exists(startd))
+                    StartLog.warn("Should not use both %s and %s",getBaseHome().toShortForm(startd),getBaseHome().toShortForm(startini));
+                else if (startArgs.isUseStartd())
+                    throw new UsageException("Cannot --add-to-startd when %s exists",getBaseHome().toShortForm(startini));
+            }
+            
+            boolean useStartD=startArgs.isUseStartd() || Files.exists(startd);            
+            builder.set(useStartD?new StartDirBuilder(this):new StartIniBuilder(this));
+            startModules.stream().map(n->modules.get(n)).forEach(module ->
+            {
+                try
+                {
+                    if (module.isSkipFilesValidation())
+                    {
+                        StartLog.debug("Skipping [files] validation on %s",module.getName());
+                    } 
+                    else 
+                    {
+                        if (builder.get().addModule(module))
+                            modified.set(true);
+                        for (String file : module.getFiles())
+                            files.add(new FileArg(module,startArgs.getProperties().expand(file)));
+                    }
+                }
+                catch(Exception e)
+                {
+                    throw new RuntimeException(e);
+                }
+            });
         }
 
         files.addAll(startArgs.getFiles());

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

@@ -416,7 +416,7 @@ public class Main
         BaseBuilder baseBuilder = new BaseBuilder(baseHome,args);
         if(baseBuilder.build())
             StartLog.info("Base directory was modified");
-        else if (args.isDownload() || !args.getAddToStartdIni().isEmpty() || !args.getAddToStartIni().isEmpty())
+        else if (args.isDownload() || !args.getStartModules().isEmpty())
             StartLog.info("Base directory was not modified");
         
         // Informational command line, don't run jetty

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

@@ -150,14 +150,8 @@ public class StartArgs
     private List<String> rawLibs = new ArrayList<>();
 
     // jetty.base - build out commands
-    /** --add-to-startd=[module,[module]] */
-    private List<String> addToStartdIni = new ArrayList<>();
-    /** --add-to-start=[module,[module]] */
-    private List<String> addToStartIni = new ArrayList<>();
-
-    /** Tri-state True if modules should be added to StartdFirst, false if StartIni first, else null */
-    private Boolean addToStartdFirst;
-    
+    /** --add-to-start[d]=[module,[module]] */
+    private List<String> startModules = new ArrayList<>();
     
     // module inspection commands
     /** --write-module-graph=[filename] */
@@ -165,6 +159,7 @@ public class StartArgs
 
     /** Collection of all modules */
     private Modules allModules;
+    
     /** Should the server be run? */
     private boolean run = true;
 
@@ -180,6 +175,7 @@ public class StartArgs
     private boolean listConfig = false;
     private boolean version = false;
     private boolean dryRun = false;
+    private boolean useStartd = false;
 
     private boolean exec = false;
     private String exec_properties;
@@ -517,14 +513,9 @@ public class StartArgs
         }
     }
 
-    public List<String> getAddToStartdIni()
+    public List<String> getStartModules()
     {
-        return addToStartdIni;
-    }
-
-    public List<String> getAddToStartIni()
-    {
-        return addToStartIni;
+        return startModules;
     }
 
     public Modules getAllModules()
@@ -783,12 +774,10 @@ public class StartArgs
     {
         return version;
     }
-
-    public boolean isAddToStartdFirst()
+    
+    public boolean isUseStartd()
     {
-        if (addToStartdFirst==null)
-            throw new IllegalStateException();
-        return addToStartdFirst.booleanValue();
+        return useStartd;
     }
     
     public void parse(ConfigSources sources)
@@ -956,30 +945,19 @@ public class StartArgs
             run = false;
             return;
         }
-
-        // jetty.base build-out : add to ${jetty.base}/start.d/
-        if (arg.startsWith("--add-to-startd="))
-        {
-            List<String> moduleNames = Props.getValues(arg);
-            addToStartdIni.addAll(moduleNames);
-            run = false;
-            download = true;
-            licenseCheckRequired = true;
-            if (addToStartdFirst==null)
-                addToStartdFirst=Boolean.TRUE;
-            return;
-        }
-
+       
         // jetty.base build-out : add to ${jetty.base}/start.ini
-        if (arg.startsWith("--add-to-start="))
+        if (arg.startsWith("--add-to-startd=")||
+            arg.startsWith("--add-to-start="))
         {
+            if (arg.startsWith("--add-to-startd="))
+                useStartd=true;
+
             List<String> moduleNames = Props.getValues(arg);
-            addToStartIni.addAll(moduleNames);
+            startModules.addAll(moduleNames);
             run = false;
             download = true;
             licenseCheckRequired = true;
-            if (addToStartdFirst==null)
-                addToStartdFirst=Boolean.FALSE;
             return;
         }
 

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

@@ -170,7 +170,7 @@ public class ConfigurationAssert
         textFile.stream()
         .filter(s->s.startsWith("EXISTS|"))
         .map(s->baseHome.getPath(s.substring(7)).toFile())
-        .forEach(f->Assert.assertTrue(f+" exists",f.exists()));
+        .forEach(f->Assert.assertTrue(f+" exists?",f.exists()));
     }
 
     private static String shorten(BaseHome baseHome, Path path, Path testResourcesDir)

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

@@ -113,12 +113,14 @@ public class TestUseCases
         {
             if (prepare != null && prepare.length>0)
             {
-                Main main = new Main();
-                List<String> cmdLine = new ArrayList<>();
-                cmdLine.add("--testing-mode");
                 for (String arg : prepare)
-                    cmdLine.add(arg);
-                main.start(main.processCommandLine(cmdLine));
+                {
+                    Main main = new Main();
+                    List<String> cmdLine = new ArrayList<>();
+                    cmdLine.add("--testing-mode");
+                    cmdLine.addAll(Arrays.asList(arg.split(" ")));
+                    main.start(main.processCommandLine(cmdLine));
+                }
             }
 
             Main main = new Main();

jetty-start/src/test/resources/usecases/alternate.4.prepare.txt

@@ -1 +1 @@
---add-to-startd=noDftOptionB
+--add-to-start=noDftOptionB

jetty-start/src/test/resources/usecases/barebones.addToStart.assert.txt

@@ -0,0 +1,18 @@
+## The XMLs we expect (order is important)
+XML|${jetty.home}/etc/base.xml
+XML|${jetty.home}/etc/main.xml
+XML|etc/optional.xml
+
+# The LIBs we expect (order is irrelevant)
+LIB|${jetty.home}/lib/base.jar
+LIB|${jetty.home}/lib/main.jar
+LIB|${jetty.home}/lib/other.jar
+LIB|${jetty.home}/lib/optional.jar
+
+# The Properties we expect (order is irrelevant)
+PROP|main.prop=value0
+PROP|optional.prop=value0
+
+# Files / Directories to create
+FILE|maindir/
+EXISTS|start.ini

jetty-start/src/test/resources/usecases/barebones.addToStart.prepare.txt

@@ -0,0 +1 @@
+--add-to-start=optional

jetty-start/src/test/resources/usecases/barebones.addToStartd.assert.txt

@@ -1,18 +1 @@
-## The XMLs we expect (order is important)
-XML|${jetty.home}/etc/base.xml
-XML|${jetty.home}/etc/main.xml
-XML|etc/optional.xml
-
-# The LIBs we expect (order is irrelevant)
-LIB|${jetty.home}/lib/base.jar
-LIB|${jetty.home}/lib/main.jar
-LIB|${jetty.home}/lib/other.jar
-LIB|${jetty.home}/lib/optional.jar
-
-# The Properties we expect (order is irrelevant)
-PROP|main.prop=value0
-PROP|optional.prop=value0
-
-# Files / Directories to create
-FILE|maindir/
-EXISTS|start.d/optional.ini
+EX|org.eclipse.jetty.start.UsageException: Cannot --add-to-startd when ${jetty.base}/start.ini exists

jetty-start/src/test/resources/usecases/dynamic-loop.1.prepare.txt

@@ -1 +1 @@
---add-to-startd=other
+--add-to-start=other

jetty-start/src/test/resources/usecases/empty.addToStart.assert.txt

@@ -0,0 +1,22 @@
+## The XMLs we expect (order is important)
+XML|${jetty.home}/etc/optional.xml
+XML|${jetty.home}/etc/base.xml
+XML|${jetty.home}/etc/main.xml
+XML|${jetty.home}/etc/extra.xml
+
+# The LIBs we expect (order is irrelevant)
+LIB|${jetty.home}/lib/optional.jar
+LIB|${jetty.home}/lib/base.jar
+LIB|${jetty.home}/lib/main.jar
+LIB|${jetty.home}/lib/other.jar
+LIB|${jetty.home}/lib/extra/extra0.jar
+LIB|${jetty.home}/lib/extra/extra1.jar
+
+# The Properties we expect (order is irrelevant)
+PROP|extra.prop=value0
+PROP|main.prop=valueT
+PROP|optional.prop=value0
+
+# Files / Directories to create
+FILE|maindir/
+EXISTS|start.ini

jetty-start/src/test/resources/usecases/empty.addToStart.prepare.txt

@@ -0,0 +1 @@
+--add-to-start=extra,optional

jetty-start/src/test/resources/usecases/empty.addToStartAddToStartd.assert.txt

@@ -0,0 +1 @@
+EX|org.eclipse.jetty.start.UsageException: Cannot --add-to-startd when ${jetty.base}/start.ini exists

jetty-start/src/test/resources/usecases/empty.addToStartAddToStartd.prepare.txt

@@ -0,0 +1,2 @@
+--add-to-start=extra
+--add-to-startd=optional

jetty-start/src/test/resources/usecases/empty.addToStartd.assert.txt

@@ -0,0 +1,24 @@
+## The XMLs we expect (order is important)
+XML|${jetty.home}/etc/optional.xml
+XML|${jetty.home}/etc/base.xml
+XML|${jetty.home}/etc/main.xml
+XML|${jetty.home}/etc/extra.xml
+
+# The LIBs we expect (order is irrelevant)
+LIB|${jetty.home}/lib/optional.jar
+LIB|${jetty.home}/lib/base.jar
+LIB|${jetty.home}/lib/main.jar
+LIB|${jetty.home}/lib/other.jar
+LIB|${jetty.home}/lib/extra/extra0.jar
+LIB|${jetty.home}/lib/extra/extra1.jar
+
+# The Properties we expect (order is irrelevant)
+PROP|extra.prop=value0
+PROP|main.prop=valueT
+PROP|optional.prop=value0
+
+# Files / Directories to create
+FILE|maindir/
+EXISTS|start.d/extra.ini
+EXISTS|start.d/optional.ini
+EXISTS|start.d/main.ini

jetty-start/src/test/resources/usecases/empty.addToStartd.prepare.txt

@@ -0,0 +1 @@
+--add-to-startd=extra,optional

jetty-start/src/test/resources/usecases/empty.addToStartdAddToStart.assert.txt

@@ -0,0 +1,24 @@
+## The XMLs we expect (order is important)
+XML|${jetty.home}/etc/optional.xml
+XML|${jetty.home}/etc/base.xml
+XML|${jetty.home}/etc/main.xml
+XML|${jetty.home}/etc/extra.xml
+
+# The LIBs we expect (order is irrelevant)
+LIB|${jetty.home}/lib/optional.jar
+LIB|${jetty.home}/lib/base.jar
+LIB|${jetty.home}/lib/main.jar
+LIB|${jetty.home}/lib/other.jar
+LIB|${jetty.home}/lib/extra/extra0.jar
+LIB|${jetty.home}/lib/extra/extra1.jar
+
+# The Properties we expect (order is irrelevant)
+PROP|extra.prop=value0
+PROP|main.prop=valueT
+PROP|optional.prop=value0
+
+# Files / Directories to create
+FILE|maindir/
+EXISTS|start.d/extra.ini
+EXISTS|start.d/optional.ini
+EXISTS|start.d/main.ini

jetty-start/src/test/resources/usecases/empty.addToStartdAddToStart.prepare.txt

@@ -0,0 +1,2 @@
+--add-to-startd=extra
+--add-to-start=optional

jetty-start/src/test/resources/usecases/transientWithIniTemplate.assert.txt

@@ -15,4 +15,3 @@ PROP|direct.option=direct
 
 # Files / Directories to create
 FILE|maindir/
-EXISTS|start.d/direct.ini

jetty-start/src/test/resources/usecases/transientWithIniTemplate.prepare.txt

@@ -1 +1 @@
---add-to-startd=direct
+--add-to-start=direct

jetty-start/src/test/resources/usecases/transientWithoutIniTemplate.assert.txt

@@ -16,5 +16,3 @@ PROP|direct.option=direct
 
 # Files / Directories to create
 FILE|maindir/
-EXISTS|start.d/direct.ini
-EXISTS|start.d/transient.ini

jetty-start/src/test/resources/usecases/transientWithoutIniTemplate.prepare.txt

@@ -1 +1 @@
---add-to-startd=direct
+--add-to-start=direct