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

TSCTestSupport: avoid dropping path components #323

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

compnerd
Copy link
Collaborator

Given that the AbsolutePath constructor invoked here is
AbsolutePath(_:relativeTo:) dropping the first component is not
meaningful for UNIX paths as an absolute path relative to root is going
be the same path as the path relative to the root being re-formed to be
relative to the root. However, this meaningfully impacts paths on
Windows as the shared path representation currently does not include the
drive or server & share (in the UNC case) and the resulting absolute
path representation is a drive-relative path, where dropping the first
component alters the path representation.

Given that the `AbsolutePath` constructor invoked here is
`AbsolutePath(_:relativeTo:)` dropping the first component is not
meaningful for UNIX paths as an absolute path relative to root is going
be the same path as the path relative to the root being re-formed to be
relative to the root.  However, this meaningfully impacts paths on
Windows as the shared path representation currently does not include the
drive or server & share (in the UNC case) and the resulting absolute
path representation is a drive-relative path, where dropping the first
component alters the path representation.
@compnerd
Copy link
Collaborator Author

@swift-ci please test

@tomerd
Copy link
Member

tomerd commented May 21, 2022

done have tests to make sure this still works correctly on all platforms?

@compnerd
Copy link
Collaborator Author

The tools-support-core tests don't work on Windows (I'm tempted to spend the necessary energy towards removing the dependency). The SPM tests will definitely exercise this heavily, and the assertion in AbsolutePath (i.e. path.pathString.utf8.first! == "/"). I think that this should be sufficiently covered. But, if there is something that you think might be useful to add - I can try to add something blind.

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.

None yet

2 participants