Skip to content

Conversation

@tomerd
Copy link
Contributor

@tomerd tomerd commented Dec 24, 2023

motivation: better support for Windows style paths

changes:

  • refactor AbsolutePath based on URL
  • refactor RelativePath based on [String]
  • remove redundant API surface area from RelativePath (anything not used outside tests)
  • remove RelativePath tests that covered redundant APIs

[TODO]

  • there are a number of tests in PathTests that are failing because the semantics is slightly different and the tests are coded to check for the existing semantics which may or may not be the right one. need to discuss which semantic we prefer and update the code or tests
  • this works for *nix style path, but should be adjusted to work for Windows. the fact we are using URL and [String] array should make that easier than the previous implementation and the genesis for this PR. cc @compnerd for help getting this work correctly on Windows.
  • improve the implementation to make it more efficient, address FIXME's etc
@tomerd tomerd marked this pull request as draft December 24, 2023 19:59
motivation: better support for Windows style paths changes: * refactor AbsolutePath based on URL * refactor RelativePath based on [String] * remove redundant API surface area from RelativePath (anything not used outside tests) * remove RelativePath tests that covered redundant APIs
@compnerd
Copy link
Member

From a quick look, we still have some API shape changes that need to be made. The concept of a root is not portable. The Windows model is a forest, not a tree. Furthermore, there are technically an infinite set of roots, so we cannot really enumerate them, but can check if a given path is a root of a tree.

@rauhul
Copy link
Member

rauhul commented Dec 26, 2023

From a quick look, we still have some API shape changes that need to be made. The concept of a root is not portable. The Windows model is a forest, not a tree. Furthermore, there are technically an infinite set of roots, so we cannot really enumerate them, but can check if a given path is a root of a tree.

Does system's FilePath type handle this?

@MaxDesiatov
Copy link
Contributor

MaxDesiatov commented Dec 29, 2023

Does system's FilePath type handle this?

AFAIU it unfortunately does not, and the lack of Windows CI on swift-system prevents us from checking that regressions in that area don't happen.

@tomerd
Copy link
Contributor Author

tomerd commented Jan 6, 2024

From a quick look, we still have some API shape changes that need to be made.

@compnerd agreed, and your help is needed to define how to make it so that it works better for Windows style paths

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

Labels

None yet

4 participants