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

[CI] Enable long paths on Windows #1297

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

RUIJIEZHONG66166
Copy link
Contributor

Enable long paths on Windows CI

@@ -72,6 +72,9 @@ jobs:
call conda install -y libuv
call conda install -y rust
git config --system core.longpaths true
git config --global core.symlinks true
git config --global core.fsmonitor false
powershell -Command "Set-ItemProperty -Path "HKLM:\\SYSTEM\CurrentControlSet\Control\FileSystem" -Name "LongPathsEnabled" -Value 1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Long path issues in XXI century :). Yeah, that's known problem. However, setting long paths as you are doing above might not fully help. It did not help for me in one of the projects I was doing before. The full solution which ultimately resolved the issue was twofold with the setting you applied being one of the fixes. There is a chance that you will still see issues with MSVC toolchain and to resolve that it is required to patch the toolchain executables with long paths support.

I hope you won't need that, but if you will still see the issues, below is a second part of the solution which helped us on that other project. Below is a snapshot of commit message and a relevant script (I can't just post a link since it was inhouse project):

We've stepped into Windows long files limitation which is that by default Windows
does not allow file and path name lenght to be more than 260. We step into this
threefold: 1) with git clone, 2) with meson subproject archives fetch and 3) with MSVC
toolchain. First 2 issues are getting fixed by Windows setting to enable long files.
Toolchain issue requires a workaround to apply manifest to toolchain executables to
enable long files support. Since it's not good to modify system binaries, this commit
copies toolchain executables and patches them with required manifest.

The script:

$ErrorActionPreference = "Stop"

function PatchToolset  {
    mkdir _bin >$null
    Set-Location _bin

    $cvtres = $(Get-Command cvtres -TotalCount 1).Source
    $link = $(Get-Command link -TotalCount 1).Source
    $rc = $(Get-Command rc -TotalCount 1).Source

    # Windows does not allow symlinks for generic users by default
    # so we will just copy some DLLs which link, rc, etc. tools require.
    cp "$(Split-Path $link -Parent)\mspdb140.dll" .\
    cp "$(Split-Path $link -Parent)\mspdbcore.dll" .\
    cp "$(Split-Path $link -Parent)\tbbmalloc.dll" .\

    cp $cvtres .\
    cp $rc .\
    cp $link .\

    # Apply longPathAware manifest to copied executables
    mt -nologo -manifest "$PSScriptRoot\longPathAware.manifest" -outputresource:"rc.exe;#1"
    if( -not $? ) { exit 1 }
    mt -nologo -manifest "$PSScriptRoot\longPathAware.manifest" -outputresource:"cvtres.exe;#1"
    if( -not $? ) { exit 1 }
    mt -nologo -manifest "$PSScriptRoot\longPathAware.manifest" -outputresource:"link.exe;#1"
    if( -not $? ) { exit 1 }

    # Export patched toolset location to github environment
    $env:Path = "$(Get-Location);$env:Path"
    Write-Output "Path=$env:Path" >> $env:GITHUB_ENV

    # Print out search queries for the tools we patched. First line
    # should point to .\_bin, second line - to original executable.
    Write-Output "Where cvtres?"
    $(Get-Command cvtres -TotalCount 2).Source
    Write-Output "Where rc?"
    $(Get-Command rc -TotalCount 2).Source
    Write-Output "Where link?"
    $(Get-Command link -TotalCount 2).Source
}

PatchToolset

With the longPathAware.manifest being:

<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
<application  xmlns="urn:schemas-microsoft-com:asm.v3">
    <windowsSettings xmlns:ws2="http://schemas.microsoft.com/SMI/2016/WindowsSettings">
        <ws2:longPathAware>true</ws2:longPathAware>
    </windowsSettings>
</application>
</assembly>

Copy link
Contributor

@Stonepia Stonepia Jan 21, 2025

Choose a reason for hiding this comment

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

Thanks for the suggestion! The usage of using mt to add a manifest to the application is quite interesting!
I also noticed the manifest change from https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=registry#application-manifest-updates-to-declare-long-path-capability but didn't know how to use that before.

Currently, we still encounter long path issues during torch.compile, I am trying to reduce the path length from the Python side (for generating shorter path). We will try to patch the corresponding exe if we still have that issue.

On the other side, the document also suggests using "\?" prefix. That could also be handled in the Python side I think.

@CuiYifeng
Copy link
Contributor

@RUIJIEZHONG66166 Due to more than one issue with Windows Build, I have temporarily applied a patch from #1287.

@RUIJIEZHONG66166 RUIJIEZHONG66166 force-pushed the ruijie/enable_long_path_for_windows branch from 09d1102 to 065b252 Compare January 20, 2025 03:45
@CuiYifeng CuiYifeng force-pushed the ruijie/enable_long_path_for_windows branch from bc56120 to 10bae41 Compare January 20, 2025 06:05
@CuiYifeng
Copy link
Contributor

Temporarily apply patches from #1303 to test Windows Build.

@CuiYifeng
Copy link
Contributor

@RUIJIEZHONG66166 There's a new Windows Build error. Seems it's related to the file system.

@RUIJIEZHONG66166 RUIJIEZHONG66166 marked this pull request as draft January 23, 2025 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants