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

Fix setting creation date for OSX-like platforms #49555

Merged
merged 45 commits into from
Oct 20, 2021
Merged
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
10379ce
Fix setting creation date for OSX
hamarb123 Mar 12, 2021
2ade343
Fix setting creation date for OSX - fix for setattrlist
hamarb123 Mar 16, 2021
aea57ed
Fix setting creation date for OSX - fix for caching when set time & p…
hamarb123 Mar 17, 2021
be1774c
Fix setting creation date for OSX - changes for PR 1
hamarb123 Mar 18, 2021
e39f5a2
Fix setting creation date for OSX - changes for PR 2
hamarb123 Mar 18, 2021
69f7c09
Fix setting creation date for OSX - changes for PR 3
hamarb123 Mar 20, 2021
edaf35e
Fix setting creation date for OSX - fix test for Windows and browser …
hamarb123 Mar 21, 2021
4abaa29
Fix setting creation date for OSX - make modification time use normal…
hamarb123 Mar 21, 2021
b85f323
Fix setting creation date for OSX - hotfix: Remove ATTR_CMN_MODTIME
hamarb123 Mar 21, 2021
a617a01
Fix setting creation date for OSX - revert windows changes & simplify…
hamarb123 Mar 23, 2021
f84f10e
Fix setting creation date for OSX - hotfix: change to CULong for mars…
hamarb123 Mar 23, 2021
9ce96e5
Fix setting creation date for OSX - hotfix: change to CULong for mars…
hamarb123 Mar 23, 2021
54a6266
Fix setting creation date for OSX - consolidate unix nanosecond calcu…
hamarb123 Mar 24, 2021
c80ef95
Merge branch 'main' into main
hamarb123 May 9, 2021
50945f8
Use InvalidateCaches instead of Invalidate and EnsureCachesInitialize…
hamarb123 May 9, 2021
f325717
Fixes for reviews in PR #49555
hamarb123 May 17, 2021
19cbb7f
Hotfix for missing parameter
hamarb123 May 17, 2021
2b702e3
Fix naming
hamarb123 May 17, 2021
f1a303e
Revert changes re SettingUpdatesPropertiesAfterAnother (mainly) and r…
hamarb123 May 18, 2021
0bbfbed
Fix errors of #49555 due to missing variables
hamarb123 May 25, 2021
42ca7e1
Remove the parameters that were meant to be removed
hamarb123 May 25, 2021
b15ceeb
Merge remote-tracking branch 'upstream/main'
hamarb123 May 29, 2021
7440772
Finish merge
hamarb123 May 29, 2021
e550082
Remove duplicate file
hamarb123 May 30, 2021
17868b1
Add custom error handling for setattrlist
hamarb123 May 30, 2021
f842935
Remove symlink specific code from this PR
hamarb123 Sep 7, 2021
1ca19ff
Remove custom ENOTDIR handling, enable tests for other OSX-like platf…
hamarb123 Sep 9, 2021
347f750
Rename files according to review
hamarb123 Sep 9, 2021
9928e3e
Change names of functions to improve readability and refactor code
hamarb123 Sep 10, 2021
15e9fc9
Fix the wrongly named variable and add the missing 'unsafe' keyword
hamarb123 Sep 10, 2021
5d20106
Merge branch 'dotnet:main' into main
hamarb123 Sep 11, 2021
581a669
Better comments and minor improvements
hamarb123 Sep 15, 2021
0623c42
Add additional tests and improvements
hamarb123 Oct 9, 2021
1ac43cc
Fix typos
hamarb123 Oct 9, 2021
11b26d8
Merge remote-tracking branch 'upstream/main'
hamarb123 Oct 9, 2021
8190220
Revert to <TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetCoreApp…
hamarb123 Oct 11, 2021
8aa9685
Remove SettingOneTimeMaintainsOtherDefaultTimes and AllDefaultTimesAr…
hamarb123 Oct 11, 2021
d8ec4fb
Fix: setattrlist can only return 0 or -1
hamarb123 Oct 19, 2021
6931bd0
Merge remote-tracking branch 'upstream/main'
hamarb123 Oct 19, 2021
57a1229
Update code comment for SettingUpdatesPropertiesAfterAnother test
hamarb123 Oct 19, 2021
9c6ab2b
Update code comment for SettingUpdatesPropertiesAfterAnother test
hamarb123 Oct 19, 2021
1fdc325
Update comment for SettingUpdatesPropertiesAfterAnother for consistency
hamarb123 Oct 19, 2021
6aa1852
Fixes for compilation and update to comment
hamarb123 Oct 19, 2021
31db20b
Update FileStatus.SetTimes.OSX.cs
hamarb123 Oct 19, 2021
c7c11d2
Move comments and add explicit types to SettingUpdatesPropertiesAfter…
hamarb123 Oct 20, 2021
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
1 change: 1 addition & 0 deletions src/libraries/Common/src/Interop/OSX/Interop.Libraries.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ internal static partial class Libraries
internal const string SystemConfigurationLibrary = "/System/Library/Frameworks/SystemConfiguration.framework/SystemConfiguration";
internal const string AppleCryptoNative = "libSystem.Security.Cryptography.Native.Apple";
internal const string MsQuic = "libmsquic.dylib";
internal const string libc = "libc";
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
}
}
29 changes: 29 additions & 0 deletions src/libraries/Common/src/Interop/OSX/Interop.libc.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Runtime.InteropServices;

internal static partial class Interop
{
internal static partial class libc
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved
{
[StructLayout(LayoutKind.Sequential)]
internal struct AttrList
{
public ushort bitmapCount;
public ushort reserved;
public uint commonAttr;
public uint volAttr;
public uint dirAttr;
public uint fileAttr;
public uint forkAttr;

public const ushort ATTR_BIT_MAP_COUNT = 5;
public const uint ATTR_CMN_CRTIME = 0x00000200;
}

[DllImport(Libraries.libc, EntryPoint = "setattrlist", SetLastError = true)]
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
internal static unsafe extern int setattrlist(string path, AttrList* attrList, void* attrBuf, nint attrBufSize, CULong options);
}
}
57 changes: 57 additions & 0 deletions src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Linq;
using System.Threading;
using Xunit;

Expand Down Expand Up @@ -70,6 +71,62 @@ public void SettingUpdatesProperties()
});
}

[Fact]
[PlatformSpecific(~TestPlatforms.Browser)]
public void SettingUpdatesPropertiesAfterAnother()
{
// Browser is excluded as there is only 1 effective time store.
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved

T item = GetExistingItem();

// These linq calls make an IEnumerable of pairs of functions that are not identical
// (eg. not (creationtime, creationtime)), includes both orders as seperate entries
// as they it have different behavior in reverse order (of functions), in addition
// to the pairs of functions, there is a reverse bool that allows a test for both
// increasing and decreasing timestamps as to not limit the test unnecessarily.
// Only testing with utc because it would be hard to check if lastwrite utc was the
// same type of method as lastwrite local since their .Getter fields are different.
// This test is required as some apis change more dates then they're supposed to.
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
// There were issues while developing this PR with specific orders of changes, so
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
// this code should almost fully eliminate any possibilities of that in the future
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
// by having a proper test for it. Also, it should be noted that the combination
// (A, B, false) is not the same as (B, A, true).

// The order that these LINQ expression creates is (when all 3 are available):
// [0] = (creation, access, False), [1] = (creation, access, True), [2] = (creation, write, False),
// [3] = (creation, write, True), [4] = (access, creation, False), [5] = (access, creation, True),
// [6] = (access, write, False), [7] = (access, write, True), [8] = (write, creation, False),
// [9] = (write, creation, True), [10] = (write, access, False), [11] = (write, access, True)
// Or, when creation time setting is not available:
// [0] = (access, write, False), [1] = (access, write, True),
// [2] = (write, access, False), [3] = (write, access, True)

IEnumerable<TimeFunction> timeFunctionsUtc = TimeFunctions(requiresRoundtripping: true).Where((f) => f.Kind == DateTimeKind.Utc);
var booleanArray = new bool[] { false, true };
Assert.All(timeFunctionsUtc.SelectMany((x) => timeFunctionsUtc.SelectMany((y) => booleanArray.Select((reverse) => (x, y, reverse)))).Where((fs) => fs.x.Getter != fs.y.Getter), (functions) =>
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
{
var function1 = functions.x;
var function2 = functions.y;
var reverse = functions.reverse;
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved

// Checking that milliseconds are not dropped after setter.
DateTime dt1 = new DateTime(2002, 12, 1, 12, 3, 3, LowTemporalResolution ? 0 : 321, DateTimeKind.Utc);
DateTime dt2 = new DateTime(2001, 12, 1, 12, 3, 3, LowTemporalResolution ? 0 : 321, DateTimeKind.Utc);
DateTime dt3 = new DateTime(2000, 12, 1, 12, 3, 3, LowTemporalResolution ? 0 : 321, DateTimeKind.Utc);
if (reverse) //reverse the order of setting dates
{
(dt1, dt3) = (dt3, dt1);
}
function1.Setter(item, dt1);
function2.Setter(item, dt2);
function1.Setter(item, dt3);
DateTime result1 = function1.Getter(item);
DateTime result2 = function2.Getter(item);
Assert.Equal(dt3, result1);
Assert.Equal(dt2, result2);
});
}

[Fact]
public void CanGetAllTimesAfterCreation()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@

internal static class IOInputs
{
public static bool SupportsSettingCreationTime => OperatingSystem.IsWindows();
public static bool SupportsGettingCreationTime => OperatingSystem.IsWindows() || OperatingSystem.IsMacOS();
public static bool SupportsSettingCreationTime => PlatformDetection.IsWindows || PlatformDetection.IsOSXLike;
public static bool SupportsGettingCreationTime => PlatformDetection.IsWindows || PlatformDetection.IsOSXLike;

// Max path length (minus trailing \0). Unix values vary system to system; just using really long values here likely to be more than on the average system.
public static readonly int MaxPath = OperatingSystem.IsWindows() ? 259 : 10000;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2147,6 +2147,9 @@
<Compile Include="$(MSBuildThisFileDirectory)System\TimeZoneInfo.Unix.Android.cs" Condition="'$(TargetsAndroid)' == 'true'" />
<Compile Include="$(MSBuildThisFileDirectory)System\TimeZoneInfo.Unix.NonAndroid.cs" Condition="'$(TargetsAndroid)' != 'true'" />
</ItemGroup>
<ItemGroup Condition="('$(TargetsUnix)' == 'true' or '$(TargetsBrowser)' == 'true') and '$(IsOSXLike)' != 'true'">
<Compile Include="$(MSBuildThisFileDirectory)System\IO\FileStatus.SetTimes.OtherUnix.cs" />
</ItemGroup>
<ItemGroup Condition="'$(TargetsUnix)' == 'true'">
<Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.GetEUid.cs">
<Link>Common\Interop\Unix\System.Native\Interop.GetEUid.cs</Link>
Expand Down Expand Up @@ -2202,6 +2205,10 @@
<Compile Include="$(CommonPath)Interop\OSX\Interop.Libraries.cs">
<Link>Common\Interop\OSX\Interop.Libraries.cs</Link>
</Compile>
<Compile Include="$(CommonPath)Interop\OSX\Interop.libc.cs">
<Link>Common\Interop\OSX\Interop.libc.cs</Link>
</Compile>
<Compile Include="$(MSBuildThisFileDirectory)System\IO\FileStatus.SetTimes.OSX.cs" />
</ItemGroup>
<ItemGroup Condition="'$(TargetsMacCatalyst)' == 'true'">
<Compile Include="$(CommonPath)Interop\OSX\System.Native\Interop.iOSSupportVersion.cs"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.InteropServices;

namespace System.IO
{
internal partial struct FileStatus
{
internal void SetCreationTime(string path, DateTimeOffset time)
{
// Try to set the attribute on the file system entry using setattrlist,
// if we get ENOTSUP then it means that "The volume does not support
// setattrlist()", so we fall back to the method used on other unix
// platforms, otherwise we throw an error if we get one, or invalidate
// the cache if successful because otherwise it has invalid information.
// Note: the unix fallback implementation doesn't have a test as we are
// yet to determine which volume types it can fail on, so modify with
// great care.
long seconds = time.ToUnixTimeSeconds();
long nanoseconds = UnixTimeSecondsToNanoseconds(time, seconds);
Interop.Error error = SetCreationTimeCore(path, seconds, nanoseconds);

if (error == Interop.Error.SUCCESS)
{
InvalidateCaches();
}
else if (error == Interop.Error.ENOTSUP)
{
SetAccessOrWriteTimeCore(path, time, isAccessTime: false, checkCreationTime: false);
}
else
{
Interop.CheckIo(error, path, InitiallyDirectory);
}
}

private unsafe Interop.Error SetCreationTimeCore(string path, long seconds, long nanoseconds)
{
Interop.Sys.TimeSpec timeSpec = default;

timeSpec.TvSec = seconds;
timeSpec.TvNsec = nanoseconds;

Interop.libc.AttrList attrList = default;
attrList.bitmapCount = Interop.libc.AttrList.ATTR_BIT_MAP_COUNT;
attrList.commonAttr = Interop.libc.AttrList.ATTR_CMN_CRTIME;

Interop.Error error = (Interop.Error)Interop.libc.setattrlist(path, &attrList, &timeSpec, sizeof(Interop.Sys.TimeSpec), default(CULong));
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved

return error;
}

private void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime) =>
SetAccessOrWriteTimeCore(path, time, isAccessTime, checkCreationTime: true);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.InteropServices;

namespace System.IO
{
internal partial struct FileStatus
{
internal void SetCreationTime(string path, DateTimeOffset time) =>
SetLastWriteTime(path, time);

private void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime) =>
SetAccessOrWriteTimeCore(path, time, isAccessTime, checkCreationTime: false);

// This is not used on these platforms, but is needed for source compat
private Interop.Error SetCreationTimeCore(string path, long seconds, long nanoseconds) =>
throw new InvalidOperationException();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

namespace System.IO
{
internal struct FileStatus
internal partial struct FileStatus
{
private const int NanosecondsPerTick = 100;

Expand Down Expand Up @@ -245,20 +245,6 @@ internal DateTimeOffset GetCreationTime(ReadOnlySpan<char> path, bool continueOn
return UnixTimeToDateTimeOffset(_fileCache.CTime, _fileCache.CTimeNsec);
}

internal void SetCreationTime(string path, DateTimeOffset time)
{
// Unix provides APIs to update the last access time (atime) and last modification time (mtime).
// There is no API to update the CreationTime.
// Some platforms (e.g. Linux) don't store a creation time. On those platforms, the creation time
// is synthesized as the oldest of last status change time (ctime) and last modification time (mtime).
// We update the LastWriteTime (mtime).
// This triggers a metadata change for FileSystemWatcher NotifyFilters.CreationTime.
// Updating the mtime, causes the ctime to be set to 'now'. So, on platforms that don't store a
// CreationTime, GetCreationTime will return the value that was previously set (when that value
// wasn't in the future).
SetLastWriteTime(path, time);
}

internal DateTimeOffset GetLastAccessTime(ReadOnlySpan<char> path, bool continueOnError = false)
{
EnsureCachesInitialized(path, continueOnError);
Expand All @@ -284,20 +270,29 @@ private DateTimeOffset UnixTimeToDateTimeOffset(long seconds, long nanoseconds)
return DateTimeOffset.FromUnixTimeSeconds(seconds).AddTicks(nanoseconds / NanosecondsPerTick);
}

private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime)
private unsafe void SetAccessOrWriteTimeCore(string path, DateTimeOffset time, bool isAccessTime, bool checkCreationTime)
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
{
// This api is used to set creation time on non OSX platforms, and as a fallback for OSX platforms.
// The reason why we use it to set 'creation time' is the below comment:
// Unix provides APIs to update the last access time (atime) and last modification time (mtime).
// There is no API to update the CreationTime.
// Some platforms (e.g. Linux) don't store a creation time. On those platforms, the creation time
// is synthesized as the oldest of last status change time (ctime) and last modification time (mtime).
// We update the LastWriteTime (mtime).
// This triggers a metadata change for FileSystemWatcher NotifyFilters.CreationTime.
// Updating the mtime, causes the ctime to be set to 'now'. So, on platforms that don't store a
// CreationTime, GetCreationTime will return the value that was previously set (when that value
// wasn't in the future).

// force a refresh so that we have an up-to-date times for values not being overwritten
_initializedFileCache = -1;
InvalidateCaches();
EnsureCachesInitialized(path);

// we use utimes()/utimensat() to set the accessTime and writeTime
Interop.Sys.TimeSpec* buf = stackalloc Interop.Sys.TimeSpec[2];

long seconds = time.ToUnixTimeSeconds();

const long TicksPerMillisecond = 10000;
const long TicksPerSecond = TicksPerMillisecond * 1000;
long nanoseconds = (time.UtcDateTime.Ticks - DateTimeOffset.UnixEpoch.Ticks - seconds * TicksPerSecond) * NanosecondsPerTick;
long nanoseconds = UnixTimeSecondsToNanoseconds(time, seconds);

#if TARGET_BROWSER
buf[0].TvSec = seconds;
Expand All @@ -321,7 +316,28 @@ private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool
}
#endif
Interop.CheckIo(Interop.Sys.UTimensat(path, buf), path, InitiallyDirectory);
_initializedFileCache = -1;

// On OSX-like platforms, when the modification time is less than the creation time (including
// when the modification time is already less than but access time is being set), the creation
// time is set to the modification time due to the api we're currently using; this is not
// desirable behaviour since it is inconsistent with windows behaviour and is not logical to
// the programmer (ie. we'd have to document it), so these api calls revert the creation time
// when it shouldn't be set (since we're setting modification time and access time here).
// checkCreationTime is only true on OSX-like platforms.
// allowFallbackToLastWriteTime is ignored on non OSX-like platforms.
bool updateCreationTime = checkCreationTime && (_fileCache.Flags & Interop.Sys.FileStatusFlags.HasBirthTime) != 0 &&
(buf[1].TvSec < _fileCache.BirthTime || (buf[1].TvSec == _fileCache.BirthTime && buf[1].TvNsec < _fileCache.BirthTimeNsec));
jozkee marked this conversation as resolved.
Show resolved Hide resolved

InvalidateCaches();

if (updateCreationTime)
{
Interop.Error error = SetCreationTimeCore(path, _fileCache.BirthTime, _fileCache.BirthTimeNsec);
if (error != Interop.Error.SUCCESS && error != Interop.Error.ENOTSUP)
{
Interop.CheckIo(error, path, InitiallyDirectory);
}
}
}

internal long GetLength(ReadOnlySpan<char> path, bool continueOnError = false)
Expand Down Expand Up @@ -399,6 +415,13 @@ private void ThrowOnCacheInitializationError(ReadOnlySpan<char> path)
}
}

private static long UnixTimeSecondsToNanoseconds(DateTimeOffset time, long seconds)
{
const long TicksPerMillisecond = 10000;
const long TicksPerSecond = TicksPerMillisecond * 1000;
return (time.UtcDateTime.Ticks - DateTimeOffset.UnixEpoch.Ticks - seconds * TicksPerSecond) * NanosecondsPerTick;
}

private bool TryRefreshFileCache(ReadOnlySpan<char> path) =>
VerifyStatCall(Interop.Sys.LStat(path, out _fileCache), out _initializedFileCache);

Expand Down