Skip to content
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

Error converting relative paths to absolute paths in Windows if relative path has leading slash or backslash #201

Closed
philwalk opened this issue Aug 25, 2023 · 0 comments · Fixed by #202
Milestone

Comments

@philwalk
Copy link
Contributor

philwalk commented Aug 25, 2023

Version(s)

os-lib release 0.9.1

Describe the bug
The following idiom is intended to convert a relative path to an absolute path:

val abspath = Path(relativeDir, pwd)

On Windows, this idiom fails, due to there being two types of relative path, unlike on other platforms.

Windows paths are described here: windows-file-path-formats. A reference to what might be called rootRelative is here.

To Reproduce

The following code illustrates the problem with rootRelative paths in Windows.

Example code:

    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:

isAbsolute: true
/omg
/omg

On Windows:

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)

Expected behaviour
A rootRelative path is supported by java.nio.file.Paths.get() but not by os.Path().

In Windows, calls to java.io.File#isAbsolute are insufficient to determine whether a path is relative to pwd.
A rootRelative path in Windows is relative to the current working drive (an immutable value captured at jvm startup).

There are 3 use-cases when creating a java.nio.file.Path object:

  • path is relative - interpret path as immediately below pwd
  • path is absolute - java.nio.file.Paths.get(path)
  • path is rootRelative - interpret path as relative to the current working drive.
lihaoyi pushed a commit that referenced this issue Sep 12, 2023
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>
@lefou lefou added this to the after 0.9.1 milestone Sep 15, 2023
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 a pull request may close this issue.

2 participants