Repository navigation
Fix Could not chdir in tw.exe for long runfiles paths on Windows - #30610
rdesgroppes wants to merge 1 commit into
Conversation
9f38717 to
2386018
Compare
| # Long enough that both the runfiles chdir target and the executable's | ||
| # own path exceed MAX_PATH, exercising ChdirToRunfiles's longPathAware | ||
| # manifest and StartSubprocess's explicit-cwd fix. | ||
| long_name = 'x' * 150 |
There was a problem hiding this comment.
| long_name = 'x' * 150 | |
| long_name = 'x' * 256 |
would make it more obvious that this is long enough.
There was a problem hiding this comment.
Tried 'x' * 256 first, but it breaks the build for an unrelated reason: bisecting, 233 still passes but 234 fails with ERROR_INVALID_NAME, because Bazel writes a <name>.exe.runfiles_manifest file for each test target.
It turns out individual path components are capped at 255 characters, per Naming and referencing shares, directories, files, and metadata and What are file path length limits?.
At 234, <name>.exe.runfiles_manifest is 234 + 22 = 256 characters, one over the limit (233 lands exactly on the 255 boundary).
In short, that's a different limit than the MAX_PATH one this test targets.
=> landed on splitting the length across both the package directory and the target name instead (_130_chars, to make the length self-explanatory in the spirit of your suggestion), so the overall runfiles path still exceeds MAX_PATH while each individual path component stays comfortably under the 255-char ceiling.
|
@bazel-io fork 9.3.0 |
On Windows, `bazel test` fails for tests run through the native test wrapper (`tw.exe`) whose runfiles directory path exceeds `MAX_PATH` (260 characters), with `ERROR_FILENAME_EXCED_RANGE` (206) from `ChdirToRunfiles`'s call to `SetCurrentDirectoryW`. The `\\?\` extended-length prefix used by bazelbuild#29921 for `CreateProcessW`'s `lpApplicationName` would not help here: `SetCurrentDirectoryW` ignores that prefix regardless of length, as stated by MicrosoftDocs/feedback#1441. => declare `longPathAware` in `tw.exe`'s manifest (`tw_manifest.xml`, embedded via `tw_resources.rc` and `windows_resources` in `tools/test/BUILD`), which Windows 10 1607+ honors for `SetCurrentDirectoryW` once paired with the `LongPathsEnabled` registry opt-in. Once `ChdirToRunfiles` succeeds into a long directory, `StartSubprocess`'s `CreateProcessW` call also needs its current directory passed explicitly: implicit inheritance (`lpCurrentDirectory=nullptr`) otherwise fails with `ERROR_INVALID_PARAMETER` once `cwd` exceeds `MAX_PATH`, _even from a `longPathAware` process_. => populate it so that `windows::WaitableProcess::Create` can shorten it internally (through `AsShortPath`), like it does for the executable's own path. The new `testChdirToRunfilesWithPathLongerThanMaxPath` in `test_wrapper_test.py` exercises both fixes: the cwd fix always fails in `StartSubprocess` when missing, while a missing manifest can surface as either `ChdirToRunfiles`'s error or `rules_cc`'s `Runfiles::Create` (`FindTestBinary`) failing first, depending on path length.
2386018 to
4859368
Compare
### What does this PR do? Bump Bazel from 9.2.0 to 9.3.0 and drop `--rewind_lost_inputs`, which it enables by default. ### Motivation Over the last 2 weeks, 388 macOS jobs (295 of them `agent_dmg-x64-a7`) failed to read disk cache entries deleted by a concurrent garbage collection, e.g. https://gitlab.ddbuild.io/DataDog/datadog-agent/-/jobs/2095610767: ``` WARNING: Remote Cache: /Users/ec2-user/builds/t3_C7z16h/0/DataDog/datadog-agent.tmp/bazel/disk-cache/cas/07/07a895b2e1df38798e8d736acded35a61816f0e5e7f5fc827329f63f5c9894bc -> /Users/ec2-user/builds/t3_C7z16h/0/DataDog/datadog-agent.tmp/bazel/_bazel_ec2-user/6728ec18f7c139e0336a1063fd8d2d55/execroot/_main/bazel-out/_tmp/actions/remote/11171.tmp (No such file or directory) ``` ([log explorer](https://app.datadoghq.com/logs?storage=flex_tier&index=ci-app-pipeline-logs-gitlab-datadog-agent&from_ts=1790185510102&to_ts=1791395110102&live=false&query=service%3Agitlab-ci%20%40ci.pipeline.name%3A%22DataDog%2Fdatadog-agent%22%20%22disk-cache%2Fcas%2F%22%20%22No%20such%20file%20or%20directory%22)) => Bazel 9.3.0 now treats such entries as cache misses (bazelbuild/bazel#31021). Over the same period, remote cache calls hit our 60s `--remote_timeout` in 119 jobs, 71 of them on Windows, e.g. https://gitlab.ddbuild.io/DataDog/datadog-agent/-/jobs/2118147858: ``` WARNING: Remote Cache: DEADLINE_EXCEEDED: CallOptions deadline exceeded after 59.945817300s. Name resolution delay 0.000000000 seconds. [closed=[], open=[[buffered_nanos=687377900, remote_addr=buildbarn-frontend-datadog-agent.us1.ddbuild.io/172.19.249.13:443]]] ``` ([log explorer](https://app.datadoghq.com/logs?storage=flex_tier&index=ci-app-pipeline-logs-gitlab-datadog-agent&from_ts=1790185510102&to_ts=1791395110102&live=false&query=service%3Agitlab-ci%20%40ci.pipeline.name%3A%22DataDog%2Fdatadog-agent%22%20%22DEADLINE_EXCEEDED%3A%20CallOptions%20deadline%20exceeded%22)) => Bazel 9.3.0 can give downloads a longer deadline than action cache lookups (`--remote_grpc_service_config`, bazelbuild/bazel#30666) and an idle timeout (`--remote_grpc_download_idle_timeout`, bazelbuild/bazel#30667), to be sized from our data in a follow-up. Bazel 9.3.0 also stops serving stale action cache entries after switching `--remote_cache` or `--remote_instance_name` (bazelbuild/bazel#30986), which our cache selection does across invocations. ### Describe how you validated your changes Ran builds and tests on my machine (earlier Bazel cache, then cold cache), relying on CI for the rest. ### Additional Notes Alas, 2 of our fixes could not be cherry-picked into the release. bazelbuild/bazel#30960, interrupting a command once its client goes away, has its cherry-pick blocked on merge conflicts: bazelbuild/bazel#31383 (comment) bazelbuild/bazel#30610, fixing `tw.exe` for long runfiles paths on Windows, got its fork request a week before being merged, and no tracking issue was ever created from it: bazelbuild/bazel#30610 (comment) No worries, I'll see what I can do.
### What does this PR do? Bump Bazel from 9.2.0 to 9.3.0 and drop `--rewind_lost_inputs`, which it enables by default. ### Motivation First, over the [last 2 weeks](https://app.datadoghq.com/logs?storage=flex_tier&index=ci-app-pipeline-logs-gitlab-datadog-agent&from_ts=1790185510102&to_ts=1791395110102&live=false&query=service%3Agitlab-ci%20%40ci.pipeline.name%3A%22DataDog%2Fdatadog-agent%22%20%22disk-cache%2Fcas%2F%22%20%22No%20such%20file%20or%20directory%22), 388 macOS jobs (295 of them `agent_dmg-x64-a7`) failed to read disk cache entries deleted by a concurrent garbage collection, e.g. https://gitlab.ddbuild.io/DataDog/datadog-agent/-/jobs/2095610767: ``` WARNING: Remote Cache: /Users/ec2-user/builds/t3_C7z16h/0/DataDog/datadog-agent.tmp/bazel/disk-cache/cas/07/07a895b2e1df38798e8d736acded35a61816f0e5e7f5fc827329f63f5c9894bc -> /Users/ec2-user/builds/t3_C7z16h/0/DataDog/datadog-agent.tmp/bazel/_bazel_ec2-user/6728ec18f7c139e0336a1063fd8d2d55/execroot/_main/bazel-out/_tmp/actions/remote/11171.tmp (No such file or directory) ``` => Bazel 9.3.0 now treats such entries as cache misses (bazelbuild/bazel#31021). Second, over the [same period](https://app.datadoghq.com/logs?storage=flex_tier&index=ci-app-pipeline-logs-gitlab-datadog-agent&from_ts=1790185510102&to_ts=1791395110102&live=false&query=service%3Agitlab-ci%20%40ci.pipeline.name%3A%22DataDog%2Fdatadog-agent%22%20%22DEADLINE_EXCEEDED%3A%20CallOptions%20deadline%20exceeded%22), remote cache calls hit our 60s `--remote_timeout` in 119 jobs, 71 of them on Windows, e.g. https://gitlab.ddbuild.io/DataDog/datadog-agent/-/jobs/2118147858: ``` WARNING: Remote Cache: DEADLINE_EXCEEDED: CallOptions deadline exceeded after 59.945817300s. Name resolution delay 0.000000000 seconds. [closed=[], open=[[buffered_nanos=687377900, remote_addr=buildbarn-frontend-datadog-agent.us1.ddbuild.io/172.19.249.13:443]]] ``` => Bazel 9.3.0 can give downloads a longer deadline than action cache lookups (`--remote_grpc_service_config`, bazelbuild/bazel#30666) and an idle timeout (`--remote_grpc_download_idle_timeout`, bazelbuild/bazel#30667), to be sized from our data in a follow-up. Finally, Bazel 9.3.0 also stops serving stale action cache entries after switching `--remote_cache` or `--remote_instance_name` (bazelbuild/bazel#30986). ### Describe how you validated your changes Ran builds and tests on my machine (earlier Bazel cache, then cold cache), relying on CI for the rest. ### Additional Notes Alas, 2 of our fixes could not be cherry-picked into the release. bazelbuild/bazel#30960, interrupting a command once its client goes away, has its cherry-pick blocked on merge conflicts: bazelbuild/bazel#31383 (comment). bazelbuild/bazel#30610, fixing `tw.exe` for long runfiles paths on Windows, got its fork request a week before being merged, and no tracking issue was ever created from it: bazelbuild/bazel#30610 (comment). I'll see what I can do for the next release, but Bazel 10 already has a first RC that includes our fixes. Co-authored-by: regis.desgroppes <regis.desgroppes@datadoghq.com>
Description
On Windows,
bazel testfails for tests run through the native test wrapper (tw.exe) whose runfiles directory path exceedsMAX_PATH(260 characters), withERROR_FILENAME_EXCED_RANGE(206) fromChdirToRunfiles's call toSetCurrentDirectoryW.The
\\?\extended-length prefix used by #29921 forCreateProcessW'slpApplicationNamewould not help here:SetCurrentDirectoryWignores that prefix regardless of length, as stated by MicrosoftDocs/feedback#1441.=> declare
longPathAwareintw.exe's manifest (tw_manifest.xml, embedded viatw_resources.rcandwindows_resourcesintools/test/BUILD), which Windows 10 1607+ honors forSetCurrentDirectoryWonce paired with theLongPathsEnabledregistry opt-in.Once
ChdirToRunfilessucceeds into a long directory,StartSubprocess'sCreateProcessWcall also needs its current directory passed explicitly: implicit inheritance (lpCurrentDirectory=nullptr) otherwise fails withERROR_INVALID_PARAMETERoncecwdexceedsMAX_PATH, even from alongPathAwareprocess.=> populate it so that
windows::WaitableProcess::Createcan shorten it internally (throughAsShortPath), like it does for the executable's own path.The new
testLongRunfilesPathChdirintest_wrapper_test.pyexercises both fixes: the cwd fix always fails inStartSubprocesswhen missing, while a missing manifest can surface as eitherChdirToRunfiles's error orrules_cc'sRunfiles::Create(FindTestBinary) failing first, depending on path length.Motivation
Fixes #30609
Build API Changes
No
Checklist
Release Notes
RELNOTES: Fix
Could not chdirintw.exefor long runfiles paths on Windows.