Skip to content

[windows] path: drop the trailing backslash before invoking PathCchRe…#479

Merged
compnerd merged 2 commits into
mainfrom
eng/windows-dirname-fix
Aug 5, 2024
Merged

[windows] path: drop the trailing backslash before invoking PathCchRe…#479
compnerd merged 2 commits into
mainfrom
eng/windows-dirname-fix

Conversation

@hyp

@hyp hyp commented Aug 2, 2024

Copy link
Copy Markdown
Contributor

…moveFileSpec to compute the dirname

PathCchRemoveFileSpec's is documented to not remove the file spec if it comes with a backslash suffix. Thus, we should remove the trailing backslashes before calling this API, to ensure we actually drop the current file/directory name.

This regressed after adoption of new swift-foundation, as it's filesystemRepresentation no longer drops the trailing backslash.

…moveFileSpec to compute the dirname

PathCchRemoveFileSpec's is documented to not remove the file spec if it comes with a backslash suffix. Thus, we should remove the trailing backslashes before calling this API, to ensure we actually drop the current file/directory name
@hyp hyp requested a review from compnerd August 2, 2024 19:58
@hyp

hyp commented Aug 2, 2024

Copy link
Copy Markdown
Contributor Author

@swift-ci please test

Comment thread Sources/TSCBasic/Path.swift
@hyp

hyp commented Aug 2, 2024

Copy link
Copy Markdown
Contributor Author

@swift-ci please tset

@hyp

hyp commented Aug 2, 2024

Copy link
Copy Markdown
Contributor Author

@swift-ci please test

@xedin

xedin commented Aug 3, 2024

Copy link
Copy Markdown

@swift-ci please test Windows platform

@xedin xedin left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thank you! @rintaro and I spent some time today figure out what is going on with Windows build of swiftpm to find this exact problem.

@xedin

xedin commented Aug 3, 2024

Copy link
Copy Markdown

@swift-ci please test Windows platform

1 similar comment
@xedin

xedin commented Aug 3, 2024

Copy link
Copy Markdown

@swift-ci please test Windows platform

@compnerd compnerd merged commit a76104d into main Aug 5, 2024
@compnerd compnerd deleted the eng/windows-dirname-fix branch August 5, 2024 14:36
@hyp

hyp commented Aug 5, 2024

Copy link
Copy Markdown
Contributor Author

Thanks, I will cherry-pick this into swift 6.

@hyp

hyp commented Aug 5, 2024

Copy link
Copy Markdown
Contributor Author

swift 6 PR: #480

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.

3 participants