From 10379ce35d02f79f501641612da0504d24e6194e Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Sat, 13 Mar 2021 08:12:18 +1100 Subject: [PATCH 01/40] Fix setting creation date for OSX Fix for #39132 --- .../src/Interop/OSX/Interop.Libraries.cs | 1 + .../Common/src/Interop/OSX/Interop.libc.cs | 32 +++++++++++++++ .../src/System.IO.FileSystem.csproj | 12 +++++- .../src/System/IO/FileStatus.OSX.cs | 39 +++++++++++++++++++ .../src/System/IO/FileStatus.OtherUnix.cs | 26 +++++++++++++ .../src/System/IO/FileStatus.Unix.cs | 20 ++-------- .../tests/PortedCommon/IOInputs.cs | 2 +- .../tests/System.IO.FileSystem.Tests.csproj | 2 +- 8 files changed, 114 insertions(+), 20 deletions(-) create mode 100644 src/libraries/Common/src/Interop/OSX/Interop.libc.cs create mode 100644 src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs create mode 100644 src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs diff --git a/src/libraries/Common/src/Interop/OSX/Interop.Libraries.cs b/src/libraries/Common/src/Interop/OSX/Interop.Libraries.cs index 1c2f75faed082..ba28f28cf22ce 100644 --- a/src/libraries/Common/src/Interop/OSX/Interop.Libraries.cs +++ b/src/libraries/Common/src/Interop/OSX/Interop.Libraries.cs @@ -17,5 +17,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"; } } diff --git a/src/libraries/Common/src/Interop/OSX/Interop.libc.cs b/src/libraries/Common/src/Interop/OSX/Interop.libc.cs new file mode 100644 index 0000000000000..6e0a38aa49615 --- /dev/null +++ b/src/libraries/Common/src/Interop/OSX/Interop.libc.cs @@ -0,0 +1,32 @@ +// 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 + { + [StructLayout(LayoutKind.Sequential)] + internal struct AttrList + { + [MarshalAs(UnmanagedType.U2)] public ushort bitmapCount; + [MarshalAs(UnmanagedType.U2)] public ushort reserved; + [MarshalAs(UnmanagedType.U4)] public uint commonAttr; + [MarshalAs(UnmanagedType.U4)] public uint volAttr; + [MarshalAs(UnmanagedType.U4)] public uint dirAttr; + [MarshalAs(UnmanagedType.U4)] public uint fileAttr; + [MarshalAs(UnmanagedType.U4)] public uint forkAttr; + + public const ushort ATTR_BIT_MAP_COUNT = 5; + public const uint ATTR_CMN_CRTIME = 0x00000200; + public const uint ATTR_CMN_MODTIME = 0x00000400; + } + + [DllImport(Libraries.libc, EntryPoint = "setattrlist", SetLastError = true)] + internal static unsafe extern int setattrlist(string path, AttrList* attrList, void* attrBuf, nint attrBufSize, uint options); + + internal const uint FSOPT_NOFOLLOW = 0x00000001; + } +} diff --git a/src/libraries/System.IO.FileSystem/src/System.IO.FileSystem.csproj b/src/libraries/System.IO.FileSystem/src/System.IO.FileSystem.csproj index a8ce9dc085800..da14ae7cbe7c4 100644 --- a/src/libraries/System.IO.FileSystem/src/System.IO.FileSystem.csproj +++ b/src/libraries/System.IO.FileSystem/src/System.IO.FileSystem.csproj @@ -2,7 +2,7 @@ true true - $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser + $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser;$(NetCoreAppCurrent)-OSX enable @@ -231,6 +231,16 @@ + + + + + + + + diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs new file mode 100644 index 0000000000000..9f1611481e718 --- /dev/null +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs @@ -0,0 +1,39 @@ +// 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) => SetTimeOnFile(path, time, Interop.libc.AttrList.ATTR_CMN_CRTIME); + + internal void SetLastWriteTime(string path, DateTimeOffset time) => SetTimeOnFile(path, time, Interop.libc.AttrList.ATTR_CMN_MODTIME); + + private unsafe void SetTimeOnFile(string path, DateTimeOffset time, uint commonAttr) + { + Interop.Sys.TimeSpec timeSpec = default; + + long seconds = time.ToUnixTimeSeconds(); + + const long TicksPerMillisecond = 10000; + const long TicksPerSecond = TicksPerMillisecond * 1000; + long nanoseconds = (time.UtcDateTime.Ticks - DateTimeOffset.UnixEpoch.Ticks - seconds * TicksPerSecond) * NanosecondsPerTick; + + timeSpec.TvSec = seconds; + timeSpec.TvNsec = nanoseconds; + + Interop.libc.AttrList attrList = default; + attrList.bitmapCount = Interop.libc.AttrList.ATTR_BIT_MAP_COUNT; + attrList.reserved = 0; + attrList.commonAttr = commonAttr; + attrList.dirAttr = 0; + attrList.fileAttr = 0; + attrList.forkAttr = 0; + attrList.volAttr = 0; + + Interop.libc.setattrlist(path, &attrList, &timeSpec, sizeof(Interop.Sys.TimeSpec), Interop.libc.FSOPT_NOFOLLOW); + } + } +} diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs new file mode 100644 index 0000000000000..5cadb86639055 --- /dev/null +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs @@ -0,0 +1,26 @@ +// 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) + { + // 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 void SetLastWriteTime(string path, DateTimeOffset time) => SetAccessOrWriteTime(path, time, isAccessTime: false); + } +} diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs index 45b0ed8b04f8d..462e0dcbf0bc7 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs @@ -5,7 +5,7 @@ namespace System.IO { - internal struct FileStatus + internal partial struct FileStatus { private const int NanosecondsPerTick = 100; @@ -186,20 +186,6 @@ internal DateTimeOffset GetCreationTime(ReadOnlySpan path, bool continueOn return UnixTimeToDateTimeOffset(_fileStatus.CTime, _fileStatus.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 path, bool continueOnError = false) { EnsureStatInitialized(path, continueOnError); @@ -218,8 +204,6 @@ internal DateTimeOffset GetLastWriteTime(ReadOnlySpan path, bool continueO return UnixTimeToDateTimeOffset(_fileStatus.MTime, _fileStatus.MTimeNsec); } - internal void SetLastWriteTime(string path, DateTimeOffset time) => SetAccessOrWriteTime(path, time, isAccessTime: false); - private DateTimeOffset UnixTimeToDateTimeOffset(long seconds, long nanoseconds) { return DateTimeOffset.FromUnixTimeSeconds(seconds).AddTicks(nanoseconds / NanosecondsPerTick).ToLocalTime(); @@ -227,6 +211,8 @@ private DateTimeOffset UnixTimeToDateTimeOffset(long seconds, long nanoseconds) private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime) { + //only used for access time on OSX, used for both on other unix platforms + // force a refresh so that we have an up-to-date times for values not being overwritten _fileStatusInitialized = -1; EnsureStatInitialized(path); diff --git a/src/libraries/System.IO.FileSystem/tests/PortedCommon/IOInputs.cs b/src/libraries/System.IO.FileSystem/tests/PortedCommon/IOInputs.cs index d6516795762ca..61feb6bb240e8 100644 --- a/src/libraries/System.IO.FileSystem/tests/PortedCommon/IOInputs.cs +++ b/src/libraries/System.IO.FileSystem/tests/PortedCommon/IOInputs.cs @@ -8,7 +8,7 @@ internal static class IOInputs { - public static bool SupportsSettingCreationTime => OperatingSystem.IsWindows(); + public static bool SupportsSettingCreationTime => OperatingSystem.IsWindows() || OperatingSystem.IsMacOS(); public static bool SupportsGettingCreationTime => OperatingSystem.IsWindows() || OperatingSystem.IsMacOS(); // 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. diff --git a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj index 7a81b2e7b1f3d..6b8b1467d328b 100644 --- a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj +++ b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj @@ -2,7 +2,7 @@ true true - $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser + $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser;$(NetCoreAppCurrent)-OSX --working-dir=/test-dir From 2ade34380edb19bc5769612e8b8994f1e82a40b0 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Tue, 16 Mar 2021 20:29:09 +1100 Subject: [PATCH 02/40] Fix setting creation date for OSX - fix for setattrlist This commit adds a fallback for setattrlist failing. It can sometimes fail on specific volume types, so this is a workaround to use the old behaviour on unsupported filesystems. Fix for #39132 --- .../src/System/IO/FileStatus.OSX.cs | 17 ++++++++++++++++- .../src/System/IO/FileStatus.OtherUnix.cs | 17 ++--------------- .../src/System/IO/FileStatus.Unix.cs | 18 +++++++++++++++++- 3 files changed, 35 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs index 9f1611481e718..98d1269815a94 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs @@ -33,7 +33,22 @@ private unsafe void SetTimeOnFile(string path, DateTimeOffset time, uint commonA attrList.forkAttr = 0; attrList.volAttr = 0; - Interop.libc.setattrlist(path, &attrList, &timeSpec, sizeof(Interop.Sys.TimeSpec), Interop.libc.FSOPT_NOFOLLOW); + // Try to set the attribute on the file system entry using setattrlist, + // otherwise fall back to the method used on other unix platforms as the + // path may either be on an unsupported volume type (for setattrlist) or it + // could be another error (eg. no file) which the fallback implementation can throw. + bool succeeded = Interop.libc.setattrlist(path, &attrList, &timeSpec, sizeof(Interop.Sys.TimeSpec), Interop.libc.FSOPT_NOFOLLOW) == 0; + if (!succeeded) + { + if (commonAttr == Interop.libc.AttrList.ATTR_CMN_CRTIME) + { + SetCreationTime_OtherUnix(path, time); + } + else if (commonAttr == Interop.libc.AttrList.ATTR_CMN_MODTIME) + { + SetLastWriteTime_OtherUnix(path, time); + } + } } } } diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs index 5cadb86639055..8329b2a571cf8 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs @@ -7,20 +7,7 @@ namespace System.IO { internal partial struct FileStatus { - 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 void SetLastWriteTime(string path, DateTimeOffset time) => SetAccessOrWriteTime(path, time, isAccessTime: false); + internal void SetCreationTime(string path, DateTimeOffset time) => SetCreationTime_OtherUnix(path, time); + internal void SetLastWriteTime(string path, DateTimeOffset time) => SetLastWriteTime_OtherUnix(path, time); } } diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs index 462e0dcbf0bc7..46856ca0ad9ab 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs @@ -186,6 +186,20 @@ internal DateTimeOffset GetCreationTime(ReadOnlySpan path, bool continueOn return UnixTimeToDateTimeOffset(_fileStatus.CTime, _fileStatus.CTimeNsec); } + private void SetCreationTime_OtherUnix(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 path, bool continueOnError = false) { EnsureStatInitialized(path, continueOnError); @@ -204,6 +218,8 @@ internal DateTimeOffset GetLastWriteTime(ReadOnlySpan path, bool continueO return UnixTimeToDateTimeOffset(_fileStatus.MTime, _fileStatus.MTimeNsec); } + private void SetLastWriteTime_OtherUnix(string path, DateTimeOffset time) => SetAccessOrWriteTime(path, time, isAccessTime: false); + private DateTimeOffset UnixTimeToDateTimeOffset(long seconds, long nanoseconds) { return DateTimeOffset.FromUnixTimeSeconds(seconds).AddTicks(nanoseconds / NanosecondsPerTick).ToLocalTime(); @@ -211,7 +227,7 @@ private DateTimeOffset UnixTimeToDateTimeOffset(long seconds, long nanoseconds) private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime) { - //only used for access time on OSX, used for both on other unix platforms + //only used for access time on OSX (unless being used as a fallback), used for both on other unix platforms // force a refresh so that we have an up-to-date times for values not being overwritten _fileStatusInitialized = -1; From aea57ed701014f02355632e6987b62649bb76009 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Wed, 17 Mar 2021 20:15:46 +1100 Subject: [PATCH 03/40] Fix setting creation date for OSX - fix for caching when set time & possible stack overflow This commit adds _fileStatusInitialized = -1; which resets the cache for the file status after setting a time value to the file using the setattrlist api version so that it will not simply return the last cached time. It also fixes SetCreationTime_OtherUnix method so that it calls SetLastWriteTime_OtherUnix directly instead of SetLastWriteTime because otherwise you'll get stuck in an infinite loop on OSX when the fallback is theoretically needed. Fix for #39132 --- .../System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs | 3 ++- .../System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs index 98d1269815a94..5d7e6f77b976a 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs @@ -8,7 +8,6 @@ namespace System.IO internal partial struct FileStatus { internal void SetCreationTime(string path, DateTimeOffset time) => SetTimeOnFile(path, time, Interop.libc.AttrList.ATTR_CMN_CRTIME); - internal void SetLastWriteTime(string path, DateTimeOffset time) => SetTimeOnFile(path, time, Interop.libc.AttrList.ATTR_CMN_MODTIME); private unsafe void SetTimeOnFile(string path, DateTimeOffset time, uint commonAttr) @@ -49,6 +48,8 @@ private unsafe void SetTimeOnFile(string path, DateTimeOffset time, uint commonA SetLastWriteTime_OtherUnix(path, time); } } + + _fileStatusInitialized = -1; } } } diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs index 46856ca0ad9ab..01da0609c84ad 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs @@ -197,7 +197,7 @@ private void SetCreationTime_OtherUnix(string path, DateTimeOffset time) // 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); + SetLastWriteTime_OtherUnix(path, time); } internal DateTimeOffset GetLastAccessTime(ReadOnlySpan path, bool continueOnError = false) From be1774c778c3eb38b23c3da1bbc1a7821022f653 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Thu, 18 Mar 2021 18:54:38 +1100 Subject: [PATCH 04/40] Fix setting creation date for OSX - changes for PR 1 Fix the behaviour of SetLastWriteTime (OSX) when setting lastwritetime before creationtime. Use Invalidate() instead of _fileStatusInitialized = -1, and only when appropriate as per PR #49555. Add SettingUpdatesPropertiesAfterAnother test to test things such as setting lastwritetime to before creationtime. Rename the added _OtherUnix methods to _StandardUnixImpl, a more logical name given their purpose (#49555) Fix for #39132 and issues in #49555. --- .../src/System/IO/FileStatus.OSX.cs | 18 ++++++--- .../src/System/IO/FileStatus.OtherUnix.cs | 4 +- .../src/System/IO/FileStatus.Unix.cs | 6 +-- .../tests/Base/BaseGetSetTimes.cs | 38 +++++++++++++++++++ 4 files changed, 56 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs index 5d7e6f77b976a..052932afafbd6 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs @@ -8,7 +8,13 @@ namespace System.IO internal partial struct FileStatus { internal void SetCreationTime(string path, DateTimeOffset time) => SetTimeOnFile(path, time, Interop.libc.AttrList.ATTR_CMN_CRTIME); - internal void SetLastWriteTime(string path, DateTimeOffset time) => SetTimeOnFile(path, time, Interop.libc.AttrList.ATTR_CMN_MODTIME); + + internal void SetLastWriteTime(string path, DateTimeOffset time) + { + var creationTime = GetCreationTime(path); + SetTimeOnFile(path, time, Interop.libc.AttrList.ATTR_CMN_MODTIME); + if (time < creationTime) SetCreationTime(path, creationTime); + } private unsafe void SetTimeOnFile(string path, DateTimeOffset time, uint commonAttr) { @@ -41,15 +47,17 @@ private unsafe void SetTimeOnFile(string path, DateTimeOffset time, uint commonA { if (commonAttr == Interop.libc.AttrList.ATTR_CMN_CRTIME) { - SetCreationTime_OtherUnix(path, time); + SetCreationTime_StandardUnixImpl(path, time); } else if (commonAttr == Interop.libc.AttrList.ATTR_CMN_MODTIME) { - SetLastWriteTime_OtherUnix(path, time); + SetLastWriteTime_StandardUnixImpl(path, time); } } - - _fileStatusInitialized = -1; + else + { + Invalidate(); + } } } } diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs index 8329b2a571cf8..c9699086e532d 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs @@ -7,7 +7,7 @@ namespace System.IO { internal partial struct FileStatus { - internal void SetCreationTime(string path, DateTimeOffset time) => SetCreationTime_OtherUnix(path, time); - internal void SetLastWriteTime(string path, DateTimeOffset time) => SetLastWriteTime_OtherUnix(path, time); + internal void SetCreationTime(string path, DateTimeOffset time) => SetCreationTime_StandardUnixImpl(path, time); + internal void SetLastWriteTime(string path, DateTimeOffset time) => SetLastWriteTime_StandardUnixImpl(path, time); } } diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs index 01da0609c84ad..c5155547ad6a5 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs @@ -186,7 +186,7 @@ internal DateTimeOffset GetCreationTime(ReadOnlySpan path, bool continueOn return UnixTimeToDateTimeOffset(_fileStatus.CTime, _fileStatus.CTimeNsec); } - private void SetCreationTime_OtherUnix(string path, DateTimeOffset time) + private void SetCreationTime_StandardUnixImpl(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. @@ -197,7 +197,7 @@ private void SetCreationTime_OtherUnix(string path, DateTimeOffset time) // 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_OtherUnix(path, time); + SetLastWriteTime_StandardUnixImpl(path, time); } internal DateTimeOffset GetLastAccessTime(ReadOnlySpan path, bool continueOnError = false) @@ -218,7 +218,7 @@ internal DateTimeOffset GetLastWriteTime(ReadOnlySpan path, bool continueO return UnixTimeToDateTimeOffset(_fileStatus.MTime, _fileStatus.MTimeNsec); } - private void SetLastWriteTime_OtherUnix(string path, DateTimeOffset time) => SetAccessOrWriteTime(path, time, isAccessTime: false); + private void SetLastWriteTime_StandardUnixImpl(string path, DateTimeOffset time) => SetAccessOrWriteTime(path, time, isAccessTime: false); private DateTimeOffset UnixTimeToDateTimeOffset(long seconds, long nanoseconds) { diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index a959e317682f4..337ec83631153 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -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; @@ -70,6 +71,43 @@ public void SettingUpdatesProperties() }); } + [Fact] + public void SettingUpdatesPropertiesAfterAnother() + { + 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. + var 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) => + { + // Checking that milliseconds are not dropped after setter. + // Emscripten drops milliseconds in Browser + 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 (functions.reverse) + { + var swap = dt3; + dt3 = dt1; + dt1 = swap; + } + functions.x.Setter(item, dt1); + functions.y.Setter(item, dt2); + functions.x.Setter(item, dt3); + DateTime result_x = functions.x.Getter(item); + DateTime result_y = functions.y.Getter(item); + Assert.Equal(dt3, result_x); + Assert.Equal(dt2, result_y); + }); + } + [Fact] public void CanGetAllTimesAfterCreation() { From e39f5a262a369fcacb91247dabeb0f8c71731c60 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Fri, 19 Mar 2021 05:50:55 +1100 Subject: [PATCH 05/40] Fix setting creation date for OSX - changes for PR 2 Added a new test to test the behaviour of the file time functions on symlinks as per standard Windows behaviour, and also what I use in my code today. It makes sure that the times are set on the symlink itself, not the target. It is likely to fail on many unix platforms, so these will be given the [ActiveIssue] attribute when we discover which ones. Make access time be set using setattrlist on OSX to fix following symlinks issue that would've failed the new test. Fix and update wording of comment in SetAccessOrWriteTime. Add T CreateSymlinkToItem(T item) to BaseGetSetTimes (and implementation in inheritors) for the new test. Made the SettingUpdatesPropertiesAfterAnother test skip on browser since it only, in effect, has one date attribute. Fix for #39132 and issues in #49555. --- .../Common/src/Interop/OSX/Interop.libc.cs | 1 + .../src/System/IO/FileStatus.OSX.cs | 6 ++++ .../src/System/IO/FileStatus.OtherUnix.cs | 1 + .../src/System/IO/FileStatus.Unix.cs | 4 +-- .../tests/Base/BaseGetSetTimes.cs | 36 ++++++++++++++++++- .../tests/Directory/GetSetTimes.cs | 8 +++++ .../tests/DirectoryInfo/GetSetTimes.cs | 10 ++++++ .../tests/File/GetSetTimes.cs | 8 +++++ .../tests/FileInfo/GetSetTimes.cs | 10 ++++++ 9 files changed, 81 insertions(+), 3 deletions(-) diff --git a/src/libraries/Common/src/Interop/OSX/Interop.libc.cs b/src/libraries/Common/src/Interop/OSX/Interop.libc.cs index 6e0a38aa49615..4292056e58e86 100644 --- a/src/libraries/Common/src/Interop/OSX/Interop.libc.cs +++ b/src/libraries/Common/src/Interop/OSX/Interop.libc.cs @@ -22,6 +22,7 @@ internal struct AttrList public const ushort ATTR_BIT_MAP_COUNT = 5; public const uint ATTR_CMN_CRTIME = 0x00000200; public const uint ATTR_CMN_MODTIME = 0x00000400; + public const uint ATTR_CMN_ACCTIME = 0x00001000; } [DllImport(Libraries.libc, EntryPoint = "setattrlist", SetLastError = true)] diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs index 052932afafbd6..a1650fe14e7ab 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs @@ -16,6 +16,8 @@ internal void SetLastWriteTime(string path, DateTimeOffset time) if (time < creationTime) SetCreationTime(path, creationTime); } + internal void SetLastAccessTime(string path, DateTimeOffset time) => SetTimeOnFile(path, time, Interop.libc.AttrList.ATTR_CMN_ACCTIME); + private unsafe void SetTimeOnFile(string path, DateTimeOffset time, uint commonAttr) { Interop.Sys.TimeSpec timeSpec = default; @@ -53,6 +55,10 @@ private unsafe void SetTimeOnFile(string path, DateTimeOffset time, uint commonA { SetLastWriteTime_StandardUnixImpl(path, time); } + else if (commonAttr == Interop.libc.AttrList.ATTR_CMN_ACCTIME) + { + SetLastAccessTime_StandardUnixImpl(path, time); + } } else { diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs index c9699086e532d..64cb0c988970f 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs @@ -9,5 +9,6 @@ internal partial struct FileStatus { internal void SetCreationTime(string path, DateTimeOffset time) => SetCreationTime_StandardUnixImpl(path, time); internal void SetLastWriteTime(string path, DateTimeOffset time) => SetLastWriteTime_StandardUnixImpl(path, time); + internal void SetLastAccessTime(string path, DateTimeOffset time) => SetLastAccessTime_StandardUnixImpl(path, time); } } diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs index c5155547ad6a5..5d5bc0dc854e2 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs @@ -208,7 +208,7 @@ internal DateTimeOffset GetLastAccessTime(ReadOnlySpan path, bool continue return UnixTimeToDateTimeOffset(_fileStatus.ATime, _fileStatus.ATimeNsec); } - internal void SetLastAccessTime(string path, DateTimeOffset time) => SetAccessOrWriteTime(path, time, isAccessTime: true); + private void SetLastAccessTime_StandardUnixImpl(string path, DateTimeOffset time) => SetAccessOrWriteTime(path, time, isAccessTime: true); internal DateTimeOffset GetLastWriteTime(ReadOnlySpan path, bool continueOnError = false) { @@ -227,7 +227,7 @@ private DateTimeOffset UnixTimeToDateTimeOffset(long seconds, long nanoseconds) private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime) { - //only used for access time on OSX (unless being used as a fallback), used for both on other unix platforms + // Only used as a fallback on OSX, used always on other Unix platforms. // force a refresh so that we have an up-to-date times for values not being overwritten _fileStatusInitialized = -1; diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index 337ec83631153..694d9f59ca5e7 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -23,6 +23,7 @@ public abstract class BaseGetSetTimes : FileSystemTest protected abstract T GetExistingItem(); protected abstract T GetMissingItem(); + protected abstract T CreateSymlinkToItem(T item); protected abstract string GetItemPath(T item); @@ -72,11 +73,12 @@ public void SettingUpdatesProperties() } [Fact] + [PlatformSpecific(~TestPlatforms.Browser)] public void SettingUpdatesPropertiesAfterAnother() { T item = GetExistingItem(); - // these linq calls make an IEnumerable of pairs of functions that are not identical + // 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 @@ -108,6 +110,38 @@ public void SettingUpdatesPropertiesAfterAnother() }); } + [Fact] + public void SettingUpdatesPropertiesOnSymlink() + { + // This test is currently consistent with the Windows behavior of setting the + // times on the symlink itself. It is needed as on OSX for example, the default + // for most APIs is to follow the symlink to completion and set the time on + // that entry instead (eg. the setattrlist will do this without the flag set). + T item = CreateSymlinkToItem(GetExistingItem()); + + Assert.All(TimeFunctions(requiresRoundtripping: true), (function) => + { + // Checking that milliseconds are not dropped after setter. + // Emscripten drops milliseconds in Browser + DateTime dt = new DateTime(2004, 12, 1, 12, 3, 3, LowTemporalResolution ? 0 : 321, function.Kind); + function.Setter(item, dt); + DateTime result = function.Getter(item); + Assert.Equal(dt, result); + Assert.Equal(dt.ToLocalTime(), result.ToLocalTime()); + + // File and Directory UTC APIs treat a DateTimeKind.Unspecified as UTC whereas + // ToUniversalTime treats it as local. + if (function.Kind == DateTimeKind.Unspecified) + { + Assert.Equal(dt, result.ToUniversalTime()); + } + else + { + Assert.Equal(dt.ToUniversalTime(), result.ToUniversalTime()); + } + }); + } + [Fact] public void CanGetAllTimesAfterCreation() { diff --git a/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs index 0a23d638c138a..7531ccae83019 100644 --- a/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs @@ -51,5 +51,13 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi ((path) => Directory.GetLastWriteTimeUtc(path)), DateTimeKind.Utc); } + + protected override string CreateSymlinkToItem(string item) + { + var link = item + ".link"; + if (Directory.Exists(link)) Directory.Delete(link); + if (!MountHelper.CreateSymbolicLink(link, item, true) || !Directory.Exists(link)) throw new Exception("Could not create symlink."); + return link; + } } } diff --git a/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs index 2dad268a79a46..cce3d1853c1d0 100644 --- a/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs @@ -57,5 +57,15 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi ((testDir) => testDir.LastWriteTimeUtc), DateTimeKind.Utc); } + + protected override DirectoryInfo CreateSymlinkToItem(DirectoryInfo item) + { + var link = new DirectoryInfo(item.FullName + ".link"); + if (link.Exists) link.Delete(); + bool failed = !MountHelper.CreateSymbolicLink(link.FullName, item.FullName, true); + link.Refresh(); + if (failed || !link.Exists) throw new Exception("Could not create symlink."); + return link; + } } } diff --git a/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs index 67a13bcf7df2b..551a5115106b1 100644 --- a/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs @@ -103,6 +103,14 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi DateTimeKind.Utc); } + protected override string CreateSymlinkToItem(string item) + { + var link = item + ".link"; + if (File.Exists(link)) File.Delete(link); + if (!MountHelper.CreateSymbolicLink(link, item, false) || !File.Exists(link)) throw new Exception("Could not create symlink."); + return link; + } + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotInAppContainer))] // Can't read root in appcontainer [PlatformSpecific(TestPlatforms.Windows)] public void PageFileHasTimes() diff --git a/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs index d3b9764951cbf..7abf30a1a20e1 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs @@ -103,6 +103,16 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi DateTimeKind.Utc); } + protected override FileInfo CreateSymlinkToItem(FileInfo item) + { + var link = new FileInfo(item.FullName + ".link"); + if (link.Exists) link.Delete(); + bool failed = !MountHelper.CreateSymbolicLink(link.FullName, item.FullName, false); + link.Refresh(); + if (failed || !link.Exists) throw new Exception("Could not create symlink."); + return link; + } + [ConditionalFact(nameof(HighTemporalResolution))] public void CopyToMillisecondPresent() { From 69f7c098cd474620dabc8c4bb299fdda8b65ab07 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Sat, 20 Mar 2021 19:00:03 +1100 Subject: [PATCH 06/40] Fix setting creation date for OSX - changes for PR 3 Revert change in last commit: Make access time be set using setattrlist on OSX to fix following symlinks issue that would've failed the new test. It is now using the other API as otherwise some file watcher tests fail. Set AT_SYMLINK_NOFOLLOW in pal_time.c to not follow symlinks for access time on OSX, or any times on other unix systems. --- src/libraries/Common/src/Interop/OSX/Interop.libc.cs | 1 - src/libraries/Native/Unix/System.Native/pal_time.c | 2 +- .../System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs | 6 ------ .../src/System/IO/FileStatus.OtherUnix.cs | 1 - .../System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs | 4 ++-- 5 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/libraries/Common/src/Interop/OSX/Interop.libc.cs b/src/libraries/Common/src/Interop/OSX/Interop.libc.cs index 4292056e58e86..6e0a38aa49615 100644 --- a/src/libraries/Common/src/Interop/OSX/Interop.libc.cs +++ b/src/libraries/Common/src/Interop/OSX/Interop.libc.cs @@ -22,7 +22,6 @@ internal struct AttrList public const ushort ATTR_BIT_MAP_COUNT = 5; public const uint ATTR_CMN_CRTIME = 0x00000200; public const uint ATTR_CMN_MODTIME = 0x00000400; - public const uint ATTR_CMN_ACCTIME = 0x00001000; } [DllImport(Libraries.libc, EntryPoint = "setattrlist", SetLastError = true)] diff --git a/src/libraries/Native/Unix/System.Native/pal_time.c b/src/libraries/Native/Unix/System.Native/pal_time.c index 56e23138a7b77..ef8a27a175c0f 100644 --- a/src/libraries/Native/Unix/System.Native/pal_time.c +++ b/src/libraries/Native/Unix/System.Native/pal_time.c @@ -33,7 +33,7 @@ int32_t SystemNative_UTimensat(const char* path, TimeSpec* times) updatedTimes[1].tv_sec = (time_t)times[1].tv_sec; updatedTimes[1].tv_nsec = (long)times[1].tv_nsec; - while (CheckInterrupted(result = utimensat(AT_FDCWD, path, updatedTimes, 0))); + while (CheckInterrupted(result = utimensat(AT_FDCWD, path, updatedTimes, AT_SYMLINK_NOFOLLOW))); #else struct timeval updatedTimes[2]; updatedTimes[0].tv_sec = (long)times[0].tv_sec; diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs index a1650fe14e7ab..052932afafbd6 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs @@ -16,8 +16,6 @@ internal void SetLastWriteTime(string path, DateTimeOffset time) if (time < creationTime) SetCreationTime(path, creationTime); } - internal void SetLastAccessTime(string path, DateTimeOffset time) => SetTimeOnFile(path, time, Interop.libc.AttrList.ATTR_CMN_ACCTIME); - private unsafe void SetTimeOnFile(string path, DateTimeOffset time, uint commonAttr) { Interop.Sys.TimeSpec timeSpec = default; @@ -55,10 +53,6 @@ private unsafe void SetTimeOnFile(string path, DateTimeOffset time, uint commonA { SetLastWriteTime_StandardUnixImpl(path, time); } - else if (commonAttr == Interop.libc.AttrList.ATTR_CMN_ACCTIME) - { - SetLastAccessTime_StandardUnixImpl(path, time); - } } else { diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs index 64cb0c988970f..c9699086e532d 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs @@ -9,6 +9,5 @@ internal partial struct FileStatus { internal void SetCreationTime(string path, DateTimeOffset time) => SetCreationTime_StandardUnixImpl(path, time); internal void SetLastWriteTime(string path, DateTimeOffset time) => SetLastWriteTime_StandardUnixImpl(path, time); - internal void SetLastAccessTime(string path, DateTimeOffset time) => SetLastAccessTime_StandardUnixImpl(path, time); } } diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs index 5d5bc0dc854e2..4541f9ce65c1e 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs @@ -197,7 +197,7 @@ private void SetCreationTime_StandardUnixImpl(string path, DateTimeOffset time) // 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_StandardUnixImpl(path, time); + SetLastWriteTime(path, time); } internal DateTimeOffset GetLastAccessTime(ReadOnlySpan path, bool continueOnError = false) @@ -208,7 +208,7 @@ internal DateTimeOffset GetLastAccessTime(ReadOnlySpan path, bool continue return UnixTimeToDateTimeOffset(_fileStatus.ATime, _fileStatus.ATimeNsec); } - private void SetLastAccessTime_StandardUnixImpl(string path, DateTimeOffset time) => SetAccessOrWriteTime(path, time, isAccessTime: true); + internal void SetLastAccessTime(string path, DateTimeOffset time) => SetAccessOrWriteTime(path, time, isAccessTime: true); internal DateTimeOffset GetLastWriteTime(ReadOnlySpan path, bool continueOnError = false) { From edaf35e05ae1e752302159a8512773532830d2ee Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Mon, 22 Mar 2021 06:57:25 +1100 Subject: [PATCH 07/40] Fix setting creation date for OSX - fix test for Windows and browser + update comments Fix the SettingUpdatesPropertiesOnSymlink test for Windows by setting FILE_FLAG_OPEN_REPARSE_POINT in the CreateFile used for setting times. Disable the SettingUpdatesPropertiesOnSymlink test for Browser as it can't use symlinks with the current API in the test project. Update comments. Add FILE_FLAG_OPEN_REPARSE_POINT. Fix for #39132 and issues in #49555. --- .../Interop/Windows/Kernel32/Interop.FileOperations.cs | 1 + .../src/System/IO/FileStatus.Unix.cs | 2 +- .../src/System/IO/FileSystem.Windows.cs | 2 +- .../System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs | 9 +++++---- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FileOperations.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FileOperations.cs index 8a2724dc2eddf..f8bf953f4c68a 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FileOperations.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FileOperations.cs @@ -19,6 +19,7 @@ internal partial class FileOperations internal const int FILE_FLAG_BACKUP_SEMANTICS = 0x02000000; internal const int FILE_FLAG_FIRST_PIPE_INSTANCE = 0x00080000; internal const int FILE_FLAG_OVERLAPPED = 0x40000000; + internal const int FILE_FLAG_OPEN_REPARSE_POINT = 0x00200000; internal const int FILE_LIST_DIRECTORY = 0x0001; } diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs index 4541f9ce65c1e..b2c4cd56b3c0c 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs @@ -227,7 +227,7 @@ private DateTimeOffset UnixTimeToDateTimeOffset(long seconds, long nanoseconds) private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime) { - // Only used as a fallback on OSX, used always on other Unix platforms. + // Used for access time or as a fallback on OSX, used always on other Unix platforms. // force a refresh so that we have an up-to-date times for values not being overwritten _fileStatusInitialized = -1; diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Windows.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Windows.cs index 5b5f9a86bc744..c83b8463ae7c5 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Windows.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Windows.cs @@ -158,7 +158,7 @@ private static SafeFileHandle OpenHandle(string fullPath, bool asDirectory) Interop.Kernel32.GenericOperations.GENERIC_WRITE, FileShare.ReadWrite | FileShare.Delete, FileMode.Open, - asDirectory ? Interop.Kernel32.FileOperations.FILE_FLAG_BACKUP_SEMANTICS : 0); + (asDirectory ? Interop.Kernel32.FileOperations.FILE_FLAG_BACKUP_SEMANTICS : 0) | Interop.Kernel32.FileOperations.FILE_FLAG_OPEN_REPARSE_POINT); if (handle.IsInvalid) { diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index 694d9f59ca5e7..5eec35a523b21 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -111,12 +111,13 @@ public void SettingUpdatesPropertiesAfterAnother() } [Fact] + [PlatformSpecific(~TestPlatforms.Browser)] public void SettingUpdatesPropertiesOnSymlink() { - // This test is currently consistent with the Windows behavior of setting the - // times on the symlink itself. It is needed as on OSX for example, the default - // for most APIs is to follow the symlink to completion and set the time on - // that entry instead (eg. the setattrlist will do this without the flag set). + // This test makes sure that the times are set on the symlink itself. + // It is needed as on OSX for example, the default for most APIs is + // to follow the symlink to completion and set the time on that entry + // instead (eg. the setattrlist will do this without the flag set). T item = CreateSymlinkToItem(GetExistingItem()); Assert.All(TimeFunctions(requiresRoundtripping: true), (function) => From 4abaa29593bb80f9c137806cd3e49eb7bcff48a3 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Mon, 22 Mar 2021 07:43:09 +1100 Subject: [PATCH 08/40] Fix setting creation date for OSX - make modification time use normal unix APIs Only implement creation time setting with setattrlist. Here is why: eg. the old code doesn't account for setting creation time to 2002, then setting modification time to 2001, and then setting access time (since access time didn't have the creation date check). I've split up SetAccessOrWriteTime into 3 parts - SetAccessOrWriteTime, SetAccessOrWriteTime_StandardUnixImpl, and SetAccessOrWriteTimeImpl (this last one contains the logic of the method without the things like refreshing the times so it's not called twice on OSX). In this process I replaced the _fileStatusInitialized = -1 calls with Invalidate() calls. And to make sure that if the creation time is needed to be set by SetAccessOrWriteTime, then we don't get in an infinite loop in case setattrlist fails, so we call SetAccessOrWriteTime_StandardUnixImpl. --- .../src/System/IO/FileStatus.OSX.cs | 45 +++++++++++-------- .../src/System/IO/FileStatus.OtherUnix.cs | 2 +- .../src/System/IO/FileStatus.Unix.cs | 14 +++--- 3 files changed, 36 insertions(+), 25 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs index 052932afafbd6..7084e5794712d 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs @@ -7,16 +7,7 @@ namespace System.IO { internal partial struct FileStatus { - internal void SetCreationTime(string path, DateTimeOffset time) => SetTimeOnFile(path, time, Interop.libc.AttrList.ATTR_CMN_CRTIME); - - internal void SetLastWriteTime(string path, DateTimeOffset time) - { - var creationTime = GetCreationTime(path); - SetTimeOnFile(path, time, Interop.libc.AttrList.ATTR_CMN_MODTIME); - if (time < creationTime) SetCreationTime(path, creationTime); - } - - private unsafe void SetTimeOnFile(string path, DateTimeOffset time, uint commonAttr) + internal unsafe void SetCreationTime(string path, DateTimeOffset time) { Interop.Sys.TimeSpec timeSpec = default; @@ -32,7 +23,7 @@ private unsafe void SetTimeOnFile(string path, DateTimeOffset time, uint commonA Interop.libc.AttrList attrList = default; attrList.bitmapCount = Interop.libc.AttrList.ATTR_BIT_MAP_COUNT; attrList.reserved = 0; - attrList.commonAttr = commonAttr; + attrList.commonAttr = Interop.libc.AttrList.ATTR_CMN_CRTIME; attrList.dirAttr = 0; attrList.fileAttr = 0; attrList.forkAttr = 0; @@ -45,14 +36,30 @@ private unsafe void SetTimeOnFile(string path, DateTimeOffset time, uint commonA bool succeeded = Interop.libc.setattrlist(path, &attrList, &timeSpec, sizeof(Interop.Sys.TimeSpec), Interop.libc.FSOPT_NOFOLLOW) == 0; if (!succeeded) { - if (commonAttr == Interop.libc.AttrList.ATTR_CMN_CRTIME) - { - SetCreationTime_StandardUnixImpl(path, time); - } - else if (commonAttr == Interop.libc.AttrList.ATTR_CMN_MODTIME) - { - SetLastWriteTime_StandardUnixImpl(path, time); - } + SetCreationTime_StandardUnixImpl(path, time); + } + else + { + Invalidate(); + } + } + + private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime) + { + // Force an update so GetCreationTime is up-to-date. + Invalidate(); + EnsureStatInitialized(path); + + // Get the creation time here in case the modification time is less than it. + var creationTime = GetCreationTime(path); + + SetAccessOrWriteTimeImpl(path, time, isAccessTime); + + if ((isAccessTime ? GetLastWriteTime(path) : time) < creationTime) + { + // In this case, the creation time is moved back to the modification time on OSX. + // So this code makes sure that the creation time is not changed when it shouldn't be. + SetCreationTime(path, creationTime); } else { diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs index c9699086e532d..08a12035e6dfc 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs @@ -8,6 +8,6 @@ namespace System.IO internal partial struct FileStatus { internal void SetCreationTime(string path, DateTimeOffset time) => SetCreationTime_StandardUnixImpl(path, time); - internal void SetLastWriteTime(string path, DateTimeOffset time) => SetLastWriteTime_StandardUnixImpl(path, time); + private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime) => SetAccessOrWriteTime_StandardUnixImpl(path, time, isAccessTime); } } diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs index b2c4cd56b3c0c..2ca44cf46b0fe 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs @@ -197,7 +197,7 @@ private void SetCreationTime_StandardUnixImpl(string path, DateTimeOffset time) // 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); + SetAccessOrWriteTime_StandardUnixImpl(path, time, isAccessTime: false); } internal DateTimeOffset GetLastAccessTime(ReadOnlySpan path, bool continueOnError = false) @@ -218,21 +218,26 @@ internal DateTimeOffset GetLastWriteTime(ReadOnlySpan path, bool continueO return UnixTimeToDateTimeOffset(_fileStatus.MTime, _fileStatus.MTimeNsec); } - private void SetLastWriteTime_StandardUnixImpl(string path, DateTimeOffset time) => SetAccessOrWriteTime(path, time, isAccessTime: false); + internal void SetLastWriteTime(string path, DateTimeOffset time) => SetAccessOrWriteTime(path, time, isAccessTime: false); private DateTimeOffset UnixTimeToDateTimeOffset(long seconds, long nanoseconds) { return DateTimeOffset.FromUnixTimeSeconds(seconds).AddTicks(nanoseconds / NanosecondsPerTick).ToLocalTime(); } - private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime) + private unsafe void SetAccessOrWriteTime_StandardUnixImpl(string path, DateTimeOffset time, bool isAccessTime) { // Used for access time or as a fallback on OSX, used always on other Unix platforms. // force a refresh so that we have an up-to-date times for values not being overwritten - _fileStatusInitialized = -1; + Invalidate(); EnsureStatInitialized(path); + SetAccessOrWriteTimeImpl(path, time, isAccessTime); + Invalidate(); + } + private unsafe void SetAccessOrWriteTimeImpl(string path, DateTimeOffset time, bool isAccessTime) + { // we use utimes()/utimensat() to set the accessTime and writeTime Interop.Sys.TimeSpec* buf = stackalloc Interop.Sys.TimeSpec[2]; @@ -264,7 +269,6 @@ private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool } #endif Interop.CheckIo(Interop.Sys.UTimensat(path, buf), path, InitiallyDirectory); - _fileStatusInitialized = -1; } internal long GetLength(ReadOnlySpan path, bool continueOnError = false) From b85f3236ea54eb96c220a8eb0c3771142837506d Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Mon, 22 Mar 2021 07:47:03 +1100 Subject: [PATCH 09/40] Fix setting creation date for OSX - hotfix: Remove ATTR_CMN_MODTIME Adding this commit now to avoid 2x CI. It was meant to be in the last one, but I forgot to save the file. Fix for #39132 and issues in #49555. --- src/libraries/Common/src/Interop/OSX/Interop.libc.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/Common/src/Interop/OSX/Interop.libc.cs b/src/libraries/Common/src/Interop/OSX/Interop.libc.cs index 6e0a38aa49615..76eba13f1d342 100644 --- a/src/libraries/Common/src/Interop/OSX/Interop.libc.cs +++ b/src/libraries/Common/src/Interop/OSX/Interop.libc.cs @@ -21,7 +21,6 @@ internal struct AttrList public const ushort ATTR_BIT_MAP_COUNT = 5; public const uint ATTR_CMN_CRTIME = 0x00000200; - public const uint ATTR_CMN_MODTIME = 0x00000400; } [DllImport(Libraries.libc, EntryPoint = "setattrlist", SetLastError = true)] From a617a01195b4dad3cb5f300962ad843b46d4f175 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Wed, 24 Mar 2021 08:56:12 +1100 Subject: [PATCH 10/40] Fix setting creation date for OSX - revert windows changes & simplify code Revert the changes for Windows & disable the SettingUpdatesPropertiesOnSymlink test on Windows. Remove unnecessary setting to 0 on the attrList variable. Remove the unnecessary MarshalAs attributes in the AttrList type. Fix for #39132 and issues in #49555. --- .../Common/src/Interop/OSX/Interop.libc.cs | 14 +++++++------- .../Windows/Kernel32/Interop.FileOperations.cs | 1 - .../src/System/IO/FileStatus.OSX.cs | 5 ----- .../src/System/IO/FileSystem.Windows.cs | 2 +- .../tests/Base/BaseGetSetTimes.cs | 2 +- 5 files changed, 9 insertions(+), 15 deletions(-) diff --git a/src/libraries/Common/src/Interop/OSX/Interop.libc.cs b/src/libraries/Common/src/Interop/OSX/Interop.libc.cs index 76eba13f1d342..9b7722f36df47 100644 --- a/src/libraries/Common/src/Interop/OSX/Interop.libc.cs +++ b/src/libraries/Common/src/Interop/OSX/Interop.libc.cs @@ -11,13 +11,13 @@ internal static partial class libc [StructLayout(LayoutKind.Sequential)] internal struct AttrList { - [MarshalAs(UnmanagedType.U2)] public ushort bitmapCount; - [MarshalAs(UnmanagedType.U2)] public ushort reserved; - [MarshalAs(UnmanagedType.U4)] public uint commonAttr; - [MarshalAs(UnmanagedType.U4)] public uint volAttr; - [MarshalAs(UnmanagedType.U4)] public uint dirAttr; - [MarshalAs(UnmanagedType.U4)] public uint fileAttr; - [MarshalAs(UnmanagedType.U4)] public uint forkAttr; + 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; diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FileOperations.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FileOperations.cs index f8bf953f4c68a..8a2724dc2eddf 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FileOperations.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FileOperations.cs @@ -19,7 +19,6 @@ internal partial class FileOperations internal const int FILE_FLAG_BACKUP_SEMANTICS = 0x02000000; internal const int FILE_FLAG_FIRST_PIPE_INSTANCE = 0x00080000; internal const int FILE_FLAG_OVERLAPPED = 0x40000000; - internal const int FILE_FLAG_OPEN_REPARSE_POINT = 0x00200000; internal const int FILE_LIST_DIRECTORY = 0x0001; } diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs index 7084e5794712d..2dee15a9958bc 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs @@ -22,12 +22,7 @@ internal unsafe void SetCreationTime(string path, DateTimeOffset time) Interop.libc.AttrList attrList = default; attrList.bitmapCount = Interop.libc.AttrList.ATTR_BIT_MAP_COUNT; - attrList.reserved = 0; attrList.commonAttr = Interop.libc.AttrList.ATTR_CMN_CRTIME; - attrList.dirAttr = 0; - attrList.fileAttr = 0; - attrList.forkAttr = 0; - attrList.volAttr = 0; // Try to set the attribute on the file system entry using setattrlist, // otherwise fall back to the method used on other unix platforms as the diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Windows.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Windows.cs index c83b8463ae7c5..5b5f9a86bc744 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Windows.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Windows.cs @@ -158,7 +158,7 @@ private static SafeFileHandle OpenHandle(string fullPath, bool asDirectory) Interop.Kernel32.GenericOperations.GENERIC_WRITE, FileShare.ReadWrite | FileShare.Delete, FileMode.Open, - (asDirectory ? Interop.Kernel32.FileOperations.FILE_FLAG_BACKUP_SEMANTICS : 0) | Interop.Kernel32.FileOperations.FILE_FLAG_OPEN_REPARSE_POINT); + asDirectory ? Interop.Kernel32.FileOperations.FILE_FLAG_BACKUP_SEMANTICS : 0); if (handle.IsInvalid) { diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index 5eec35a523b21..c51ed5edd7b7d 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -111,7 +111,7 @@ public void SettingUpdatesPropertiesAfterAnother() } [Fact] - [PlatformSpecific(~TestPlatforms.Browser)] + [PlatformSpecific(~(TestPlatforms.Browser | TestPlatforms.Windows))] public void SettingUpdatesPropertiesOnSymlink() { // This test makes sure that the times are set on the symlink itself. From f84f10e73f5bd9f9cc969f27695f846987298d4e Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Wed, 24 Mar 2021 09:05:54 +1100 Subject: [PATCH 11/40] Fix setting creation date for OSX - hotfix: change to CULong for marshalling with setattrlist Use CULong as per https://github.com/dotnet/runtime/pull/49555#discussion_r599986776 Fix for #39132 and issues in #49555. --- src/libraries/Common/src/Interop/OSX/Interop.libc.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Common/src/Interop/OSX/Interop.libc.cs b/src/libraries/Common/src/Interop/OSX/Interop.libc.cs index 9b7722f36df47..89ba51305689f 100644 --- a/src/libraries/Common/src/Interop/OSX/Interop.libc.cs +++ b/src/libraries/Common/src/Interop/OSX/Interop.libc.cs @@ -24,7 +24,7 @@ internal struct AttrList } [DllImport(Libraries.libc, EntryPoint = "setattrlist", SetLastError = true)] - internal static unsafe extern int setattrlist(string path, AttrList* attrList, void* attrBuf, nint attrBufSize, uint options); + internal static unsafe extern int setattrlist(string path, AttrList* attrList, void* attrBuf, nint attrBufSize, CULong options); internal const uint FSOPT_NOFOLLOW = 0x00000001; } From 9ce96e5b97f9245e0f5a7cb52981c17a302c3c76 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Wed, 24 Mar 2021 09:08:05 +1100 Subject: [PATCH 12/40] Fix setting creation date for OSX - hotfix: change to CULong for marshalling with setattrlist 2 Use CULong as per #49555 (comment) Forgot to save the other file's changes Fix for #39132 and issues in #49555. --- .../System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs index 2dee15a9958bc..c73e2e67ef79f 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs @@ -28,7 +28,7 @@ internal unsafe void SetCreationTime(string path, DateTimeOffset time) // otherwise fall back to the method used on other unix platforms as the // path may either be on an unsupported volume type (for setattrlist) or it // could be another error (eg. no file) which the fallback implementation can throw. - bool succeeded = Interop.libc.setattrlist(path, &attrList, &timeSpec, sizeof(Interop.Sys.TimeSpec), Interop.libc.FSOPT_NOFOLLOW) == 0; + bool succeeded = Interop.libc.setattrlist(path, &attrList, &timeSpec, sizeof(Interop.Sys.TimeSpec), new CULong(Interop.libc.FSOPT_NOFOLLOW)) == 0; if (!succeeded) { SetCreationTime_StandardUnixImpl(path, time); From 54a62662105e450e01f0f9b8a696f38943ce2975 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Wed, 24 Mar 2021 12:38:01 +1100 Subject: [PATCH 13/40] Fix setting creation date for OSX - consolidate unix nanosecond calculation Consolidate unix nanosecond calculation into UnixTimeSecondsToNanoseconds as per https://github.com/dotnet/runtime/pull/49555#discussion_r600004629. Fix for #39132 and issues in #49555. --- .../src/System/IO/FileStatus.OSX.cs | 5 +---- .../src/System/IO/FileStatus.Unix.cs | 12 ++++++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs index c73e2e67ef79f..a396a33ffeecb 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs @@ -12,10 +12,7 @@ internal unsafe void SetCreationTime(string path, DateTimeOffset time) Interop.Sys.TimeSpec timeSpec = default; 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); timeSpec.TvSec = seconds; timeSpec.TvNsec = nanoseconds; diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs index 2ca44cf46b0fe..5e722b33fe3c8 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs @@ -242,10 +242,7 @@ private unsafe void SetAccessOrWriteTimeImpl(string path, DateTimeOffset time, b 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; @@ -338,5 +335,12 @@ internal void EnsureStatInitialized(ReadOnlySpan path, bool continueOnErro throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(errno), new string(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; + } } } From 50945f8fe3e43db086ddef035ea1d34ecd30a3b3 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Sun, 9 May 2021 15:08:30 +1000 Subject: [PATCH 14/40] Use InvalidateCaches instead of Invalidate and EnsureCachesInitialized instead of EnsureStatInitialized --- .../System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs | 8 ++++---- .../System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs index a396a33ffeecb..aba2f4e0863f3 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs @@ -32,15 +32,15 @@ internal unsafe void SetCreationTime(string path, DateTimeOffset time) } else { - Invalidate(); + InvalidateCaches(); } } private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime) { // Force an update so GetCreationTime is up-to-date. - Invalidate(); - EnsureStatInitialized(path); + InvalidateCaches(); + EnsureCachesInitialized(path); // Get the creation time here in case the modification time is less than it. var creationTime = GetCreationTime(path); @@ -55,7 +55,7 @@ private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool } else { - Invalidate(); + InvalidateCaches(); } } } diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs index 8169fde45842f..b69914b377c56 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs @@ -300,7 +300,7 @@ private unsafe void SetAccessOrWriteTime_StandardUnixImpl(string path, DateTimeO // force a refresh so that we have an up-to-date times for values not being overwritten InvalidateCaches(); - EnsureStatInitialized(path); + EnsureCachesInitialized(path); SetAccessOrWriteTimeImpl(path, time, isAccessTime); InvalidateCaches(); } From f325717b3c5d074e9a69dbc2c6bbcf5c20cbdb09 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Tue, 18 May 2021 04:29:49 +1000 Subject: [PATCH 15/40] Fixes for reviews in PR #49555 Add an identifying field to the TimeFunction tuple and a function to get a TimeFunction from the name. This function is then used so that SettingUpdatesPropertiesAfterAnother can be written as a Theory, and is a lot more readable. Fix some comments that were incorrect. --- .../tests/Base/BaseGetSetTimes.cs | 97 +++++++++++-------- .../tests/Directory/GetSetTimes.cs | 27 ++++-- .../tests/DirectoryInfo/GetSetTimes.cs | 27 ++++-- .../tests/File/GetSetTimes.cs | 27 ++++-- .../tests/FileInfo/GetSetTimes.cs | 27 ++++-- 5 files changed, 131 insertions(+), 74 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index c51ed5edd7b7d..a37baf63722a1 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -29,19 +29,25 @@ public abstract class BaseGetSetTimes : FileSystemTest public abstract IEnumerable TimeFunctions(bool requiresRoundtripping = false); - public class TimeFunction : Tuple + public TimeFunction TimeFunction(string name, bool requiresRoundtripping = false) { - public TimeFunction(SetTime setter, GetTime getter, DateTimeKind kind) - : base(item1: setter, item2: getter, item3: kind) + return TimeFunctions(requiresRoundtripping: name).FirstOrDefault((x) => x.Name == name); + } + + public class TimeFunction : Tuple + { + public TimeFunction(SetTime setter, GetTime getter, DateTimeKind kind, string name) + : base(item1: setter, item2: getter, item3: kind, item4: name) { } - public static TimeFunction Create(SetTime setter, GetTime getter, DateTimeKind kind) - => new TimeFunction(setter, getter, kind); + public static TimeFunction Create(SetTime setter, GetTime getter, DateTimeKind kind, string name) + => new TimeFunction(setter, getter, kind, name); public SetTime Setter => Item1; public GetTime Getter => Item2; public DateTimeKind Kind => Item3; + public string Name => Item4; } [Fact] @@ -72,48 +78,64 @@ public void SettingUpdatesProperties() }); } - [Fact] + [Theory] + [InlineData("CreationTime_Utc", "LastAccessTime_Utc", false)] + [InlineData("CreationTime_Utc", "LastAccessTime_Utc", true)] + [InlineData("CreationTime_Utc", "LastWriteTime_Utc", false)] + [InlineData("CreationTime_Utc", "LastWriteTime_Utc", true)] + [InlineData("LastAccessTime_Utc", "CreationTime_Utc", false)] + [InlineData("LastAccessTime_Utc", "CreationTime_Utc", true)] + [InlineData("LastAccessTime_Utc", "LastWriteTime_Utc", false)] + [InlineData("LastAccessTime_Utc", "LastWriteTime_Utc", true)] + [InlineData("LastWriteTime_Utc", "CreationTime_Utc", false)] + [InlineData("LastWriteTime_Utc", "CreationTime_Utc", true)] + [InlineData("LastWriteTime_Utc", "LastAccessTime_Utc", false)] + [InlineData("LastWriteTime_Utc", "LastAccessTime_Utc", true)] [PlatformSpecific(~TestPlatforms.Browser)] - public void SettingUpdatesPropertiesAfterAnother() + public void SettingUpdatesPropertiesAfterAnother(string function1Name, string function2Name, bool reverse) { + // Browser is excluded as there is only 1 effective time store. + 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. - var 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) => + // This test tests setting function1 to 2002, then function2 to 2001, then function1 + // to 2000 (or reverse if reverse = true). This test is required as some apis change + // more dates then they're supposed to. There were issues while developing this PR + // with specific orders of changes, so this code should almost fully eliminate any + // possibilities of that in the future 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). + + var function1 = TimeFunction(function1Name); + if (function1 == null) return; //function not supported on this platform anyway (eg. creation time on linux) + + var function2 = TimeFunction(function2Name); + if (function2 == null) return; //function not supported on this platform anyway (eg. creation time on linux) + + // 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 { - // Checking that milliseconds are not dropped after setter. - // Emscripten drops milliseconds in Browser - 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 (functions.reverse) - { - var swap = dt3; - dt3 = dt1; - dt1 = swap; - } - functions.x.Setter(item, dt1); - functions.y.Setter(item, dt2); - functions.x.Setter(item, dt3); - DateTime result_x = functions.x.Getter(item); - DateTime result_y = functions.y.Getter(item); - Assert.Equal(dt3, result_x); - Assert.Equal(dt2, result_y); - }); + var swap = dt3; + dt3 = dt1; + dt1 = swap; + } + 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] [PlatformSpecific(~(TestPlatforms.Browser | TestPlatforms.Windows))] public void SettingUpdatesPropertiesOnSymlink() { + // Browser is excluded as there is only 1 effective time store. + // This test makes sure that the times are set on the symlink itself. // It is needed as on OSX for example, the default for most APIs is // to follow the symlink to completion and set the time on that entry @@ -122,8 +144,7 @@ public void SettingUpdatesPropertiesOnSymlink() Assert.All(TimeFunctions(requiresRoundtripping: true), (function) => { - // Checking that milliseconds are not dropped after setter. - // Emscripten drops milliseconds in Browser + // Checking that milliseconds are not dropped after setter on supported platforms. DateTime dt = new DateTime(2004, 12, 1, 12, 3, 3, LowTemporalResolution ? 0 : 321, function.Kind); function.Setter(item, dt); DateTime result = function.Getter(item); diff --git a/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs index 7531ccae83019..44f3e3104026d 100644 --- a/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs @@ -16,40 +16,49 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi yield return TimeFunction.Create( ((path, time) => Directory.SetCreationTime(path, time)), ((path) => Directory.GetCreationTime(path)), - DateTimeKind.Local); + DateTimeKind.Local, + "CreationTime_Local"); yield return TimeFunction.Create( ((path, time) => Directory.SetCreationTimeUtc(path, time)), ((path) => Directory.GetCreationTimeUtc(path)), - DateTimeKind.Unspecified); + DateTimeKind.Unspecified, + "CreationTime_Unspecified"); yield return TimeFunction.Create( ((path, time) => Directory.SetCreationTimeUtc(path, time)), ((path) => Directory.GetCreationTimeUtc(path)), - DateTimeKind.Utc); + DateTimeKind.Utc, + "CreationTime_Utc"); } yield return TimeFunction.Create( ((path, time) => Directory.SetLastAccessTime(path, time)), ((path) => Directory.GetLastAccessTime(path)), - DateTimeKind.Local); + DateTimeKind.Local, + "LastAccessTime_Local"); yield return TimeFunction.Create( ((path, time) => Directory.SetLastAccessTimeUtc(path, time)), ((path) => Directory.GetLastAccessTimeUtc(path)), - DateTimeKind.Unspecified); + DateTimeKind.Unspecified, + "LastAccessTime_Unspecified"); yield return TimeFunction.Create( ((path, time) => Directory.SetLastAccessTimeUtc(path, time)), ((path) => Directory.GetLastAccessTimeUtc(path)), - DateTimeKind.Utc); + DateTimeKind.Utc, + "LastAccessTime_Utc"); yield return TimeFunction.Create( ((path, time) => Directory.SetLastWriteTime(path, time)), ((path) => Directory.GetLastWriteTime(path)), - DateTimeKind.Local); + DateTimeKind.Local, + "LastWriteTime_Local"); yield return TimeFunction.Create( ((path, time) => Directory.SetLastWriteTimeUtc(path, time)), ((path) => Directory.GetLastWriteTimeUtc(path)), - DateTimeKind.Unspecified); + DateTimeKind.Unspecified, + "LastWriteTime_Unspecified"); yield return TimeFunction.Create( ((path, time) => Directory.SetLastWriteTimeUtc(path, time)), ((path) => Directory.GetLastWriteTimeUtc(path)), - DateTimeKind.Utc); + DateTimeKind.Utc, + "LastWriteTime_Utc"); } protected override string CreateSymlinkToItem(string item) diff --git a/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs index cce3d1853c1d0..1b284e6c3288c 100644 --- a/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs @@ -22,40 +22,49 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi yield return TimeFunction.Create( ((testDir, time) => {testDir.CreationTime = time; }), ((testDir) => testDir.CreationTime), - DateTimeKind.Local); + DateTimeKind.Local, + "CreationTime_Local"); yield return TimeFunction.Create( ((testDir, time) => {testDir.CreationTimeUtc = time; }), ((testDir) => testDir.CreationTimeUtc), - DateTimeKind.Unspecified); + DateTimeKind.Unspecified, + "CreationTime_Unspecified"); yield return TimeFunction.Create( ((testDir, time) => { testDir.CreationTimeUtc = time; }), ((testDir) => testDir.CreationTimeUtc), - DateTimeKind.Utc); + DateTimeKind.Utc, + "CreationTime_Utc"); } yield return TimeFunction.Create( ((testDir, time) => {testDir.LastAccessTime = time; }), ((testDir) => testDir.LastAccessTime), - DateTimeKind.Local); + DateTimeKind.Local, + "LastAccessTime_Local"); yield return TimeFunction.Create( ((testDir, time) => {testDir.LastAccessTimeUtc = time; }), ((testDir) => testDir.LastAccessTimeUtc), - DateTimeKind.Unspecified); + DateTimeKind.Unspecified, + "LastAccessTime_Unspecified"); yield return TimeFunction.Create( ((testDir, time) => { testDir.LastAccessTimeUtc = time; }), ((testDir) => testDir.LastAccessTimeUtc), - DateTimeKind.Utc); + DateTimeKind.Utc, + "LastAccessTime_Utc"); yield return TimeFunction.Create( ((testDir, time) => {testDir.LastWriteTime = time; }), ((testDir) => testDir.LastWriteTime), - DateTimeKind.Local); + DateTimeKind.Local, + "LastWriteTime_Local"); yield return TimeFunction.Create( ((testDir, time) => {testDir.LastWriteTimeUtc = time; }), ((testDir) => testDir.LastWriteTimeUtc), - DateTimeKind.Unspecified); + DateTimeKind.Unspecified, + "LastWriteTime_Unspecified"); yield return TimeFunction.Create( ((testDir, time) => { testDir.LastWriteTimeUtc = time; }), ((testDir) => testDir.LastWriteTimeUtc), - DateTimeKind.Utc); + DateTimeKind.Utc, + "LastWriteTime_Utc"); } protected override DirectoryInfo CreateSymlinkToItem(DirectoryInfo item) diff --git a/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs index 551a5115106b1..5a574f3dfdaac 100644 --- a/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs @@ -67,40 +67,49 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi yield return TimeFunction.Create( ((path, time) => File.SetCreationTime(path, time)), ((path) => File.GetCreationTime(path)), - DateTimeKind.Local); + DateTimeKind.Local, + "CreationTime_Local"); yield return TimeFunction.Create( ((path, time) => File.SetCreationTimeUtc(path, time)), ((path) => File.GetCreationTimeUtc(path)), - DateTimeKind.Unspecified); + DateTimeKind.Unspecified, + "CreationTime_Unspecified"); yield return TimeFunction.Create( ((path, time) => File.SetCreationTimeUtc(path, time)), ((path) => File.GetCreationTimeUtc(path)), - DateTimeKind.Utc); + DateTimeKind.Utc, + "CreationTime_Utc"); } yield return TimeFunction.Create( ((path, time) => File.SetLastAccessTime(path, time)), ((path) => File.GetLastAccessTime(path)), - DateTimeKind.Local); + DateTimeKind.Local, + "LastAccessTime_Local"); yield return TimeFunction.Create( ((path, time) => File.SetLastAccessTimeUtc(path, time)), ((path) => File.GetLastAccessTimeUtc(path)), - DateTimeKind.Unspecified); + DateTimeKind.Unspecified, + "LastAccessTime_Unspecified"); yield return TimeFunction.Create( ((path, time) => File.SetLastAccessTimeUtc(path, time)), ((path) => File.GetLastAccessTimeUtc(path)), - DateTimeKind.Utc); + DateTimeKind.Utc, + "LastAccessTime_Utc"); yield return TimeFunction.Create( ((path, time) => File.SetLastWriteTime(path, time)), ((path) => File.GetLastWriteTime(path)), - DateTimeKind.Local); + DateTimeKind.Local, + "LastWriteTime_Local"); yield return TimeFunction.Create( ((path, time) => File.SetLastWriteTimeUtc(path, time)), ((path) => File.GetLastWriteTimeUtc(path)), - DateTimeKind.Unspecified); + DateTimeKind.Unspecified, + "LastWriteTime_Unspecified"); yield return TimeFunction.Create( ((path, time) => File.SetLastWriteTimeUtc(path, time)), ((path) => File.GetLastWriteTimeUtc(path)), - DateTimeKind.Utc); + DateTimeKind.Utc, + "LastWriteTime_Utc"); } protected override string CreateSymlinkToItem(string item) diff --git a/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs index 7abf30a1a20e1..a11f273360768 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs @@ -67,40 +67,49 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi yield return TimeFunction.Create( ((testFile, time) => { testFile.CreationTime = time; }), ((testFile) => testFile.CreationTime), - DateTimeKind.Local); + DateTimeKind.Local, + "CreationTime_Local"); yield return TimeFunction.Create( ((testFile, time) => { testFile.CreationTimeUtc = time; }), ((testFile) => testFile.CreationTimeUtc), - DateTimeKind.Unspecified); + DateTimeKind.Unspecified, + "CreationTime_Unspecified"); yield return TimeFunction.Create( ((testFile, time) => { testFile.CreationTimeUtc = time; }), ((testFile) => testFile.CreationTimeUtc), - DateTimeKind.Utc); + DateTimeKind.Utc, + "CreationTime_Utc"); } yield return TimeFunction.Create( ((testFile, time) => { testFile.LastAccessTime = time; }), ((testFile) => testFile.LastAccessTime), - DateTimeKind.Local); + DateTimeKind.Local, + "LastAccessTime_Local"); yield return TimeFunction.Create( ((testFile, time) => { testFile.LastAccessTimeUtc = time; }), ((testFile) => testFile.LastAccessTimeUtc), - DateTimeKind.Unspecified); + DateTimeKind.Unspecified, + "LastAccessTime_Unspecified"); yield return TimeFunction.Create( ((testFile, time) => { testFile.LastAccessTimeUtc = time; }), ((testFile) => testFile.LastAccessTimeUtc), - DateTimeKind.Utc); + DateTimeKind.Utc, + "LastAccessTime_Utc"); yield return TimeFunction.Create( ((testFile, time) => { testFile.LastWriteTime = time; }), ((testFile) => testFile.LastWriteTime), - DateTimeKind.Local); + DateTimeKind.Local, + "LastWriteTime_Local"); yield return TimeFunction.Create( ((testFile, time) => { testFile.LastWriteTimeUtc = time; }), ((testFile) => testFile.LastWriteTimeUtc), - DateTimeKind.Unspecified); + DateTimeKind.Unspecified, + "LastWriteTime_Unspecified"); yield return TimeFunction.Create( ((testFile, time) => { testFile.LastWriteTimeUtc = time; }), ((testFile) => testFile.LastWriteTimeUtc), - DateTimeKind.Utc); + DateTimeKind.Utc, + "LastWriteTime_Utc"); } protected override FileInfo CreateSymlinkToItem(FileInfo item) From 19cbb7f815099b25c991ab249ef9b1e3b621275c Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Tue, 18 May 2021 04:38:58 +1000 Subject: [PATCH 16/40] Hotfix for missing parameter --- .../System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index a37baf63722a1..07b45c02ab5be 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -31,6 +31,7 @@ public abstract class BaseGetSetTimes : FileSystemTest public TimeFunction TimeFunction(string name, bool requiresRoundtripping = false) { + //if this returns null, it means that the function is not supported on the platform (eg. creation time on linux) return TimeFunctions(requiresRoundtripping: name).FirstOrDefault((x) => x.Name == name); } @@ -105,11 +106,11 @@ public void SettingUpdatesPropertiesAfterAnother(string function1Name, string fu // possibilities of that in the future 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). - var function1 = TimeFunction(function1Name); - if (function1 == null) return; //function not supported on this platform anyway (eg. creation time on linux) + var function1 = TimeFunction(function1Name, requiresRoundtripping: true); + if (function1 == null) return; - var function2 = TimeFunction(function2Name); - if (function2 == null) return; //function not supported on this platform anyway (eg. creation time on linux) + var function2 = TimeFunction(function2Name, requiresRoundtripping: true); + if (function2 == null) return; // Checking that milliseconds are not dropped after setter. DateTime dt1 = new DateTime(2002, 12, 1, 12, 3, 3, LowTemporalResolution ? 0 : 321, DateTimeKind.Utc); From 2b702e3f88181ba05c81c72c15cbe2ff32477af4 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Tue, 18 May 2021 05:49:38 +1000 Subject: [PATCH 17/40] Fix naming --- .../System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index 07b45c02ab5be..eaf95eb0f9e98 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -29,7 +29,7 @@ public abstract class BaseGetSetTimes : FileSystemTest public abstract IEnumerable TimeFunctions(bool requiresRoundtripping = false); - public TimeFunction TimeFunction(string name, bool requiresRoundtripping = false) + public TimeFunction GetTimeFunction(string name, bool requiresRoundtripping = false) { //if this returns null, it means that the function is not supported on the platform (eg. creation time on linux) return TimeFunctions(requiresRoundtripping: name).FirstOrDefault((x) => x.Name == name); @@ -106,10 +106,10 @@ public void SettingUpdatesPropertiesAfterAnother(string function1Name, string fu // possibilities of that in the future 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). - var function1 = TimeFunction(function1Name, requiresRoundtripping: true); + var function1 = GetTimeFunction(function1Name, requiresRoundtripping: true); if (function1 == null) return; - var function2 = TimeFunction(function2Name, requiresRoundtripping: true); + var function2 = GetTimeFunction(function2Name, requiresRoundtripping: true); if (function2 == null) return; // Checking that milliseconds are not dropped after setter. From f1a303e7e1f005b5067429c94aab6ab015d1d171 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Tue, 18 May 2021 11:12:18 +1000 Subject: [PATCH 18/40] Revert changes re SettingUpdatesPropertiesAfterAnother (mainly) and revert using a Theory Reversion as per https://github.com/dotnet/runtime/pull/49555#discussion_r633895912 This change can be unreverted if that's what's decided. --- .../tests/Base/BaseGetSetTimes.cs | 103 +++++++++--------- .../tests/Directory/GetSetTimes.cs | 27 ++--- .../tests/DirectoryInfo/GetSetTimes.cs | 27 ++--- .../tests/File/GetSetTimes.cs | 27 ++--- .../tests/FileInfo/GetSetTimes.cs | 27 ++--- 5 files changed, 85 insertions(+), 126 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index eaf95eb0f9e98..9134000178f6e 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -29,26 +29,19 @@ public abstract class BaseGetSetTimes : FileSystemTest public abstract IEnumerable TimeFunctions(bool requiresRoundtripping = false); - public TimeFunction GetTimeFunction(string name, bool requiresRoundtripping = false) + public class TimeFunction : Tuple { - //if this returns null, it means that the function is not supported on the platform (eg. creation time on linux) - return TimeFunctions(requiresRoundtripping: name).FirstOrDefault((x) => x.Name == name); - } - - public class TimeFunction : Tuple - { - public TimeFunction(SetTime setter, GetTime getter, DateTimeKind kind, string name) - : base(item1: setter, item2: getter, item3: kind, item4: name) + public TimeFunction(SetTime setter, GetTime getter, DateTimeKind kind) + : base(item1: setter, item2: getter, item3: kind) { } - public static TimeFunction Create(SetTime setter, GetTime getter, DateTimeKind kind, string name) - => new TimeFunction(setter, getter, kind, name); + public static TimeFunction Create(SetTime setter, GetTime getter, DateTimeKind kind) + => new TimeFunction(setter, getter, kind); public SetTime Setter => Item1; public GetTime Getter => Item2; public DateTimeKind Kind => Item3; - public string Name => Item4; } [Fact] @@ -79,19 +72,7 @@ public void SettingUpdatesProperties() }); } - [Theory] - [InlineData("CreationTime_Utc", "LastAccessTime_Utc", false)] - [InlineData("CreationTime_Utc", "LastAccessTime_Utc", true)] - [InlineData("CreationTime_Utc", "LastWriteTime_Utc", false)] - [InlineData("CreationTime_Utc", "LastWriteTime_Utc", true)] - [InlineData("LastAccessTime_Utc", "CreationTime_Utc", false)] - [InlineData("LastAccessTime_Utc", "CreationTime_Utc", true)] - [InlineData("LastAccessTime_Utc", "LastWriteTime_Utc", false)] - [InlineData("LastAccessTime_Utc", "LastWriteTime_Utc", true)] - [InlineData("LastWriteTime_Utc", "CreationTime_Utc", false)] - [InlineData("LastWriteTime_Utc", "CreationTime_Utc", true)] - [InlineData("LastWriteTime_Utc", "LastAccessTime_Utc", false)] - [InlineData("LastWriteTime_Utc", "LastAccessTime_Utc", true)] + [Fact] [PlatformSpecific(~TestPlatforms.Browser)] public void SettingUpdatesPropertiesAfterAnother(string function1Name, string function2Name, bool reverse) { @@ -99,36 +80,50 @@ public void SettingUpdatesPropertiesAfterAnother(string function1Name, string fu T item = GetExistingItem(); - // This test tests setting function1 to 2002, then function2 to 2001, then function1 - // to 2000 (or reverse if reverse = true). This test is required as some apis change - // more dates then they're supposed to. There were issues while developing this PR - // with specific orders of changes, so this code should almost fully eliminate any - // possibilities of that in the future 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). - - var function1 = GetTimeFunction(function1Name, requiresRoundtripping: true); - if (function1 == null) return; - - var function2 = GetTimeFunction(function2Name, requiresRoundtripping: true); - if (function2 == null) return; - - // 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 + // 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. + // There were issues while developing this PR with specific orders of changes, so + // this code should almost fully eliminate any possibilities of that in the future + // 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) + + var 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) => { - var swap = dt3; - dt3 = dt1; - dt1 = swap; - } - 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); + // 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 + { + var swap = dt3; + dt3 = dt1; + dt1 = swap; + } + 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] diff --git a/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs index 44f3e3104026d..7531ccae83019 100644 --- a/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs @@ -16,49 +16,40 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi yield return TimeFunction.Create( ((path, time) => Directory.SetCreationTime(path, time)), ((path) => Directory.GetCreationTime(path)), - DateTimeKind.Local, - "CreationTime_Local"); + DateTimeKind.Local); yield return TimeFunction.Create( ((path, time) => Directory.SetCreationTimeUtc(path, time)), ((path) => Directory.GetCreationTimeUtc(path)), - DateTimeKind.Unspecified, - "CreationTime_Unspecified"); + DateTimeKind.Unspecified); yield return TimeFunction.Create( ((path, time) => Directory.SetCreationTimeUtc(path, time)), ((path) => Directory.GetCreationTimeUtc(path)), - DateTimeKind.Utc, - "CreationTime_Utc"); + DateTimeKind.Utc); } yield return TimeFunction.Create( ((path, time) => Directory.SetLastAccessTime(path, time)), ((path) => Directory.GetLastAccessTime(path)), - DateTimeKind.Local, - "LastAccessTime_Local"); + DateTimeKind.Local); yield return TimeFunction.Create( ((path, time) => Directory.SetLastAccessTimeUtc(path, time)), ((path) => Directory.GetLastAccessTimeUtc(path)), - DateTimeKind.Unspecified, - "LastAccessTime_Unspecified"); + DateTimeKind.Unspecified); yield return TimeFunction.Create( ((path, time) => Directory.SetLastAccessTimeUtc(path, time)), ((path) => Directory.GetLastAccessTimeUtc(path)), - DateTimeKind.Utc, - "LastAccessTime_Utc"); + DateTimeKind.Utc); yield return TimeFunction.Create( ((path, time) => Directory.SetLastWriteTime(path, time)), ((path) => Directory.GetLastWriteTime(path)), - DateTimeKind.Local, - "LastWriteTime_Local"); + DateTimeKind.Local); yield return TimeFunction.Create( ((path, time) => Directory.SetLastWriteTimeUtc(path, time)), ((path) => Directory.GetLastWriteTimeUtc(path)), - DateTimeKind.Unspecified, - "LastWriteTime_Unspecified"); + DateTimeKind.Unspecified); yield return TimeFunction.Create( ((path, time) => Directory.SetLastWriteTimeUtc(path, time)), ((path) => Directory.GetLastWriteTimeUtc(path)), - DateTimeKind.Utc, - "LastWriteTime_Utc"); + DateTimeKind.Utc); } protected override string CreateSymlinkToItem(string item) diff --git a/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs index 1b284e6c3288c..cce3d1853c1d0 100644 --- a/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs @@ -22,49 +22,40 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi yield return TimeFunction.Create( ((testDir, time) => {testDir.CreationTime = time; }), ((testDir) => testDir.CreationTime), - DateTimeKind.Local, - "CreationTime_Local"); + DateTimeKind.Local); yield return TimeFunction.Create( ((testDir, time) => {testDir.CreationTimeUtc = time; }), ((testDir) => testDir.CreationTimeUtc), - DateTimeKind.Unspecified, - "CreationTime_Unspecified"); + DateTimeKind.Unspecified); yield return TimeFunction.Create( ((testDir, time) => { testDir.CreationTimeUtc = time; }), ((testDir) => testDir.CreationTimeUtc), - DateTimeKind.Utc, - "CreationTime_Utc"); + DateTimeKind.Utc); } yield return TimeFunction.Create( ((testDir, time) => {testDir.LastAccessTime = time; }), ((testDir) => testDir.LastAccessTime), - DateTimeKind.Local, - "LastAccessTime_Local"); + DateTimeKind.Local); yield return TimeFunction.Create( ((testDir, time) => {testDir.LastAccessTimeUtc = time; }), ((testDir) => testDir.LastAccessTimeUtc), - DateTimeKind.Unspecified, - "LastAccessTime_Unspecified"); + DateTimeKind.Unspecified); yield return TimeFunction.Create( ((testDir, time) => { testDir.LastAccessTimeUtc = time; }), ((testDir) => testDir.LastAccessTimeUtc), - DateTimeKind.Utc, - "LastAccessTime_Utc"); + DateTimeKind.Utc); yield return TimeFunction.Create( ((testDir, time) => {testDir.LastWriteTime = time; }), ((testDir) => testDir.LastWriteTime), - DateTimeKind.Local, - "LastWriteTime_Local"); + DateTimeKind.Local); yield return TimeFunction.Create( ((testDir, time) => {testDir.LastWriteTimeUtc = time; }), ((testDir) => testDir.LastWriteTimeUtc), - DateTimeKind.Unspecified, - "LastWriteTime_Unspecified"); + DateTimeKind.Unspecified); yield return TimeFunction.Create( ((testDir, time) => { testDir.LastWriteTimeUtc = time; }), ((testDir) => testDir.LastWriteTimeUtc), - DateTimeKind.Utc, - "LastWriteTime_Utc"); + DateTimeKind.Utc); } protected override DirectoryInfo CreateSymlinkToItem(DirectoryInfo item) diff --git a/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs index 5a574f3dfdaac..551a5115106b1 100644 --- a/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs @@ -67,49 +67,40 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi yield return TimeFunction.Create( ((path, time) => File.SetCreationTime(path, time)), ((path) => File.GetCreationTime(path)), - DateTimeKind.Local, - "CreationTime_Local"); + DateTimeKind.Local); yield return TimeFunction.Create( ((path, time) => File.SetCreationTimeUtc(path, time)), ((path) => File.GetCreationTimeUtc(path)), - DateTimeKind.Unspecified, - "CreationTime_Unspecified"); + DateTimeKind.Unspecified); yield return TimeFunction.Create( ((path, time) => File.SetCreationTimeUtc(path, time)), ((path) => File.GetCreationTimeUtc(path)), - DateTimeKind.Utc, - "CreationTime_Utc"); + DateTimeKind.Utc); } yield return TimeFunction.Create( ((path, time) => File.SetLastAccessTime(path, time)), ((path) => File.GetLastAccessTime(path)), - DateTimeKind.Local, - "LastAccessTime_Local"); + DateTimeKind.Local); yield return TimeFunction.Create( ((path, time) => File.SetLastAccessTimeUtc(path, time)), ((path) => File.GetLastAccessTimeUtc(path)), - DateTimeKind.Unspecified, - "LastAccessTime_Unspecified"); + DateTimeKind.Unspecified); yield return TimeFunction.Create( ((path, time) => File.SetLastAccessTimeUtc(path, time)), ((path) => File.GetLastAccessTimeUtc(path)), - DateTimeKind.Utc, - "LastAccessTime_Utc"); + DateTimeKind.Utc); yield return TimeFunction.Create( ((path, time) => File.SetLastWriteTime(path, time)), ((path) => File.GetLastWriteTime(path)), - DateTimeKind.Local, - "LastWriteTime_Local"); + DateTimeKind.Local); yield return TimeFunction.Create( ((path, time) => File.SetLastWriteTimeUtc(path, time)), ((path) => File.GetLastWriteTimeUtc(path)), - DateTimeKind.Unspecified, - "LastWriteTime_Unspecified"); + DateTimeKind.Unspecified); yield return TimeFunction.Create( ((path, time) => File.SetLastWriteTimeUtc(path, time)), ((path) => File.GetLastWriteTimeUtc(path)), - DateTimeKind.Utc, - "LastWriteTime_Utc"); + DateTimeKind.Utc); } protected override string CreateSymlinkToItem(string item) diff --git a/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs index a11f273360768..7abf30a1a20e1 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs @@ -67,49 +67,40 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi yield return TimeFunction.Create( ((testFile, time) => { testFile.CreationTime = time; }), ((testFile) => testFile.CreationTime), - DateTimeKind.Local, - "CreationTime_Local"); + DateTimeKind.Local); yield return TimeFunction.Create( ((testFile, time) => { testFile.CreationTimeUtc = time; }), ((testFile) => testFile.CreationTimeUtc), - DateTimeKind.Unspecified, - "CreationTime_Unspecified"); + DateTimeKind.Unspecified); yield return TimeFunction.Create( ((testFile, time) => { testFile.CreationTimeUtc = time; }), ((testFile) => testFile.CreationTimeUtc), - DateTimeKind.Utc, - "CreationTime_Utc"); + DateTimeKind.Utc); } yield return TimeFunction.Create( ((testFile, time) => { testFile.LastAccessTime = time; }), ((testFile) => testFile.LastAccessTime), - DateTimeKind.Local, - "LastAccessTime_Local"); + DateTimeKind.Local); yield return TimeFunction.Create( ((testFile, time) => { testFile.LastAccessTimeUtc = time; }), ((testFile) => testFile.LastAccessTimeUtc), - DateTimeKind.Unspecified, - "LastAccessTime_Unspecified"); + DateTimeKind.Unspecified); yield return TimeFunction.Create( ((testFile, time) => { testFile.LastAccessTimeUtc = time; }), ((testFile) => testFile.LastAccessTimeUtc), - DateTimeKind.Utc, - "LastAccessTime_Utc"); + DateTimeKind.Utc); yield return TimeFunction.Create( ((testFile, time) => { testFile.LastWriteTime = time; }), ((testFile) => testFile.LastWriteTime), - DateTimeKind.Local, - "LastWriteTime_Local"); + DateTimeKind.Local); yield return TimeFunction.Create( ((testFile, time) => { testFile.LastWriteTimeUtc = time; }), ((testFile) => testFile.LastWriteTimeUtc), - DateTimeKind.Unspecified, - "LastWriteTime_Unspecified"); + DateTimeKind.Unspecified); yield return TimeFunction.Create( ((testFile, time) => { testFile.LastWriteTimeUtc = time; }), ((testFile) => testFile.LastWriteTimeUtc), - DateTimeKind.Utc, - "LastWriteTime_Utc"); + DateTimeKind.Utc); } protected override FileInfo CreateSymlinkToItem(FileInfo item) From 0bbfbed5246549cb62ea00a2e8d8e66efe581560 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Tue, 25 May 2021 19:41:16 +1000 Subject: [PATCH 19/40] Fix errors of #49555 due to missing variables --- .../System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index 9134000178f6e..197df52bf8f46 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -106,6 +106,10 @@ public void SettingUpdatesPropertiesAfterAnother(string function1Name, string fu 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) => { + var function1 = functions.x; + var function2 = functions.y; + var reverse = functions.reverse; + // 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); From 42ca7e198ac61959178cfe7d08cf887b9f2fa992 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Tue, 25 May 2021 20:14:34 +1000 Subject: [PATCH 20/40] Remove the parameters that were meant to be removed --- .../System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index 197df52bf8f46..a8d6a052a5436 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -74,7 +74,7 @@ public void SettingUpdatesProperties() [Fact] [PlatformSpecific(~TestPlatforms.Browser)] - public void SettingUpdatesPropertiesAfterAnother(string function1Name, string function2Name, bool reverse) + public void SettingUpdatesPropertiesAfterAnother() { // Browser is excluded as there is only 1 effective time store. From 74407728cc335569376c7a44f794b301706a2168 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Sun, 30 May 2021 08:03:05 +1000 Subject: [PATCH 21/40] Finish merge --- .../src/System.Private.CoreLib.Shared.projitems | 12 ++++++++++++ .../src/System/IO/FileStatus.OSX.cs | 0 .../src/System/IO/FileStatus.OtherUnix.cs | 0 3 files changed, 12 insertions(+) rename src/libraries/{System.IO.FileSystem => System.Private.CoreLib}/src/System/IO/FileStatus.OSX.cs (100%) rename src/libraries/{System.IO.FileSystem => System.Private.CoreLib}/src/System/IO/FileStatus.OtherUnix.cs (100%) diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index bbbd804c83e6f..a20eef82d4eaf 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -2052,6 +2052,18 @@ + + + + + + Common\Interop\OSX\Interop.Libraries.cs + + + Common\Interop\OSX\Interop.libc.cs + + + Common\Interop\Unix\System.Native\Interop.GetEUid.cs diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.OSX.cs similarity index 100% rename from src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OSX.cs rename to src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.OSX.cs diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.OtherUnix.cs similarity index 100% rename from src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.OtherUnix.cs rename to src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.OtherUnix.cs From e5500822633425312f8d947a63bf6f7abd00c225 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Sun, 30 May 2021 10:13:05 +1000 Subject: [PATCH 22/40] Remove duplicate file --- .../src/System.Private.CoreLib.Shared.projitems | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index a20eef82d4eaf..69535e8fedf99 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -2056,9 +2056,6 @@ - - Common\Interop\OSX\Interop.Libraries.cs - Common\Interop\OSX\Interop.libc.cs From 17868b1ea6d2f3651d1e83d6e112910849d9fd12 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Sun, 30 May 2021 13:51:16 +1000 Subject: [PATCH 23/40] Add custom error handling for setattrlist Add custom error handling for setattrlist as per https://github.com/dotnet/runtime/pull/49555#discussion_r633788259. ENOTDIR means it is not a directory, so directory not found seems logical to me, I think the other errors can be dealt with by an IOException, happy to change this. --- .../Common/src/Interop/Unix/Interop.IOErrors.cs | 1 + .../src/System/IO/FileStatus.OSX.cs | 17 +++++++++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs b/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs index 14aeddd5adcf4..6b6e185119e08 100644 --- a/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs +++ b/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs @@ -114,6 +114,7 @@ internal static Exception GetExceptionForIoErrno(ErrorInfo errorInfo, string? pa switch (errorInfo.Error) { case Error.ENOENT: + case Error.ENOTDIR: if (isDirectory) { return !string.IsNullOrEmpty(path) ? diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.OSX.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.OSX.cs index aba2f4e0863f3..7f0edeed39804 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.OSX.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.OSX.cs @@ -22,18 +22,23 @@ internal unsafe void SetCreationTime(string path, DateTimeOffset time) attrList.commonAttr = Interop.libc.AttrList.ATTR_CMN_CRTIME; // Try to set the attribute on the file system entry using setattrlist, - // otherwise fall back to the method used on other unix platforms as the - // path may either be on an unsupported volume type (for setattrlist) or it - // could be another error (eg. no file) which the fallback implementation can throw. - bool succeeded = Interop.libc.setattrlist(path, &attrList, &timeSpec, sizeof(Interop.Sys.TimeSpec), new CULong(Interop.libc.FSOPT_NOFOLLOW)) == 0; - if (!succeeded) + // 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. + Interop.Error result = (Interop.Error)Interop.libc.setattrlist(path, &attrList, &timeSpec, sizeof(Interop.Sys.TimeSpec), new CULong(Interop.libc.FSOPT_NOFOLLOW)); + if (result == Interop.Error.ENOTSUP) { SetCreationTime_StandardUnixImpl(path, time); } - else + else if (result == Interop.Error.SUCCESS) { InvalidateCaches(); } + else + { + Interop.CheckIo(result, path, InitiallyDirectory); + } } private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime) From f84293583bb0c8e1060bdacfb80158a6c64c5fbd Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Wed, 8 Sep 2021 09:08:11 +1000 Subject: [PATCH 24/40] Remove symlink specific code from this PR Removes the symlink specific code as per https://github.com/dotnet/runtime/pull/49555#issuecomment-904413721. This will be reverted (and modified) as per https://github.com/dotnet/runtime/pull/49555#issuecomment-904413721 into #52639. --- .../Common/src/Interop/OSX/Interop.libc.cs | 2 -- .../Native/Unix/System.Native/pal_time.c | 2 +- .../tests/Base/BaseGetSetTimes.cs | 35 ------------------- .../tests/Directory/GetSetTimes.cs | 8 ----- .../tests/DirectoryInfo/GetSetTimes.cs | 10 ------ .../tests/File/GetSetTimes.cs | 8 ----- .../tests/FileInfo/GetSetTimes.cs | 10 ------ .../src/System/IO/FileStatus.OSX.cs | 2 +- 8 files changed, 2 insertions(+), 75 deletions(-) diff --git a/src/libraries/Common/src/Interop/OSX/Interop.libc.cs b/src/libraries/Common/src/Interop/OSX/Interop.libc.cs index 89ba51305689f..d0b377977a1d6 100644 --- a/src/libraries/Common/src/Interop/OSX/Interop.libc.cs +++ b/src/libraries/Common/src/Interop/OSX/Interop.libc.cs @@ -25,7 +25,5 @@ internal struct AttrList [DllImport(Libraries.libc, EntryPoint = "setattrlist", SetLastError = true)] internal static unsafe extern int setattrlist(string path, AttrList* attrList, void* attrBuf, nint attrBufSize, CULong options); - - internal const uint FSOPT_NOFOLLOW = 0x00000001; } } diff --git a/src/libraries/Native/Unix/System.Native/pal_time.c b/src/libraries/Native/Unix/System.Native/pal_time.c index ef8a27a175c0f..56e23138a7b77 100644 --- a/src/libraries/Native/Unix/System.Native/pal_time.c +++ b/src/libraries/Native/Unix/System.Native/pal_time.c @@ -33,7 +33,7 @@ int32_t SystemNative_UTimensat(const char* path, TimeSpec* times) updatedTimes[1].tv_sec = (time_t)times[1].tv_sec; updatedTimes[1].tv_nsec = (long)times[1].tv_nsec; - while (CheckInterrupted(result = utimensat(AT_FDCWD, path, updatedTimes, AT_SYMLINK_NOFOLLOW))); + while (CheckInterrupted(result = utimensat(AT_FDCWD, path, updatedTimes, 0))); #else struct timeval updatedTimes[2]; updatedTimes[0].tv_sec = (long)times[0].tv_sec; diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index a8d6a052a5436..437f331be1f42 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -23,7 +23,6 @@ public abstract class BaseGetSetTimes : FileSystemTest protected abstract T GetExistingItem(); protected abstract T GetMissingItem(); - protected abstract T CreateSymlinkToItem(T item); protected abstract string GetItemPath(T item); @@ -130,40 +129,6 @@ public void SettingUpdatesPropertiesAfterAnother() }); } - [Fact] - [PlatformSpecific(~(TestPlatforms.Browser | TestPlatforms.Windows))] - public void SettingUpdatesPropertiesOnSymlink() - { - // Browser is excluded as there is only 1 effective time store. - - // This test makes sure that the times are set on the symlink itself. - // It is needed as on OSX for example, the default for most APIs is - // to follow the symlink to completion and set the time on that entry - // instead (eg. the setattrlist will do this without the flag set). - T item = CreateSymlinkToItem(GetExistingItem()); - - Assert.All(TimeFunctions(requiresRoundtripping: true), (function) => - { - // Checking that milliseconds are not dropped after setter on supported platforms. - DateTime dt = new DateTime(2004, 12, 1, 12, 3, 3, LowTemporalResolution ? 0 : 321, function.Kind); - function.Setter(item, dt); - DateTime result = function.Getter(item); - Assert.Equal(dt, result); - Assert.Equal(dt.ToLocalTime(), result.ToLocalTime()); - - // File and Directory UTC APIs treat a DateTimeKind.Unspecified as UTC whereas - // ToUniversalTime treats it as local. - if (function.Kind == DateTimeKind.Unspecified) - { - Assert.Equal(dt, result.ToUniversalTime()); - } - else - { - Assert.Equal(dt.ToUniversalTime(), result.ToUniversalTime()); - } - }); - } - [Fact] public void CanGetAllTimesAfterCreation() { diff --git a/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs index 7531ccae83019..0a23d638c138a 100644 --- a/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs @@ -51,13 +51,5 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi ((path) => Directory.GetLastWriteTimeUtc(path)), DateTimeKind.Utc); } - - protected override string CreateSymlinkToItem(string item) - { - var link = item + ".link"; - if (Directory.Exists(link)) Directory.Delete(link); - if (!MountHelper.CreateSymbolicLink(link, item, true) || !Directory.Exists(link)) throw new Exception("Could not create symlink."); - return link; - } } } diff --git a/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs index cce3d1853c1d0..2dad268a79a46 100644 --- a/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs @@ -57,15 +57,5 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi ((testDir) => testDir.LastWriteTimeUtc), DateTimeKind.Utc); } - - protected override DirectoryInfo CreateSymlinkToItem(DirectoryInfo item) - { - var link = new DirectoryInfo(item.FullName + ".link"); - if (link.Exists) link.Delete(); - bool failed = !MountHelper.CreateSymbolicLink(link.FullName, item.FullName, true); - link.Refresh(); - if (failed || !link.Exists) throw new Exception("Could not create symlink."); - return link; - } } } diff --git a/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs index 551a5115106b1..67a13bcf7df2b 100644 --- a/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs @@ -103,14 +103,6 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi DateTimeKind.Utc); } - protected override string CreateSymlinkToItem(string item) - { - var link = item + ".link"; - if (File.Exists(link)) File.Delete(link); - if (!MountHelper.CreateSymbolicLink(link, item, false) || !File.Exists(link)) throw new Exception("Could not create symlink."); - return link; - } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotInAppContainer))] // Can't read root in appcontainer [PlatformSpecific(TestPlatforms.Windows)] public void PageFileHasTimes() diff --git a/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs index 7abf30a1a20e1..d3b9764951cbf 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs @@ -103,16 +103,6 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi DateTimeKind.Utc); } - protected override FileInfo CreateSymlinkToItem(FileInfo item) - { - var link = new FileInfo(item.FullName + ".link"); - if (link.Exists) link.Delete(); - bool failed = !MountHelper.CreateSymbolicLink(link.FullName, item.FullName, false); - link.Refresh(); - if (failed || !link.Exists) throw new Exception("Could not create symlink."); - return link; - } - [ConditionalFact(nameof(HighTemporalResolution))] public void CopyToMillisecondPresent() { diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.OSX.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.OSX.cs index 7f0edeed39804..5be8f88c4fa1c 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.OSX.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.OSX.cs @@ -26,7 +26,7 @@ internal unsafe void SetCreationTime(string path, DateTimeOffset time) // 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. - Interop.Error result = (Interop.Error)Interop.libc.setattrlist(path, &attrList, &timeSpec, sizeof(Interop.Sys.TimeSpec), new CULong(Interop.libc.FSOPT_NOFOLLOW)); + Interop.Error result = (Interop.Error)Interop.libc.setattrlist(path, &attrList, &timeSpec, sizeof(Interop.Sys.TimeSpec), default(CULong)); if (result == Interop.Error.ENOTSUP) { SetCreationTime_StandardUnixImpl(path, time); From 1ca19ff3ff16d0279d133380d9e856c8d9f45b12 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Thu, 9 Sep 2021 17:24:11 +1000 Subject: [PATCH 25/40] Remove custom ENOTDIR handling, enable tests for other OSX-like platforms + move items in project file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Removed ENOTDIR handling as per https://github.com/dotnet/runtime/pull/49555#discussion_r705012504 • Enabled tests for other OSX-like platforms since the code was included on them (IsOSXLike is presumably defined at src/libraries/Common/tests/TestUtilities/System/PlatformDetection.Unix.cs). If it fails on any of these platforms, they'll be fully explicitly excluded everywhere for this PR or fixed. • Move item in project file (System.Private.CoreLib.Shared.projitems) as per https://github.com/dotnet/runtime/pull/49555#discussion_r705032218 --- .../Common/src/Interop/Unix/Interop.IOErrors.cs | 1 - .../tests/PortedCommon/IOInputs.cs | 4 ++-- .../tests/System.IO.FileSystem.Tests.csproj | 2 +- .../src/System.Private.CoreLib.Shared.projitems | 10 ++++------ 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs b/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs index 6b6e185119e08..14aeddd5adcf4 100644 --- a/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs +++ b/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs @@ -114,7 +114,6 @@ internal static Exception GetExceptionForIoErrno(ErrorInfo errorInfo, string? pa switch (errorInfo.Error) { case Error.ENOENT: - case Error.ENOTDIR: if (isDirectory) { return !string.IsNullOrEmpty(path) ? diff --git a/src/libraries/System.IO.FileSystem/tests/PortedCommon/IOInputs.cs b/src/libraries/System.IO.FileSystem/tests/PortedCommon/IOInputs.cs index 2f7aca7a8b0fd..2dec8cfcf9e1f 100644 --- a/src/libraries/System.IO.FileSystem/tests/PortedCommon/IOInputs.cs +++ b/src/libraries/System.IO.FileSystem/tests/PortedCommon/IOInputs.cs @@ -8,8 +8,8 @@ internal static class IOInputs { - public static bool SupportsSettingCreationTime => OperatingSystem.IsWindows() || OperatingSystem.IsMacOS(); - public static bool SupportsGettingCreationTime => OperatingSystem.IsWindows() || OperatingSystem.IsMacOS(); + public static bool SupportsSettingCreationTime => OperatingSystem.IsWindows() || OperatingSystem.IsMacOS() || OperatingSystem.IsIOS() || OperatingSystem.IsTvOS() || OperatingSystem.IsMacCatalyst(); + public static bool SupportsGettingCreationTime => OperatingSystem.IsWindows() || OperatingSystem.IsMacOS() || OperatingSystem.IsIOS() || OperatingSystem.IsTvOS() || OperatingSystem.IsMacCatalyst(); // 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; diff --git a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj index 5844f7330241f..4b7577eb955eb 100644 --- a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj +++ b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj @@ -2,7 +2,7 @@ true true - $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser;$(NetCoreAppCurrent)-OSX + $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser;$(NetCoreAppCurrent)-OSX;$(NetCoreAppCurrent)-iOS;$(NetCoreAppCurrent)-tvOS;$(NetCoreAppCurrent)-MacCatalyst --working-dir=/test-dir diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index 69535e8fedf99..fd0b741d6feb8 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -2055,12 +2055,6 @@ - - - Common\Interop\OSX\Interop.libc.cs - - - Common\Interop\Unix\System.Native\Interop.GetEUid.cs @@ -2109,6 +2103,10 @@ Common\Interop\OSX\Interop.Libraries.cs + + Common\Interop\OSX\Interop.libc.cs + + Date: Thu, 9 Sep 2021 17:48:29 +1000 Subject: [PATCH 26/40] Rename files according to review Rename files according to https://github.com/dotnet/runtime/pull/49555#discussion_r705073538 (and nearby comments). --- .../src/System.Private.CoreLib.Shared.projitems | 4 ++-- .../IO/{FileStatus.OSX.cs => FileStatus.SetTimes.OSX.cs} | 0 ...leStatus.OtherUnix.cs => FileStatus.SetTimes.OtherUnix.cs} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename src/libraries/System.Private.CoreLib/src/System/IO/{FileStatus.OSX.cs => FileStatus.SetTimes.OSX.cs} (100%) rename src/libraries/System.Private.CoreLib/src/System/IO/{FileStatus.OtherUnix.cs => FileStatus.SetTimes.OtherUnix.cs} (100%) diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index fd0b741d6feb8..75b7a98c43c6f 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -2053,7 +2053,7 @@ - + @@ -2106,7 +2106,7 @@ Common\Interop\OSX\Interop.libc.cs - + Date: Fri, 10 Sep 2021 18:02:30 +1000 Subject: [PATCH 27/40] Change names of functions to improve readability and refactor code This commit changes and refactors the core functions of this PR to have clearer names and to create a more logical flow of the code. It also updates some comments and fixes some oversights. Changes are based on the discussion https://github.com/dotnet/runtime/pull/49555#discussion_r705180391 (read whole conversation). --- .../src/System/IO/FileStatus.SetTimes.OSX.cs | 61 ++++++++----------- .../IO/FileStatus.SetTimes.OtherUnix.cs | 11 +++- .../src/System/IO/FileStatus.Unix.cs | 51 +++++++++------- 3 files changed, 64 insertions(+), 59 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs index 5be8f88c4fa1c..a5aec8620adf1 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs @@ -7,61 +7,50 @@ namespace System.IO { internal partial struct FileStatus { - internal unsafe void SetCreationTime(string path, DateTimeOffset time) + internal void SetCreationTime(string path, DateTimeOffset time) { - Interop.Sys.TimeSpec timeSpec = default; - - long seconds = time.ToUnixTimeSeconds(); - long nanoseconds = UnixTimeSecondsToNanoseconds(time, seconds); - - 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; - // 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. - Interop.Error result = (Interop.Error)Interop.libc.setattrlist(path, &attrList, &timeSpec, sizeof(Interop.Sys.TimeSpec), default(CULong)); - if (result == Interop.Error.ENOTSUP) - { - SetCreationTime_StandardUnixImpl(path, time); - } - else if (result == Interop.Error.SUCCESS) + Interop.Error error = SetCreationTimeCore(path, time); + + if (error == Interop.Error.ENOTSUP) { - InvalidateCaches(); + SetAccessOrWriteTimeCore(path, time, isAccessTime: false, checkCreationTime: false); } - else + else if (error != Interop.Error.SUCCESS) { - Interop.CheckIo(result, path, InitiallyDirectory); + Interop.CheckIo(error, path, InitiallyDirectory); } } - private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime) + private Interop.Error SetCreationTimeCore(string path, DateTimeOffset time) { - // Force an update so GetCreationTime is up-to-date. - InvalidateCaches(); - EnsureCachesInitialized(path); + Interop.Sys.TimeSpec timeSpec = default; - // Get the creation time here in case the modification time is less than it. - var creationTime = GetCreationTime(path); + long seconds = time.ToUnixTimeSeconds(); + long nanoseconds = UnixTimeSecondsToNanoseconds(time, seconds); - SetAccessOrWriteTimeImpl(path, time, isAccessTime); + timeSpec.TvSec = seconds; + timeSpec.TvNsec = nanoseconds; - if ((isAccessTime ? GetLastWriteTime(path) : time) < creationTime) - { - // In this case, the creation time is moved back to the modification time on OSX. - // So this code makes sure that the creation time is not changed when it shouldn't be. - SetCreationTime(path, creationTime); - } - else + 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)); + + if (error == Interop.Error.SUCCESS) { InvalidateCaches(); } + + return error; } + + private void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime) => + SetAccessOrWriteTimeCore(path, time, isAccessTime, checkCreationTime: true); } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OtherUnix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OtherUnix.cs index 08a12035e6dfc..04bb6b4c71efe 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OtherUnix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OtherUnix.cs @@ -7,7 +7,14 @@ namespace System.IO { internal partial struct FileStatus { - internal void SetCreationTime(string path, DateTimeOffset time) => SetCreationTime_StandardUnixImpl(path, time); - private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime) => SetAccessOrWriteTime_StandardUnixImpl(path, time, isAccessTime); + 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, DateTimeOffset time) => + default(Interop.Error); } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs index b69914b377c56..a4f4e84d8f1dd 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs @@ -255,20 +255,6 @@ internal DateTimeOffset GetCreationTime(ReadOnlySpan path, bool continueOn return UnixTimeToDateTimeOffset(_fileCache.CTime, _fileCache.CTimeNsec); } - private void SetCreationTime_StandardUnixImpl(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). - SetAccessOrWriteTime_StandardUnixImpl(path, time, isAccessTime: false); - } - internal DateTimeOffset GetLastAccessTime(ReadOnlySpan path, bool continueOnError = false) { EnsureCachesInitialized(path, continueOnError); @@ -294,19 +280,24 @@ private DateTimeOffset UnixTimeToDateTimeOffset(long seconds, long nanoseconds) return DateTimeOffset.FromUnixTimeSeconds(seconds).AddTicks(nanoseconds / NanosecondsPerTick).ToLocalTime(); } - private unsafe void SetAccessOrWriteTime_StandardUnixImpl(string path, DateTimeOffset time, bool isAccessTime) + private unsafe void SetAccessOrWriteTimeCore(string path, DateTimeOffset time, bool isAccessTime, bool checkCreationTime) { - // Used for access time or as a fallback on OSX, used always on other Unix platforms. + // 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 InvalidateCaches(); EnsureCachesInitialized(path); - SetAccessOrWriteTimeImpl(path, time, isAccessTime); - InvalidateCaches(); - } - private unsafe void SetAccessOrWriteTimeImpl(string path, DateTimeOffset time, bool isAccessTime) - { // we use utimes()/utimensat() to set the accessTime and writeTime Interop.Sys.TimeSpec* buf = stackalloc Interop.Sys.TimeSpec[2]; @@ -335,6 +326,24 @@ private unsafe void SetAccessOrWriteTimeImpl(string path, DateTimeOffset time, b } #endif Interop.CheckIo(Interop.Sys.UTimensat(path, buf), path, InitiallyDirectory); + + // On OSX platforms, when the modification time is less than the creation time, the creation time + // is broken; 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 + // platforms. allowFallbackToLastWriteTime is ignored on non OSX 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)); + + InvalidateCaches(); + + if (updateCreationTime) + { + Interop.Error result = SetCreationTimeCore(path, UnixTimeToDateTimeOffset(_fileCache.BirthTime, _fileCache.BirthTimeNsec)); + if (error != Interop.Error.SUCCESS && error != Interop.Error.ENOTSUP) + { + Interop.CheckIo(error, path, InitiallyDirectory); + } + } } internal long GetLength(ReadOnlySpan path, bool continueOnError = false) From 15e9fc94c895077e1f1d034b9a00a7b3e53b461e Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Sat, 11 Sep 2021 09:28:28 +1000 Subject: [PATCH 28/40] Fix the wrongly named variable and add the missing 'unsafe' keyword Fix the wrongly named variable and add the missing 'unsafe' keyword --- .../src/System/IO/FileStatus.SetTimes.OSX.cs | 2 +- .../System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs index a5aec8620adf1..802f64e257d4a 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs @@ -26,7 +26,7 @@ internal void SetCreationTime(string path, DateTimeOffset time) } } - private Interop.Error SetCreationTimeCore(string path, DateTimeOffset time) + private unsafe Interop.Error SetCreationTimeCore(string path, DateTimeOffset time) { Interop.Sys.TimeSpec timeSpec = default; diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs index a4f4e84d8f1dd..2d272833be56a 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs @@ -338,7 +338,7 @@ private unsafe void SetAccessOrWriteTimeCore(string path, DateTimeOffset time, b if (updateCreationTime) { - Interop.Error result = SetCreationTimeCore(path, UnixTimeToDateTimeOffset(_fileCache.BirthTime, _fileCache.BirthTimeNsec)); + Interop.Error error = SetCreationTimeCore(path, UnixTimeToDateTimeOffset(_fileCache.BirthTime, _fileCache.BirthTimeNsec)); if (error != Interop.Error.SUCCESS && error != Interop.Error.ENOTSUP) { Interop.CheckIo(error, path, InitiallyDirectory); From 581a66940135c0d637d59994a2c29a6e2d97edc7 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Wed, 15 Sep 2021 17:50:26 +1000 Subject: [PATCH 29/40] Better comments and minor improvements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changes according to https://github.com/dotnet/runtime/pull/49555#pullrequestreview-753354257 • Use explicit type `IEnumerable` instead of var • Use PlatformDetection rather than OperatingSystem • Move the InvalidateCaches code for creation time setting to avoid unnecessary checks • Update explanatory comments to be more useful --- .../tests/Base/BaseGetSetTimes.cs | 2 +- .../tests/PortedCommon/IOInputs.cs | 4 ++-- .../src/System/IO/FileStatus.SetTimes.OSX.cs | 16 +++++++++------- .../src/System/IO/FileStatus.Unix.cs | 12 ++++++++---- 4 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index 437f331be1f42..7de7fbf954bc3 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -101,7 +101,7 @@ public void SettingUpdatesPropertiesAfterAnother() // [0] = (access, write, False), [1] = (access, write, True), // [2] = (write, access, False), [3] = (write, access, True) - var timeFunctionsUtc = TimeFunctions(requiresRoundtripping: true).Where((f) => f.Kind == DateTimeKind.Utc); + IEnumerable 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) => { diff --git a/src/libraries/System.IO.FileSystem/tests/PortedCommon/IOInputs.cs b/src/libraries/System.IO.FileSystem/tests/PortedCommon/IOInputs.cs index 2dec8cfcf9e1f..677a027a732ea 100644 --- a/src/libraries/System.IO.FileSystem/tests/PortedCommon/IOInputs.cs +++ b/src/libraries/System.IO.FileSystem/tests/PortedCommon/IOInputs.cs @@ -8,8 +8,8 @@ internal static class IOInputs { - public static bool SupportsSettingCreationTime => OperatingSystem.IsWindows() || OperatingSystem.IsMacOS() || OperatingSystem.IsIOS() || OperatingSystem.IsTvOS() || OperatingSystem.IsMacCatalyst(); - public static bool SupportsGettingCreationTime => OperatingSystem.IsWindows() || OperatingSystem.IsMacOS() || OperatingSystem.IsIOS() || OperatingSystem.IsTvOS() || OperatingSystem.IsMacCatalyst(); + 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; diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs index 802f64e257d4a..03b65a5d5a655 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs @@ -14,13 +14,20 @@ internal void SetCreationTime(string path, DateTimeOffset time) // 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. Interop.Error error = SetCreationTimeCore(path, time); - if (error == Interop.Error.ENOTSUP) + if (error == Interop.Error.SUCCESS) + { + InvalidateCaches(); + } + else if (error == Interop.Error.ENOTSUP) { SetAccessOrWriteTimeCore(path, time, isAccessTime: false, checkCreationTime: false); } - else if (error != Interop.Error.SUCCESS) + else { Interop.CheckIo(error, path, InitiallyDirectory); } @@ -42,11 +49,6 @@ private unsafe Interop.Error SetCreationTimeCore(string path, DateTimeOffset tim Interop.Error error = (Interop.Error)Interop.libc.setattrlist(path, &attrList, &timeSpec, sizeof(Interop.Sys.TimeSpec), default(CULong)); - if (error == Interop.Error.SUCCESS) - { - InvalidateCaches(); - } - return error; } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs index 9c2166084d932..0ae995b8cdbdf 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs @@ -317,10 +317,14 @@ private unsafe void SetAccessOrWriteTimeCore(string path, DateTimeOffset time, b #endif Interop.CheckIo(Interop.Sys.UTimensat(path, buf), path, InitiallyDirectory); - // On OSX platforms, when the modification time is less than the creation time, the creation time - // is broken; 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 - // platforms. allowFallbackToLastWriteTime is ignored on non OSX platforms. + // 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)); From 0623c427e16939ab0dae9763300dff5d3e7e223c Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Sat, 9 Oct 2021 19:15:48 +1100 Subject: [PATCH 30/40] Add additional tests and improvements - Use tuple destruction to swap variables - Add SettingOneTimeMaintainsOtherDefaultTimes and AllDefaultTimesAreSame test as per https://github.com/dotnet/runtime/pull/49555#discussion_r712787328 - Change TFM(/s) as per https://github.com/dotnet/runtime/pull/49555#discussion_r717042126 - Avoid unnecessary call to `UnixTimeToDateTimeOffset` as per https://github.com/dotnet/runtime/pull/49555#discussion_r712606875 - Use InvalidOperationException as per https://github.com/dotnet/runtime/pull/49555#discussion_r717040240 --- .../tests/Base/BaseGetSetTimes.cs | 70 ++++++++++++++++++- .../tests/System.IO.FileSystem.Tests.csproj | 2 +- .../src/System/IO/FileStatus.SetTimes.OSX.cs | 9 ++- .../IO/FileStatus.SetTimes.OtherUnix.cs | 4 +- .../src/System/IO/FileStatus.Unix.cs | 2 +- 5 files changed, 75 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index 7de7fbf954bc3..1344c08763366 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -115,9 +115,7 @@ public void SettingUpdatesPropertiesAfterAnother() DateTime dt3 = new DateTime(2000, 12, 1, 12, 3, 3, LowTemporalResolution ? 0 : 321, DateTimeKind.Utc); if (reverse) //reverse the order of setting dates { - var swap = dt3; - dt3 = dt1; - dt1 = swap; + (dt1, dt3) = (dt3, dt1); } function1.Setter(item, dt1); function2.Setter(item, dt2); @@ -129,6 +127,72 @@ public void SettingUpdatesPropertiesAfterAnother() }); } + [Fact] + [PlatformSpecific(~TestPlatforms.Browser)] + public void SettingOneTimeMaintainsOtherDefaultTimes() + { + // Browser is excluded as there is only 1 effective time store. + + T item = GetExistingItem(); + + // Only bother with UTC as we're only interested in 1 function per time value + IEnumerable functions = TimeFunctions(requiresRoundtripping: true).Where((f) => f.Kind == DateTimeKind.Utc); + + Assert.All(functions, (function) => + { + // Here we are resetting all of the times to 'default' and then storing what its value is + IEnumerable ResetTimes() + { + foreach (TimeFunction function2 in functions) + { + function2.Setter(item, default(DateTime)); + yield return function2.GetTime(item); + } + } + DateTime[] defaultTimeValues = ResetTimes().ToArray(); + + // Now we set that specific time value + function.SetTime(item, DateTime.UtcNow); + + // Now we compare the times to what they should be, except for the function that we're currently testing + int defaultTimeValuesIndex = 0; + Assert.All(functions, (function2) => + { + if (function != function2) + { + Assert.Equal(function2.GetTime(item), defaultTimeValues[defaultTimeValuesIndex]); + } + defaultTimeValuesIndex++; + }); + }); + } + + [Fact] + [PlatformSpecific(~TestPlatforms.Browser)] + public void AllDefaultTimesAreSame() + { + // Browser is excluded as there is only 1 effective time store. + + T item = GetExistingItem(); + + // Only bother with UTC as we're only interested in 1 function per time value + IEnumerable functions = TimeFunctions(requiresRoundtripping: true).Where((f) => f.Kind == DateTimeKind.Utc); + + // Here we are resetting all of the times to 'default' and then storing what its value is + IEnumerable ResetTimes() + { + foreach (TimeFunction function2 in functions) + { + function2.Setter(item, default(DateTime)); + yield return function2.GetTime(item); + } + } + DateTime[] defaultTimeValues = ResetTimes().ToArray(); + + // Here we are comparing it (it's easy to just compare the first item to the rest) + Assert.Equal(Enumerable.Repeat(defaultTimeValues[0], defaultTimeValues.Length), defaultTimeValues); + } + [Fact] public void CanGetAllTimesAfterCreation() { diff --git a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj index fc28fa2098854..8b492f5c41427 100644 --- a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj +++ b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj @@ -2,7 +2,7 @@ true true - $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser;$(NetCoreAppCurrent)-OSX;$(NetCoreAppCurrent)-iOS;$(NetCoreAppCurrent)-tvOS;$(NetCoreAppCurrent)-MacCatalyst + $(NetCoreAppCurrent) --working-dir=/test-dir diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs index 03b65a5d5a655..ba4c71c00a48f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs @@ -17,7 +17,9 @@ internal void SetCreationTime(string path, DateTimeOffset time) // 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. - Interop.Error error = SetCreationTimeCore(path, time); + long seconds = time.ToUnixTimeSeconds(); + long nanoseconds = UnixTimeSecondsToNanoseconds(time, seconds); + Interop.Error error = SetCreationTimeCore(path, seconds, nanoseconds); if (error == Interop.Error.SUCCESS) { @@ -33,13 +35,10 @@ internal void SetCreationTime(string path, DateTimeOffset time) } } - private unsafe Interop.Error SetCreationTimeCore(string path, DateTimeOffset time) + private unsafe Interop.Error SetCreationTimeCore(string path, long seconds, long nanoseconds) { Interop.Sys.TimeSpec timeSpec = default; - long seconds = time.ToUnixTimeSeconds(); - long nanoseconds = UnixTimeSecondsToNanoseconds(time, seconds); - timeSpec.TvSec = seconds; timeSpec.TvNsec = nanoseconds; diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OtherUnix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OtherUnix.cs index 04bb6b4c71efe..d9d9b1eeb7c92 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OtherUnix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OtherUnix.cs @@ -14,7 +14,7 @@ private void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAcces 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, DateTimeOffset time) => - default(Interop.Error); + private Interop.Error SetCreationTimeCore(string path, long seconds, long nanoseconds) => + throw new InvalidOperationException(); } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs index 0ae995b8cdbdf..3171852eba1a6 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs @@ -332,7 +332,7 @@ private unsafe void SetAccessOrWriteTimeCore(string path, DateTimeOffset time, b if (updateCreationTime) { - Interop.Error error = SetCreationTimeCore(path, UnixTimeToDateTimeOffset(_fileCache.BirthTime, _fileCache.BirthTimeNsec)); + Interop.Error error = SetCreationTimeCore(path, _fileCache.BirthTime, _fileCache.BirthTimeNsec); if (error != Interop.Error.SUCCESS && error != Interop.Error.ENOTSUP) { Interop.CheckIo(error, path, InitiallyDirectory); From 1ac43cc7427a2c56ffb9dd9c5105181542f80da0 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Sun, 10 Oct 2021 07:02:25 +1100 Subject: [PATCH 31/40] Fix typos --- .../System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index 1344c08763366..38f2b16b390bf 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -146,13 +146,13 @@ IEnumerable ResetTimes() foreach (TimeFunction function2 in functions) { function2.Setter(item, default(DateTime)); - yield return function2.GetTime(item); + yield return function2.Getter(item); } } DateTime[] defaultTimeValues = ResetTimes().ToArray(); // Now we set that specific time value - function.SetTime(item, DateTime.UtcNow); + function.Setter(item, DateTime.UtcNow); // Now we compare the times to what they should be, except for the function that we're currently testing int defaultTimeValuesIndex = 0; @@ -160,7 +160,7 @@ IEnumerable ResetTimes() { if (function != function2) { - Assert.Equal(function2.GetTime(item), defaultTimeValues[defaultTimeValuesIndex]); + Assert.Equal(function2.Getter(item), defaultTimeValues[defaultTimeValuesIndex]); } defaultTimeValuesIndex++; }); @@ -184,7 +184,7 @@ IEnumerable ResetTimes() foreach (TimeFunction function2 in functions) { function2.Setter(item, default(DateTime)); - yield return function2.GetTime(item); + yield return function2.Getter(item); } } DateTime[] defaultTimeValues = ResetTimes().ToArray(); From 819022052438c9f5ca833d80de9b82117e73d7c6 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Mon, 11 Oct 2021 11:42:44 +1100 Subject: [PATCH 32/40] Revert to $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser Revert to using $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser since $(NetCoreAppCurrent) failed; I wouldn't be surprised if this failed too. --- .../tests/System.IO.FileSystem.Tests.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj index b0dbb39350f9f..1d523e70a1c8d 100644 --- a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj +++ b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj @@ -2,7 +2,7 @@ true true - $(NetCoreAppCurrent) + $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser --working-dir=/test-dir From 8aa9685ec2e3b1abf2881a5869bb31a6c1766c95 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Mon, 11 Oct 2021 12:52:23 +1100 Subject: [PATCH 33/40] Remove SettingOneTimeMaintainsOtherDefaultTimes and AllDefaultTimesAreSame tests Remove SettingOneTimeMaintainsOtherDefaultTimes and AllDefaultTimesAreSame tests since they don't work on windows because it is apparently an invalid time. Additionally, the function SettingOneTimeMaintainsOtherDefaultTimes wasn't working and needed to be fixed. --- .../tests/Base/BaseGetSetTimes.cs | 66 ------------------- 1 file changed, 66 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index 38f2b16b390bf..7fc458f0c3433 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -127,72 +127,6 @@ public void SettingUpdatesPropertiesAfterAnother() }); } - [Fact] - [PlatformSpecific(~TestPlatforms.Browser)] - public void SettingOneTimeMaintainsOtherDefaultTimes() - { - // Browser is excluded as there is only 1 effective time store. - - T item = GetExistingItem(); - - // Only bother with UTC as we're only interested in 1 function per time value - IEnumerable functions = TimeFunctions(requiresRoundtripping: true).Where((f) => f.Kind == DateTimeKind.Utc); - - Assert.All(functions, (function) => - { - // Here we are resetting all of the times to 'default' and then storing what its value is - IEnumerable ResetTimes() - { - foreach (TimeFunction function2 in functions) - { - function2.Setter(item, default(DateTime)); - yield return function2.Getter(item); - } - } - DateTime[] defaultTimeValues = ResetTimes().ToArray(); - - // Now we set that specific time value - function.Setter(item, DateTime.UtcNow); - - // Now we compare the times to what they should be, except for the function that we're currently testing - int defaultTimeValuesIndex = 0; - Assert.All(functions, (function2) => - { - if (function != function2) - { - Assert.Equal(function2.Getter(item), defaultTimeValues[defaultTimeValuesIndex]); - } - defaultTimeValuesIndex++; - }); - }); - } - - [Fact] - [PlatformSpecific(~TestPlatforms.Browser)] - public void AllDefaultTimesAreSame() - { - // Browser is excluded as there is only 1 effective time store. - - T item = GetExistingItem(); - - // Only bother with UTC as we're only interested in 1 function per time value - IEnumerable functions = TimeFunctions(requiresRoundtripping: true).Where((f) => f.Kind == DateTimeKind.Utc); - - // Here we are resetting all of the times to 'default' and then storing what its value is - IEnumerable ResetTimes() - { - foreach (TimeFunction function2 in functions) - { - function2.Setter(item, default(DateTime)); - yield return function2.Getter(item); - } - } - DateTime[] defaultTimeValues = ResetTimes().ToArray(); - - // Here we are comparing it (it's easy to just compare the first item to the rest) - Assert.Equal(Enumerable.Repeat(defaultTimeValues[0], defaultTimeValues.Length), defaultTimeValues); - } - [Fact] public void CanGetAllTimesAfterCreation() { From d8ec4fb182787f96e59991663dff74fda02e732c Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Tue, 19 Oct 2021 13:56:49 +1100 Subject: [PATCH 34/40] Fix: setattrlist can only return 0 or -1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As per https://github.com/dotnet/runtime/pull/49555#discussion_r731451808, use GetLastErrorInfo to get the error when setattrlist indicates that there is one. Co-authored-by: David Cantú --- .../src/System/IO/FileStatus.SetTimes.OSX.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs index ba4c71c00a48f..3bc7109f41ad5 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs @@ -46,7 +46,10 @@ private unsafe Interop.Error SetCreationTimeCore(string path, long seconds, long 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)); + Interop.Error error = + Interop.libc.setattrlist(path, &attrList, &timeSpec, sizeof(Interop.Sys.TimeSpec), default(CULong)) == 0 ? + Interop.Error.Success : + Interop.Sys.GetLastErrorInfo(); return error; } From 57a1229be67ed240351db968ebbb92ffa15b5a2c Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Tue, 19 Oct 2021 14:10:22 +1100 Subject: [PATCH 35/40] Update code comment for SettingUpdatesPropertiesAfterAnother test The comment is now clearer and grammatically correct :) --- .../System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index 7fc458f0c3433..495bf359e4eed 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -86,11 +86,11 @@ public void SettingUpdatesPropertiesAfterAnother() // 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. - // There were issues while developing this PR with specific orders of changes, so - // this code should almost fully eliminate any possibilities of that in the future - // 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). + // This test is required as some apis change more dates than would be desired (eg. + // utimensat). There were issues related to the order in which the dates are set, + // so this test should almost fully eliminate any possibilities of that in the + // future 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), From 9c6ab2be55fb528361ec29c4f8d2e201d8f34fcb Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Tue, 19 Oct 2021 14:11:32 +1100 Subject: [PATCH 36/40] Update code comment for SettingUpdatesPropertiesAfterAnother test Describe how utimensat change more dates than would be desired --- .../System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index 495bf359e4eed..9b36114f2a5cb 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -87,10 +87,10 @@ public void SettingUpdatesPropertiesAfterAnother() // 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 than would be desired (eg. - // utimensat). There were issues related to the order in which the dates are set, - // so this test should almost fully eliminate any possibilities of that in the - // future 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). + // utimensat sets creation time too). There were issues related to the order in + // which the dates are set, so this test should almost fully eliminate any + // possibilities of that in the future 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), From 1fdc325c56aa15ff6fd86ab723a40fb4ba56c19c Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Tue, 19 Oct 2021 14:15:59 +1100 Subject: [PATCH 37/40] Update comment for SettingUpdatesPropertiesAfterAnother for consistency Update for consistency with the comment in FileStatus.Unix.cs --- .../System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index 9b36114f2a5cb..fdda4c521a679 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -87,8 +87,8 @@ public void SettingUpdatesPropertiesAfterAnother() // 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 than would be desired (eg. - // utimensat sets creation time too). There were issues related to the order in - // which the dates are set, so this test should almost fully eliminate any + // utimes()/utimensat() set creation time too). There were issues related to the + // order in which the dates are set, so this test should almost fully eliminate any // possibilities of that in the future 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). From 6aa185276d2f86b14c95a55480e59123fa0ce969 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Tue, 19 Oct 2021 14:37:34 +1100 Subject: [PATCH 38/40] Fixes for compilation and update to comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Fix trailing spaces and incorrect capitalisation in FileStatus.SetTimes.OSX.cs • Add more info to the comment in the SettingUpdatesPropertiesAfterAnother test --- .../System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs | 10 ++++++---- .../src/System/IO/FileStatus.SetTimes.OSX.cs | 6 +++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index fdda4c521a679..7ac1d04f2a56f 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -87,10 +87,12 @@ public void SettingUpdatesPropertiesAfterAnother() // 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 than would be desired (eg. - // utimes()/utimensat() set creation time too). There were issues related to the - // order in which the dates are set, so this test should almost fully eliminate any - // possibilities of that in the future 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). + // utimes()/utimensat() set the write and access times, but as a side effect of + // the implementation, it sets creation time too when the write time is less than + // the creation time). There were issues related to the order in which the dates are + // set, so this test should almost fully eliminate any possibilities of that in the + // future 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), diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs index 3bc7109f41ad5..4efa81721fbe1 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs @@ -46,9 +46,9 @@ private unsafe Interop.Error SetCreationTimeCore(string path, long seconds, long attrList.bitmapCount = Interop.libc.AttrList.ATTR_BIT_MAP_COUNT; attrList.commonAttr = Interop.libc.AttrList.ATTR_CMN_CRTIME; - Interop.Error error = - Interop.libc.setattrlist(path, &attrList, &timeSpec, sizeof(Interop.Sys.TimeSpec), default(CULong)) == 0 ? - Interop.Error.Success : + Interop.Error error = + Interop.libc.setattrlist(path, &attrList, &timeSpec, sizeof(Interop.Sys.TimeSpec), default(CULong)) == 0 ? + Interop.Error.SUCCESS : Interop.Sys.GetLastErrorInfo(); return error; From 31db20b6ed85a635b982ee1e29cafe3ed2c30742 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Tue, 19 Oct 2021 15:33:29 +1100 Subject: [PATCH 39/40] Update FileStatus.SetTimes.OSX.cs Use the Error property --- .../src/System/IO/FileStatus.SetTimes.OSX.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs index 4efa81721fbe1..db485033649ca 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.SetTimes.OSX.cs @@ -49,7 +49,7 @@ private unsafe Interop.Error SetCreationTimeCore(string path, long seconds, long Interop.Error error = Interop.libc.setattrlist(path, &attrList, &timeSpec, sizeof(Interop.Sys.TimeSpec), default(CULong)) == 0 ? Interop.Error.SUCCESS : - Interop.Sys.GetLastErrorInfo(); + Interop.Sys.GetLastErrorInfo().Error; return error; } From c7c11d2c44d926293c4c070c146ce992276dc734 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Wed, 20 Oct 2021 13:54:26 +1100 Subject: [PATCH 40/40] Move comments and add explicit types to SettingUpdatesPropertiesAfterAnother test Move comments and add explicit types to SettingUpdatesPropertiesAfterAnother test --- .../tests/Base/BaseGetSetTimes.cs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index 7ac1d04f2a56f..8df736132ea3f 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -72,11 +72,9 @@ public void SettingUpdatesProperties() } [Fact] - [PlatformSpecific(~TestPlatforms.Browser)] + [PlatformSpecific(~TestPlatforms.Browser)] // Browser is excluded as there is only 1 effective time store. public void SettingUpdatesPropertiesAfterAnother() { - // Browser is excluded as there is only 1 effective time store. - T item = GetExistingItem(); // These linq calls make an IEnumerable of pairs of functions that are not identical @@ -104,12 +102,12 @@ public void SettingUpdatesPropertiesAfterAnother() // [2] = (write, access, False), [3] = (write, access, True) IEnumerable timeFunctionsUtc = TimeFunctions(requiresRoundtripping: true).Where((f) => f.Kind == DateTimeKind.Utc); - var booleanArray = new bool[] { false, true }; + bool[] 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) => { - var function1 = functions.x; - var function2 = functions.y; - var reverse = functions.reverse; + TimeFunction function1 = functions.x; + TimeFunction function2 = functions.y; + bool reverse = functions.reverse; // Checking that milliseconds are not dropped after setter. DateTime dt1 = new DateTime(2002, 12, 1, 12, 3, 3, LowTemporalResolution ? 0 : 321, DateTimeKind.Utc);