Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clone files on OSX-like platforms when possible, instead of copying the whole file #79243

Merged
merged 85 commits into from
Jun 13, 2023
Merged
Show file tree
Hide file tree
Changes from 70 commits
Commits
Show all changes
85 commits
Select commit Hold shift + click to select a range
a373c59
Initial implementation of fix for #77835 - cloning files on macOS
hamarb123 Dec 5, 2022
f21a5ae
Fix some build issues from previous commit for #77835
hamarb123 Dec 5, 2022
1890115
Fix some build issues from previous commit for #77835 again
hamarb123 Dec 5, 2022
e490d5e
Fix some build issues from previous commit for #77835 again (2)
hamarb123 Dec 5, 2022
1e43a91
Fix some build issues from previous commit for #77835 again (3)
hamarb123 Dec 5, 2022
4c5a995
Fix some build issues from previous commit for #77835 again (4)
hamarb123 Dec 5, 2022
c65e229
Fix some build issues from previous commit for #77835 again (5)
hamarb123 Dec 5, 2022
5377efb
Fix copying a file onto itself logic for #77835
hamarb123 Dec 6, 2022
8d8328a
Fix some nullability issues from previous commit for #77835
hamarb123 Dec 6, 2022
835fc2f
Fix some nullability issues from previous commit for #77835 (again)
hamarb123 Dec 6, 2022
05f9d26
Fix bug introduced in 'Fix copying a file onto itself logic for #77835'
hamarb123 Dec 6, 2022
5fe4cd2
Merge remote-tracking branch 'upstream'
hamarb123 Dec 8, 2022
28ef104
Merge branch 'main' of https://github.com/dotnet/runtime
hamarb123 Jan 24, 2023
21144e0
Add extra test
hamarb123 Jan 24, 2023
049627f
Use correct comment formatting
hamarb123 Jan 24, 2023
ab18c05
Use clonefile API instead
hamarb123 Jan 24, 2023
776f500
Remove redundant equality
hamarb123 Jan 24, 2023
dcddd8f
Clone ACLs and make CopyOntoLockedFile test conditional
hamarb123 Jan 30, 2023
ac27788
Remove retry logic
hamarb123 Jan 30, 2023
f320a70
Address feedback
hamarb123 May 14, 2023
3d4a962
Implement feedback to remove StartedCopyFileState
hamarb123 May 14, 2023
a908a79
Fix compilation issues by me thinking I was smart
hamarb123 May 14, 2023
16cedc6
Fix nullability
hamarb123 May 14, 2023
d15fed3
Implement feedback
hamarb123 May 16, 2023
617afbf
Merge remote-tracking branch 'upstream/main'
hamarb123 May 16, 2023
eb97585
Fix compilation issues
hamarb123 May 16, 2023
70c6e8b
Fix missed line in FileSystem.CopyFile.OSX.cs
hamarb123 May 16, 2023
f866a3d
Update src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.…
hamarb123 May 16, 2023
bf67649
Update src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.…
hamarb123 May 16, 2023
ff108db
Update src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.…
hamarb123 May 16, 2023
69071eb
Update src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.…
hamarb123 May 16, 2023
20c42f9
Replace var with actual type as per feedback
hamarb123 May 16, 2023
1280a8a
Update comment for clarity
hamarb123 May 16, 2023
f33ae9c
Implement feedback to remove OpenCopyFileDstHandle
hamarb123 May 16, 2023
068a8aa
Fix compilation errors
hamarb123 May 16, 2023
2c48aad
Remove some comments as per feedback
hamarb123 May 17, 2023
ef84010
Changes from feedback
hamarb123 May 17, 2023
8924afe
Remove redundant line as per feedback
hamarb123 May 17, 2023
1649d43
Remove old comment
hamarb123 May 17, 2023
a82767b
Implement some feedback
hamarb123 May 18, 2023
c0c7e33
Implement suggestion that cuts down on duplication
hamarb123 May 18, 2023
18b8c2e
Remove unnecessary using
hamarb123 May 18, 2023
00b68ed
Remove some other unnecessary usings
hamarb123 May 18, 2023
afa73ca
Fix filePermissions missing as per feedback
hamarb123 May 18, 2023
a684785
Fix compilation issues from files having wrong name in projitems
hamarb123 May 18, 2023
61c0576
Make the CopyFile implementation more similar to how it was before
hamarb123 May 18, 2023
7c6c80b
Fix copied code compilation issue
hamarb123 May 18, 2023
9cdb4b3
Remove filePermissions parameter as per feedback
hamarb123 May 23, 2023
2dc08e4
Remove isReadOnly as per feedback
hamarb123 May 23, 2023
02cfaf6
Fix whitespace
hamarb123 May 23, 2023
1fc02f7
Fix compilation errors
hamarb123 May 23, 2023
71bc96e
Fix more compilation issues
hamarb123 May 23, 2023
aa76f24
Try clonefile immediately as per feedback
hamarb123 May 23, 2023
3945a9f
Add error handling to it, instead of always falling back
hamarb123 May 23, 2023
3457dcc
Add missing cast
hamarb123 May 23, 2023
788216e
Remove unneeded filePermissions specification
hamarb123 May 23, 2023
233217a
Change indentation of delete section
hamarb123 May 23, 2023
20caacd
Temporarily comment out the stat section
hamarb123 May 23, 2023
6d40467
Remove some unneeded indentation
hamarb123 May 23, 2023
705aa4f
Remove special casing of EINVAL
hamarb123 May 23, 2023
a5de060
Remove section which does stat as it's unneeded
hamarb123 May 23, 2023
7221560
Improve some of the comments.
hamarb123 May 23, 2023
1db28fd
Add handler for case of EEXIST still
hamarb123 May 23, 2023
d360ae4
Remove partial method definitions
hamarb123 May 23, 2023
323cf67
Add some code to deal with EINVAL
hamarb123 May 23, 2023
44ef8f3
Remove srcStat parameter as it's no longer needed
hamarb123 May 23, 2023
50195af
Revert now-unnecessary changes to OpenReadOnly
hamarb123 May 23, 2023
53f52bc
Use Unlink instead of File.Delete
hamarb123 May 23, 2023
aee3ea8
Remove unnecessary diffs
hamarb123 May 23, 2023
c6174e6
Fix compilation errors
hamarb123 May 24, 2023
0838d9f
Update src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.…
hamarb123 May 24, 2023
013c775
Reorder flags check as per feedback
hamarb123 May 24, 2023
487aee9
Fix test failures
hamarb123 May 24, 2023
37b5183
Extract attempting clonefile to a separate function
hamarb123 May 24, 2023
bc99ec9
Use CreateOpenException when deleting the destination file
hamarb123 May 24, 2023
e1c7739
Use a debug statement to check EINVAL instead
hamarb123 May 24, 2023
aec1eac
Improve & fix comment
hamarb123 May 24, 2023
483858d
Fix compilation errors
hamarb123 May 24, 2023
f67bd18
Implement feedback
hamarb123 May 24, 2023
5cf74af
Move the Debug.Assert calls together
hamarb123 May 24, 2023
10011a5
Implement feedback to use partial functions
hamarb123 Jun 6, 2023
a793bb7
Implement feedback to use pattern matching, and move comments
hamarb123 Jun 6, 2023
550a76a
Implement other feedback
hamarb123 Jun 6, 2023
32c5463
Fix visibility of partial method
hamarb123 Jun 6, 2023
de964d0
Merge branch 'main' of https://github.com/dotnet/runtime
hamarb123 Jun 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/libraries/Common/src/Interop/OSX/Interop.libc.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,10 @@ internal static unsafe int fsetattrlist(SafeHandle handle, AttrList* attrList, v
handle.DangerousRelease();
}
}

[LibraryImport(Libraries.libc, EntryPoint = "clonefile", StringMarshalling = StringMarshalling.Utf8, SetLastError = true)]
internal static unsafe partial int clonefile(string src, string dst, int flags);

internal const int CLONE_ACL = 0x0004;
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
}
}
13 changes: 13 additions & 0 deletions src/libraries/System.IO.FileSystem/tests/File/Copy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,19 @@ public void WindowsAlternateDataStreamOverwrite(string defaultStream, string alt
Assert.Throws<IOException>(() => Copy(testFileAlternateStream, testFile2 + alternateStream, overwrite: true));
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingEnabled))]
public void CopyOntoLockedFile()
{
string testFileSource = GetTestFilePath();
string testFileDest = GetTestFilePath();
File.Create(testFileSource).Dispose();
File.Create(testFileDest).Dispose();
using (var stream = new FileStream(testFileDest, FileMode.Open, FileAccess.Read, FileShare.None))
{
Assert.Throws<IOException>(() => Copy(testFileSource, testFileDest, overwrite: true));
}
}

[Fact]
public void DestinationFileIsTruncatedWhenItsLargerThanSourceFile()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2373,6 +2373,7 @@
</ItemGroup>
<ItemGroup Condition="('$(TargetsUnix)' == 'true' or '$(TargetsBrowser)' == 'true' or '$(TargetsWasi)' == 'true') and '$(IsOSXLike)' != 'true'">
<Compile Include="$(MSBuildThisFileDirectory)System\IO\FileStatus.SetTimes.OtherUnix.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\IO\FileSystem.TryCloneFile.OtherUnix.cs" />
</ItemGroup>
<ItemGroup Condition="'$(TargetsUnix)' == 'true'">
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetEUid.cs" Link="Common\Interop\Unix\Interop.GetEUid.cs" />
Expand Down Expand Up @@ -2427,6 +2428,7 @@
<Link>Common\Interop\OSX\Interop.libc.cs</Link>
</Compile>
<Compile Include="$(MSBuildThisFileDirectory)System\IO\FileStatus.SetTimes.OSX.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\IO\FileSystem.TryCloneFile.OSX.cs" />
</ItemGroup>
<ItemGroup Condition="'$(IsiOSLike)' == 'true' or '$(IsOSXLike)' == 'true'">
<Compile Include="$(CommonPath)Interop\OSX\System.Native\Interop.SearchPath.cs">
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.Win32.SafeHandles;
using System.Diagnostics;

namespace System.IO
{
internal static partial class FileSystem
{
private static bool TryCloneFile(string sourceFullPath, string destFullPath, bool overwrite)
{
// Try to clone the file immediately, this will only succeed if the
// destination doesn't exist, so we don't worry about locking for this one.
int flags = Interop.@libc.CLONE_ACL;
retry1:
if (Interop.@libc.clonefile(sourceFullPath, destFullPath, flags) == 0)
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
{
// Success.
return true;
}
Interop.Error error = Interop.Sys.GetLastError();

// Some filesystems don't support ACLs, so may fail due to trying to copy ACLs.
// This will disable them and allow trying again (a maximum of 1 time).
if (flags != 0 && error == Interop.Error.EINVAL)
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
{
flags = 0;
goto retry1;
}

// Try to delete the destination file if we're overwriting.
if (error == Interop.Error.EEXIST && overwrite)
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
{
// Delete the destination. This should fail on directories. Get a lock to the dest file to ensure we don't copy onto it when
// it's locked by something else, and then delete it. It should also fail if destination == source since it's already locked.
danmoseley marked this conversation as resolved.
Show resolved Hide resolved
try
{
using SafeFileHandle? dstHandle = SafeFileHandle.Open(destFullPath, FileMode.Open, FileAccess.ReadWrite,
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
FileShare.None, FileOptions.None, preallocationSize: 0);
if (Interop.Sys.Unlink(destFullPath) < 0)
{
Interop.Error errorInfo = Interop.Sys.GetLastError();
if (error != Interop.Error.ENOENT)
{
// Fall back to standard copy as an unexpected error has occured.
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
return false;
}
}
}
catch (FileNotFoundException)
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
{
// We don't want to throw if it's just the file not existing, since we're trying to delete it.
}

// Try clonefile now we've deleted the destination file.
retry2:
if (Interop.@libc.clonefile(sourceFullPath, destFullPath, flags) == 0)
{
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
// Success.
return true;
}
error = Interop.Sys.GetLastError();
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved

// Some filesystems don't support ACLs, so may fail due to trying to copy ACLs.
// This will disable them and allow trying again (a maximum of 1 time).
if (flags != 0 && error == Interop.Error.EINVAL)
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
{
flags = 0;
goto retry2;
}
}

// Check if it's not supported, if they're on different filesystems, the destination file still exists, or if we have unsupported flags still.
// EINVAL is checked again here since clonefile is implemented by filesystem-specific drivers, therefore it could potentially fail with flags=0.
if (error == Interop.Error.ENOTSUP || error == Interop.Error.EXDEV || error == Interop.Error.EEXIST || error == Interop.Error.EINVAL)
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
{
// Fall back to normal copy.
return false;
}

// Throw the appropriate exception.
Debug.Assert(error != Interop.Error.SUCCESS); // We shouldn't fail with success.
throw Interop.GetExceptionForIoErrno(error.Info());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace System.IO
{
internal static partial class FileSystem
{
#pragma warning disable IDE0060 // Remove unused parameter
private static bool TryCloneFile(string sourceFullPath, string destFullPath, bool overwrite)
#pragma warning restore IDE0060 // Remove unused parameter
{
// No such functionality is available on unix OSes (other than OSX-like ones).
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ public static void CopyFile(string sourceFullPath, string destFullPath, bool ove
long fileLength;
UnixFileMode filePermissions;
using SafeFileHandle src = SafeFileHandle.OpenReadOnly(sourceFullPath, FileOptions.None, out fileLength, out filePermissions);

// Try to clone the file first.
if (TryCloneFile(sourceFullPath, destFullPath, overwrite))
{
return;
}

using SafeFileHandle dst = SafeFileHandle.Open(destFullPath, overwrite ? FileMode.Create : FileMode.CreateNew,
FileAccess.ReadWrite, FileShare.None, FileOptions.None, preallocationSize: 0, filePermissions,
CreateOpenException);
Expand Down