fix(windows): use extended paths in Python bootstraps - #4071
Open
belitskiy wants to merge 9 commits into
Open
Conversation
Implicit long-path support is not universal across the Win32 API. The documented set of APIs covered by the long-path opt-in does not include DLL loading functions, e.g., LoadLibraryExW: https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation#functions-without-max_path-restrictions Always use extended-length paths in Windows bootstrap code and correctly convert UNC paths to the \\?\UNC\ form.
…otstrap path order Extended-length path prefixes (\\?\ and \\?\UNC\) added by bootstrap templates caused Runfiles.CurrentRepository() to fail with a mount mismatch ValueError when computing relative paths against standard drive roots. Additionally, forward-slash paths could be corrupted when checking the prefix before converting to an absolute path. - Normalize \\?\ and \\?\UNC\ prefixes and path separators in Runfiles._normalize_windows_path() and use ntpath on Windows. - Call abspath() before checking extended path prefixes across all bootstrap templates. - Fix inverted Windows logic in zip_main_template.py create_runfiles_root(). - Add unit tests for Windows path normalization in tests/runfiles/runfiles_test.py. - Add news fragment news/4071.fixed.md.
Unconditionally prepending \\?\ to short Windows paths broke cmd.exe batch wrapper execution, relative path resolution (..) in importlib.metadata, and zipapp runfiles discovery. Restrict extended path prefixing in bootstrap templates to paths that exceed MAX_PATH (260 characters).
…nfiles Unconditionally apply extended-length path prefixing on Windows without arbitrary MAX_PATH length checks. Normalize relative path separators in find_runfiles_root across bootstrap templates so os.path.exists checks succeed under \\?\ paths. Ensure zipapp symlink extraction resolves relative targets to absolute paths on Windows, and normalize RECORD paths in importlib metadata tests.
Collaborator
|
CI flagged a variety of edge cases: https://buildkite.com/bazel/rules-python-python/builds/17072/list Would you be able to create a repro for this case? i.e. one that triggers LoadLibraryExW with a long path. There's now a py_extension rule that can create windows DLLs, so it should possible to create a small no-op python c module at a long path location to verify it can be loaded I'm gonna throw my ai at this and see if it can hash out the failures, too. |
Add a test exercising Python C extension dynamic loading at a path longer than 260 characters (MAX_PATH). On Windows, this exercises the extended-length path prefix (\\?\) in sys.path and runfiles required by LoadLibraryExW.
LoadLibraryExW is a Windows-specific API and long path names are restricted to Windows target compatibility to avoid POSIX path length limits on other operating systems.
…ap_impls Move the extended-length path test from tests/cc/py_extension to tests/bootstrap_impls/long_path to better reflect that it validates bootstrap path handling and LoadLibraryExW runtime behavior.
…ompatibility Shorten the intermediate directory segments so that the compile-time cl.exe params file path remains under MAX_PATH (213 characters), while the full runtime path in Bazel runfiles on Windows exceeds MAX_PATH (388 characters), exercising LoadLibraryExW with extended path prefixing.
…able tests Normalize sys.executable paths when comparing across processes, and read metadata files with relative '..' components via resolved paths to prevent Win32 invalid argument errors under extended-length prefixes.
Author
|
Sorry, I expected to get emails about replies here, but I didn't for some reason (or they were filtered out, etc.), so I missed this. (fixed notifications, they were marked Spam for some reason...) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implicit long-path support is not universal across the Win32 API. The documented set of APIs covered by the long-path opt-in does not include DLL loading functions, e.g., LoadLibraryExW:
https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation#functions-without-max_path-restrictions
Always use extended-length paths in Windows bootstrap code and correctly convert UNC paths to the \?\UNC\ form.
Experienced this breakage in JAX after the project being switched over to Bzlmod, which made some paths too long:
https://github.com/jax-ml/jax/actions/runs/31674295160/job/94365413380
Already used as a patch in jax-ml/jax#39961