Skip to content

Fix mill for Windows#1258

Draft
ArmborstL wants to merge 3 commits intodevfrom
mill-windows-fix
Draft

Fix mill for Windows#1258
ArmborstL wants to merge 3 commits intodevfrom
mill-windows-fix

Conversation

@ArmborstL
Copy link
Contributor

Checklist:

  • No wiki changes required

PR description

A recent commit by @superaxander added quotes to file paths to allow spaces in paths. However, on my Windows machine, the quotes in the .classpath file resulted in Error: Could not find or load main class vct.main.Main, so we removed those quotes again.
The CI did not catch this, because https://github.com/utwente-fmt/vercors/blob/dev/.github/workflows/scalatest.yml uses a release jar, bypassing the classpath file.

ToDo:

  • figure out how to integrate paths with spaces into the classpath
    • wrapping each path in the classpath file individually in quotes also does not help
  • add a test to the CI? (or are we fine with dev having problems as long as the release works?)

@bobismijnnaam
Copy link
Contributor

Maybe one more version of quoting that we can try: https://www.digizol.com/2006/12/setting-java-classpath-option-with.html . So quoting individual elements of the classpath.

java -classpath "C:/Documents and Settings/user/project/lib/axis.jar";"C:/Documents and Settings/user/project/lib/axis-ant.jar" TestClient

Haven't tested it myself...

@ArmborstL
Copy link
Contributor Author

As @superaxander figured out: without quotes, the Windows paths in the classpath file are used unchanged, i.e. the \ path separator remains unchanged. The moment you quote it, the parser treats \ as an escape character, mangling up the Windows paths. So the solution is to replace all \ in all the paths in classpath with \\.

@superaxander
Copy link
Member

Backslash issue should be resolved with this but I can't test it since the compilation fails due to the path being to long. This happens during the generation of the protobuf files. ScalaPBC should use the runWithArgumentFile option in protoc-bridge but it doesn't at the moment have an option to turn that on (so by extension the ScalaPB module in Mill also does not have this option)

I guess we should open some issues on those projects' repos.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments