Fix Windows launcher #791
Fix Windows launcher #791
Conversation
| PathRef(millPath) | ||
| } | ||
|
|
||
| def prependShellScript = T{ | ||
| val (millArgs, otherArgs) = forkArgs().partition(arg => arg.startsWith("-DMILL") && !arg.startsWith("-DMILL_VERSION")) |
There was a problem hiding this comment.
We pass the MILL_SOMETHING properties via file, so that Windows doesn't complain!
Everything except MILL_VERSION because it's used in lots of places.
| classpath, | ||
| classpath | ||
| Agg(pathingJar().path.toString) // TODO not working yet on Windows! see #791 |
There was a problem hiding this comment.
On Windows, the classpath is only the "pathingJar", it contains all the dependencies.
It works in batch mode, interactive needs bit more debugging.
There was a problem hiding this comment.
@lihaoyi this only relates to the dev.assembly version..
| String millOptionsPath = System.getProperty("MILL_OPTIONS_PATH"); | ||
| if(millOptionsPath != null) { | ||
| // read MILL_CLASSPATH from file MILL_OPTIONS_PATH | ||
| Properties millProps = new Properties(); |
There was a problem hiding this comment.
On Windows we read the classpath from the MILL_OPTIONS_PATH file instead of env var..
|
@sake92 Is this ready to review? If so I can take a look |
|
@lihaoyi yes please! |
|
@sake92 this looks good to me I think. Before I merge, could you flesh out the PR description a bit to summarize (1) exactly which user-facing problems you are fixing, (2) what the root cause of those problems is, (3) how your PR solves those root causes and (4) how you tested it to verify your PR solves the things you think it solves? I'm still a bit fuzzy on the different problems with dev, assembly, dev.assembly, dev.launcher, etc. and which ones I should expect to behave differently due to this PR. This will also help future maintenance. Once that's done I'm happy to merge! |
|
@lihaoyi I updated the description, hope it's clearer now.
|
|
This looks good, thanks for the detailed PR description! |
There are 2 problems this PR tries to fix:
assemblytaskdev.launchertaskThese problems manifest with java 1.8.
In Hotspots 9+ there exists an option
-XX:VMOptionsFile...Production version (
assembly)Problem
mill.batmill foo.compileFirst time you run it you'll get this message:
If you kill it and try again, it just hangs...
I couldnt attach VisualVM for some reason to debug further.
I guess it cant start a server and pipe between client-server breaks!?
Fix
Remove
-XX:VMOptionsFilestuff.This is enough to get working Mill for Windows users! (even on java 1.8)
Testing
Build Mill with
publish-local.shcommands.Then use built Mill version on projects. See that it works. 馃槃
Development version (
dev.launcher)Here is where it gets interesting.
Problem
mill -i dev.launcherWhy? Because
-XX:VMOptionsFiledoesnt exist in java 1.8...Why is it needed in first place? Because Windows has command-line limitation of only ~8000 characters!
Most lengthy operations are
MILL_STUFFand classpath, of course (lot of JARs from Coursier local repo...)Fix
MILL_OPTIONS_PATH=$path/mill.propertiesto bypass long-DMILL_..props-cpparameterTesting
Build
dev.launcherand see it works in client-server mode.Only thing that isnt working on Windows is
-imode ofdev.launcher:Minor NOTE: Once you do
-iit stays during that shell session (SETLOCALwasn't used), so if you runmillafter that it will still use interactive mode... This PR fixes that also.