Skip to content
Open
1 change: 1 addition & 0 deletions documentation/wiki/ChangeWaves.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ Change wave checks around features will be removed in the release that accompani
- [TaskHostTask forwards request-level global properties (e.g. MSBuildRestoreSessionId) to out-of-proc TaskHost in -mt mode](https://github.com/dotnet/msbuild/pull/13443)
- [Fix ShouldTreatWarningAsError in OOP TaskHost checking wrong collection (WarningsAsMessages instead of WarningsAsErrors)](https://github.com/dotnet/msbuild/issues/11952)
- [Fix ToolTask hang when tool spawns grandchild processes that inherit stdout/stderr pipe handles](https://github.com/dotnet/msbuild/issues/2981)
- [Copy task retries on ERROR_ACCESS_DENIED on non-Windows platforms to handle transient lock conflicts (e.g. macOS CoW filesystems)](https://github.com/dotnet/msbuild/issues/13463)

### 18.5
- [FindUnderPath and AssignTargetPath tasks no longer throw on invalid path characters when using TaskEnvironment.GetAbsolutePath](https://github.com/dotnet/msbuild/pull/13069)
Expand Down
68 changes: 67 additions & 1 deletion src/Tasks.UnitTests/Copy_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,18 @@ public void DoNotNormallyCopyOverReadOnlyFile(bool isUseHardLinks, bool isUseSym
string destinationContent = File.ReadAllText(destination);
Assert.Equal("This is a destination file.", destinationContent);

((MockEngine)t.BuildEngine).AssertLogDoesntContain("MSB3026"); // did not do retries as it was r/o
// On Windows, ERROR_ACCESS_DENIED is not retried (it's a real ACL or r/o bit issue).
// On non-Windows with Wave18_6 enabled (default), 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.
if (NativeMethodsShared.IsWindows)
{
((MockEngine)t.BuildEngine).AssertLogDoesntContain("MSB3026");
}
else
{
// non-Windows: retries are expected due to possible transient EACCES
((MockEngine)t.BuildEngine).AssertLogContains("MSB3026");
}
}
Comment on lines +755 to 764
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot generated this review using guidance from repository custom instructions.
finally
{
Expand All @@ -764,6 +775,61 @@ public void DoNotNormallyCopyOverReadOnlyFile(bool isUseHardLinks, bool isUseSym
}
}

/// <summary>
/// When Wave18_6 is disabled, non-Windows should NOT retry on ERROR_ACCESS_DENIED (old behavior preserved via opt-out).
/// </summary>
[Theory]
[MemberData(nameof(GetHardLinksSymLinks))]
public void DoNotRetryCopyOverReadOnlyFileWhenWave18_6Disabled(bool isUseHardLinks, bool isUseSymbolicLinks)
{
using TestEnvironment env = TestEnvironment.Create(_testOutputHelper);

// TODO: Remove test when Wave18_6 rotates out
ChangeWaves.ResetStateForTests();
env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", ChangeWaves.Wave18_6.ToString());

TransientTestFile source = env.CreateFile("source.tmp", "This is a source file.");
TransientTestFile destination = env.CreateFile("destination.tmp", "This is a destination file.");

File.SetAttributes(destination.Path, FileAttributes.ReadOnly);
try
{
ITaskItem sourceItem = new TaskItem(source.Path);
ITaskItem destinationItem = new TaskItem(destination.Path);
ITaskItem[] sourceFiles = { sourceItem };
ITaskItem[] destinationFiles = { destinationItem };

var engine = new MockEngine(_testOutputHelper);
var t = new Copy
{
TaskEnvironment = TaskEnvironmentHelper.CreateForTest(),
RetryDelayMilliseconds = 1, // speed up tests!
BuildEngine = engine,
SourceFiles = sourceFiles,
DestinationFiles = destinationFiles,
SkipUnchangedFiles = true,
// OverwriteReadOnlyFiles defaults to false
UseHardlinksIfPossible = isUseHardLinks,
UseSymboliclinksIfPossible = isUseSymbolicLinks,
};

// Should fail: target is readonly
t.Execute().ShouldBeFalse();

// Expect for there to have been no copies.
t.CopiedFiles.ShouldBeEmpty();

File.ReadAllText(destination.Path).ShouldBe("This is a destination file.");

// With Wave18_6 disabled, ERROR_ACCESS_DENIED should not be retried on any platform (old behavior).
engine.AssertLogDoesntContain("MSB3026");
}
finally
{
File.SetAttributes(destination.Path, FileAttributes.Normal);
}
}

/// <summary>
/// If MSBUILDALWAYSOVERWRITEREADONLYFILES is set, then overwrite read-only even when
/// OverwriteReadOnlyFiles is false
Expand Down
5 changes: 4 additions & 1 deletion src/Tasks/Copy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1033,7 +1033,10 @@ private bool DoCopyWithRetries(FileState sourceFileState, FileState destinationF
// to a failure to reset the readonly bit properly, in which case retrying will succeed. This seems to be
// a pretty edge scenario, but since some of our internal builds appear to be hitting it, provide a secret
// environment variable to allow overriding the default behavior and forcing retries in this circumstance as well.
if (!s_alwaysRetryCopy)
// On at least some non-Windows platforms (e.g. macOS https://github.com/dotnet/msbuild/issues/13463), access denied can also indicate a
// transient lock conflict (e.g. EACCES from a concurrent clonefile/flock), so we only skip retries for
// access denied on Windows.
if (!s_alwaysRetryCopy && (NativeMethodsShared.IsWindows || !ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave18_6)))
{
throw;
}
Expand Down
Loading