Skip to content

Commit 17c6b78

Browse files
Pre-parse Mill CLI config in launcher to allow -i/--interactive flag to be passed anywhere (#5823)
Fixes #5773 Experimentally, it adds ~250ms to the Mill JVM launcher when `-i` or `--bsp` is passed, adds nothing to the Mill JVM launcher when they are not passed, and adds nothing to the Mill Native launcher in either case. Given that the Mill JVM launcher with `-i` already takes ~2s to run, bumping it up to ~2.25s isn't a huge deal, especially since only people on Windows-ARM would be affected (since that doesn't support Graal Native Images). In exchange we get a much more consistent user experience without this weird _"`-i` must come first"_ rule --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent baf1b7d commit 17c6b78

File tree

2 files changed

+23
-40
lines changed

2 files changed

+23
-40
lines changed

runner/daemon/src/mill/daemon/MillMain0.scala

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import mill.util.BuildInfo
2222
import mill.api
2323
import mill.api.daemon.internal.bsp.BspServerResult
2424

25-
import java.io.{InputStream, PipedInputStream, PrintStream, PrintWriter, StringWriter}
25+
import java.io.{InputStream, PrintStream, PrintWriter, StringWriter}
2626
import java.lang.reflect.InvocationTargetException
2727
import java.util.Locale
2828
import java.util.concurrent.{ThreadPoolExecutor, TimeUnit}
@@ -150,14 +150,6 @@ object MillMain0 {
150150
)
151151
(true, RunnerState.empty)
152152

153-
case Result.Success(config)
154-
if config.noDaemonEnabled > 0 && streams.in.getClass == classOf[PipedInputStream] =>
155-
// because we have stdin as dummy, we assume we were already started in server process
156-
streams.err.println(
157-
"-i/--interactive/--no-daemon/--bsp must be passed in as the first argument"
158-
)
159-
(false, RunnerState.empty)
160-
161153
case Result.Success(config) if config.noDaemonEnabled > 1 =>
162154
streams.err.println(
163155
"Only one of -i/--interactive, --no-daemon or --bsp may be given"

runner/launcher/src/mill/launcher/MillLauncherMain.java

Lines changed: 22 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -18,27 +18,32 @@
1818
* A Scala implementation would result in the JVM loading much more classes almost doubling the start-up times.
1919
*/
2020
public class MillLauncherMain {
21-
static final String bspFlag = "--bsp";
2221

2322
public static void main(String[] args) throws Exception {
24-
boolean runNoServer = false;
25-
if (args.length > 0) {
26-
String firstArg = args[0];
27-
runNoServer =
28-
Arrays.asList("--interactive", "--no-server", "--no-daemon", "--repl", bspFlag, "--help")
29-
.contains(firstArg)
30-
|| firstArg.startsWith("-i");
23+
var needParsedConfig = false;
24+
if (Arrays.stream(args)
25+
.anyMatch(f -> f.startsWith("-") && !f.startsWith("--") && f.contains("i"))) {
26+
needParsedConfig = true;
3127
}
32-
if (!runNoServer) {
33-
// WSL2 has the directory /run/WSL/ and WSL1 not.
34-
String osVersion = System.getProperty("os.version");
35-
if (osVersion != null && (osVersion.contains("icrosoft") || osVersion.contains("WSL"))) {
36-
// Server-Mode not supported under WSL1
37-
runNoServer = true;
38-
}
28+
for (String token :
29+
Arrays.asList("--interactive", "--no-server", "--no-daemon", "--repl", "--bsp", "--help")) {
30+
if (Arrays.stream(args).anyMatch(f -> f.equals(token))) needParsedConfig = true;
31+
}
32+
33+
boolean runNoDaemon = false;
34+
boolean bspMode = false;
35+
36+
// Only use MillCliConfig and other Scala classes if we detect that a relevant flag
37+
// might have been passed, to avoid loading those classes on the common path for performance
38+
if (needParsedConfig) {
39+
var config = MillCliConfig.parse(args).toOption();
40+
if (config.exists(c -> c.bsp().value())) bspMode = true;
41+
if (config.exists(
42+
c -> c.interactive().value() || c.noServer().value() || c.noDaemon().value()))
43+
runNoDaemon = true;
3944
}
4045

41-
var outMode = containsBspFlag(args) ? OutFolderMode.BSP : OutFolderMode.REGULAR;
46+
var outMode = bspMode ? OutFolderMode.BSP : OutFolderMode.REGULAR;
4247
exitInTestsAfterBspCheck();
4348
var outDir = OutFiles.outFor(outMode);
4449

@@ -57,7 +62,7 @@ public static void main(String[] args) throws Exception {
5762
+ " make it less responsive.");
5863
}
5964

60-
if (runNoServer) {
65+
if (runNoDaemon) {
6166
// start in no-server mode
6267
System.exit(MillProcessLauncher.launchMillNoDaemon(args, outMode));
6368
} else {
@@ -107,20 +112,6 @@ public void prepareDaemonDir(Path daemonDir) throws Exception {
107112
}
108113
}
109114

110-
private static boolean containsBspFlag(String[] args) {
111-
// First do a simple check because it is faster, as it only uses java stdlib.
112-
var simpleCheck = Arrays.asList(args).contains(bspFlag);
113-
if (!simpleCheck) return false;
114-
115-
// If the simple check passed, do a more expensive check which loads more classes, thus
116-
// introduces startup
117-
// overhead.
118-
//
119-
// This is done to prevent false positives in the simple check, for example when the user passes
120-
// "--bsp" as a flag to `run` task.
121-
return MillCliConfig.parse(args).toOption().exists(config -> config.bsp().value());
122-
}
123-
124115
private static void exitInTestsAfterBspCheck() {
125116
if (System.getenv("MILL_TEST_EXIT_AFTER_BSP_CHECK") != null) {
126117
System.exit(0);

0 commit comments

Comments
 (0)