Skip to content

Commit

Permalink
CoW, Artifacts: Handle symlinked same-file path (#492)
Browse files Browse the repository at this point in the history
Avoid throwing when source and destination are the same via a symlink.
  • Loading branch information
erikmav authored Oct 23, 2023
1 parent dddcbe2 commit efa8411
Show file tree
Hide file tree
Showing 16 changed files with 531 additions and 56 deletions.
2 changes: 1 addition & 1 deletion Directory.Build.targets
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
7 changes: 4 additions & 3 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
6 changes: 6 additions & 0 deletions MSBuildSdks.sln
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions src/Artifacts.UnitTests/RobocopyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
3 changes: 3 additions & 0 deletions src/Artifacts/Microsoft.Build.Artifacts.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
2 changes: 1 addition & 1 deletion src/Artifacts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `?` | |
Expand Down
33 changes: 14 additions & 19 deletions src/Artifacts/Tasks/Robocopy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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; }

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}
}
}
Expand Down Expand Up @@ -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);
}
}
}
Expand All @@ -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))
Expand All @@ -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);
}
}
}
Expand Down Expand Up @@ -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;
}
Expand Down
61 changes: 61 additions & 0 deletions src/CopyOnWrite.UnitTests/CopyExceptionHandlingTests.cs
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
@@ -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>
14 changes: 1 addition & 13 deletions src/CopyOnWrite/Copy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down
21 changes: 12 additions & 9 deletions src/CopyOnWrite/MSBuild/NativeMethods.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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)
Expand All @@ -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();
}

Expand Down
Loading

0 comments on commit efa8411

Please sign in to comment.