Skip to content

Conversation

@tannergooding
Copy link
Member

This was broken yesterday in #114187

On some versions of CMake (potentially just v4.0.0 and later) the install will fail as C:\Program Files is interpreted as containing an escape character \P which is unrecognized.

Copilot AI review requested due to automatic review settings May 14, 2025 17:28
@tannergooding
Copy link
Member Author

CC. @elinor-fung

@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 14, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR normalizes Windows install paths using CMake’s file(TO_CMAKE_PATH) to ensure environment variables are interpreted correctly on CMake v4.0+ and updates the CRT file globbing to use the normalized paths.

  • Normalize VCToolsRedistDir and ExtensionSdkDir with file(TO_CMAKE_PATH)
  • Update file(GLOB_RECURSE ...) patterns to reference the new normalized variables
Comments suppressed due to low confidence (1)

src/native/corehost/test/ijw/CMakeLists.txt:16

  • Consider adding a check to ensure $ENV{VCToolsRedistDir} is defined (e.g., if(NOT DEFINED ENV{VCToolsRedistDir}) ...) before calling file(TO_CMAKE_PATH) to fail early if the environment variable is missing.
file(TO_CMAKE_PATH "$ENV{VCToolsRedistDir}" CMAKE_VS_VCTOOLSREDISTDIR)

Comment on lines 15 to +16
# Copy over the debug CRT so that it is available for test runs
file(TO_CMAKE_PATH "$ENV{VCToolsRedistDir}" CMAKE_VS_VCTOOLSREDISTDIR)
Copy link

Copilot AI May 14, 2025

Choose a reason for hiding this comment

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

[nitpick] Add a brief comment explaining that file(TO_CMAKE_PATH) is used to normalize Windows paths (transform backslashes to forward slashes) for compatibility with CMake 4.0+.

Suggested change
# Copy over the debug CRT so that it is available for test runs
file(TO_CMAKE_PATH "$ENV{VCToolsRedistDir}" CMAKE_VS_VCTOOLSREDISTDIR)
# Copy over the debug CRT so that it is available for test runs
# Normalize the Windows path (transform backslashes to forward slashes) for compatibility with CMake 4.0+
file(TO_CMAKE_PATH "$ENV{VCToolsRedistDir}" CMAKE_VS_VCTOOLSREDISTDIR)
# Normalize the Windows path (transform backslashes to forward slashes) for compatibility with CMake 4.0+

Copilot uses AI. Check for mistakes.
@tannergooding tannergooding added area-Infrastructure-installer and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels May 14, 2025
Copy link
Member

@elinor-fung elinor-fung left a comment

Choose a reason for hiding this comment

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

Thanks!

@tannergooding tannergooding merged commit 8c8b218 into dotnet:main May 14, 2025
148 of 152 checks passed
@tannergooding tannergooding deleted the fix-ijw-test-install branch May 14, 2025 21:17
@github-actions github-actions bot locked and limited conversation to collaborators Jun 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants