Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow .mill-jvm-opts to interpolate environment variables, add .mill-opts #3841

Merged
merged 11 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 2 additions & 44 deletions build.mill
Original file line number Diff line number Diff line change
Expand Up @@ -643,8 +643,7 @@ def launcherScript(
shellCommands = {
val jvmArgsStr = shellJvmArgs.mkString(" ")
def java(mainClass: String, passMillJvmOpts: Boolean) = {
val millJvmOpts = if (passMillJvmOpts) "$mill_jvm_opts" else ""
s"""exec "$$JAVACMD" $jvmArgsStr $$JAVA_OPTS $millJvmOpts -cp "${shellClassPath.mkString(
s"""exec "$$JAVACMD" $jvmArgsStr $$JAVA_OPTS -cp "${shellClassPath.mkString(
":"
)}" $mainClass "$$@""""
}
Expand All @@ -655,49 +654,19 @@ def launcherScript(
| JAVACMD="$$JAVA_HOME/bin/java"
|fi
|
|mill_jvm_opts=""
|init_mill_jvm_opts () {
| if [ -z $$MILL_JVM_OPTS_PATH ] ; then
| mill_jvm_opts_file=".mill-jvm-opts"
| else
| mill_jvm_opts_file=$$MILL_JVM_OPTS_PATH
| fi
|
| if [ -f "$$mill_jvm_opts_file" ] ; then
| # We need to append a newline at the end to fix
| # https://github.com/com-lihaoyi/mill/issues/2140
| newline="
|"
| mill_jvm_opts="$$(
| echo "$$newline" | cat "$$mill_jvm_opts_file" - | (
| while IFS= read line
| do
| mill_jvm_opts="$${mill_jvm_opts} $$(echo $$line | grep -v "^[[:space:]]*[#]")"
| done
| # we are in a sub-shell, so need to return it explicitly
| echo "$${mill_jvm_opts}"
| )
| )"
| mill_jvm_opts="$${mill_jvm_opts} -Dmill.jvm_opts_applied=true"
| fi
|}
|
Comment on lines -658 to -684
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lefou do you remember what this stuff is? AFAIK, we currently do all MILL_JVM_OPTS processing in the mill client Java code, so not sure why we have to do some bash logic here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also the relevant integration tests seem to pass with this stuff deleted

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be remnant from back when mill -i ran everything within a single process, so we needed to make sure it was initialized correctly before that process started. Should be unnecessary now

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly. This was prior to processing this in Java. This is a left over and should be no longer needed since we no longer run any task evaluation in the client JVM.

|# Client-server mode doesn't seem to work on WSL, just disable it for now
|# https://stackoverflow.com/a/43618657/871202
|if grep -qEi "(Microsoft|WSL)" /proc/version > /dev/null 2> /dev/null ; then
| init_mill_jvm_opts
| if [ -z $$COURSIER_CACHE ] ; then
| COURSIER_CACHE=.coursier
| fi
| ${java(millMainClass, true)}
|else
| if [ "$${1%"-i"*}" != "$$1" ] ; then # first arg starts with "-i"
| init_mill_jvm_opts
| ${java(millMainClass, true)}
| else
| case "$$1" in
| -i | --interactive | --repl | --no-server | --bsp )
| init_mill_jvm_opts
| ${java(millMainClass, true)}
| ;;
| *)
Expand All @@ -711,8 +680,7 @@ def launcherScript(
cmdCommands = {
val jvmArgsStr = cmdJvmArgs.mkString(" ")
def java(mainClass: String, passMillJvmOpts: Boolean) = {
val millJvmOpts = if (passMillJvmOpts) "!mill_jvm_opts!" else ""
s""""%JAVACMD%" $jvmArgsStr %JAVA_OPTS% $millJvmOpts -cp "${cmdClassPath.mkString(
s""""%JAVACMD%" $jvmArgsStr %JAVA_OPTS% -cp "${cmdClassPath.mkString(
";"
)}" $mainClass %*"""
}
Expand All @@ -726,17 +694,7 @@ def launcherScript(
|if "%1" == "--no-server" set _I_=true
|if "%1" == "--bsp" set _I_=true
|
|set "mill_jvm_opts="
|set "mill_jvm_opts_file=.mill-jvm-opts"
|if not "%MILL_JVM_OPTS_PATH%"=="" set "mill_jvm_opts_file=%MILL_JVM_OPTS_PATH%"
|
|if defined _I_ (
| if exist %mill_jvm_opts_file% (
| for /f "delims=" %%G in (%mill_jvm_opts_file%) do (
| set line=%%G
| if "!line:~0,2!"=="-X" set "mill_jvm_opts=!mill_jvm_opts! !line!"
| )
| )
| ${java(millMainClass, true)}
|) else (
| ${java(millMainClass, false)}
Expand Down
37 changes: 31 additions & 6 deletions docs/modules/ROOT/partials/Installation_IDE_Support.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -196,18 +196,43 @@ JVM options, one per line.
For example, if your build requires a lot of memory and bigger stack size, your
`.mill-jvm-opts` could look like this:

_.mill-jvm-opts_
----
-Xss10m
-Xmx10G
----

The file name for passing JVM options to the Mill launcher is configurable. If
for some reason you don't want to use `.mill-jvm-opts` file name, add
`MILL_JVM_OPTS_PATH` environment variable with any other file name.
Note that `.mill-jvm-opts` requires each CLI token to be on a separate line, so
`-Xss10m -Xmx10G` on a single line is not allowed (as it would pass `"-Xss10m -Xmx10G"`
as a single token and fail argument parsing)

NOTE: `.mill-jvm-opts` is for passing JVM options to the JVM running Mill itself.
If you want to pass JVM options to the project that Mill is building and running,
see the section on
`.mill-jvm-opts` also supports environment variable interpolation, e.g.

_.mill-jvm-opts_
----
-Dmy.jvm.property=$PWD
----

The file name `.mill-jvm-opts` can be overridden via the `MILL_JVM_OPTS_PATH` environment
variable.

== Custom Mill Options

Mill also supports the `.mill-opts` file for passing a default set of command line
options to Mill itself. For example, if your project's tasks are CPU heavy, you
may want everyone using your project to run only 0.5 concurrent tasks per CPU. This
can be done by setting `.mill-opts` to:

_.mill-opts_
----
--jobs=0.5C
----

The file name `.mill-opts` can be overridden via the `MILL_OPTS_PATH` environment variable.

NOTE: `.mill-jvm-opts` is for passing JVM options to the JVM running Mill,
and `.mill-opts` is for passing options to Mill itself. If you want to pass JVM options
to the project that Mill is building and running, see the section on
xref:javalib/module-config.adoc#_compilation_execution_flags[Compilation and Execution Flags].

== Other installation methods
Expand Down
3 changes: 2 additions & 1 deletion integration/feature/mill-jvm-opts/resources/.mill-jvm-opts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@

# comment after an empty line
-DPROPERTY_PROPERLY_SET_VIA_JVM_OPTS=value-from-file
-Dproperty.properly.set.via.jvm.opts=value-from-file
-Dproperty.with.interpolated.working.dir=value-with-$PWD
-Xss120m
3 changes: 3 additions & 0 deletions integration/feature/mill-jvm-opts/resources/.mill-opts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@

# comment after an empty line
--jobs=17
15 changes: 13 additions & 2 deletions integration/feature/mill-jvm-opts/resources/build.mill
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,26 @@ import java.lang.management.ManagementFactory
import scala.jdk.CollectionConverters._

def checkJvmOpts() = Task.Command {
val prop = System.getProperty("PROPERTY_PROPERLY_SET_VIA_JVM_OPTS")
val prop = System.getProperty("property.properly.set.via.jvm.opts")
if (prop != "value-from-file") sys.error("jvm-opts not correctly applied, value was: " + prop)
val runtime = ManagementFactory.getRuntimeMXBean()
val args = runtime.getInputArguments().asScala.toSet
if (!args.contains("-DPROPERTY_PROPERLY_SET_VIA_JVM_OPTS=value-from-file")) {
if (!args.contains("-Dproperty.properly.set.via.jvm.opts=value-from-file")) {
sys.error("jvm-opts not correctly applied, args were: " + args.mkString)
}
if (!args.contains("-Xss120m")) {
sys.error("jvm-opts not correctly applied, args were: " + args.mkString)
}
()
}

def checkEnvJvmOpts() = Task.Command {
val prop = System.getProperty("property.with.interpolated.working.dir")
assert(prop.startsWith("value-with-"))
assert(prop.contains("integration/feature/mill-jvm-opts"))
}

def checkNonJvmOpts() = Task.Command {
System.err.println(Task.ctx().asInstanceOf[mill.api.Ctx.Jobs].jobs)
assert(Task.ctx().asInstanceOf[mill.api.Ctx.Jobs].jobs == 17)
}
15 changes: 13 additions & 2 deletions integration/feature/mill-jvm-opts/src/MillJvmOptsTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,20 @@ import utest._

object MillJvmOptsTests extends UtestIntegrationTestSuite {
val tests: Tests = Tests {
test("JVM options from file .mill-jvm-opts are properly read") - integrationTest { tester =>
test("simple") - integrationTest { tester =>
import tester._
assert(eval("checkJvmOpts").isSuccess)
val res = eval("checkJvmOpts")
assert(res.isSuccess)
}
test("interpolatedEnvVars") - integrationTest { tester =>
import tester._
val res = eval("checkEnvJvmOpts")
assert(res.isSuccess)
}
test("nonJvmOpts") - integrationTest { tester =>
import tester._
val res = eval("checkNonJvmOpts")
assert(res.isSuccess)
}
}
}
1 change: 1 addition & 0 deletions main/client/src/mill/main/client/EnvVars.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ public class EnvVars {


public static final String MILL_JVM_OPTS_PATH = "MILL_JVM_OPTS_PATH";
public static final String MILL_OPTS_PATH = "MILL_OPTS_PATH";


/**
Expand Down
29 changes: 25 additions & 4 deletions main/client/src/mill/main/client/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import java.util.List;
import java.util.LinkedList;
import java.util.Scanner;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class Util {
// use methods instead of constants to avoid inlining by compiler
Expand Down Expand Up @@ -139,14 +141,12 @@ static String sha1Hash(String path) throws NoSuchAlgorithmException {
*/
public static List<String> readOptsFileLines(final File file) {
final List<String> vmOptions = new LinkedList<>();
try (
final Scanner sc = new Scanner(file)
) {
try (final Scanner sc = new Scanner(file)) {
while (sc.hasNextLine()) {
String arg = sc.nextLine();
String trimmed = arg.trim();
if (!trimmed.isEmpty() && !trimmed.startsWith("#")) {
vmOptions.add(arg);
vmOptions.add(interpolateEnvVars(arg, System.getenv()));
}
}
} catch (FileNotFoundException e) {
Expand All @@ -155,4 +155,25 @@ public static List<String> readOptsFileLines(final File file) {
return vmOptions;
}

public static String interpolateEnvVars(String input, Map<String, String> env){
Matcher matcher = envInterpolatorPattern.matcher(input);
// StringBuilder to store the result after replacing
StringBuffer result = new StringBuffer();

while (matcher.find()) {
String match = matcher.group(1);
if (match.equals("$")) {
matcher.appendReplacement(result, "\\$");
} else {
String envVarValue = env.get(match);
matcher.appendReplacement(result, ""+envVarValue);
}
}

matcher.appendTail(result); // Append the remaining part of the string
return result.toString();
}

static Pattern envInterpolatorPattern = Pattern.compile("\\$(\\$|[A-Z_][A-Z0-9_]*)");

}
2 changes: 1 addition & 1 deletion main/server/src/mill/main/server/Server.scala
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ abstract class Server[T](
os.remove.all(socketPath)

val relFile = socketPath.relativeTo(os.pwd).toNIO.toFile
serverLog("listening on socket " + relFile)
serverLog("listening on socket " + relFile + " " + os.pwd)
// Use relative path because otherwise the full path might be too long for the socket API
val addr = AFUNIXSocketAddress.of(relFile)
AFUNIXServerSocket.bindOn(addr)
Expand Down
13 changes: 12 additions & 1 deletion runner/client/src/mill/runner/client/MillClientMain.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.Collections;

import mill.main.client.ServerLauncher;
import mill.main.client.Util;
import mill.main.client.lock.Locks;
import mill.main.client.OutFiles;
import mill.main.client.ServerCouldNotBeStarted;

import static mill.runner.client.MillProcessLauncher.millOptsFile;

/**
* This is a Java implementation to speed up repetitive starts.
* A Scala implementation would result in the JVM loading much more classes almost doubling the start-up times.
Expand Down Expand Up @@ -36,7 +40,14 @@ public static void main(String[] args) throws Exception {
MillNoServerLauncher.runMain(args);
} else try {
// start in client-server mode
ServerLauncher launcher = new ServerLauncher(System.in, System.out, System.err, System.getenv(), args, null, -1){
java.util.List<String> optsArgs = Util.readOptsFileLines(millOptsFile());
Collections.addAll(optsArgs, args);

ServerLauncher launcher = new ServerLauncher(
System.in, System.out, System.err,
System.getenv(), optsArgs.toArray(new String[0]),
null, -1
){
public void initServer(Path serverDir, boolean setJnaNoSys, Locks locks) throws Exception{
MillProcessLauncher.launchMillServer(serverDir, setJnaNoSys);
}
Expand Down
9 changes: 9 additions & 0 deletions runner/client/src/mill/runner/client/MillProcessLauncher.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ static int launchMillNoServer(String[] args) throws Exception {
l.addAll(millLaunchJvmCommand(setJnaNoSys));
l.add("mill.runner.MillMain");
l.add(processDir.toAbsolutePath().toString());
l.addAll(Util.readOptsFileLines(millOptsFile()));
l.addAll(Arrays.asList(args));

final ProcessBuilder builder = new ProcessBuilder()
Expand Down Expand Up @@ -91,6 +92,14 @@ static File millJvmOptsFile() {
return new File(millJvmOptsPath).getAbsoluteFile();
}

static File millOptsFile() {
String millJvmOptsPath = System.getenv(EnvVars.MILL_OPTS_PATH);
if (millJvmOptsPath == null || millJvmOptsPath.trim().equals("")) {
millJvmOptsPath = ".mill-opts";
}
return new File(millJvmOptsPath).getAbsoluteFile();
}

static boolean millJvmOptsAlreadyApplied() {
final String propAppliedProp = System.getProperty("mill.jvm_opts_applied");
return propAppliedProp != null && propAppliedProp.equals("true");
Expand Down
2 changes: 2 additions & 0 deletions runner/src/mill/runner/MillServerMain.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import scala.util.Try

object MillServerMain {
def main(args0: Array[String]): Unit = SystemStreams.withTopLevelSystemStreamProxy {
println("MillServerMain")
pprint.log(sys.props)
// Disable SIGINT interrupt signal in the Mill server.
//
// This gets passed through from the client to server whenever the user
Expand Down
3 changes: 2 additions & 1 deletion testkit/src/mill/testkit/ExampleTester.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package mill.testkit
import mill.util.Util
import mill.main.client.EnvVars.MILL_TEST_SUITE
import utest._

/**
Expand Down Expand Up @@ -114,7 +115,7 @@ class ExampleTester(
stderr = os.Pipe,
cwd = workspacePath,
mergeErrIntoOut = true,
env = Map("MILL_TEST_SUITE" -> this.getClass().toString()),
env = Map(MILL_TEST_SUITE -> this.getClass().toString()),
check = false
)

Expand Down
3 changes: 2 additions & 1 deletion testkit/src/mill/testkit/IntegrationTester.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package mill.testkit

import mill.main.client.EnvVars.MILL_TEST_SUITE
import mill.define.Segments
import mill.eval.Evaluator
import mill.resolve.SelectMode
Expand Down Expand Up @@ -91,7 +92,7 @@ object IntegrationTester {
)
}

def millTestSuiteEnv: Map[String, String] = Map("MILL_TEST_SUITE" -> this.getClass().toString())
def millTestSuiteEnv: Map[String, String] = Map(MILL_TEST_SUITE -> this.getClass().toString())

/**
* Helpers to read the `.json` metadata files belonging to a particular task
Expand Down
Loading