Make os.pwd modifiable via the os.pwd0 dynamic variable
#298
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lolgab
approved these changes
Sep 7, 2024
lefou
reviewed
Sep 7, 2024
os/src-jvm/package.scala
Outdated
| @@ -38,7 +39,9 @@ package object os { | |||
| /** | |||
| * The current working directory for this process. | |||
| */ | |||
| val pwd: Path = os.Path(java.nio.file.Paths.get(".").toAbsolutePath) | |||
| def pwd: Path = pwd0.value | |||
| val pwd0: DynamicVariable[Path] = | |||
There was a problem hiding this comment.
Can we find a better name? pwdContext or pwdDynamic?
Would be nice, if reading code gives a clue what's happening.
Example:
val y = os.pwd0.withValue(os.pwd / "hello") {
os.pwd
}vs.
val y = os.pwdContext.withValue(os.pwd / "hello") {
os.pwd
}
lihaoyi
added a commit
that referenced
this pull request
Sep 7, 2024
…nction` (#299) Follow up to #298, which it turns out was not flexible enough to do what we need in Mill. Mill uses references to the `pwd` to create the `.dest` folder lazily, thus we cannot just assign a value to the `os.pwd` but actually need to assign a lambda that can be replaced to implement the lazy evaluation I pulled the trigger too quickly on releasing 0.10.6 with `os.dynamicPwd`, so this one has to be named differently. I chose `os.dynamicPwdFunction` and will make sure to test it with an unstable release before releasing a stable tag
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Basically the same thing we did in #295 and #283, except rather than being
os.SubProcess-specific this applies to all usage ofos.pwd. And with similar limitations: this works for code paths that go through OS-Lib, but not for code that usesjava.ioorjava.niodirectly.AFAIK this is the last of the "global" things we need to redirect in Mill tasks, after env variables and default subprocess streams. I'd like it to go into Mill 0.12.0 together with the other sandboxing-related changes
Added unit tests and updated the documentation
Pull Request: #298