Windows: support long paths when calling WinAPI functions #2181

laszlocsomor · 2016-12-05T12:58:15Z

Description of the problem / feature request / question:

We're hitting path length limitations because we use the ASCII version of Win32 functions and paths are limited to 260 characters (MAX_PATH).

See for example #2178 (comment).

We have to use the widechar versions and prefix all long paths with \\?\.
Caveats: such prefixed paths may not use / as the path separator, only \. Using / only works with the ASCII versions.

If possible, provide a minimal example to reproduce the problem:

$ git checkout c9e363c674e4a9cd2422e80da2636f6620798fcb
$ bazel test //src/test/shell/bazel:bazel_windows_example_test

Environment info

Operating System: Windows (every version)
If bazel info release returns "development version" or "(@non-git)", please tell us what source tree you compiled Bazel from; git commit hash is appreciated (git rev-parse HEAD):

$ git rev-parse HEAD
c9e363c674e4a9cd2422e80da2636f6620798fcb

Have you found anything relevant by searching the web? (e.g. GitHub issues, email threads in the bazel-discuss@googlegroups.com archive)

Naupe.

Anything else, information or logs or outputs that would be helpful?

(If they are large, please upload as attachment or provide link).

The text was updated successfully, but these errors were encountered:

laszlocsomor · 2016-12-05T13:27:58Z

Bumping to P1 because this makes our CI flaky / failing.

This way it's easier to find the functions we need to change to widechar version. See #2181 -- PiperOrigin-RevId: 141043389 MOS_MIGRATED_REVID=141043389

This change: - renames windows_error_handling.* to windows_util.* - moves most stuff except for the JNI method implementations into the new windows_util namespace - implements a jstring to wchar string converter - uses GetFileAttributesW in windows_file_operations.cc See #2181 -- PiperOrigin-RevId: 141187291 MOS_MIGRATED_REVID=141187291

Implement conversion methods between char strings and wchar_t strings. This is necessary to use widechar Windows functions. See #2181 -- PiperOrigin-RevId: 142009930 MOS_MIGRATED_REVID=142009930

In this change: - implement PrintErrorW (copy of PrintError but uses FormatMessageW) - use GetModuleFileNameW in GetSelfPath and do not worry about UNC paths, we'll automatically handle them in the future when we use widechar functions everywhere. Until then, if a path happens to have the "\\?\" prefix and that causes an error, we won't be worse off than today; today we just call `pdie` - use GetTempPathW in GetOutputRoot; in theory this might also return an UNC path but again that shouldn't be a problem - add comments for the arguments of CreateFileA and CreateProcessA calls, to make the code a bit more readable See #2181 -- PiperOrigin-RevId: 142118854 MOS_MIGRATED_REVID=142118854

Move GetJvmVersion from blaze_util to blaze_util_platform, and remove the RunProgram declaration from blaze_util.h. Since GetJvmVersion is the only user of RunProgram this is safe to do, and allows making RunProgram static as well as simplifying its implementation on Windows, while also changing it to use CreateProcessW instead of CreateProcessA. See #2181 -- PiperOrigin-RevId: 142122045 MOS_MIGRATED_REVID=142122045

This method can normalize paths with "." and ".." and multiple "/" characters. E.g. normalize("../foo/./bar/../baz") = "foo/baz" This method enables us implementing PathExists on Windows. If the path to check is too long, we need to prefix it with "\\?\" for the Windows API functions to work, but then the path must be fully normalized and in Windows format. We already have functions to convert a path to Windows format but that doesn't normalize; with this function we can finally convert paths like "/c/foo/../bar" to L"\\?\c:\foo" and check if it exists. See #2107 See #2181 -- PiperOrigin-RevId: 142648194 MOS_MIGRATED_REVID=142648194

petemounce · 2017-01-04T23:13:03Z

Anything I can do to assist with this? It's one of the things that blocks our Windows adoption.

laszlocsomor · 2017-01-05T10:27:11Z

Thanks, I think we're close to fixing this. I'm about to update the CreateFileA calls to CreateFileW in blaze_util_windows, then update the CreateProcessA ones to W, and that should do it.

Checking if a path exists is surprisingly hard on Windows. The most convenient API functions are PathFileExists and GetFileAttributes but neither of them follows junctions. To check if a junction is dangling, we have to resolve it all the way. This change adds a JunctionResolver class to file_windows, which can resolve junctions (if they aren't dangling) and non-junctions (in this case just checks their existence). See #2107 See #2181 -- PiperOrigin-RevId: 143645274 MOS_MIGRATED_REVID=143645274

See #2107 See #2181 -- PiperOrigin-RevId: 143655738 MOS_MIGRATED_REVID=143655738

laszlocsomor · 2017-01-05T16:57:51Z

TIL: CreateProcessW doesn't support long paths, not even with "\\?\".

(..a moment of silence..)

Luckily, there's GetShortPathName so we can shorten those long paths and CreateProcess can spawn a process with them.

laszlocsomor · 2017-01-05T17:58:52Z

Another poor soul who ran into this, and tried everything I tried and some more:
https://social.msdn.microsoft.com/Forums/windowsdesktop/en-US/944e2977-ad09-4b10-818b-ba383a3f8db0/createprocess-with-extended-length-path-fails-errorinsufficientbuffer?forum=windowsgeneraldevelopmentissues

petemounce · 2017-01-06T09:30:49Z

Happy New Year...?

See #2107 See #2181 -- PiperOrigin-RevId: 143947877 MOS_MIGRATED_REVID=143947877

Because CreateProcessW doesn't support long paths, not even with the "\\?\" prefix [1], we need to convert long paths to short ones to spawn processes. This change implements the corresponding function and uses it in blaze_util_windows. [1] #2181 (comment) See #2107 See #2181 -- PiperOrigin-RevId: 144062404 MOS_MIGRATED_REVID=144062404

Add a separate argument to nativeCreateProcess for argv[0] specifically, and another for the rest of the args. In a subsequent change I'll add code to compute the 8dot3 style short name of the argv[0] so we can use longer paths for executables in CreateProcessA than we normally could. This is the same approach as used in commit 44ecf9a See #2107 See #2181 -- PiperOrigin-RevId: 144311562 MOS_MIGRATED_REVID=144311562

Use CreateFileW to create the output files for redirected stdout/stderr in nativeCreateProcess. This way we can support long paths for these files. See #2107 See #2181 -- PiperOrigin-RevId: 144311729 MOS_MIGRATED_REVID=144311729

When spawning a new process with CreateProcessA, convert argv0 to a 8dot3 style short path so we can support longer paths than MAX_PATH. This is the same approach we did in commit 44ecf9a. See #2107 See #2181 -- PiperOrigin-RevId: 144613589 MOS_MIGRATED_REVID=144613589

Use CreateFileW in blaze_util_windows.cc when opening the "jvm.out" file. This allows supporting long paths. Also use AsWindowsPathWithUncPrefix instead of just AsWindowsPath plus manually adding the UNC prefix. Also fix a compilation error in file_windows_test.cc, I'm surprised the CI system didn't catch this, maybe we aren't running this test there. See #2107 See #2181 -- PiperOrigin-RevId: 144813245 MOS_MIGRATED_REVID=144813245

Add tests for the AsExecutablePathForCreateProcess method, since its logic is pretty complex. Unfortunately testing it also requires complex logic, as we need to test what exactly happens when the input path is shorter than MAX_PATH or when it's longer than it. To test that reliably, we need a base path that we know will not get shortened. Creating that base path under the temp directory is a nightmare, we need to: (1) retrieve the temp dir, shorten it so we know that it won't be shortened further (2) keep creating subdirectories that have a short name so they also won't get shortened, but keep the entire path below MAX_PATH while leaving enough space for a file name in the end (3) append a file name such that the path is just below MAX_PATH, or is exactly that long, or is longer than it. Because of steps (1) and (2) we can be sure that no other component in the path will get shortened, so we can test exactly what's going on with the shortener logic and its error handling. But oh boy is it complicated. Side note, we need to use the Widechar WinAPI functions to create/delete the directories and files, because the POSIX API on Windows appears to be backed by the ASCII API functions, so attempting to `mkdir` with a path longer than CreateDirectoryA's limit is going to fail. But on the positive side, adding tests caught two bugs in the method, so we have that going for us which is nice. See #2107 See #2181 -- PiperOrigin-RevId: 144823029 MOS_MIGRATED_REVID=144823029

Implement ForEachDirectoryEntry on Windows using FindFirstFileW / FindNextFileW. Supports long paths and traversing junctions. See #2107 See #2181 -- PiperOrigin-RevId: 145062749 MOS_MIGRATED_REVID=145062749

*** Reason for rollback *** Breaks Bazel-Install-Trigger on CI. *** Original change description *** Bazel client, Windows: impl. ForEachDirectoryEntry Implement ForEachDirectoryEntry on Windows using FindFirstFileW / FindNextFileW. Supports long paths and traversing junctions. See #2107 See #2181 -- PiperOrigin-RevId: 145282158 MOS_MIGRATED_REVID=145282158

laszlocsomor · 2017-03-03T14:43:23Z

I have a change that fixes about 5 long path related bugs. Now I can build bazel with a long output_user_root. I'll break it down and start sending out.

The hard limit for SetCurrentDirectory{A,W} is MAX_PATH-1, even with UNC prefix, therefore a process' cwd may also not be longer than that. See #2107 See #2406 See #2181 -- PiperOrigin-RevId: 149290147 MOS_MIGRATED_REVID=149290147

Fix blaze_util_windows.ConvertPath: in the MSVC version this is using the actual %PATH% value, we don't need to convert it. Fix blaze_util_windows.PathAsJvmFlag: shorten the path so we can pass it to the JVM process (long paths aren't understood by the shell), but also converrt backslashes to forward slashes so the JVM won't believe we are passing paths with escaped characters. See #2107 See #2181 -- PiperOrigin-RevId: 149396971 MOS_MIGRATED_REVID=149396971

dslomov · 2017-04-04T06:54:27Z

Is there still work to do here?

laszlocsomor · 2017-04-04T06:57:35Z

Not that I'm aware of. It's my great pleasure to hit "Close and comment".

petemounce · 2017-04-04T07:12:17Z

We're going to eagerly try this out this week as it's one of the things that blocks our use of Bazel on Windows. Thanks and we'll keep you posted!

laszlocsomor · 2017-04-04T07:29:03Z

We're also going to start dogfooding the MSYS-less version with 0.5.0, stay tuned!

laszlocsomor added platform: windows P2 We'll consider working on this in future. (Assignee optional) type: feature request labels Dec 5, 2016

laszlocsomor self-assigned this Dec 5, 2016

laszlocsomor added P1 I'll work on this now. (Assignee required) and removed P2 We'll consider working on this in future. (Assignee optional) labels Dec 5, 2016

bazel-io pushed a commit that referenced this issue Dec 5, 2016

Windows: explicitly use ASCII-version of Win32 API

d86ae8c

This way it's easier to find the functions we need to change to widechar version. See #2181 -- PiperOrigin-RevId: 141043389 MOS_MIGRATED_REVID=141043389

laszlocsomor mentioned this issue Dec 6, 2016

Windows: don't shell out to cmd.exe #2190

Closed

laszlocsomor added P2 We'll consider working on this in future. (Assignee optional) and removed P1 I'll work on this now. (Assignee required) labels Dec 6, 2016

dslomov added this to the 0.5 milestone Dec 6, 2016

bazel-io pushed a commit that referenced this issue Jan 5, 2017

blaze_util_windows: use CreateFileW

116d039

See #2107 See #2181 -- PiperOrigin-RevId: 143655738 MOS_MIGRATED_REVID=143655738

bazel-io pushed a commit that referenced this issue Jan 9, 2017

Bazel client, Windows: implement GetCwd

2e08b9a

See #2107 See #2181 -- PiperOrigin-RevId: 143947877 MOS_MIGRATED_REVID=143947877

This was referenced Feb 14, 2017

Use unicode Paths on Windows #2101

Closed

Make Windows-style paths work #933

Closed

laszlocsomor closed this as completed Apr 4, 2017

petemounce mentioned this issue Apr 4, 2017

MSVC / wrapper script bug: cannot compile files with long path #2406

Closed

Xjs mentioned this issue Aug 14, 2018

Windows: Use absolute and shortened path for GOROOT environment variable bazel-contrib/rules_go#1647

Merged

Iorek73 mentioned this issue Sep 11, 2019

Rules are not allowed to cross the boundary of subpackages after bazel 0.26 #9370

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Windows: support long paths when calling WinAPI functions #2181

Windows: support long paths when calling WinAPI functions #2181

laszlocsomor commented Dec 5, 2016

laszlocsomor commented Dec 5, 2016

petemounce commented Jan 4, 2017

laszlocsomor commented Jan 5, 2017

laszlocsomor commented Jan 5, 2017 •

edited

Loading

laszlocsomor commented Jan 5, 2017

petemounce commented Jan 6, 2017

laszlocsomor commented Mar 3, 2017

dslomov commented Apr 4, 2017

laszlocsomor commented Apr 4, 2017

petemounce commented Apr 4, 2017

laszlocsomor commented Apr 4, 2017

Windows: support long paths when calling WinAPI functions #2181

Windows: support long paths when calling WinAPI functions #2181

Comments

laszlocsomor commented Dec 5, 2016

Description of the problem / feature request / question:

If possible, provide a minimal example to reproduce the problem:

Environment info

Have you found anything relevant by searching the web? (e.g. GitHub issues, email threads in the bazel-discuss@googlegroups.com archive)

Anything else, information or logs or outputs that would be helpful?

laszlocsomor commented Dec 5, 2016

petemounce commented Jan 4, 2017

laszlocsomor commented Jan 5, 2017

laszlocsomor commented Jan 5, 2017 • edited Loading

laszlocsomor commented Jan 5, 2017

petemounce commented Jan 6, 2017

laszlocsomor commented Mar 3, 2017

dslomov commented Apr 4, 2017

laszlocsomor commented Apr 4, 2017

petemounce commented Apr 4, 2017

laszlocsomor commented Apr 4, 2017

laszlocsomor commented Jan 5, 2017 •

edited

Loading