Fix Copy with UseSymbolicLinksIfPossible = true (#8157)

* Properly create symlinks * Fix test * Version test and use good name * Add osVersion * Add && * Remove version check * Update src/Tasks.UnitTests/Copy_Tests.cs Co-authored-by: Rainer Sigwald <raines@microsoft.com> Co-authored-by: Forgind <Forgind@users.noreply.github.com> Co-authored-by: Rainer Sigwald <raines@microsoft.com>
dotnet · Nov 23, 2022 · d797c48 · d797c48
1 parent 0ac748c
commit d797c48
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 60 deletions.
diff --git a/src/Tasks.UnitTests/Copy_Tests.cs b/src/Tasks.UnitTests/Copy_Tests.cs
@@ -2397,77 +2397,63 @@ public CopySymbolicLink_Tests(ITestOutputHelper testOutputHelper)
         [Fact]
         public void CopyToDestinationFolderWithSymbolicLinkCheck()
         {
-            var isPrivileged = true;
-
-            if (NativeMethodsShared.IsWindows)
-            {
-                if (!new WindowsPrincipal(WindowsIdentity.GetCurrent()).IsInRole(new SecurityIdentifier(WellKnownSidType.BuiltinAdministratorsSid, null)))
-                {
-                    isPrivileged = false;
-                    Assert.True(true, "It seems that you don't have the permission to create symbolic links. Try to run this test again with higher privileges");
-                }
-            }
-
-            if (isPrivileged)
+            string sourceFile = FileUtilities.GetTemporaryFile();
+            string temp = Path.GetTempPath();
+            string destFolder = Path.Combine(temp, "2A333ED756AF4dc392E728D0F864A398");
+            string destFile = Path.Combine(destFolder, Path.GetFileName(sourceFile));
+            try
             {
-                string sourceFile = FileUtilities.GetTemporaryFile();
-                string temp = Path.GetTempPath();
-                string destFolder = Path.Combine(temp, "2A333ED756AF4dc392E728D0F864A398");
-                string destFile = Path.Combine(destFolder, Path.GetFileName(sourceFile));
-                try
-                {
-                    File.WriteAllText(sourceFile, "This is a source temp file."); // HIGHCHAR: Test writes in UTF8 without preamble.
-
-                    // Don't create the dest folder, let task do that
+                File.WriteAllText(sourceFile, "This is a source temp file."); // HIGHCHAR: Test writes in UTF8 without preamble.
 
-                    ITaskItem[] sourceFiles = { new TaskItem(sourceFile) };
+                // Don't create the dest folder, let task do that
+                ITaskItem[] sourceFiles = { new TaskItem(sourceFile) };
 
-                    var me = new MockEngine(true);
-                    var t = new Copy
-                    {
-                        RetryDelayMilliseconds = 1,  // speed up tests!
-                        BuildEngine = me,
-                        SourceFiles = sourceFiles,
-                        DestinationFolder = new TaskItem(destFolder),
-                        SkipUnchangedFiles = true,
-                        UseSymboliclinksIfPossible = true
-                    };
+                var me = new MockEngine(true);
+                var t = new Copy
+                {
+                    RetryDelayMilliseconds = 1,  // speed up tests!
+                    BuildEngine = me,
+                    SourceFiles = sourceFiles,
+                    DestinationFolder = new TaskItem(destFolder),
+                    SkipUnchangedFiles = true,
+                    UseSymboliclinksIfPossible = true
+                };
 
-                    bool success = t.Execute();
+                bool success = t.Execute();
 
-                    Assert.True(success); // "success"
-                    Assert.True(File.Exists(destFile)); // "destination exists"
+                Assert.True(success); // "success"
+                Assert.True(File.Exists(destFile)); // "destination exists"
+                Assert.True((File.GetAttributes(destFile) & FileAttributes.ReparsePoint) != 0, "File was copied but is not a symlink");
 
-                    MockEngine.GetStringDelegate resourceDelegate = AssemblyResources.GetString;
+                MockEngine.GetStringDelegate resourceDelegate = AssemblyResources.GetString;
 
-                    me.AssertLogContainsMessageFromResource(resourceDelegate, "Copy.SymbolicLinkComment", sourceFile, destFile);
+                me.AssertLogContainsMessageFromResource(resourceDelegate, "Copy.SymbolicLinkComment", sourceFile, destFile);
 
-                    string destinationFileContents = File.ReadAllText(destFile);
-                    Assert.Equal("This is a source temp file.", destinationFileContents); // "Expected the destination symbolic linked file to contain the contents of source file."
+                string destinationFileContents = File.ReadAllText(destFile);
+                Assert.Equal("This is a source temp file.", destinationFileContents); // "Expected the destination symbolic linked file to contain the contents of source file."
 
-                    Assert.Single(t.DestinationFiles);
-                    Assert.Single(t.CopiedFiles);
-                    Assert.Equal(destFile, t.DestinationFiles[0].ItemSpec);
-                    Assert.Equal(destFile, t.CopiedFiles[0].ItemSpec);
+                Assert.Single(t.DestinationFiles);
+                Assert.Single(t.CopiedFiles);
+                Assert.Equal(destFile, t.DestinationFiles[0].ItemSpec);
+                Assert.Equal(destFile, t.CopiedFiles[0].ItemSpec);
 
-                    // Now we will write new content to the source file
-                    // we'll then check that the destination file automatically
-                    // has the same content (i.e. it's been hard linked)
+                // Now we will write new content to the source file
+                // we'll then check that the destination file automatically
+                // has the same content (i.e. it's been hard linked)
 
-                    File.WriteAllText(sourceFile, "This is another source temp file."); // HIGHCHAR: Test writes in UTF8 without preamble.
+                File.WriteAllText(sourceFile, "This is another source temp file."); // HIGHCHAR: Test writes in UTF8 without preamble.
 
-                    // Read the destination file (it should have the same modified content as the source)
-                    destinationFileContents = File.ReadAllText(destFile);
-                    Assert.Equal("This is another source temp file.", destinationFileContents); // "Expected the destination hard linked file to contain the contents of source file. Even after modification of the source"
+                // Read the destination file (it should have the same modified content as the source)
+                destinationFileContents = File.ReadAllText(destFile);
+                Assert.Equal("This is another source temp file.", destinationFileContents); // "Expected the destination hard linked file to contain the contents of source file. Even after modification of the source"
 
-                    ((MockEngine)t.BuildEngine).AssertLogDoesntContain("MSB3891"); // Didn't do retries
-                }
-                finally
-                {
-                    File.Delete(sourceFile);
-                    File.Delete(destFile);
-                    FileUtilities.DeleteWithoutTrailingBackslash(destFolder, true);
-                }
+                ((MockEngine)t.BuildEngine).AssertLogDoesntContain("MSB3891"); // Didn't do retries
+            }
+            finally
+            {
+                File.Delete(sourceFile);
+                File.Delete(destFile);
+                FileUtilities.DeleteWithoutTrailingBackslash(destFolder, true);
             }
         }
 

diff --git a/src/Tasks/NativeMethods.cs b/src/Tasks/NativeMethods.cs
@@ -517,7 +517,8 @@ internal struct PROCESS_INFORMATION
     internal enum SymbolicLink
     {
         File = 0,
-        Directory = 1
+        Directory = 1,
+        AllowUnprivilegedCreate = 2,
     }
 
     /// <summary>
@@ -833,7 +834,14 @@ internal static bool MakeSymbolicLink(string newFileName, string exitingFileName
             bool symbolicLinkCreated;
             if (NativeMethodsShared.IsWindows)
             {
-                symbolicLinkCreated = CreateSymbolicLink(newFileName, exitingFileName, SymbolicLink.File);
+                Version osVersion = Environment.OSVersion.Version;
+                SymbolicLink flags = SymbolicLink.File;
+                if (osVersion.Major >= 11 || (osVersion.Major == 10 && osVersion.Build >= 14972))
+                {
+                    flags |= SymbolicLink.AllowUnprivilegedCreate;
+                }
+
+                symbolicLinkCreated = CreateSymbolicLink(newFileName, exitingFileName, flags);
                 errorMessage = symbolicLinkCreated ? null : Marshal.GetExceptionForHR(Marshal.GetHRForLastWin32Error()).Message;
             }
             else