diff --git a/Directory.Packages.props b/Directory.Packages.props index 62059eed..64674ded 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -4,6 +4,7 @@ 17.6.3 + @@ -15,9 +16,10 @@ + + - diff --git a/src/Artifacts.UnitTests/Microsoft.Build.Artifacts.UnitTests.csproj b/src/Artifacts.UnitTests/Microsoft.Build.Artifacts.UnitTests.csproj index ce540feb..0b3e2811 100644 --- a/src/Artifacts.UnitTests/Microsoft.Build.Artifacts.UnitTests.csproj +++ b/src/Artifacts.UnitTests/Microsoft.Build.Artifacts.UnitTests.csproj @@ -8,6 +8,7 @@ + diff --git a/src/Artifacts.UnitTests/RobocopyTests.cs b/src/Artifacts.UnitTests/RobocopyTests.cs index d4852ea1..e47bb187 100644 --- a/src/Artifacts.UnitTests/RobocopyTests.cs +++ b/src/Artifacts.UnitTests/RobocopyTests.cs @@ -45,7 +45,7 @@ public void NonRecursiveWildcards() [nameof(RobocopyMetadata.IsRecursive)] = "false", }, }, - Sleep = duration => { }, + Sleep = _ => { }, }; copyArtifacts.Execute().ShouldBeTrue(buildEngine.GetConsoleLog()); @@ -58,7 +58,8 @@ public void NonRecursiveWildcards() "bar.txt", "foo.txt", }.Select(i => Path.Combine(destination.FullName, i)), - ignoreOrder: true); + ignoreOrder: true, + customMessage: buildEngine.GetConsoleLog()); } [Fact] @@ -95,7 +96,7 @@ public void RecursiveWildcards() ["FileMatch"] = "*exe *dll *exe.config", }, }, - Sleep = duration => { }, + Sleep = _ => { }, }; copyArtifacts.Execute().ShouldBeTrue(buildEngine.GetConsoleLog()); @@ -111,7 +112,8 @@ public void RecursiveWildcards() "foo.exe.config", Path.Combine("baz", "baz.dll"), }.Select(i => Path.Combine(destination.FullName, i)), - ignoreOrder: true); + ignoreOrder: true, + customMessage: buildEngine.GetConsoleLog()); } [Fact] @@ -148,17 +150,19 @@ public void SingleFileMatch() ["FileMatch"] = "foo.pdb", }, }, - Sleep = duration => { }, + Sleep = _ => { }, }; copyArtifacts.Execute().ShouldBeTrue(buildEngine.GetConsoleLog()); destination.GetFiles("*", SearchOption.AllDirectories) .Select(i => i.FullName) - .ShouldBe(new[] - { - "foo.pdb", - }.Select(i => Path.Combine(destination.FullName, i))); + .ShouldBe( + new[] + { + "foo.pdb", + }.Select(i => Path.Combine(destination.FullName, i)), + customMessage: buildEngine.GetConsoleLog()); } [Fact] @@ -196,18 +200,75 @@ public void SingleFileMatchRecursive() ["FileMatch"] = "foo.pdb", }, }, - Sleep = duration => { }, + Sleep = _ => { }, }; copyArtifacts.Execute().ShouldBeTrue(buildEngine.GetConsoleLog()); destination.GetFiles("*", SearchOption.AllDirectories) .Select(i => i.FullName) - .ShouldBe(new[] + .ShouldBe( + new[] + { + "foo.pdb", + Path.Combine("foo", "foo", "foo", "foo.pdb"), + }.Select(i => Path.Combine(destination.FullName, i)), + customMessage: buildEngine.GetConsoleLog()); + } + + [Fact] + public void DuplicatedItemsShouldResultInOneCopy() + { + DirectoryInfo source = CreateFiles( + "source", + @"foo.txt"); + + DirectoryInfo destination = new DirectoryInfo(Path.Combine(TestRootPath, "destination")); + + BuildEngine buildEngine = BuildEngine.Create(); + + MockTaskItem singleFileItem = new MockTaskItem(Path.Combine(source.FullName, "foo.txt")) + { + ["DestinationFolder"] = destination.FullName, + }; + + MockTaskItem recursiveDirItem = new MockTaskItem(source.FullName) + { + ["DestinationFolder"] = destination.FullName, + }; + + Robocopy copyArtifacts = new Robocopy + { + BuildEngine = buildEngine, + Sources = new ITaskItem[] { - "foo.pdb", - Path.Combine("foo", "foo", "foo", "foo.pdb"), - }.Select(i => Path.Combine(destination.FullName, i))); + singleFileItem, + singleFileItem, + singleFileItem, + singleFileItem, + singleFileItem, + singleFileItem, + + recursiveDirItem, + recursiveDirItem, + recursiveDirItem, + recursiveDirItem, + recursiveDirItem, + recursiveDirItem, + }, + Sleep = _ => { }, + }; + + copyArtifacts.Execute().ShouldBeTrue(buildEngine.GetConsoleLog()); + + destination.GetFiles("*", SearchOption.AllDirectories) + .Select(i => i.FullName) + .ShouldBe( + new[] + { + "foo.txt", + }.Select(i => Path.Combine(destination.FullName, i)), + customMessage: buildEngine.GetConsoleLog()); } } } \ No newline at end of file diff --git a/src/Artifacts/FileSystem.cs b/src/Artifacts/FileSystem.cs index 44ca48d8..055084f8 100644 --- a/src/Artifacts/FileSystem.cs +++ b/src/Artifacts/FileSystem.cs @@ -2,6 +2,9 @@ // // Licensed under the MIT license. +#if NETSTANDARD2_0_OR_GREATER +using Microsoft.CopyOnWrite; +#endif using System.Collections.Generic; using System.IO; @@ -9,6 +12,10 @@ namespace Microsoft.Build.Artifacts { internal sealed class FileSystem : IFileSystem { +#if NETSTANDARD2_0_OR_GREATER + private static readonly ICopyOnWriteFilesystem CoW = CopyOnWriteFilesystemFactory.GetInstance(); +#endif + private FileSystem() { } @@ -59,5 +66,27 @@ public bool FileExists(FileInfo file) { return file.Exists; } + + public bool TryCloneFile(FileInfo sourceFile, FileInfo destinationFile) + { +#if NETSTANDARD2_0_OR_GREATER + string sourcePath = sourceFile.FullName; + string destPath = destinationFile.FullName; + if (CoW.CopyOnWriteLinkSupportedBetweenPaths(sourcePath, destPath)) + { + if (destinationFile.Exists) + { + // CoW doesn't overwrite destination files. + destinationFile.Delete(); + } + + // PathIsFullyResolved: FileInfo.FullName is fully resolved. + CoW.CloneFile(destPath, destPath, CloneFlags.PathIsFullyResolved); + return true; + } +#endif + + return false; + } } } \ No newline at end of file diff --git a/src/Artifacts/IFileSystem.cs b/src/Artifacts/IFileSystem.cs index 21527633..dbdadb41 100644 --- a/src/Artifacts/IFileSystem.cs +++ b/src/Artifacts/IFileSystem.cs @@ -26,5 +26,11 @@ internal interface IFileSystem bool FileExists(string path); bool FileExists(FileInfo file); + + /// + /// Attempts to create a copy-on-write link (file clone) if supported. + /// + /// True if the clone was created, false if unsupported. + bool TryCloneFile(FileInfo sourceFile, FileInfo destinationFile); } } \ No newline at end of file diff --git a/src/Artifacts/Microsoft.Build.Artifacts.csproj b/src/Artifacts/Microsoft.Build.Artifacts.csproj index d7728476..d264df90 100644 --- a/src/Artifacts/Microsoft.Build.Artifacts.csproj +++ b/src/Artifacts/Microsoft.Build.Artifacts.csproj @@ -12,7 +12,9 @@ true + + diff --git a/src/Artifacts/Tasks/Robocopy.cs b/src/Artifacts/Tasks/Robocopy.cs index 37b81d5a..decd51fc 100644 --- a/src/Artifacts/Tasks/Robocopy.cs +++ b/src/Artifacts/Tasks/Robocopy.cs @@ -5,16 +5,41 @@ using Microsoft.Build.Framework; using Microsoft.Build.Utilities; using System; +using System.Collections.Concurrent; using System.Collections.Generic; +using System.ComponentModel; using System.IO; using System.Linq; using System.Threading; +using System.Threading.Tasks.Dataflow; + +#nullable enable namespace Microsoft.Build.Artifacts.Tasks { public class Robocopy : Task { + // Similar but somewhat higher parallelism vs. MSBuild Copy task https://github.com/dotnet/msbuild/blob/main/src/Tasks/Copy.cs + private static readonly int DefaultCopyParallelism = Environment.ProcessorCount > 4 ? 8 : 4; + private static readonly ExecutionDataflowBlockOptions ActionBlockOptions = new () { MaxDegreeOfParallelism = GetMsBuildCopyTaskParallelism() }; + private static readonly bool DisableCoW = Environment.GetEnvironmentVariable("MSBUILDDISABLECLONEFILE") is not null; + + private readonly ConcurrentDictionary _dirsCreated = new (StringComparer.OrdinalIgnoreCase); + private readonly ActionBlock _copyFileBlock; + private readonly HashSet _filesCopied = new (CopyFileDedupKey.ComparerInstance); private TimeSpan _retryWaitInMilliseconds = TimeSpan.Zero; + private int _numFilesCopied; + private int _numFilesSkipped; + + public Robocopy() + { + _copyFileBlock = new (async job => + { + // Break from synchronous thread context of caller to get onto global thread pool thread for synchronous copy operations. + await System.Threading.Tasks.Task.Yield(); + CopyFileImpl(job.SourceFile, job.DestFile, job.Metadata); + }, ActionBlockOptions); + } public int RetryCount { get; set; } @@ -25,7 +50,7 @@ public class Robocopy : Task public bool ShowErrorOnRetry { get; set; } [Required] - public ITaskItem[] Sources { get; set; } + public ITaskItem[] Sources { get; set; } = Array.Empty(); internal IFileSystem FileSystem { get; set; } = Artifacts.FileSystem.Instance; @@ -36,20 +61,46 @@ public override bool Execute() RetryCount = Math.Max(0, RetryCount); _retryWaitInMilliseconds = RetryWait < 0 ? TimeSpan.Zero : TimeSpan.FromMilliseconds(RetryWait); - int count = 0; foreach (IList bucket in GetBuckets()) { CopyItems(bucket); - count++; } - Log.LogMessage("Processed {0} bucket(s)", count); + _copyFileBlock.Complete(); + _copyFileBlock.Completion.GetAwaiter().GetResult(); + + Log.LogMessage("Copied {0} files{1}", _numFilesCopied, _numFilesSkipped == 0 ? string.Empty : $", skipped {_numFilesSkipped}"); return !Log.HasLoggedErrors; } - private void CopyFile(FileInfo sourceFile, FileInfo destFile, bool createDirs, RobocopyMetadata metadata) + private static int GetMsBuildCopyTaskParallelism() { - if (createDirs) + // Use the MSBuild Copy task override parallelism setting if present. + // https://github.com/dotnet/msbuild/blob/1ff019aaa7cc17f22990548bb19498dfbbdebaec/src/Framework/Traits.cs#L83 + string? par = Environment.GetEnvironmentVariable("MSBUILDCOPYTASKPARALLELISM"); + if (string.IsNullOrEmpty(par) || !int.TryParse(par, out int parallelism)) + { + return DefaultCopyParallelism; + } + + return parallelism; + } + + private void CopyFile(FileInfo sourceFile, FileInfo destFile, RobocopyMetadata metadata) + { + if (_filesCopied.Add(new CopyFileDedupKey(sourceFile.FullName, destFile.FullName))) + { + _copyFileBlock.Post(new CopyJob(sourceFile, destFile, metadata)); + } + else + { + Log.LogMessage(MessageImportance.Low, "Skipped {0} to {1} as duplicate copy", sourceFile.FullName, destFile.FullName); + } + } + + private void CopyFileImpl(FileInfo sourceFile, FileInfo destFile, RobocopyMetadata metadata) + { + if (destFile.DirectoryName is not null) { CreateDirectoryWithRetries(destFile.DirectoryName); } @@ -60,14 +111,46 @@ private void CopyFile(FileInfo sourceFile, FileInfo destFile, bool createDirs, R { if (metadata.ShouldCopy(FileSystem, sourceFile, destFile)) { - destFile = FileSystem.CopyFile(sourceFile, destFile.FullName, true); + string sourcePath = sourceFile.FullName; + string destPath = destFile.FullName; + bool cowLinked = false; + if (!DisableCoW) + { + // Fast-path: We're on a CoW capable filesystem. + // On any problem fall back to real copy. + try + { + cowLinked = FileSystem.TryCloneFile(sourceFile, destFile); + if (cowLinked) + { + destFile.Refresh(); + } + } + catch (Win32Exception win32Ex) when (win32Ex.NativeErrorCode == 32) + { + // ERROR_SHARING_VIOLATION - just retry with copy. + Log.LogMessage("Sharing violation creating copy-on-write link from {0} to {1}, retrying with copy", sourcePath, destPath); + } + catch (Exception ex) + { + Log.LogMessage("Exception creating copy-on-write link from {0} to {1}, retrying with copy: {2}", sourcePath, destPath, ex); + } + } + + if (!cowLinked) + { + destFile = sourceFile.CopyTo(destFile.FullName, overwrite: true); + } + destFile.Attributes = FileAttributes.Normal; - destFile.LastWriteTime = sourceFile.LastWriteTime; - Log.LogMessage(MessageImportance.Low, "Copied {0} to {1}", sourceFile.FullName, destFile.FullName); + destFile.LastWriteTimeUtc = sourceFile.LastWriteTimeUtc; + Log.LogMessage(MessageImportance.Low, "{0} {1} to {2}", cowLinked ? "Created copy-on-write link" : "Copied", sourceFile.FullName, destFile.FullName); + Interlocked.Increment(ref _numFilesCopied); } else { Log.LogMessage(MessageImportance.Low, "Skipped copying {0} to {1}", sourceFile.FullName, destFile.FullName); + Interlocked.Increment(ref _numFilesSkipped); } break; @@ -85,7 +168,7 @@ private void CopyItems(IList items) RobocopyMetadata first = items.First(); bool isRecursive = first.IsRecursive; bool hasWildcards = first.HasWildcardMatches; - DirectoryInfo source = new DirectoryInfo(first.SourceFolder); + var source = new DirectoryInfo(first.SourceFolder); if (hasWildcards || isRecursive) { @@ -103,7 +186,6 @@ private void CopyItems(IList items, DirectoryInfo source) { foreach (RobocopyMetadata item in items) { - bool createDirs = true; foreach (string file in item.FileMatches) { FileInfo sourceFile = new FileInfo(Path.Combine(source.FullName, file)); @@ -114,18 +196,15 @@ private void CopyItems(IList items, DirectoryInfo source) FileInfo destFile = new FileInfo(Path.Combine(destination, file)); if (Verify(destFile, false, false)) { - CopyFile(sourceFile, destFile, createDirs, item); + CopyFile(sourceFile, destFile, item); } } - - // only try to create the dirs for the first set of files - createDirs = false; } } } } - private void CopySearch(IList bucket, bool isRecursive, string match, DirectoryInfo source, string subDirectory) + private void CopySearch(IList bucket, bool isRecursive, string match, DirectoryInfo source, string? subDirectory) { bool hasSubDirectory = !string.IsNullOrEmpty(subDirectory); foreach (FileInfo sourceFile in FileSystem.EnumerateFiles(source, match)) @@ -136,11 +215,11 @@ private void CopySearch(IList bucket, bool isRecursive, string { foreach (string destination in item.DestinationFolders) { - string fullDest = hasSubDirectory ? Path.Combine(destination, subDirectory) : destination; + string fullDest = hasSubDirectory ? Path.Combine(destination, subDirectory!) : destination; FileInfo destFile = new FileInfo(Path.Combine(fullDest, sourceFile.Name)); Verify(destFile, false, false); - CopyFile(sourceFile, destFile, true, item); + CopyFile(sourceFile, destFile, item); } } } @@ -152,7 +231,7 @@ private void CopySearch(IList bucket, bool isRecursive, string foreach (DirectoryInfo childSource in FileSystem.EnumerateDirectories(source)) { // per dir we need to re-items for those items excluding a specific dir - string childSubDirectory = hasSubDirectory ? Path.Combine(subDirectory, childSource.Name) : childSource.Name; + string childSubDirectory = hasSubDirectory ? Path.Combine(subDirectory!, childSource.Name) : childSource.Name; IList subBucket = new List(); foreach (RobocopyMetadata item in bucket) { @@ -174,13 +253,23 @@ private void CreateDirectoryWithRetries(string directory) { try { - FileSystem.CreateDirectory(directory); - + // Minimize filesystem calls for directory creation. AddOrUpdate must be used instead of GetOrAdd for this pattern to work. + _dirsCreated.AddOrUpdate(directory, d => + { + // Create directory before exiting add lock - + // this logic may be executed multiple times on different threads + // but must create the directory before exiting. + FileSystem.CreateDirectory(d); + return true; + }, + #pragma warning disable SA1117 + (_, f) => f); + #pragma warning restore SA1117 break; } catch (IOException) { - /* ignore failures - we'll catch them later on copy */ + // Ignore failures - we'll catch them later on copy. Sleep(TimeSpan.FromMilliseconds(200)); } } @@ -191,7 +280,7 @@ private IEnumerable> GetBuckets() IList allSources = new List(); IList> allBuckets = new List>(); - foreach (ITaskItem item in Sources ?? Enumerable.Empty()) + foreach (ITaskItem item in Sources) { if (RobocopyMetadata.TryParse(item, Log, FileSystem.DirectoryExists, out RobocopyMetadata metadata)) { @@ -289,5 +378,49 @@ private bool Verify(FileInfo file, bool shouldExist, bool verifyExists) return false; } + + private readonly struct CopyFileDedupKey + { + private readonly string _sourcePath; + private readonly string _destPath; + + public CopyFileDedupKey(string source, string dest) + { + _sourcePath = source; + _destPath = dest; + } + + public static Comparer ComparerInstance { get; } = new (); + + public sealed class Comparer : IEqualityComparer + { + public bool Equals(CopyFileDedupKey x, CopyFileDedupKey y) + { + return x._sourcePath.Equals(y._sourcePath, StringComparison.OrdinalIgnoreCase) && + x._destPath.Equals(y._destPath, StringComparison.OrdinalIgnoreCase); + } + + public int GetHashCode(CopyFileDedupKey obj) + { + return StringComparer.OrdinalIgnoreCase.GetHashCode(obj._destPath) ^ StringComparer.OrdinalIgnoreCase.GetHashCode(obj._sourcePath); + } + } + } + + private sealed class CopyJob + { + public CopyJob(FileInfo sourceFile, FileInfo destFile, RobocopyMetadata metadata) + { + SourceFile = sourceFile; + DestFile = destFile; + Metadata = metadata; + } + + public FileInfo SourceFile { get; } + + public FileInfo DestFile { get; } + + public RobocopyMetadata Metadata { get; } + } } } diff --git a/src/Artifacts/Tasks/RobocopyMetadata.cs b/src/Artifacts/Tasks/RobocopyMetadata.cs index 9f6bcd56..5120ed6b 100644 --- a/src/Artifacts/Tasks/RobocopyMetadata.cs +++ b/src/Artifacts/Tasks/RobocopyMetadata.cs @@ -25,7 +25,7 @@ private RobocopyMetadata() public bool AlwaysCopy { get; private set; } - public List DestinationFolders { get; } = new List(); + public List DestinationFolders { get; } = new (); public string[] DirExcludes { get; private set; } @@ -279,7 +279,7 @@ public bool ShouldCopy(IFileSystem fileSystem, FileInfo source, FileInfo dest) private string DumpString(string[] noWild, Regex[] wild) { - StringBuilder builder = new StringBuilder(); + var builder = new StringBuilder(128); if (noWild != null) { foreach (string item in noWild) @@ -312,9 +312,9 @@ private string DumpString(string[] noWild, Regex[] wild) private void SplitItems(string metadata, ITaskItem taskItem) { string items = taskItem.GetMetadata(metadata); - List strings = new List(); - List preRegex = new List(); - List regularExpressions = new List(); + var strings = new List(); + var preRegex = new List(); + var regularExpressions = new List(); bool doMatchAll = false; if (!string.IsNullOrEmpty(items)) { diff --git a/src/Artifacts/version.json b/src/Artifacts/version.json index 4e6e966c..1f07002f 100644 --- a/src/Artifacts/version.json +++ b/src/Artifacts/version.json @@ -1,4 +1,4 @@ { "inherit": true, - "version": "4.2" + "version": "4.3" } diff --git a/src/CopyOnWrite/Copy.cs b/src/CopyOnWrite/Copy.cs index a7a99bcb..be43f124 100644 --- a/src/CopyOnWrite/Copy.cs +++ b/src/CopyOnWrite/Copy.cs @@ -976,7 +976,8 @@ private static int ParseIntFromEnvironmentVariableOrDefault(string environmentVa } #region CopyOnWrite functionality - private static readonly ICopyOnWriteFilesystem _cow = CopyOnWriteFilesystemFactory.GetInstance(); + private static readonly ICopyOnWriteFilesystem CoW = CopyOnWriteFilesystemFactory.GetInstance(); + private static readonly bool DisableCoW = Environment.GetEnvironmentVariable("MSBUILDDISABLECOPYONWRITE") is not null; /// /// Attempt to clone source file to destination. @@ -988,7 +989,7 @@ private bool TryCopyOnWrite(string source, string dest) { try { - _cow.CloneFile(source, dest, CloneFlags.PathIsFullyResolved); + CoW.CloneFile(source, dest, CloneFlags.PathIsFullyResolved); Log.LogMessage(MessageImportance.Low, $"CloneFile '{source}' to '{dest}'."); } catch (Exception e) @@ -1000,8 +1001,6 @@ private bool TryCopyOnWrite(string source, string dest) return true; } - private static readonly ConcurrentDictionary> _reFsDrives = new(StringComparer.OrdinalIgnoreCase); - /// /// Check for CopyOnWrite support. Result is cached by drive root. /// @@ -1010,38 +1009,17 @@ private bool TryCopyOnWrite(string source, string dest) /// True when CopyOnWrite appears to be supported. private bool IsCopyOnWriteSupported(string source, string dest) { - if (source.StartsWith(@"\\", StringComparison.OrdinalIgnoreCase) || dest.StartsWith(@"\\")) + if (DisableCoW) { return false; } - var sourceDrive = Path.GetPathRoot(source); - var destDrive = Path.GetPathRoot(dest); - - if (!sourceDrive.Equals(destDrive, StringComparison.OrdinalIgnoreCase)) + if (source.StartsWith(@"\\", StringComparison.Ordinal) || dest.StartsWith(@"\\", StringComparison.Ordinal)) { return false; } - try - { - return _reFsDrives.GetOrAdd( - sourceDrive, - (key) => new Lazy(() => - { - var supportsCloneFile = _cow.CopyOnWriteLinkSupportedInDirectoryTree(key); - Log.LogMessage(MessageImportance.Low, - supportsCloneFile - ? $"Drive {key} has support for CloneFile. All copies will attempt to use CloneFile." - : $"Drive {key} does not have support for CloneFile. File.Copy will be used."); - return supportsCloneFile; - })).Value; - } - catch (Exception ex) - { - Log.LogMessage(MessageImportance.Low, $"Couldn't determine if CloneFile is supported for {sourceDrive} : {ex}"); - return false; - } + return CoW.CopyOnWriteLinkSupportedBetweenPaths(source, dest); } #endregion }