Enable batchfiles to run via CreateProcessW - #30490
ahewitson-ops wants to merge 6 commits into
Conversation
|
As Marc pointed out in the Windows channel, in theory this could be working but slow because windows first tries to run it as a PE binary and then silently rewrites to |
|
✅ Bazel docs preview is ready! Updated pages
Preview URL: https://bazel-pr-30490.mintlify.app/ Updated for |
9e4cf69 to
9516afd
Compare
9516afd to
dbf737a
Compare
Do I understand correctly this only works under tw.exe because we have enabled longpathAware in the mainifest file? For example, this wouldn't work in process_test.exe or bazel.exe since they don't have longpathAware enabled. |
|
What matters for Bazel is the manifest of the embedded |
|
I'm fine with fixing tw.exe only, but we should be clear in the PR description about what are the actual conditions for this to work |
Using long paths in general? Yes, it's only because we have a manifest file for I've updated the PR description a little and the release notes. |
|
I believe the host Windows machine also has to enable |
Yes, which I believe is the default nowadays for Windows machines. But this PR doesn't really change the need for that registry key, as it was already added in #29921 |
| @@ -314,14 +314,18 @@ wstring AsExecutablePathForCreateProcess(wstring path, wstring* quoted_path, | |||
| // lpApplicationName: it is not subject to MAX_PATH, and providing it lifts | |||
| // that limit from the executable part of CreateProcessW's lpCommandLine too. | |||
| // This works only for a plain executable with an absolute, normalized path. | |||
There was a problem hiding this comment.
nit: should we also update comment here?
| std::wstring proc_error; | ||
| if (!proc.Create(bat_path, L"", nullptr, L".", devnull, pipe_write, | ||
| stderr_dup, nullptr, &proc_error)) { | ||
| GTEST_SKIP() << "WaitableProcess::Create failed: " |
There was a problem hiding this comment.
Should this be:
ASSERT_TRUE(proc.Create(bat_path, L"", nullptr, L".", devnull, pipe_write,
stderr_dup, nullptr, &proc_error))
<< blaze_util::WstringToCstring(proc_error);
?
Description
Enables Batchfiles to be ran via CreateProcessW even when their paths exceed
MAX_PATHwhen running intw.exe.Looks to fix #30431
Motivation
#29921 helped us Windows users by enabling long paths for all .exe files, but specifically left out Batch/cmd files. Specifically because
.batand.cmdfiles must be run through an interpreter likecmd.exe, as stated in the CreateProcessW documentation.However, testing shows that CreateProcessW can in fact handle being given a .bat or .cmd file above long paths, and silently rewrites the call to shunt it over to
cmd.exewithout the user having to do anything. The only caveat is thatcmd.exedoes not understand the\\?\prefix for long paths and so a .bat/.cmd path must be passed without it.I want to also note the manifest changes came from @nwatkiss in another PR for handling long paths HERE.
Note that this only works in places where a
longpathAwaremanifest is applied to the binary.Build API Changes
No
Checklist
Release Notes
RELNOTES: Allows Batchfiles ran by CreateProcessW in tw.exe to work under longpathAware.