Skip to content

Making Paths more consistent using variable substitution #4870

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

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

albassort
Copy link

@albassort albassort commented Apr 5, 2025

Made to the specification in:

#3660 (comment)

3 substitutions:

JavaHome (usually os.home), CoursierCache, and the WorkingDirectory, usually out/. A forth one should potentially be made for IvyDeps, but I think that can be more dangerous.

Substitutions are done in this format. "$VarName. E.g "$WorkingDirectory". This is done because this makes the paths illegal on Windows,, however, not on macOS or Linux. There really aren't any illegal characters on these platforms....

Additionally, I only tested so far on Linux. Ubuntus 24.04.

The test is relatively simple. It recursively looks through all JSON files in the out folders, and where paths are found, compares the two. It makes sure the keys are the same, and that all the strings are the same/.

As you may not know me, this is my first contribution to a Scala project, and I am not familiar with all of the formatting requirements. I apologize if I made any mistakes, I basically just copied what I learned from reviewing the codebase, combined with my Nim syntax knowledge.

@lihaoyi
Copy link
Member

lihaoyi commented Apr 7, 2025

Please leave this as a draft until you are done working on it

@albassort
Copy link
Author

It appears, for whatever reason, the new changes rebased in have made the compiling using -Duser.home not work.

[3628-0]   --------------------------------------------
[3628-0]     com.lihaoyi:mill-scalajslib-worker-1_3:SNAPSHOT 
[3628-0]        No test override found at mill.runner.MillBuildBootstrap$$anon$1@5b7ae833/mill/local-test-overrides/com.lihaoyi-mill-scalajslib-worker-1_3
[3628-0]        not found: /mnt/.ivy2/local/com.lihaoyi/mill-scalajslib-worker-1_3/SNAPSHOT/ivys/ivy.xml
[3628-0]        not found: https://repo1.maven.org/maven2/com/lihaoyi/mill-scalajslib-worker-1_3/SNAPSHOT/mill-scalajslib-worker-1_3-SNAPSHOT.pom
[3628-0]   
[3628-0] --------------------------------------------

@albassort
Copy link
Author

The PR is mostly finished, all tests, and the ones that fail are the ones which also fail on main. It was quite tricky as a first project, but this should work.

There is one issue however. I cant get the example to compile with -Duser.home on either HEAD/MAIN or in mine, and it has nothing to do with the paths being normalized.

3623-0]   1 tasks failed
[3623-0]   client.scalaJSWorkerClasspath java.lang.RuntimeException: 
[3623-0]   Resolution failed for 1 modules:
[3623-0]   --------------------------------------------
[3623-0]     com.lihaoyi:mill-scalajslib-worker-1_3:SNAPSHOT 
[3623-0]        No test override found at mill.runner.MillBuildBootstrap$$anon$1@74826b2b/mill/local-test-overrides/com.lihaoyi-mill-scalajslib-worker-1_3
[3623-0]        not found: /home/albassort/.ivy2/local/com.lihaoyi/mill-scalajslib-worker-1_3/SNAPSHOT/ivys/ivy.xml
[3623-0]        not found: https://repo1.maven.org/maven2/com/lihaoyi/mill-scalajslib-worker-1_3/SNAPSHOT/mill-scalajslib-worker-1_3-SNAPSHOT.pom
[3623-0]   
[3623-0] --------------------------------------------

This was working last week, now its not, and im confused.

@albassort albassort marked this pull request as ready for review May 3, 2025 21:04
@albassort albassort marked this pull request as draft May 4, 2025 18:54
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.

2 participants