Skip to content

[HIPIFY][Infra] workaround for long paths on Windows build#2384

Closed
kirthana14m wants to merge 1 commit intoamd-mainlinefrom
amd/dev/kirthana14m/windows-path-shrink-mainline
Closed

[HIPIFY][Infra] workaround for long paths on Windows build#2384
kirthana14m wants to merge 1 commit intoamd-mainlinefrom
amd/dev/kirthana14m/windows-path-shrink-mainline

Conversation

@kirthana14m
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@emankov emankov left a comment

Choose a reason for hiding this comment

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

Despite the do not review label, I'm still forced to request changes.

cd L:
ccache -z
python3 build_tools/github_actions/build_configure.py
cmake -B 'B:\build' -GNinja -S 'L:\' --preset windows-release -DTHEROCK_AMDGPU_FAMILIES=gfx1151 '-DCMAKE_C_COMPILER=C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.44.35207\/bin/Hostx64/x64/cl.exe' '-DCMAKE_CXX_COMPILER=C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.44.35207\/bin/Hostx64/x64/cl.exe' '-DCMAKE_LINKER=C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.44.35207\/bin/Hostx64/x64/link.exe' -DTHEROCK_BACKGROUND_BUILD_JOBS=4
Copy link
Collaborator

@emankov emankov Feb 11, 2026

Choose a reason for hiding this comment

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

  1. You should always use "" for paths instead of '', as in Windows CMD, single quotes are not string delimiters.
  2. Targetting a very specific MSVS version, like 14.44.35207, is a bad solution. You should use vcvars64.bat beforehand.
  3. Could you explain why and where the drive letter B has appeared?

set currentDir=%cd%

REM Substitute the current directory with L: drive
subst L: %currentDir%
Copy link
Collaborator

Choose a reason for hiding this comment

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

UNSAFE! You should check a drive letter not in use.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the comments @emankov

Currently, I am putting this PR on hold, and all changes are incorporated on #2383

  1. Updated the path to use "" instead of ''.
  2. Drive check and dynamic drive allocation has been added.
  3. We are static windows image, so the paths Build and MSCV are directly fetched for the build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do not review infrastructural related to the Repo infrastructure Windows Windows only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants