Copy task: make ERROR_ACCESS_DENIED fail-fast Windows-only to fix macOS CoW retry resilience#13479
Copy task: make ERROR_ACCESS_DENIED fail-fast Windows-only to fix macOS CoW retry resilience#13479
Conversation
|
Hello @@copilot, I noticed that you’re changing an .swr file or any file under src/Package/MSBuild.VSSetup.. Please make sure to validate this change by an experimental VS insertion. This is accomplished by pushing to an exp/* branch, which requires write permissions to this repo. |
…OS CoW resilience Agent-Logs-Url: https://github.com/dotnet/msbuild/sessions/18cc673e-cbf9-44ae-b6b8-585a38d7f0a1 Co-authored-by: rainersigwald <3347530+rainersigwald@users.noreply.github.com>
|
@copilot put the new behavior behind changewave 18.6. |
There was a problem hiding this comment.
Pull request overview
This PR adjusts the built-in Copy task’s retry behavior so that ERROR_ACCESS_DENIED only triggers the “fail-fast / no retry” path on Windows, improving resilience on macOS where EACCES can be transient under concurrent CoW/clone operations.
Changes:
- Guard
ERROR_ACCESS_DENIEDfail-fast logic withNativeMethodsShared.IsWindowsso non-Windows platforms fall back to the normal retry loop. - Update the
DoNotNormallyCopyOverReadOnlyFileunit test so the “no retries logged” assertion is Windows-only.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Tasks/Copy.cs | Limits the access-denied “throw immediately” behavior to Windows to allow retries on non-Windows. |
| src/Tasks.UnitTests/Copy_Tests.cs | Adjusts the read-only overwrite test assertion to account for platform-dependent retry behavior. |
| if (NativeMethodsShared.IsWindows) | ||
| { | ||
| // On Windows, ERROR_ACCESS_DENIED is not retried (it's a real ACL or r/o bit issue). | ||
| ((MockEngine)t.BuildEngine).AssertLogDoesntContain("MSB3026"); | ||
| } | ||
| // On non-Windows, access denied can be a transient lock (e.g. macOS CoW filesystem), | ||
| // so we retry; retries will ultimately fail for a genuinely read-only file. | ||
| } |
There was a problem hiding this comment.
On non-Windows this test no longer asserts anything about the new retry behavior (it only documents it in a comment). To keep the behavior change covered, add an assertion for the non-Windows path (e.g., that the retry warning code MSB3026 appears) or add a dedicated test that simulates an UnauthorizedAccessException(ERROR_ACCESS_DENIED) and verifies the Copy task enters the retry loop on non-Windows.
Agent-Logs-Url: https://github.com/dotnet/msbuild/sessions/c0adc714-f4a2-45f7-b322-46ac4bf8d409 Co-authored-by: rainersigwald <3347530+rainersigwald@users.noreply.github.com>
Done in 9d82a6b. The non-Windows retry behavior is now gated behind |
…indows retry path Agent-Logs-Url: https://github.com/dotnet/msbuild/sessions/1a824345-719b-4d22-b427-a6a732eb5ee7 Co-authored-by: rainersigwald <3347530+rainersigwald@users.noreply.github.com>
NativeMethodsShared.IsWindows(not macOS-specific)elsebranch inDoNotNormallyCopyOverReadOnlyFileasserting MSB3026 appears on non-Windows