Skip to content

Fix some cases where AbsolutePath::relative(to: AbsolutePath) would assert on Windows#534

Merged
daveinglis merged 1 commit intoswiftlang:mainfrom
daveinglis:fix_asserts
Feb 18, 2026
Merged

Fix some cases where AbsolutePath::relative(to: AbsolutePath) would assert on Windows#534
daveinglis merged 1 commit intoswiftlang:mainfrom
daveinglis:fix_asserts

Conversation

@daveinglis
Copy link
Contributor

  • there where a couple case where relative(to:) would assert on windows when a path was long (>260) and when tailing slashes where not removed in some cases.

@daveinglis daveinglis force-pushed the fix_asserts branch 2 times, most recently from 975683d to df81ada Compare December 16, 2025 18:29
@jakepetroules
Copy link
Contributor

@swift-ci test

@daveinglis daveinglis marked this pull request as draft December 17, 2025 21:31
@daveinglis
Copy link
Contributor Author

This needs some more work/investigation, seeing some strange crashes in swiftPM that this is causing

@daveinglis daveinglis marked this pull request as ready for review December 18, 2025 14:59
if string.first?.isASCII ?? false, string.first?.isLetter ?? false, string.first?.isLowercase ?? false,
let path: String
let hasDrive: Bool
if string.first?.isASCII ?? false, string.first?.isLetter ?? false,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (possibly-blocking): Does not account for long path.

This does not take into account long path \\?\D:\<very long path>. Can we instead use Regex to determine if the passed string matches the desired conditions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that it not supporting that is fine. Foundation does go to some lengths to ensure that long paths are transparent to the user (it converts on the way in and out). Users generally would not type the NT path (and you cannot copy it from explorer as that transacts in Win32 paths and cmd would rely on DOS paths).

…ssert

- there where a couple case where relative(to:) would assert on windows
  when a path was long (>206), when tailing slashes where not removed
  in some cases and when paths where on different drives.

- strip long path prefix in appending methods.
@daveinglis
Copy link
Contributor Author

@swift-ci test

@daveinglis
Copy link
Contributor Author

@swift-ci test windows

Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that there is a small edge case that isn't being considered, but this seems good outside of that.

// Check if this is a root directory (e.g., "C:\" or just "\")
// A root directory is either just "\" or "X:\" where X is a drive letter
let isRootDir = result.count == 1 || // Just "\"
(result.count == 3 && result.dropFirst().first == ":") // "X:\"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, not sure I fully understand, how does this not also mark 1:\ as a rooted directory where it is actually a file (ADS) path?

@daveinglis daveinglis merged commit 41e3ff4 into swiftlang:main Feb 18, 2026
47 checks passed
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.

4 participants