refactor RelativePath to allow late stage canonicalization in support of windows #369

tomerd · 2022-11-17T01:13:23Z

motivation: delay canonicalization of relative path to the construction of absolute path from it, to better fit how windows paths work

changes:

remove the canonicalizing, non-throwing initializer of relative path
for tests, conform RelativePath and AbsolutePath to ExpressibleByStringLiteral and ExpressibleByStringInterpolation to make it easier to pass in string and static strings
remove incorrect validation of ~ as invalid relative path
update call sites and test

Tests/TSCBasicTests/PathTests.swift

tomerd · 2022-11-17T02:07:04Z

companion: swiftlang/swift-package-manager#5910

tomerd · 2023-04-13T22:24:16Z

testing via swiftlang/swift-package-manager#5910

compnerd · 2023-04-14T15:51:24Z

Hmm, merging this locally seems to not build on Windows? :/

compnerd · 2023-04-14T15:52:11Z

@swift-ci please test Windows platform

compnerd · 2023-04-14T15:54:54Z

@tomerd did you forget part of the commit?

S:\SourceCache\swift-project\swift-tools-support-core\Sources\TSCBasic\Path.swift:524:14: error: no exact matches in call to initializer
        self.init(normalizingAbsolutePath: path)

There is no Windowspath(normalizingAbsolutePath:) that I can see.

compnerd · 2023-04-14T15:55:00Z

@swift-ci please test Windows platform

Sources/TSCBasic/Path.swift

compnerd · 2023-04-14T16:38:43Z

This does seem to regress the test suite:

Test Case 'ManifestSourceGenerationTests.testAdvancedFeatures' started at 2023-04-14 09:37:04.984
TSCBasic/Path.swift:974: Assertion failed
Current stack trace:
0    (null)                             0x00007ffe873af220 swift_stdlib_reportFatalErrorInFile + 132

neonichu · 2023-04-17T15:56:03Z

Sources/TSCBasic/Path.swift

@@ -338,6 +321,8 @@ public struct RelativePath: Hashable, Sendable {
    }
 }

+
+


Sources/TSCBasic/Path.swift

tomerd · 2023-04-17T23:55:14Z

tested via swiftlang/swift-package-manager#5910

PR#369 - swiftlang#369 Caused the majority of tests on Windows to fail, as normalization of RelativePath was removed. This change also removed the call to 'PathAllocCanonicalize' for AbsolutePath which had the long file flag 'PATHCCH_ALLOW_LONG_PATHS'. Reintroducing canonicalization of AbsolutePath path representation to handle long paths. * Update tests dealing with RelativePath to match implementation. * Canonicalize the path representation for AbsolutePath which also allows for long path '\\?\' prefix addition when path > 260 in length. * Strip trailing backslash on string representation of AbsolutePath to match definition. Only strips for non root paths. * Add helper functions: - removeTrailingBackslash - stripPrefix - canonicalPathRepresentation * Add Windows API Error helpers * Add long path tests into each test case * Ran swift format on code * Update copyright dates

PR#369 - swiftlang#369 Caused the majority of tests on Windows to fail, as normalization of RelativePath was removed. This change also removed the call to 'PathAllocCanonicalize' for AbsolutePath which had the long file flag 'PATHCCH_ALLOW_LONG_PATHS'. Reintroducing canonicalization of AbsolutePath path representation to handle long paths. * Update tests dealing with RelativePath to match implementation. * Canonicalize the path representation for AbsolutePath which also allows for long path '\\?\' prefix addition when path > 260 in length. * Strip trailing backslash on string representation of AbsolutePath to match definition. Only strips for non root paths. * Add helper functions: - removeTrailingBackslash - stripPrefix - canonicalPathRepresentation * Add Windows API Error helpers * Add long path tests into each test case * Fix up .suffix. '.' has no suffix * Update copyright dates

* Resolves #505 - Fix handling for Windows long paths PR#369 - #369 Caused the majority of tests on Windows to fail, as normalization of RelativePath was removed. This change also removed the call to 'PathAllocCanonicalize' for AbsolutePath which had the long file flag 'PATHCCH_ALLOW_LONG_PATHS'. Reintroducing canonicalization of AbsolutePath path representation to handle long paths. * Update tests dealing with RelativePath to match implementation. * Canonicalize the path representation for AbsolutePath which also allows for long path '\\?\' prefix addition when path > 260 in length. * Strip trailing backslash on string representation of AbsolutePath to match definition. Only strips for non root paths. * Add helper functions: - removeTrailingBackslash - stripPrefix - canonicalPathRepresentation * Add Windows API Error helpers * Add long path tests into each test case * Fix up .suffix. '.' has no suffix * Update copyright dates * Make PathCchStripPrefix and PathCchRemoveBackslash use temporary buffer. - Move PathCchStripPrefix and PathCchRemoveBackslash to use a mutable temporary buffer. - Could not use buffer.withMemoryRebound and getCstring() as on Windows this seem to produce corrupt data. - Add more tests for unParsed '\\?\' and device '\\.\' paths - Remove the PATHCCH_CANONICALIZE_SLASHES flag as it is not needed. - Add Win32Error.swift to CMakeLists

tomerd requested review from aciidgh, abertelrud, neonichu and friedbunny as code owners November 17, 2022 01:13

tomerd commented Nov 17, 2022

View reviewed changes

Tests/TSCBasicTests/PathTests.swift Outdated Show resolved Hide resolved

tomerd changed the title ~~refactoring of relative path to allow late stage canonicalization in support of windows~~ [wip] refactoring of relative path to allow late stage canonicalization in support of windows Nov 17, 2022

tomerd mentioned this pull request Nov 17, 2022

refactor RelativePath to allow late stage canonicalization in support of windows swiftlang/swift-package-manager#5910

Merged

tomerd mentioned this pull request Apr 11, 2023

TSCBasic: protect against an invalid API usage #325

Closed

tomerd force-pushed the refactor/relative-path branch 2 times, most recently from 024c262 to 78feb0a Compare April 13, 2023 21:04

tomerd changed the title ~~[wip] refactoring of relative path to allow late stage canonicalization in support of windows~~ refactore of relative path to allow late stage canonicalization in support of windows Apr 13, 2023

tomerd force-pushed the refactor/relative-path branch from 78feb0a to 8edc44e Compare April 13, 2023 22:14

tomerd changed the title ~~refactore of relative path to allow late stage canonicalization in support of windows~~ refactor RelativePath to allow late stage canonicalization in support of windows Apr 14, 2023

compnerd requested changes Apr 14, 2023

View reviewed changes

Sources/TSCBasic/Path.swift Outdated Show resolved Hide resolved

compnerd reviewed Apr 14, 2023

View reviewed changes

Sources/TSCBasic/Path.swift Outdated Show resolved Hide resolved

compnerd reviewed Apr 14, 2023

View reviewed changes

Sources/TSCBasic/Path.swift Outdated Show resolved Hide resolved

Sources/TSCBasic/Path.swift Show resolved Hide resolved

neonichu reviewed Apr 17, 2023

View reviewed changes

Sources/TSCBasic/Path.swift Outdated

@@ -338,6 +321,8 @@ public struct RelativePath: Hashable, Sendable {

}

}

Copy link

Contributor

neonichu Apr 17, 2023

Choose a reason for hiding this comment

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

why

neonichu approved these changes Apr 17, 2023

View reviewed changes

tomerd commented Apr 17, 2023

View reviewed changes

Sources/TSCBasic/Path.swift Outdated Show resolved Hide resolved

tomerd requested a review from MaxDesiatov as a code owner April 17, 2023 18:13

tomerd added 3 commits April 17, 2023 11:15

wip

ddd2ee6

fixup

7512a11

fixup

13a5ea2

tomerd added 4 commits April 17, 2023 11:15

cleanup

c4ec918

more

fff8890

fixup

8c2a8e0

Update Sources/TSCBasic/Path.swift

e913e15

tomerd force-pushed the refactor/relative-path branch from 968d308 to e913e15 Compare April 17, 2023 18:15

tomerd merged commit b2527eb into swiftlang:main Apr 17, 2023

kcieplak mentioned this pull request May 9, 2025

Resolves #505 - Fix handling for Windows long paths #506

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor RelativePath to allow late stage canonicalization in support of windows #369

refactor RelativePath to allow late stage canonicalization in support of windows #369

Uh oh!

tomerd commented Nov 17, 2022 •

edited

Loading

Uh oh!

Uh oh!

tomerd commented Nov 17, 2022

Uh oh!

tomerd commented Apr 13, 2023

Uh oh!

compnerd commented Apr 14, 2023

Uh oh!

compnerd commented Apr 14, 2023

Uh oh!

compnerd commented Apr 14, 2023

Uh oh!

compnerd commented Apr 14, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

compnerd commented Apr 14, 2023

Uh oh!

neonichu Apr 17, 2023

Uh oh!

Uh oh!

tomerd commented Apr 17, 2023

Uh oh!

Uh oh!

		@@ -338,6 +321,8 @@ public struct RelativePath: Hashable, Sendable {
		}
		}

refactor RelativePath to allow late stage canonicalization in support of windows #369

refactor RelativePath to allow late stage canonicalization in support of windows #369

Uh oh!

Conversation

tomerd commented Nov 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

tomerd commented Nov 17, 2022

Uh oh!

tomerd commented Apr 13, 2023

Uh oh!

compnerd commented Apr 14, 2023

Uh oh!

compnerd commented Apr 14, 2023

Uh oh!

compnerd commented Apr 14, 2023

Uh oh!

compnerd commented Apr 14, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

compnerd commented Apr 14, 2023

Uh oh!

neonichu Apr 17, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tomerd commented Apr 17, 2023

Uh oh!

Uh oh!

tomerd commented Nov 17, 2022 •

edited

Loading