Skip to content

Commit

Permalink
fix for issue-201 (#202)
Browse files Browse the repository at this point in the history
Thanks for a great library! Hopefully this change will increase interest
among windows shell environment developers.

This fixes #201

The essence of the fix is to support paths with a leading slash (`'/' or
'\'`) on Windows.

In this PR, this path type is referred to as `driveRelative`, due to
subtle differences in how it is handled on `Windows` versus other
platforms.

Example code:
```scala
    val p1 = java.nio.file.Paths.get("/omg")
    printf("isAbsolute: %s\n", p1.isAbsolute)
    printf("%s\n", p1)
    printf("%s\n", os.Path("/omg"))
```
Output on Linux or OSX:
```sh
isAbsolute: true
/omg
/omg
```
On Windows:
```scala
isAbsolute: false
\omg
java.lang.IllegalArgumentException: requirement failed: \omg is not an absolute path
        at os.Path.<init>(Path.scala:474)
        at os.Path$.apply(Path.scala:426)
        at oslibChek$package$.main(oslibChek.sc:11)
        at oslibChek$package.main(oslibChek.sc)
```
### Background
On Windows, a `driveRelative` path is considered relative because a
drive letter prefix is required to fully resolve it.
Like other platforms, Windows also supports `posix relative` paths,
which are relative to the `current working directory`.

Because all platforms, including `Windows`, support absolute paths and
`posix relative` paths, it's convenient when writing
platform-independent code to view `driveRelative` paths as absolute
paths, as they are on other platforms.

On Windows, the `current working drive` is an immutable value that is
captured on jvm startup. Therefore, a `driveRelative` path on Windows
unambiguously refers to a unique file with a hidden drive letter.

This PR treats `driveRelative` paths as absolute paths in Windows, and
has no effect on other platforms.

#### Making `os/test/src/PathTests` platform independent

This PR enables `Unix()` tests in `os/test/src/PathTests.scala` so they
also verify required semantics in Windows.

### How this PR relates to #170 and #196
The purpose of #196 seems to be to add full support on Windows, without
resorting to `java.nio` classes and methods.
The addition of `os.root(...)` or `os.drive(...)` would be convenient
for people writing windows-only code, whereas this PR is intended to
allow writing platform-independent code, so the concerns seem to be
orthogonal.

It seems important not to alter `Path.segments` in a way that is
incompatible with `java.nio` segments.
 
An alternate way to preserve drive letter information would be to add a
method to one of the `Path.scala` traits that returns a drive letter if
appropriate on `Windows` and an empty String elsewhere.

```scala
trait PathChunk {
  def segments: Seq[String]
  def rootPrefix: String  // empty String, or Windows drive letter
  def ups: Int
}
```

With this approach, the following assertion would be valid on all
platforms:

```scala
os.Path(pwd.rootPrefix) ==> pwd
```

Then `driveRoot` in this PR would become `pwd.rootPrefix`.

There is another (perhaps never used?) very subtle feature of `Windows`
filesystem: each drive has a different value for `pwd`. It may not be
necessary to model this in `os-lib`, since these values are immutable in
a running jvm, and there are existing workarounds.


### Additional Details regarding unique aspects of Windows Filesystem
Paths
For completeness, the following lengthy `scala` REPL session illustrates
Path values returned by `java.nio.file.Paths.get()`, some of which might
be surprising.
<details>

Relevant information: My system drives are C: and F:, but not J:

First, some unsurprising results:
```scala
c:\work-directory> scala.bat
scala> import java.nio.file.Paths
Welcome to Scala 3.3.1-RC5 (17.0.2, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.

scala> Paths.get("/")
val res0: java.nio.file.Path = \

scala> Paths.get("/").isAbsolute
val res2: Boolean = false

scala> Paths.get("/").toAbsolutePath
val res1: java.nio.file.Path = C:\

scala> Paths.get("c:/").toAbsolutePath
C:\work-directory

scala> Paths.get("f:/").toAbsolutePath
F:\

scala> Paths.get("f:/..").normalize.toAbsolutePath
f:\

scala> Paths.get("c:/..").normalize.toAbsolutePath
c:\
```
Now some less obvious results:
```scala
scala> Paths.get("c:").toAbsolutePath
C:\work-directory

scala> Paths.get("f:").toAbsolutePath
F:\

scala> Paths.get("f:/..").normalize.toAbsolutePath
f:\

scala> Paths.get("c:/..").normalize.toAbsolutePath
c:\

scala> Paths.get("c:..").normalize.toAbsolutePath
C:\work-directory

scala> Paths.get("f:..").normalize.toAbsolutePath
F:\..

scala> Paths.get("F:Users")
val res1: java.nio.file.Path = F:Users

scala> Paths.get("F:Users").toAbsolutePath
val res2: java.nio.file.Path = F:\work-directory\Users

scala> Paths.get("j:").toAbsolutePath
java.io.IOError: java.io.IOException: Unable to get working directory of drive 'J'
  at java.base/sun.nio.fs.WindowsPath.toAbsolutePath(WindowsPath.java:926)
  at java.base/sun.nio.fs.WindowsPath.toAbsolutePath(WindowsPath.java:42)
  ... 35 elided
Caused by: java.io.IOException: Unable to get working directory of drive 'J'
  ... 37 more
```

The error message returned for a non-existing drive seems to imply that
existing drives have a `working directory`.
This is consistent with the `Windows API` as described here:

[GetFullPathNameA](https://learn.microsoft.com/en-gb/windows/win32/api/fileapi/nf-fileapi-getfullpathnamea)

I can set the `working drive` for F: in a `CMD` session before I start
up the `jvm`, and it does affect the output of `Paths.get()`.
```CMD
c:\work-directory>F:

f:\>cd work-directory

F:\work-directory>scala.bat
Welcome to Scala 3.3.1-RC5 (17.0.2, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.

scala> import java.nio.file.Paths

scala> Paths.get("/")
val res0: java.nio.file.Path = \

scala> Paths.get("/").toAbsolutePath
val res1: java.nio.file.Path = C:\

scala> Paths.get("f:")
val res2: java.nio.file.Path = f:

scala> Paths.get("f:").toAbsolutePath
val res3: java.nio.file.Path = F:\work-directory

scala> Paths.get("c:").toAbsolutePath
val res4: java.nio.file.Path = C:\work-directory

scala> Paths.get("c:")
val res5: java.nio.file.Path = c:
```

Regarding the interpretation of a drive letter expression not followed
by a '/' or '\\', the
rule is that it represents the `working directory` for that drive.

These values are immutable: after the `JVM` starts up it's not possible
to change a drive `working directory`.
</details>
  • Loading branch information
philwalk authored Sep 12, 2023
1 parent 9e876d3 commit f79f62f
Show file tree
Hide file tree
Showing 3 changed files with 196 additions and 161 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ target/
.project
.cache
.sbtserver
.scala-build/
.bsp/
project/.sbtserver
tags
nohup.out
Expand Down
47 changes: 39 additions & 8 deletions os/src/Path.scala
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,12 @@ sealed trait FilePath extends BasePath {
def toNIO: java.nio.file.Path
def resolveFrom(base: os.Path): os.Path
}

object FilePath {
def apply[T: PathConvertible](f0: T) = {
val f = implicitly[PathConvertible[T]].apply(f0)
if (f.isAbsolute) Path(f0)
def f = implicitly[PathConvertible[T]].apply(f0)
// if Windows root-relative path, convert it to an absolute path
if (Path.driveRelative(f0) || f.isAbsolute) Path(f0)
else {
val r = RelPath(f0)
if (r.ups == 0) r.asSubPath
Expand Down Expand Up @@ -297,7 +299,7 @@ object RelPath {
def apply[T: PathConvertible](f0: T): RelPath = {
val f = implicitly[PathConvertible[T]].apply(f0)

require(!f.isAbsolute, s"$f is not a relative path")
require(!f.isAbsolute && !Path.driveRelative(f0), s"$f is not a relative path")

val segments = BasePath.chunkify(f.normalize())
val (ups, rest) = segments.partition(_ == "..")
Expand Down Expand Up @@ -398,13 +400,16 @@ object Path {

def apply[T: PathConvertible](f: T, base: Path): Path = apply(FilePath(f), base)
def apply[T: PathConvertible](f0: T): Path = {
val f = implicitly[PathConvertible[T]].apply(f0)
// drive letter prefix is empty unless running in Windows.
val f = if (driveRelative(f0)) {
Paths.get(s"$driveRoot$f0")
} else {
implicitly[PathConvertible[T]].apply(f0)
}
if (f.iterator.asScala.count(_.startsWith("..")) > f.getNameCount / 2) {
throw PathError.AbsolutePathOutsideRoot
}

val normalized = f.normalize()
new Path(normalized)
new Path(f.normalize())
}

implicit val pathOrdering: Ordering[Path] = new Ordering[Path] {
Expand Down Expand Up @@ -436,6 +441,32 @@ object Path {
}
}

/**
* @return true if Windows OS and path begins with slash or backslash.
* Examples:
* driveRelative("/Users") // true in `Windows`, false elsewhere.
* driveRelative("\\Users") // true in `Windows`, false elsewhere.
* driveRelative("C:/Users") // false always
*/
def driveRelative[T: PathConvertible](f0: T): Boolean = {
if (driveRoot.isEmpty) {
false // non-Windows os
} else {
f0.toString.take(1) match {
case "\\" | "/" => true
case _ => false
}
}
}

/**
* @return current working drive if Windows, empty string elsewhere.
* Paths.get(driveRoot) == current working directory on all platforms.
*/
lazy val driveRoot: String = Paths.get(".").toAbsolutePath.getRoot.toString match {
case "/" => "" // implies a non-Windows platform
case s => s.take(2) // Windows current working drive (e.g., "C:")
}
}

trait ReadablePath {
Expand All @@ -452,7 +483,7 @@ class Path private[os] (val wrapped: java.nio.file.Path)
def toSource: SeekableSource =
new SeekableSource.ChannelSource(java.nio.file.Files.newByteChannel(wrapped))

require(wrapped.isAbsolute, s"$wrapped is not an absolute path")
require(wrapped.isAbsolute || Path.driveRelative(wrapped), s"$wrapped is not an absolute path")
def segments: Iterator[String] = wrapped.iterator().asScala.map(_.toString)
def getSegment(i: Int): String = wrapped.getName(i).toString
def segmentCount = wrapped.getNameCount
Expand Down
Loading

0 comments on commit f79f62f

Please sign in to comment.