Skip to content

Commit

Permalink
Issue #5263 Jetty Home warning (#5309)
Browse files Browse the repository at this point in the history
* Issue #5264 Jetty Home warning

Warn when using jetty home as a jetty base

* Issue #5304 HTTP2 HostHeader

 + updated more options doco and handling

Signed-off-by: Greg Wilkins <[email protected]>

* Issue #5264 Jetty Home warning

updates from review
  • Loading branch information
gregw authored Sep 22, 2020
1 parent 1294382 commit 592dfb8
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 60 deletions.
4 changes: 1 addition & 3 deletions jetty-deploy/src/test/resources/jetty.xml
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,7 @@
<!-- handler by context path and virtual host, and the -->
<!-- DefaultHandler, which handles any requests not handled by -->
<!-- the context handlers. -->
<!-- Other handlers may be added to the "Handlers" collection, -->
<!-- for example the jetty-requestlog.xml file adds the -->
<!-- RequestLogHandler after the default handler -->
<!-- Other handlers may be added to the "Handlers" collection. -->
<!-- =========================================================== -->
<Set name="handler">
<New id="Handlers" class="org.eclipse.jetty.server.handler.HandlerList">
Expand Down
2 changes: 1 addition & 1 deletion jetty-distribution/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@
<argument>jetty.home=${home-directory}</argument>
<argument>jetty.base=${base-directory}</argument>
<argument>--create-startd</argument>
<argument>--add-to-start=server,requestlog,deploy,websocket,ext,resources,client,annotations,jndi,servlets,jsp,jstl,http,https,demo</argument>
<argument>--add-module=server,requestlog,deploy,websocket,ext,resources,client,annotations,jndi,servlets,jsp,jstl,http,https,demo</argument>
</arguments>
</configuration>
<goals>
Expand Down
129 changes: 80 additions & 49 deletions jetty-start/src/main/java/org/eclipse/jetty/start/BaseBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@

package org.eclipse.jetty.start;

import java.io.BufferedReader;
import java.io.FileWriter;
import java.io.IOException;
import java.io.InputStreamReader;
import java.net.URI;
import java.nio.file.DirectoryStream;
import java.nio.file.Files;
Expand All @@ -28,6 +30,7 @@
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
Expand Down Expand Up @@ -176,7 +179,9 @@ else if (!licensing.acknowledgeLicenses())
{
if (!Files.exists(startini))
{
StartLog.info("create " + baseHome.toShortForm(startini));
if (Files.exists(getBaseHome().getBasePath("start.jar")))
StartLog.warn("creating start.ini in ${jetty.home} is not recommended!");
StartLog.info("create %s", baseHome.toShortForm(startini));
Files.createFile(startini);
modified.set(true);
}
Expand Down Expand Up @@ -222,72 +227,98 @@ public boolean accept(Path entry) throws IOException
}
}

if (!newlyAdded.isEmpty())
if ((startArgs.isCreateStartD() && (!Files.exists(startd) || Files.exists(startini))) ||
(!newlyAdded.isEmpty() && !Files.exists(startini) && !Files.exists(startd)))
{
if (!Files.exists(startini) && !Files.exists(startd) && FS.ensureDirectoryExists(startd))
if (Files.exists(getBaseHome().getBasePath("start.jar")) && !startArgs.isCreateStartD())
{
StartLog.warn("creating start.d in ${jetty.home} is not recommended!");
if (!startArgs.isCreateStartD())
{
BufferedReader input = new BufferedReader(new InputStreamReader(System.in));
System.err.printf("%nProceed (y/N)? ");
String response = input.readLine();

if (Utils.isBlank(response) || !response.toLowerCase(Locale.ENGLISH).startsWith("y"))
System.exit(1);
}
}

if (FS.ensureDirectoryExists(startd))
{
StartLog.info("mkdir " + baseHome.toShortForm(startd));
modified.set(true);
}

if (Files.exists(startini) && Files.exists(startd))
StartLog.warn("Use both %s and %s is deprecated", getBaseHome().toShortForm(startd), getBaseHome().toShortForm(startini));

boolean useStartD = Files.exists(startd);
builder.set(useStartD ? new StartDirBuilder(this) : new StartIniBuilder(this));
newlyAdded.stream().map(modules::get).forEach(module ->
if (Files.exists(startini) && startArgs.isCreateStartD())
{
String ini = null;
try
int ini = 0;
Path startdStartini = startd.resolve("start.ini");
while (Files.exists(startdStartini))
{
if (module.isSkipFilesValidation())
{
StartLog.debug("Skipping [files] validation on %s", module.getName());
}
else
{
// if (explicitly added and ini file modified)
if (startArgs.getStartModules().contains(module.getName()))
{
ini = builder.get().addModule(module, startArgs.getProperties());
if (ini != null)
modified.set(true);
}
for (String file : module.getFiles())
{
files.add(new FileArg(module, startArgs.getProperties().expand(file)));
}
}
ini++;
startdStartini = startd.resolve("start" + ini + ".ini");
}
catch (Exception e)
Files.move(startini, startdStartini);
modified.set(true);
}
}

boolean useStartD = Files.exists(startd);
if (useStartD && Files.exists(startini))
StartLog.warn("Use of both %s and %s is deprecated", getBaseHome().toShortForm(startd), getBaseHome().toShortForm(startini));

builder.set(useStartD ? new StartDirBuilder(this) : new StartIniBuilder(this));
newlyAdded.stream().map(modules::get).forEach(module ->
{
String ini = null;
try
{
if (module.isSkipFilesValidation())
{
throw new RuntimeException(e);
StartLog.debug("Skipping [files] validation on %s", module.getName());
}

if (module.isDynamic())
else
{
for (String s : module.getEnableSources())
// if (explicitly added and ini file modified)
if (startArgs.getStartModules().contains(module.getName()))
{
ini = builder.get().addModule(module, startArgs.getProperties());
if (ini != null)
modified.set(true);
}
for (String file : module.getFiles())
{
StartLog.info("%-15s %s", module.getName(), s);
files.add(new FileArg(module, startArgs.getProperties().expand(file)));
}
}
else if (module.isTransitive())
}
catch (Exception e)
{
throw new RuntimeException(e);
}

if (module.isDynamic())
{
for (String s : module.getEnableSources())
{
if (module.hasIniTemplate())
StartLog.info("%-15s transitively enabled, ini template available with --add-module=%s",
module.getName(),
module.getName());
else
StartLog.info("%-15s transitively enabled", module.getName());
StartLog.info("%-15s %s", module.getName(), s);
}
else
{
StartLog.info("%-15s initialized in %s",
}
else if (module.isTransitive())
{
if (module.hasIniTemplate())
StartLog.info("%-15s transitively enabled, ini template available with --add-module=%s",
module.getName(),
ini);
}
});
}
module.getName());
else
StartLog.info("%-15s transitively enabled", module.getName());
}
else
{
StartLog.info("%-15s initialized in %s", module.getName(), ini);
}
});

files.addAll(startArgs.getFiles());
if (!files.isEmpty() && processFileResources(files))
Expand Down
11 changes: 8 additions & 3 deletions jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ public class StartArgs
private boolean dryRun = false;
private final Set<String> dryRunParts = new HashSet<>();
private boolean jpms = false;
private boolean createStartD = false;
private boolean createStartIni = false;
private boolean updateIni = false;
private String mavenBaseUri;
Expand Down Expand Up @@ -716,7 +717,6 @@ public CommandLineBuilder getMainArgs(Set<String> parts) throws IOException

Prop p = processSystemProperty(key, value, null);
cmd.addRawArg("-D" + p.key + "=" + getProperties().expand(p.value));

}
else
{
Expand Down Expand Up @@ -1024,6 +1024,11 @@ public boolean isVersion()
return version;
}

public boolean isCreateStartD()
{
return createStartD;
}

public boolean isCreateStartIni()
{
return createStartIni;
Expand Down Expand Up @@ -1289,9 +1294,9 @@ public void parse(final String rawarg, String source)
licenseCheckRequired = true;
return;
}
if ("--create-startd".equals(arg))
if ("--create-startd".equals(arg) || "--create-start-d".equals(arg))
{
StartLog.warn("--create-startd option is deprecated! By default start.d is used");
createStartD = true;
run = false;
createFiles = true;
licenseCheckRequired = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ public class DirConfigSource implements ConfigSource
BANNED_ARGS.add("--create-files");
BANNED_ARGS.add("--create-startd");
BANNED_ARGS.add("--create-start-ini");
BANNED_ARGS.add("--create-start-d");
BANNED_ARGS.add("--add-to-startd");
BANNED_ARGS.add("--add-to-start");
BANNED_ARGS.add("--add-module");
Expand Down
14 changes: 10 additions & 4 deletions jetty-start/src/main/resources/org/eclipse/jetty/start/usage.txt
Original file line number Diff line number Diff line change
Expand Up @@ -120,19 +120,25 @@ Jetty Module Management:
amended in the generated file by specifying those
properties on the command line.

If a module is transitively enabled, it's ini file will not
If a module is transitively enabled, its ini file will not
be generated. An explicit --add-module is required to generate
an ini file.

This option replaces the depecated --add-to-start and --add-to-startd.
This option replaces the deprecated --add-to-start and --add-to-startd.

--update-ini Scan all start.ini and start.d/*.ini files and update
any properties with values specified on the command
line. e.g. --update-ini jetty.http.port=8888

--create-start-d
Create a ${jetty.base}/start.d directory. If a ${jetty.base}/start.ini
files exists, then it is moved into start.d. Using a start.d directory
is the default and this option is only needed to either force the creation of a
${jetty.home}/start.d or to move a start.ini file to start.d

--create-start-ini
Create a ${jetty.base}/start.ini file. If a ${jetty.base}/start.d
directory exists, then all it's contained ini files are concatinated into
directory exists, then all its contained ini files are concatenated into
the start.ini file.

--write-module-graph=<filename>
Expand Down Expand Up @@ -216,7 +222,7 @@ Properties:
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:
Each module may define its own properties. Start properties defined include:

jetty.home=[directory]
Set the home directory of the jetty distribution.
Expand Down

0 comments on commit 592dfb8

Please sign in to comment.