CoW, Artifacts: Handle symlinked same-file path

Directory.Build.targets

@@ -41,7 +41,7 @@
  </ItemGroup>
 
  <ItemGroup Condition="'$(IsTestProject)' == 'true'">
- <Compile Include="$(MSBuildThisFileDirectory)src\Shared\*.cs" />
+ <Compile Include="$(MSBuildThisFileDirectory)src\TestShared\*.cs" />
 
  <Content Include="$(MSBuildThisFileDirectory)xunit.runner.json"
  CopyToOutputDirectory="PreserveNewest" />

Directory.Packages.props

@@ -12,12 +12,13 @@
  <PackageVersion Update="Microsoft.Build.Tasks.Core" Version="16.9.0" Condition="'$(TargetFramework)' == 'netcoreapp3.1' Or '$(TargetFramework)' == 'netstandard2.0'" />
  <PackageVersion Update="Microsoft.Build.Tasks.Core" Version="15.9.20" Condition="'$(TargetFramework)' == 'net46'" />
  <PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.7.2" />
- <PackageVersion Include="Microsoft.Win32.Registry" Version="4.7.0" />
+ <PackageVersion Include="Microsoft.Win32.Registry" Version="5.0.0" />
  <PackageVersion Include="MSBuild.ProjectCreation" Version="10.0.0" />
  <PackageVersion Include="Newtonsoft.Json" Version="13.0.3" />
  <PackageVersion Include="Shouldly" Version="4.2.1" />
- <PackageVersion Include="System.Threading.Tasks.Dataflow" Version="6.0.0" Condition=" '$(TargetFramework)' != 'net46' "/>
- <PackageVersion Include="System.Threading.Tasks.Dataflow" Version="4.11.1" Condition=" '$(TargetFramework)' == 'net46' "/>
+ <PackageVersion Include="System.CodeDom" Version="7.0.0" />
+ <PackageVersion Include="System.Threading.Tasks.Dataflow" Version="6.0.0" Condition=" '$(TargetFramework)' != 'net46' " />
+ <PackageVersion Include="System.Threading.Tasks.Dataflow" Version="4.11.1" Condition=" '$(TargetFramework)' == 'net46' " />
  <PackageVersion Include="xunit" Version="2.5.1" />
  <PackageVersion Include="xunit.runner.visualstudio" Version="2.4.5" />
  </ItemGroup>

MSBuildSdks.sln

@@ -82,6 +82,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "SampleNoTargets", "SampleNo
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Build.CopyOnWrite", "src\CopyOnWrite\Microsoft.Build.CopyOnWrite.csproj", "{153D1183-2953-4D4D-A5AD-AA2CF99B0DE3}"
 EndProject
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Build.CopyOnWrite.UnitTests", "src\CopyOnWrite.UnitTests\Microsoft.Build.CopyOnWrite.UnitTests.csproj", "{AF9F2AFE-04D4-40B3-B17F-54ABD3DE7E4E}"
+EndProject
 Global
  GlobalSection(SolutionConfigurationPlatforms) = preSolution
  Debug|Any CPU = Debug|Any CPU
@@ -140,6 +142,10 @@ Global
  {153D1183-2953-4D4D-A5AD-AA2CF99B0DE3}.Debug|Any CPU.Build.0 = Debug|Any CPU
  {153D1183-2953-4D4D-A5AD-AA2CF99B0DE3}.Release|Any CPU.ActiveCfg = Release|Any CPU
  {153D1183-2953-4D4D-A5AD-AA2CF99B0DE3}.Release|Any CPU.Build.0 = Release|Any CPU
+ {AF9F2AFE-04D4-40B3-B17F-54ABD3DE7E4E}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+ {AF9F2AFE-04D4-40B3-B17F-54ABD3DE7E4E}.Debug|Any CPU.Build.0 = Debug|Any CPU
+ {AF9F2AFE-04D4-40B3-B17F-54ABD3DE7E4E}.Release|Any CPU.ActiveCfg = Release|Any CPU
+ {AF9F2AFE-04D4-40B3-B17F-54ABD3DE7E4E}.Release|Any CPU.Build.0 = Release|Any CPU
  EndGlobalSection
  GlobalSection(SolutionProperties) = preSolution
  HideSolutionNode = FALSE

src/Artifacts.UnitTests/RobocopyTests.cs

@@ -21,8 +21,6 @@ namespace Microsoft.Build.Artifacts.UnitTests
 {
  public class RobocopyTests : MSBuildSdkTestBase
  {
- private static readonly bool IsWindows = Environment.OSVersion.Platform == PlatformID.Win32NT;
-
  [Fact]
  public void DedupKeyOsDifferences()
  {

src/Artifacts/Microsoft.Build.Artifacts.csproj

@@ -11,6 +11,9 @@
  <SymbolPackageFormat>snupkg</SymbolPackageFormat>
  <EmbedUntrackedSources>true</EmbedUntrackedSources>
  </PropertyGroup>
+ <ItemGroup>
+ <Compile Include="..\Shared\CopyExceptionHandling.cs" />
+ </ItemGroup>
  <ItemGroup>
  <PackageReference Include="CopyOnWrite" GeneratePathProperty="True" PrivateAssets="All" />
  <PackageReference Include="Microsoft.Build.Utilities.Core" ExcludeAssets="Runtime" PrivateAssets="All" />

src/Artifacts/README.md

@@ -172,7 +172,7 @@ The `<Artifact />` items specify collections of artifacts to stage. These items
 | `IsRecursive` | Enables a recursive path search for artifacts to stage | `true` |
 | `VerifyExists` | Enables a check that the file exists before copying | `true` |
 | `AlwaysCopy` | Enables copies even if the destination already exists | `false` |
-| `OnlyNewer` | Enables copies only if the destnation exist and the source is newer | `false` |
+| `OnlyNewer` | Enables copies only if the destination exists and the source is newer | `false` |
 | `FileMatch` | A list of one or more file filters seperated by a space or semicolon to include. Wildcards include `*` and `?` | `*`|
 | `FileExclude` | A list of one or more file filters seperated by a space or semicolon to exclude. Wildcards include `*` and `?` | |
 | `DirExclude` | A list of one or more directory filters seperated by a space or semicolon to exclude. Wildcards include `*` and `?` | |

src/Artifacts/Tasks/Robocopy.cs

@@ -3,6 +3,7 @@
 // Licensed under the MIT license.
 
 using Microsoft.Build.Framework;
+using Microsoft.Build.Shared;
 using Microsoft.Build.Utilities;
 using System;
 using System.Collections.Concurrent;
@@ -30,7 +31,6 @@ public class Robocopy : Task
  private static readonly ExecutionDataflowBlockOptions ActionBlockOptions = new () { MaxDegreeOfParallelism = MsBuildCopyParallelism, EnsureOrdered = MsBuildCopyParallelism == 1 };
 
  private readonly ConcurrentDictionary<string, bool> _dirsCreated = new (Artifacts.FileSystem.PathComparer);
- private readonly Dictionary<string, string> _realDirPaths = new (Artifacts.FileSystem.PathComparer); // Cache results of symlink resolution to avoid I/O.
  private readonly HashSet<string> _destinationPathsStarted = new (Artifacts.FileSystem.PathComparer); // Destination paths that were dispatched to copy. Extra copies to the same destination are copied single-threaded in a second wave.
  private readonly List<CopyJob> _duplicateDestinationDelayedJobs = new (); // Jobs that were delayed because they were to a destination path that was already dispatched to copy.
  private readonly ActionBlock<CopyJob> _copyFileBlock;
@@ -66,7 +66,7 @@ public Robocopy()
  public bool ShowDiagnosticTrace { get; set; }
 
  /// <summary>
- /// Gets or sets a value indicating whether to log errors on retries
+ /// Gets or sets a value indicating whether to log errors on retries.
  /// </summary>
  public bool ShowErrorOnRetry { get; set; }
 
@@ -156,7 +156,7 @@ private void CopyFile(FileInfo sourceFile, FileInfo destFile, RobocopyMetadata m
  }
  else if (!_filesCopied.Contains(new CopyFileDedupKey(sourceFile.FullName, destFile.FullName)))
  {
- Log.LogMessage("Delaying {0} to {1} as duplicate destination", sourceFile.FullName, destFile.FullName);
+ Log.LogMessage("Delaying copying {0} to {1} as duplicate destination", sourceFile.FullName, destFile.FullName);
  _duplicateDestinationDelayedJobs.Add(new CopyJob(sourceFile, destFile, metadata));
  }
  else
@@ -216,15 +216,19 @@ private void CopyFileImpl(FileInfo sourceFile, FileInfo destFile, RobocopyMetada
  }
  else
  {
- Log.LogMessage(MessageImportance.Low, "Skipped copying {0} to {1}", sourceFile.FullName, destFile.FullName);
+ Log.LogMessage(MessageImportance.Low, "Skipped copying {0} to {1}", sourcePath, destPath);
  Interlocked.Increment(ref _numFilesSkipped);
  }
 
  break;
  }
  catch (IOException e)
  {
- LogCopyFailureAndSleep(retry, "Failed to copy {0} to {1}. {2}", sourcePath, destPath, e.Message);
+ // Avoid issuing an error if the paths are actually to the same file.
+ if (!CopyExceptionHandling.FullPathsAreIdentical(sourcePath, destPath))
+ {
+ LogCopyFailureAndSleep(retry, "Failed to copy {0} to {1}. {2}", sourcePath, destPath, e.Message);
+ }
  }
  }
  }
@@ -257,19 +261,15 @@ private void CopyItems(IList<RobocopyMetadata> items, DirectoryInfo source)
  {
  foreach (string file in item.FileMatches)
  {
- // Break down symlinks/junctions to their real paths to avoid duplicate copies.
  string sourcePath = Path.Combine(sourceDir, file);
  FileInfo sourceFile = new FileInfo(sourcePath);
- if (Verify(sourceFile, true, item.VerifyExists))
+ if (Verify(sourceFile, item.VerifyExists))
  {
  foreach (string destDir in item.DestinationFolders)
  {
  string destPath = Path.Combine(destDir, file);
  FileInfo destFile = new FileInfo(destPath);
- if (Verify(destFile, shouldExist: false, false))
- {
- CopyFile(sourceFile, destFile, item);
- }
+ CopyFile(sourceFile, destFile, item);
  }
  }
  }
@@ -278,8 +278,6 @@ private void CopyItems(IList<RobocopyMetadata> items, DirectoryInfo source)
 
  private void CopySearch(IList<RobocopyMetadata> bucket, bool isRecursive, string match, DirectoryInfo source, string? subDirectory)
  {
- string sourceDir = source.FullName;
-
  bool hasSubDirectory = !string.IsNullOrEmpty(subDirectory);
 
  foreach (FileInfo sourceFile in FileSystem.EnumerateFiles(source, match))
@@ -294,10 +292,7 @@ private void CopySearch(IList<RobocopyMetadata> bucket, bool isRecursive, string
  string destDir = hasSubDirectory ? Path.Combine(destinationDir, subDirectory!) : destinationDir;
  string destPath = Path.Combine(destDir, fileName);
  FileInfo destFile = new FileInfo(destPath);
- if (Verify(destFile, shouldExist: false, false))
- {
- CopyFile(sourceFile, destFile, item);
- }
+ CopyFile(sourceFile, destFile, item);
  }
  }
  }
@@ -444,9 +439,9 @@ private void LogCopyFailureAndSleep(int attempt, string message, params object[]
  }
  }
 
- private bool Verify(FileInfo file, bool shouldExist, bool verifyExists)
+ private bool Verify(FileInfo file, bool verifyExists)
  {
- if (!shouldExist || FileSystem.FileExists(file))
+ if (FileSystem.FileExists(file))
  {
  return true;
  }

src/CopyOnWrite.UnitTests/CopyExceptionHandlingTests.cs

@@ -0,0 +1,61 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+//
+// Licensed under the MIT license.
+
+using Microsoft.Build.Framework;
+using Microsoft.Build.Shared;
+using Microsoft.Build.UnitTests.Common;
+using System.IO;
+using Xunit;
+
+namespace Microsoft.Build.CopyOnWrite.UnitTests;
+
+public class CopyExceptionHandlingTests : MSBuildSdkTestBase
+{
+ [Fact]
+ public void PathsAreIdentical_NoSymlinks()
+ {
+ string osRoot = IsWindows ? @"C:\" : "/";
+ string osCapitalizationDependentName = IsWindows ? "NAME" : "name";
+ Assert.True(CopyExceptionHandling.PathsAreIdentical(osRoot, osRoot));
+ Assert.True(CopyExceptionHandling.PathsAreIdentical(Path.Combine(osRoot, "name"), Path.Combine(osCapitalizationDependentName)));
+ Assert.True(CopyExceptionHandling.PathsAreIdentical(osRoot, Path.Combine(osRoot, "subDir", "..")));
+ }
+
+ [Fact]
+ public void PathsAreIdentical_Symlinks()
+ {
+ if (!UserCanCreateSymlinks())
+ {
+ return;
+ }
+
+ using var tempDir = new DisposableTempDirectory();
+ string regularFilePath = Path.Combine(tempDir.Path, "regular.txt");
+ File.WriteAllText(regularFilePath, "regular");
+ Assert.True(CopyExceptionHandling.PathsAreIdentical(regularFilePath, regularFilePath));
+
+ string regularFilePathWithNonCanonicalSegments =
+ Path.Combine(tempDir.Path, "..", Path.GetFileName(tempDir.Path), ".", "regular.txt");
+ Assert.True(CopyExceptionHandling.PathsAreIdentical(regularFilePath, regularFilePathWithNonCanonicalSegments));
+
+ string symlinkPath = Path.Combine(tempDir.Path, "symlink_to_regular.txt");
+ string? errorMessage = null;
+ bool linkCreated = NativeMethods.MakeSymbolicLink(symlinkPath, regularFilePath, ref errorMessage);
+ Assert.True(linkCreated);
+ Assert.Null(errorMessage);
+ Assert.True(CopyExceptionHandling.PathsAreIdentical(regularFilePath, symlinkPath));
+
+ File.Delete(symlinkPath);
+ errorMessage = null;
+ linkCreated = NativeMethods.MakeSymbolicLink(symlinkPath, regularFilePathWithNonCanonicalSegments, ref errorMessage);
+ Assert.True(linkCreated);
+ Assert.Null(errorMessage);
+ Assert.True(CopyExceptionHandling.PathsAreIdentical(regularFilePath, symlinkPath));
+ }
+
+ private bool UserCanCreateSymlinks()
+ {
+ return !IsWindows || IsAdministratorOnWindows();
+ }
+}

src/CopyOnWrite.UnitTests/Microsoft.Build.CopyOnWrite.UnitTests.csproj

@@ -0,0 +1,24 @@
+<Project Sdk="Microsoft.NET.Sdk">
+ <PropertyGroup>
+ <TargetFrameworks>net472;netcoreapp3.1;net6.0</TargetFrameworks>
+ <IsPackable>false</IsPackable>
+ <Nullable>Enable</Nullable>
+
+ <!-- Suppress "Error : System.CodeDom 7.0.0 doesn't support netcoreapp3.1 and has not been tested with it." -->
+ <SuppressTfmSupportBuildWarnings>true</SuppressTfmSupportBuildWarnings>
+ </PropertyGroup>
+ <ItemGroup>
+ <PackageReference Include="CopyOnWrite" />
+ <PackageReference Include="Microsoft.Build.Tasks.Core" />
+ <PackageReference Include="Microsoft.NET.Test.Sdk" />
+ <PackageReference Include="Microsoft.Win32.Registry" />
+ <PackageReference Include="MSBuild.ProjectCreation" />
+ <PackageReference Include="Newtonsoft.Json" />
+ <PackageReference Include="System.CodeDom" />
+ <PackageReference Include="xunit" />
+ <PackageReference Include="xunit.runner.visualstudio" />
+ </ItemGroup>
+ <ItemGroup>
+ <ProjectReference Include="..\CopyOnWrite\Microsoft.Build.CopyOnWrite.csproj" />
+ </ItemGroup>
+</Project>

src/CopyOnWrite/Copy.cs

@@ -864,7 +864,7 @@ private bool DoCopyWithRetries(FileState sourceFileState, FileState destinationF
  // if this was just because the source and destination files are the
  // same file, that's not a failure.
  // Note -- we check this exceptional case here, not before the copy, for perf.
- if (PathsAreIdentical(sourceFileState.Name, destinationFileState.Name))
+ if (CopyExceptionHandling.PathsAreIdentical(sourceFileState.Name, destinationFileState.Name))
  {
  return true;
  }
@@ -968,18 +968,6 @@ public override bool Execute()
 
  #endregion
 
- /// <summary>
- /// Compares two paths to see if they refer to the same file. We can't solve the general
- /// canonicalization problem, so we just compare strings on the full paths.
- /// </summary>
- private static bool PathsAreIdentical(string source, string destination)
- {
- string fullSourcePath = Path.GetFullPath(source);
- string fullDestinationPath = Path.GetFullPath(destination);
- StringComparison filenameComparison = NativeMethods.IsWindows ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal;
- return String.Equals(fullSourcePath, fullDestinationPath, filenameComparison);
- }
-
  private static int GetParallelismFromEnvironment()
  {
  int parallelism = CopyTaskParallelism;

src/CopyOnWrite/MSBuild/NativeMethods.cs

@@ -2,11 +2,14 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using System.ComponentModel;
 using System.IO;
 using System.Runtime.InteropServices;
+using System.Text;
 using Microsoft.Win32;
+using Microsoft.Win32.SafeHandles;
 
-#nullable disable
+#nullable enable
 
 namespace Microsoft.Build.Framework;
 
@@ -104,7 +107,7 @@ private static bool IsLongPathsEnabledRegistry()
  {
  using (RegistryKey fileSystemKey = Registry.LocalMachine.OpenSubKey(WINDOWS_FILE_SYSTEM_REGISTRY_KEY))
  {
- object longPathsEnabledValue = fileSystemKey?.GetValue(WINDOWS_LONG_PATHS_ENABLED_VALUE_NAME, 0);
+ object? longPathsEnabledValue = fileSystemKey?.GetValue(WINDOWS_LONG_PATHS_ENABLED_VALUE_NAME, 0);
  return fileSystemKey != null && Convert.ToInt32(longPathsEnabledValue) == 1;
  }
  }
@@ -132,7 +135,7 @@ internal static bool IsWindows
  [DllImport("libc", SetLastError = true)]
  internal static extern int link(string oldpath, string newpath);
 
- internal static bool MakeHardLink(string newFileName, string exitingFileName, ref string errorMessage)
+ internal static bool MakeHardLink(string newFileName, string exitingFileName, ref string? errorMessage)
  {
  bool hardLinkCreated;
  if (IsWindows)
@@ -152,29 +155,29 @@ internal static bool MakeHardLink(string newFileName, string exitingFileName, re
  //------------------------------------------------------------------------------
  // CreateSymbolicLink
  //------------------------------------------------------------------------------
- internal enum SymbolicLink
+ private enum SymbolicLink
  {
  File = 0,
  Directory = 1
  }
  [DllImport("kernel32.dll", SetLastError = true, CharSet = CharSet.Unicode)]
  [return: MarshalAs(UnmanagedType.I1)]
- internal static extern bool CreateSymbolicLink(string symLinkFileName, string targetFileName, SymbolicLink dwFlags);
+ private static extern bool CreateSymbolicLink(string symLinkFileName, string targetFileName, SymbolicLink dwFlags);
 
  [DllImport("libc", SetLastError = true)]
- internal static extern int symlink(string oldpath, string newpath);
+ private static extern int symlink(string oldpath, string newpath);
 
- internal static bool MakeSymbolicLink(string newFileName, string exitingFileName, ref string errorMessage)
+ public static bool MakeSymbolicLink(string newFileName, string existingFileName, ref string? errorMessage)
  {
  bool symbolicLinkCreated;
  if (IsWindows)
  {
- symbolicLinkCreated = CreateSymbolicLink(newFileName, exitingFileName, SymbolicLink.File);
+ symbolicLinkCreated = CreateSymbolicLink(newFileName, existingFileName, SymbolicLink.File);
  errorMessage = symbolicLinkCreated ? null : Marshal.GetExceptionForHR(Marshal.GetHRForLastWin32Error()).Message;
  }
  else
  {
- symbolicLinkCreated = symlink(exitingFileName, newFileName) == 0;
+ symbolicLinkCreated = symlink(existingFileName, newFileName) == 0;
  errorMessage = symbolicLinkCreated ? null : "The link() library call failed with the following error code: " + Marshal.GetLastWin32Error();
  }