Skip to content

Commit 42108d5

Browse files
Fix BSP install argv path (#5061)
Fixes #5058 It seems the `mill.main.cli` property and `MILL_MAIN_CLI` environment variable were not getting propagated properly, and the server-side `java.class.path` property we were falling back to is now invalid since #5025. The solution is to propagate a single `MILL_EXECUTABLE_PATH` variable computed from `class .getProtectionDomain().getCodeSource().getLocation()`, which returns the assembly jar in `packaged` on or native binary in `native` mode. This doesn't work in `local` mode, but that's fine since the `.ide` tests don't pass in `local` mode anyway. As we now handle `MILL_EXECUTABLE_PATH` in a narrow code path from the Mill launcher to the Mill server, we no longer need special handling in all the Mill bootstrap scripts to pass in this variable Added a test case to `ide[bsp-modules]` to exercise this at least on the JVM; tested the native image case manually. Also tested the JVM launcher in intellij via `MILL_STABLE_VERSION=1 ./mill dist.installLocalCache` and verified the BSP import no longer immediately crashes --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent fdb7d20 commit 42108d5

File tree

12 files changed

+44
-94
lines changed

12 files changed

+44
-94
lines changed

ci/mill.ps1

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ if (-not (Test-Path -Path $MILL -PathType Leaf)) {
122122
Invoke-WebRequest -Uri $DOWNLOAD_URL -OutFile $MILL
123123
}
124124

125-
$MILL_MAIN_CLI = $Env:MILL_MAIN_CLI ?? $PSCommandPath
125+
126126

127127
$MILL_FIRST_ARG = $null
128128
$REMAINING_ARGUMENTS = $remainingArgs
@@ -134,4 +134,4 @@ if ($null -ne $remainingArgs) {
134134
}
135135
}
136136

137-
& $MILL $MILL_FIRST_ARG -D "mill.main.cli=$MILL_MAIN_CLI" $REMAINING_ARGUMENTS
137+
& $MILL $MILL_FIRST_ARG $REMAINING_ARGUMENTS

core/constants/src/mill/constants/EnvVars.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,4 +42,9 @@ public class EnvVars {
4242
* e.g. to turn on additional testing/debug/log-related code
4343
*/
4444
public static final String MILL_TEST_SUITE = "MILL_TEST_SUITE";
45+
46+
/**
47+
* The path to the currently executing Mill launcher executable
48+
*/
49+
public static final String MILL_EXECUTABLE_PATH = "MILL_EXECUTABLE_PATH";
4550
}

integration/feature/subprocess-stdout/resources/mill

Lines changed: 0 additions & 67 deletions
This file was deleted.

integration/ide/bsp-modules/resources/build.mill

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,7 @@ def validate() = Task.Command {
3131
os.write(file, content)
3232
PathRef(file)
3333
}
34+
35+
def checkExecutable() = Task.Command {
36+
println("checkExecutable succeeded")
37+
}

integration/ide/bsp-modules/src/BspModulesTests.scala

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,15 @@ object BspModulesTests extends UtestIntegrationTestSuite {
1414
val res = eval("mill.bsp.BSP/install")
1515
assert(res.isSuccess)
1616
os.exists(workspacePath / Constants.bspDir / s"${Constants.serverName}.json") ==> true
17+
val json = ujson.read(
18+
os.read(workspacePath / Constants.bspDir / s"${Constants.serverName}.json")
19+
)
20+
21+
val executable = json("argv").arr(0).str
22+
val checkRes = os.call((executable, "checkExecutable"), cwd = workspacePath)
23+
assert(checkRes.exitCode == 0)
24+
assert(checkRes.out.text().contains("checkExecutable succeeded"))
25+
()
1726
}
1827
test("ModuleUtils resolves all referenced transitive modules") - integrationTest { tester =>
1928
import tester._

integration/ide/bsp-server/src/BspServerTests.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ object BspServerTests extends UtestIntegrationTestSuite {
4343
stdout = os.Inherit,
4444
stderr = os.Inherit,
4545
check = true,
46-
env = Map("MILL_MAIN_CLI" -> tester.millExecutable.toString)
46+
env = Map("MILL_EXECUTABLE_PATH" -> tester.millExecutable.toString)
4747
)
4848

4949
withBspServer(

mill

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -294,10 +294,6 @@ if [ ! -s "${MILL}" ] ; then
294294
fi
295295
fi
296296

297-
if [ -z "$MILL_MAIN_CLI" ] ; then
298-
MILL_MAIN_CLI="${0}"
299-
fi
300-
301297
MILL_FIRST_ARG=""
302298
if [ "$1" = "--bsp" ] || [ "$1" = "-i" ] || [ "$1" = "--interactive" ] || [ "$1" = "--no-server" ] || [ "$1" = "--repl" ] || [ "$1" = "--help" ] ; then
303299
# Need to preserve the first position of those listed options
@@ -313,4 +309,4 @@ unset MILL_REPO_URL
313309

314310
# We don't quote MILL_FIRST_ARG on purpose, so we can expand the empty value without quotes
315311
# shellcheck disable=SC2086
316-
exec "${MILL}" $MILL_FIRST_ARG -D "mill.main.cli=${MILL_MAIN_CLI}" "$@"
312+
exec "${MILL}" $MILL_FIRST_ARG "$@"

mill.bat

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,6 @@ if [!GITHUB_RELEASE_CDN!]==[] (
4242
set "GITHUB_RELEASE_CDN="
4343
)
4444

45-
if [!MILL_MAIN_CLI!]==[] (
46-
set "MILL_MAIN_CLI=%~f0"
47-
)
4845

4946
set "MILL_REPO_URL=https://github.com/com-lihaoyi/mill"
5047

@@ -267,4 +264,4 @@ if not [!MILL_FIRST_ARG!]==[] (
267264
)
268265
)
269266

270-
"%MILL%" %MILL_FIRST_ARG% -D "mill.main.cli=%MILL_MAIN_CLI%" %MILL_PARAMS%
267+
"%MILL%" %MILL_FIRST_ARG% %MILL_PARAMS%

runner/bsp/src/mill/bsp/BSP.scala

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,8 @@ private[mill] object BSP {
3434
}
3535

3636
private def bspConnectionJson(jobs: Int, debug: Boolean): String = {
37-
val millPath = sys.env.get("MILL_MAIN_CLI")
38-
.orElse(sys.props.get("mill.main.cli"))
39-
// we assume, the classpath is an executable jar here
40-
.orElse(sys.props.get("java.class.path"))
41-
.getOrElse(throw new IllegalStateException("System property 'java.class.path' not set"))
37+
val millPath = sys.env.get("MILL_EXECUTABLE_PATH")
38+
.getOrElse(throw new IllegalStateException("Env 'MILL_EXECUTABLE_PATH' not set"))
4239

4340
upickle.default.write(
4441
BspConfigJson(

runner/client/src/mill/runner/client/MillProcessLauncher.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ static Process configureRunMillProcess(ProcessBuilder builder, Path serverDir) t
7676
Path sandbox = serverDir.resolve(ServerFiles.sandbox);
7777
Files.createDirectories(sandbox);
7878
builder.environment().put(EnvVars.MILL_WORKSPACE_ROOT, new File("").getCanonicalPath());
79+
builder.environment().put(EnvVars.MILL_EXECUTABLE_PATH, getExecutablePath());
7980

8081
String jdkJavaOptions = System.getenv("JDK_JAVA_OPTIONS");
8182
if (jdkJavaOptions == null) jdkJavaOptions = "";
@@ -386,4 +387,20 @@ public static void prepareMillRunFolder(Path serverDir) throws Exception {
386387
termInfoPropagatorThread.setDaemon(true);
387388
termInfoPropagatorThread.start();
388389
}
390+
391+
public static String getExecutablePath() {
392+
try {
393+
// Gets the location of the JAR or native image
394+
Path path = Paths.get(MillProcessLauncher.class
395+
.getProtectionDomain()
396+
.getCodeSource()
397+
.getLocation()
398+
.toURI());
399+
400+
File file = path.toFile();
401+
return file.getAbsolutePath();
402+
} catch (java.net.URISyntaxException e) {
403+
throw new RuntimeException("Failed to determine Mill client executable path", e);
404+
}
405+
}
389406
}

0 commit comments

Comments
 (0)