Skip to content

path and remote_file_cache: support windows paths#49437

Merged
scheibelp merged 2 commits intospack:developfrom
tldahlgren:url_path_windows_aware
Mar 12, 2025
Merged

path and remote_file_cache: support windows paths#49437
scheibelp merged 2 commits intospack:developfrom
tldahlgren:url_path_windows_aware

Conversation

@tldahlgren
Copy link
Copy Markdown
Contributor

@tldahlgren tldahlgren commented Mar 12, 2025

This PR restores/adds support for Windows paths (with drives except drive-relative) (post-#48784) to spack.util.path.canonicalize_path() and spack.util.remote_file_cache.local_path(). It also adds unit tests for both methods.

@spackbot-app spackbot-app bot added core PR affects Spack core functionality tests General test capability(ies) utilities labels Mar 12, 2025
Copy link
Copy Markdown
Contributor

@johnwparent johnwparent left a comment

Choose a reason for hiding this comment

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

Left a few comments. I think a lot of the logic in util.path can be removed, but as this is blocking develop, if it fixes CI, and doesn't fail the unit tests, I'd rather just merge this ASAP and cleanup in a subsequent PR, which is why this is a comment and not a change request.

@johnwparent
Copy link
Copy Markdown
Contributor

Thanks for this PR @tldahlgren, I really like the check for drive, add to file scheme trick.

@tldahlgren
Copy link
Copy Markdown
Contributor Author

BTW (Not having a windows box atm) I am confused as to where the d drive is coming from for the relative/packages.txt test case.

@tldahlgren tldahlgren force-pushed the url_path_windows_aware branch from 0049f08 to 82be1da Compare March 12, 2025 21:49
@scheibelp scheibelp enabled auto-merge (squash) March 12, 2025 22:25
@scheibelp scheibelp merged commit d518aaa into spack:develop Mar 12, 2025
35 checks passed
@tldahlgren tldahlgren deleted the url_path_windows_aware branch March 12, 2025 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core PR affects Spack core functionality tests General test capability(ies) utilities

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants