From 270c526f219e90bc1c325c707fd7cd1850a89ea3 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Wed, 12 May 2021 20:14:31 +1000 Subject: [PATCH 01/19] Implement most of the fix for #38824 Reverts the changes in the seperate PR https://github.com/dotnet/runtime/commit/a617a01195b4dad3cb5f300962ad843b46d4f175 to fix #38824. Does not re-enable the test because that relies on #49555, will add a seperate commit to whichever is merged last to enable the SettingUpdatesPropertiesOnSymlink test. --- .../src/Interop/Windows/Kernel32/Interop.FileOperations.cs | 1 + .../System.IO.FileSystem/src/System/IO/FileSystem.Windows.cs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) 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 f2a3872299d2c1..8ff31bb3a3b695 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 static 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/FileSystem.Windows.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Windows.cs index 5b5f9a86bc7446..c83b8463ae7c52 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) { From 952a168ab2046fc2450cc9a3d797d10de1248589 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Thu, 21 Oct 2021 10:01:47 +1100 Subject: [PATCH 02/19] Most of the code for PR #52639 to fix #38824 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Deal with merge conflicts • Add FSOPT_NOFOLLOW for macOS and use it in setattrlist • Use AT_SYMLINK_NOFOLLOW with utimensat (note: there doesn't seem to be an equivalent for utimes) • Add SettingUpdatesPropertiesOnSymlink test to test if it actually sets it on the symlink itself (to the best that we can anyway ie. write + read the times) • Specify FILE_FLAG_OPEN_REPARSE_POINT for CreateFile in FileSystem.Windows.cs (is there any other CreateFile calls that need this?) --- .../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 | 2 ++ .../tests/DirectoryInfo/GetSetTimes.cs | 2 ++ .../tests/File/GetSetTimes.cs | 2 ++ .../tests/FileInfo/GetSetTimes.cs | 2 ++ .../src/System/IO/FileStatus.SetTimes.OSX.cs | 2 +- .../src/System/IO/FileSystem.Windows.cs | 2 +- 9 files changed, 48 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 d0b377977a1d6e..89ba51305689fb 100644 --- a/src/libraries/Common/src/Interop/OSX/Interop.libc.cs +++ b/src/libraries/Common/src/Interop/OSX/Interop.libc.cs @@ -25,5 +25,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, 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 56e23138a7b77e..ef8a27a175c0fc 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/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index 8df736132ea3fa..3417bdcc5f95f8 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); @@ -127,6 +128,40 @@ public void SettingUpdatesPropertiesAfterAnother() }); } + [Fact] + [PlatformSpecific(~(TestPlatforms.Browser))] // Browser is excluded as there is only 1 effective time store. + public void SettingUpdatesPropertiesOnSymlink() + { + // This test is in this class since it needs all of the time functions. + // 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). + // It is also the same case on unix, with the utimensat function. + 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, 9, 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 0a23d638c138af..a4abde6f05674b 100644 --- a/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs @@ -51,5 +51,7 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi ((path) => Directory.GetLastWriteTimeUtc(path)), DateTimeKind.Utc); } + + public string CreateSymlinkToItem(string item) => Directory.CreateSymbolicLink(item + ".link", item).FullName; } } diff --git a/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs index 2dad268a79a460..9e272ddadbd64d 100644 --- a/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs @@ -57,5 +57,7 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi ((testDir) => testDir.LastWriteTimeUtc), DateTimeKind.Utc); } + + public DirectoryInfo CreateSymlinkToItem(DirectoryInfo item) => (DirectoryInfo)Directory.CreateSymbolicLink(item.FullName + ".link", item.FullName); } } diff --git a/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs index 63373576e0be13..c8d66b8fc53774 100644 --- a/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs @@ -103,6 +103,8 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi DateTimeKind.Utc); } + public string CreateSymlinkToItem(string item) => File.CreateSymbolicLink(item + ".link", item).FullName; + [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 d3b9764951cbfd..016616e3f7962d 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs @@ -103,6 +103,8 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi DateTimeKind.Utc); } + public FileInfo CreateSymlinkToItem(FileInfo item) => (FileInfo)File.CreateSymbolicLink(item.FullName + ".link", item.FullName); + [ConditionalFact(nameof(HighTemporalResolution))] public void CopyToMillisecondPresent() { 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 db485033649caa..23c46bae4a0125 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 @@ -47,7 +47,7 @@ private unsafe Interop.Error SetCreationTimeCore(string path, long seconds, long 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.libc.setattrlist(path, &attrList, &timeSpec, sizeof(Interop.Sys.TimeSpec), new CULong(Interop.libc.FSOPT_NOFOLLOW)) == 0 ? Interop.Error.SUCCESS : Interop.Sys.GetLastErrorInfo().Error; diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs index b94d7eeb74fafb..a73558a3f7b53d 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs @@ -155,7 +155,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 | Interop.Kernel32.FileOperations.FILE_FLAG_OPEN_REPARSE_POINT) : Interop.Kernel32.FileOperations.FILE_FLAG_OPEN_REPARSE_POINT); if (handle.IsInvalid) { From abdbbcb3ae8fed30f6056d074021db76d4f4ba7a Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Thu, 21 Oct 2021 11:11:35 +1100 Subject: [PATCH 03/19] Remove additional FILE_FLAG_OPEN_REPARSE_POINT I added FILE_FLAG_OPEN_REPARSE_POINT before and it was then added in another PR, this removes the duplicate entry. --- .../src/Interop/Windows/Kernel32/Interop.FileOperations.cs | 1 - 1 file changed, 1 deletion(-) 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 e564ffcae5550d..cc4896c1c52e48 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FileOperations.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FileOperations.cs @@ -21,7 +21,6 @@ internal static partial class FileOperations internal const int FILE_FLAG_FIRST_PIPE_INSTANCE = 0x00080000; internal const int FILE_FLAG_OPEN_REPARSE_POINT = 0x00200000; internal const int FILE_FLAG_OVERLAPPED = 0x40000000; - internal const int FILE_FLAG_OPEN_REPARSE_POINT = 0x00200000; internal const int FILE_LIST_DIRECTORY = 0x0001; } From 7dc18750e6e4dcaf02ff62004487c95efbe0d03d Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Thu, 21 Oct 2021 11:55:01 +1100 Subject: [PATCH 04/19] Add missing override keywords Add missing override keywords for abstract members in the tests. --- .../System.IO.FileSystem/tests/Directory/GetSetTimes.cs | 2 +- .../System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs | 2 +- src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs | 2 +- .../System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs index a4abde6f05674b..d851814599f096 100644 --- a/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs @@ -52,6 +52,6 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi DateTimeKind.Utc); } - public string CreateSymlinkToItem(string item) => Directory.CreateSymbolicLink(item + ".link", item).FullName; + public override string CreateSymlinkToItem(string item) => Directory.CreateSymbolicLink(item + ".link", item).FullName; } } diff --git a/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs index 9e272ddadbd64d..3dd170830d8465 100644 --- a/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs @@ -58,6 +58,6 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi DateTimeKind.Utc); } - public DirectoryInfo CreateSymlinkToItem(DirectoryInfo item) => (DirectoryInfo)Directory.CreateSymbolicLink(item.FullName + ".link", item.FullName); + public override DirectoryInfo CreateSymlinkToItem(DirectoryInfo item) => (DirectoryInfo)Directory.CreateSymbolicLink(item.FullName + ".link", item.FullName); } } diff --git a/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs index c8d66b8fc53774..7158fce3e39c2e 100644 --- a/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs @@ -103,7 +103,7 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi DateTimeKind.Utc); } - public string CreateSymlinkToItem(string item) => File.CreateSymbolicLink(item + ".link", item).FullName; + public override string CreateSymlinkToItem(string item) => File.CreateSymbolicLink(item + ".link", item).FullName; [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotInAppContainer))] // Can't read root in appcontainer [PlatformSpecific(TestPlatforms.Windows)] diff --git a/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs index 016616e3f7962d..bd484c3089b018 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs @@ -103,7 +103,7 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi DateTimeKind.Utc); } - public FileInfo CreateSymlinkToItem(FileInfo item) => (FileInfo)File.CreateSymbolicLink(item.FullName + ".link", item.FullName); + public override FileInfo CreateSymlinkToItem(FileInfo item) => (FileInfo)File.CreateSymbolicLink(item.FullName + ".link", item.FullName); [ConditionalFact(nameof(HighTemporalResolution))] public void CopyToMillisecondPresent() From 41d23e2beef496204153b4f61e77506a41e43e48 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Thu, 21 Oct 2021 12:34:54 +1100 Subject: [PATCH 05/19] Fix access modifiers Fix access modifiers for tests - were meant to be protected rather than public --- .../System.IO.FileSystem/tests/Directory/GetSetTimes.cs | 2 +- .../System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs | 2 +- src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs | 2 +- .../System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs index d851814599f096..05b9a0bba90669 100644 --- a/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs @@ -52,6 +52,6 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi DateTimeKind.Utc); } - public override string CreateSymlinkToItem(string item) => Directory.CreateSymbolicLink(item + ".link", item).FullName; + protected override string CreateSymlinkToItem(string item) => Directory.CreateSymbolicLink(item + ".link", item).FullName; } } diff --git a/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs index 3dd170830d8465..db76f07916dff5 100644 --- a/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs @@ -58,6 +58,6 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi DateTimeKind.Utc); } - public override DirectoryInfo CreateSymlinkToItem(DirectoryInfo item) => (DirectoryInfo)Directory.CreateSymbolicLink(item.FullName + ".link", item.FullName); + protected override DirectoryInfo CreateSymlinkToItem(DirectoryInfo item) => (DirectoryInfo)Directory.CreateSymbolicLink(item.FullName + ".link", item.FullName); } } diff --git a/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs index 7158fce3e39c2e..0e998af0cde2ad 100644 --- a/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs @@ -103,7 +103,7 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi DateTimeKind.Utc); } - public override string CreateSymlinkToItem(string item) => File.CreateSymbolicLink(item + ".link", item).FullName; + protected override string CreateSymlinkToItem(string item) => File.CreateSymbolicLink(item + ".link", item).FullName; [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotInAppContainer))] // Can't read root in appcontainer [PlatformSpecific(TestPlatforms.Windows)] diff --git a/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs index bd484c3089b018..887f5e5c029867 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs @@ -103,7 +103,7 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi DateTimeKind.Utc); } - public override FileInfo CreateSymlinkToItem(FileInfo item) => (FileInfo)File.CreateSymbolicLink(item.FullName + ".link", item.FullName); + protected override FileInfo CreateSymlinkToItem(FileInfo item) => (FileInfo)File.CreateSymbolicLink(item.FullName + ".link", item.FullName); [ConditionalFact(nameof(HighTemporalResolution))] public void CopyToMillisecondPresent() From 0a87835c562d0f1a9f4dd65b231635b1a63cd2a2 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Tue, 26 Oct 2021 11:48:24 +1100 Subject: [PATCH 06/19] Add more symlink tests, rearrange files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Move symlink creation api to better spot in non-base files • Add IsDirectory property for some of the new tests • Change abstract symlink api to CreateSymlink that accepts path and target • Create a CreateSymlinkToItem method that creates a symlink to an item that may be relative that uses the new/modified abstract CreateSymlink method • Add SettingUpdatesPropertiesCore to avoid code duplication • Add tests for the following variants: normal/symlink, target exists/doesn't exist, absolute/relative target • Exclude nonexistent symlink target tests on Unix for Directories since they are counted as files --- .../tests/Base/BaseGetSetTimes.cs | 124 ++++++++++++------ .../tests/Directory/GetSetTimes.cs | 6 +- .../tests/DirectoryInfo/GetSetTimes.cs | 6 +- .../tests/File/GetSetTimes.cs | 6 +- .../tests/FileInfo/GetSetTimes.cs | 6 +- 5 files changed, 100 insertions(+), 48 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index 3417bdcc5f95f8..f2643d42168a60 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -23,7 +23,26 @@ public abstract class BaseGetSetTimes : FileSystemTest protected abstract T GetExistingItem(); protected abstract T GetMissingItem(); - protected abstract T CreateSymlinkToItem(T item); + + protected abstract T CreateSymlink(string path, string pathToTarget); + protected abstract bool IsDirectory { get; } + + protected T CreateSymlinkToItem(T item, bool isRelative) + { + // Creates a Symlink to 'item' (may or may not exist) that is optionally + // a relative path rather than an absolute path. + var itemPath = GetItemPath(item); + var linkPath = itemPath + ".link"; + + // If a symlink was a directory on unix and references a non-existent + // path, then it manifests as a file. If a symlink was a file that now + // has a target that is a directory, it manifests as a directory. + if (Directory.Exists(linkPath)) Directory.Delete(linkPath); + else if (File.Exists(linkPath)) File.Exists(linkPath); + + string target = isRelative ? Path.GetFileName(itemPath) : itemPath; + return CreateSymlink(linkPath, target); + } protected abstract string GetItemPath(T item); @@ -44,16 +63,14 @@ public static TimeFunction Create(SetTime setter, GetTime getter, DateTimeKind k public DateTimeKind Kind => Item3; } - [Fact] - public void SettingUpdatesProperties() + private void SettingUpdatesPropertiesCore(T item, int year) { - T item = GetExistingItem(); - + // Pass the year as a parameter so it changes between different tests (in case of the same file) Assert.All(TimeFunctions(requiresRoundtripping: true), (function) => { // Checking that milliseconds are not dropped after setter. // Emscripten drops milliseconds in Browser - DateTime dt = new DateTime(2014, 12, 1, 12, 3, 3, LowTemporalResolution ? 0 : 321, function.Kind); + DateTime dt = new DateTime(year, 12, 1, 12, 3, 3, LowTemporalResolution ? 0 : 321, function.Kind); function.Setter(item, dt); DateTime result = function.Getter(item); Assert.Equal(dt, result); @@ -72,6 +89,67 @@ public void SettingUpdatesProperties() }); } + [Fact] + public void SettingUpdatesProperties() + { + T item = GetExistingItem(); + SettingUpdatesPropertiesCore(item, 2014); + } + + [Fact] + public void SettingUpdatesPropertiesOnSymlink() + { + // This test is in this class since it needs all of the time functions. + // 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). + // It is also the same case on unix, with the utimensat function. + T item = CreateSymlinkToItem(GetExistingItem(), false); + SettingUpdatesPropertiesCore(item, 2004); + } + + [Fact] + public void SettingUpdatesPropertiesOnNonexistentSymlink() + { + if (IsDirectory && !OperatingSystem.IsWindows()) + { + // On Unix, symlinks that are created as 'directories' need their + // directory target to exist for it to count as a directory symlink. + return; + } + + // Same as the above SettingUpdatesPropertiesOnSymlink test, + // except that the target of the symlink doesn't exist. + T item = CreateSymlinkToItem(GetMissingItem(), false); + SettingUpdatesPropertiesCore(item, 2005); + } + + [Fact] + public void SettingUpdatesPropertiesOnRelativeSymlink() + { + // Same as the SettingUpdatesPropertiesOnSymlink function, + // except that the symlink target is relative rather than absolute. + T item = CreateSymlinkToItem(GetExistingItem(), true); + SettingUpdatesPropertiesCore(item, 2006); + } + + [Fact] + public void SettingUpdatesPropertiesOnRelativeNonexistentSymlink() + { + if (IsDirectory && !OperatingSystem.IsWindows()) + { + // On Unix, symlinks that are created as 'directories' need their + // directory target to exist for it to count as a directory symlink. + return; + } + + // Same as the SettingUpdatesPropertiesOnNonexistentSymlink function, + // except that the symlink target is relative rather than absolute. + T item = CreateSymlinkToItem(GetMissingItem(), true); + SettingUpdatesPropertiesCore(item, 2007); + } + [Fact] [PlatformSpecific(~TestPlatforms.Browser)] // Browser is excluded as there is only 1 effective time store. public void SettingUpdatesPropertiesAfterAnother() @@ -128,40 +206,6 @@ public void SettingUpdatesPropertiesAfterAnother() }); } - [Fact] - [PlatformSpecific(~(TestPlatforms.Browser))] // Browser is excluded as there is only 1 effective time store. - public void SettingUpdatesPropertiesOnSymlink() - { - // This test is in this class since it needs all of the time functions. - // 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). - // It is also the same case on unix, with the utimensat function. - 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, 9, 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 05b9a0bba90669..50831e49640468 100644 --- a/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs @@ -9,6 +9,10 @@ public class Directory_GetSetTimes : StaticGetSetTimes { protected override string GetExistingItem() => Directory.CreateDirectory(GetTestFilePath()).FullName; + protected override string CreateSymlink(string path, string pathToTarget) => Directory.CreateSymbolicLink(path, pathToTarget).FullName; + + protected override bool IsDirectory => true; + public override IEnumerable TimeFunctions(bool requiresRoundtripping = false) { if (IOInputs.SupportsGettingCreationTime && (!requiresRoundtripping || IOInputs.SupportsSettingCreationTime)) @@ -51,7 +55,5 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi ((path) => Directory.GetLastWriteTimeUtc(path)), DateTimeKind.Utc); } - - protected override string CreateSymlinkToItem(string item) => Directory.CreateSymbolicLink(item + ".link", item).FullName; } } diff --git a/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs index db76f07916dff5..433181ed1b9cc4 100644 --- a/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs @@ -11,6 +11,10 @@ public class DirectoryInfo_GetSetTimes : InfoGetSetTimes protected override DirectoryInfo GetMissingItem() => new DirectoryInfo(GetTestFilePath()); + protected override DirectoryInfo CreateSymlink(string path, string pathToTarget) => (DirectoryInfo)Directory.CreateSymbolicLink(path, pathToTarget); + + protected override bool IsDirectory => true; + protected override string GetItemPath(DirectoryInfo item) => item.FullName; protected override void InvokeCreate(DirectoryInfo item) => item.Create(); @@ -57,7 +61,5 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi ((testDir) => testDir.LastWriteTimeUtc), DateTimeKind.Utc); } - - protected override DirectoryInfo CreateSymlinkToItem(DirectoryInfo item) => (DirectoryInfo)Directory.CreateSymbolicLink(item.FullName + ".link", item.FullName); } } diff --git a/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs index 0e998af0cde2ad..bb3b33342f0929 100644 --- a/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs @@ -22,6 +22,10 @@ protected override string GetExistingItem() return path; } + protected override FileInfo CreateSymlink(string path, string pathToTarget) => File.CreateSymbolicLink(path, pathToTarget).FullName; + + protected override bool IsDirectory => false; + [Fact] [PlatformSpecific(TestPlatforms.Linux)] public void BirthTimeIsNotNewerThanLowestOfAccessModifiedTimes() @@ -103,8 +107,6 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi DateTimeKind.Utc); } - protected override string CreateSymlinkToItem(string item) => File.CreateSymbolicLink(item + ".link", item).FullName; - [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 887f5e5c029867..b2efd8f8ef5b79 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs @@ -17,6 +17,10 @@ protected override FileInfo GetExistingItem() return new FileInfo(path); } + protected override FileInfo CreateSymlink(string path, string pathToTarget) => (FileInfo)File.CreateSymbolicLink(path, pathToTarget); + + protected override bool IsDirectory => false; + private static bool HasNonZeroNanoseconds(DateTime dt) => dt.Ticks % 10 != 0; public FileInfo GetNonZeroMilliseconds() @@ -103,8 +107,6 @@ public override IEnumerable TimeFunctions(bool requiresRoundtrippi DateTimeKind.Utc); } - protected override FileInfo CreateSymlinkToItem(FileInfo item) => (FileInfo)File.CreateSymbolicLink(item.FullName + ".link", item.FullName); - [ConditionalFact(nameof(HighTemporalResolution))] public void CopyToMillisecondPresent() { From f8c4fd6904eeb623d9da730f31ad506ad6a42421 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Tue, 26 Oct 2021 12:52:00 +1100 Subject: [PATCH 07/19] Fix return type of CreateSymlink in File/GetSetTimes.cs --- src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs index bb3b33342f0929..6e8f0d96c074db 100644 --- a/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs @@ -22,7 +22,7 @@ protected override string GetExistingItem() return path; } - protected override FileInfo CreateSymlink(string path, string pathToTarget) => File.CreateSymbolicLink(path, pathToTarget).FullName; + protected override string CreateSymlink(string path, string pathToTarget) => File.CreateSymbolicLink(path, pathToTarget).FullName; protected override bool IsDirectory => false; From 060bf1cf5e8c876504d0f2bb3b1d4ec8c5f8cfe3 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Tue, 26 Oct 2021 14:44:01 +1100 Subject: [PATCH 08/19] Remove browser from new symlink tests as it doesn't support creation of symlinks Remove browser from new symlink tests as it doesn't support creation of symlinks --- .../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 f2643d42168a60..418fa39cca36da 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -97,6 +97,7 @@ public void SettingUpdatesProperties() } [Fact] + [PlatformSpecific(~TestPlatforms.Browser)] // Browser is excluded as it doesn't support symlinks public void SettingUpdatesPropertiesOnSymlink() { // This test is in this class since it needs all of the time functions. @@ -110,6 +111,7 @@ public void SettingUpdatesPropertiesOnSymlink() } [Fact] + [PlatformSpecific(~TestPlatforms.Browser)] public void SettingUpdatesPropertiesOnNonexistentSymlink() { if (IsDirectory && !OperatingSystem.IsWindows()) @@ -126,6 +128,7 @@ public void SettingUpdatesPropertiesOnNonexistentSymlink() } [Fact] + [PlatformSpecific(~TestPlatforms.Browser)] public void SettingUpdatesPropertiesOnRelativeSymlink() { // Same as the SettingUpdatesPropertiesOnSymlink function, @@ -135,6 +138,7 @@ public void SettingUpdatesPropertiesOnRelativeSymlink() } [Fact] + [PlatformSpecific(~TestPlatforms.Browser)] public void SettingUpdatesPropertiesOnRelativeNonexistentSymlink() { if (IsDirectory && !OperatingSystem.IsWindows()) From f4bac0cc004ba0cf4db031b9c7b681ce9df2c1a4 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Thu, 28 Oct 2021 12:35:03 +1100 Subject: [PATCH 09/19] Use lutimes, improve code readability, simplify tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Use lutimes when it's available • Extract dwFlagsAndAttributes to a variable • Use same year for all tests • Checking to delete old symlink is unnecessary, so don't • Replace var with explicit type --- .../Native/Unix/Common/pal_config.h.in | 1 + .../Native/Unix/System.Native/pal_time.c | 8 ++++++ src/libraries/Native/Unix/configure.cmake | 5 ++++ .../tests/Base/BaseGetSetTimes.cs | 27 +++++++------------ .../src/System/IO/FileSystem.Windows.cs | 8 +++++- 5 files changed, 30 insertions(+), 19 deletions(-) diff --git a/src/libraries/Native/Unix/Common/pal_config.h.in b/src/libraries/Native/Unix/Common/pal_config.h.in index 9cffccbb0cc718..00b774b87341b4 100644 --- a/src/libraries/Native/Unix/Common/pal_config.h.in +++ b/src/libraries/Native/Unix/Common/pal_config.h.in @@ -112,6 +112,7 @@ #cmakedefine01 HAVE_GETDOMAINNAME #cmakedefine01 HAVE_UNAME #cmakedefine01 HAVE_UTIMENSAT +#cmakedefine01 HAVE_LUTIMES #cmakedefine01 HAVE_FUTIMES #cmakedefine01 HAVE_FUTIMENS #cmakedefine01 HAVE_MKSTEMPS diff --git a/src/libraries/Native/Unix/System.Native/pal_time.c b/src/libraries/Native/Unix/System.Native/pal_time.c index ef8a27a175c0fc..a84aa529ba30b8 100644 --- a/src/libraries/Native/Unix/System.Native/pal_time.c +++ b/src/libraries/Native/Unix/System.Native/pal_time.c @@ -34,6 +34,14 @@ 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))); +#elif HAVE_LUTIMES + struct timeval updatedTimes[2]; + updatedTimes[0].tv_sec = (long)times[0].tv_sec; + updatedTimes[0].tv_usec = (int)times[0].tv_nsec / 1000; + + updatedTimes[1].tv_sec = (long)times[1].tv_sec; + updatedTimes[1].tv_usec = (int)times[1].tv_nsec / 1000; + while (CheckInterrupted(result = lutimes(path, updatedTimes))); #else struct timeval updatedTimes[2]; updatedTimes[0].tv_sec = (long)times[0].tv_sec; diff --git a/src/libraries/Native/Unix/configure.cmake b/src/libraries/Native/Unix/configure.cmake index 1038d1b4c1f2f6..57708aff84b651 100644 --- a/src/libraries/Native/Unix/configure.cmake +++ b/src/libraries/Native/Unix/configure.cmake @@ -683,6 +683,11 @@ check_symbol_exists( sys/stat.h HAVE_UTIMENSAT) +check_symbol_exists( + lutimes + sys/time.h + HAVE_LUTIMES) + set (PREVIOUS_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS}) set (CMAKE_REQUIRED_FLAGS "-Werror -Wsign-conversion") diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index 418fa39cca36da..eddac3d6b57caf 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -31,17 +31,9 @@ protected T CreateSymlinkToItem(T item, bool isRelative) { // Creates a Symlink to 'item' (may or may not exist) that is optionally // a relative path rather than an absolute path. - var itemPath = GetItemPath(item); - var linkPath = itemPath + ".link"; - - // If a symlink was a directory on unix and references a non-existent - // path, then it manifests as a file. If a symlink was a file that now - // has a target that is a directory, it manifests as a directory. - if (Directory.Exists(linkPath)) Directory.Delete(linkPath); - else if (File.Exists(linkPath)) File.Exists(linkPath); - + string itemPath = GetItemPath(item); string target = isRelative ? Path.GetFileName(itemPath) : itemPath; - return CreateSymlink(linkPath, target); + return CreateSymlink(itemPath + ".link", target); } protected abstract string GetItemPath(T item); @@ -63,14 +55,13 @@ public static TimeFunction Create(SetTime setter, GetTime getter, DateTimeKind k public DateTimeKind Kind => Item3; } - private void SettingUpdatesPropertiesCore(T item, int year) + private void SettingUpdatesPropertiesCore(T item) { - // Pass the year as a parameter so it changes between different tests (in case of the same file) Assert.All(TimeFunctions(requiresRoundtripping: true), (function) => { // Checking that milliseconds are not dropped after setter. // Emscripten drops milliseconds in Browser - DateTime dt = new DateTime(year, 12, 1, 12, 3, 3, LowTemporalResolution ? 0 : 321, function.Kind); + 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); @@ -93,7 +84,7 @@ private void SettingUpdatesPropertiesCore(T item, int year) public void SettingUpdatesProperties() { T item = GetExistingItem(); - SettingUpdatesPropertiesCore(item, 2014); + SettingUpdatesPropertiesCore(item); } [Fact] @@ -107,7 +98,7 @@ public void SettingUpdatesPropertiesOnSymlink() // instead (eg. the setattrlist will do this without the flag set). // It is also the same case on unix, with the utimensat function. T item = CreateSymlinkToItem(GetExistingItem(), false); - SettingUpdatesPropertiesCore(item, 2004); + SettingUpdatesPropertiesCore(item); } [Fact] @@ -124,7 +115,7 @@ public void SettingUpdatesPropertiesOnNonexistentSymlink() // Same as the above SettingUpdatesPropertiesOnSymlink test, // except that the target of the symlink doesn't exist. T item = CreateSymlinkToItem(GetMissingItem(), false); - SettingUpdatesPropertiesCore(item, 2005); + SettingUpdatesPropertiesCore(item); } [Fact] @@ -134,7 +125,7 @@ public void SettingUpdatesPropertiesOnRelativeSymlink() // Same as the SettingUpdatesPropertiesOnSymlink function, // except that the symlink target is relative rather than absolute. T item = CreateSymlinkToItem(GetExistingItem(), true); - SettingUpdatesPropertiesCore(item, 2006); + SettingUpdatesPropertiesCore(item); } [Fact] @@ -151,7 +142,7 @@ public void SettingUpdatesPropertiesOnRelativeNonexistentSymlink() // Same as the SettingUpdatesPropertiesOnNonexistentSymlink function, // except that the symlink target is relative rather than absolute. T item = CreateSymlinkToItem(GetMissingItem(), true); - SettingUpdatesPropertiesCore(item, 2007); + SettingUpdatesPropertiesCore(item); } [Fact] diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs index a73558a3f7b53d..0c14567b24eea5 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs @@ -150,12 +150,18 @@ private static SafeFileHandle OpenHandle(string fullPath, bool asDirectory) throw new ArgumentException(SR.Arg_PathIsVolume, "path"); } + int dwFlagsAndAttributes = Interop.Kernel32.FileOperations.FILE_FLAG_OPEN_REPARSE_POINT; + if (asDirectory) + { + dwFlagsAndAttributes |= Interop.Kernel32.FileOperations.FILE_FLAG_BACKUP_SEMANTICS; + } + SafeFileHandle handle = Interop.Kernel32.CreateFile( fullPath, Interop.Kernel32.GenericOperations.GENERIC_WRITE, FileShare.ReadWrite | FileShare.Delete, FileMode.Open, - asDirectory ? (Interop.Kernel32.FileOperations.FILE_FLAG_BACKUP_SEMANTICS | Interop.Kernel32.FileOperations.FILE_FLAG_OPEN_REPARSE_POINT) : Interop.Kernel32.FileOperations.FILE_FLAG_OPEN_REPARSE_POINT); + dwFlagsAndAttributes); if (handle.IsInvalid) { From 97d1ed31eabcd2ea3cd4294d5d41a8556b479206 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Thu, 28 Oct 2021 12:39:23 +1100 Subject: [PATCH 10/19] Change year in test to 2014 to reduce diff --- .../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 eddac3d6b57caf..3e5ddfdcf18dc1 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -61,7 +61,7 @@ private void SettingUpdatesPropertiesCore(T item) { // 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); + DateTime dt = new DateTime(2014, 12, 1, 12, 3, 3, LowTemporalResolution ? 0 : 321, function.Kind); function.Setter(item, dt); DateTime result = function.Getter(item); Assert.Equal(dt, result); From fd9d2d5fa199996e828107e9bf848387938ab47b Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Thu, 28 Oct 2021 15:59:12 +1100 Subject: [PATCH 11/19] Rename symlink tests, use 1 core symlink times function, and check that target times don't change Rename symlink tests, use 1 core symlink times function, and check that target times don't change --- .../tests/Base/BaseGetSetTimes.cs | 76 +++++++++++++------ 1 file changed, 51 insertions(+), 25 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index 3e5ddfdcf18dc1..92b856510e020f 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -87,6 +87,50 @@ public void SettingUpdatesProperties() SettingUpdatesPropertiesCore(item); } + private void SettingUpdatesPropertiesOnSymlinkCore(bool isRelative, bool targetExists) + { + if (!targetExists && IsDirectory && !OperatingSystem.IsWindows()) + { + // On Unix, symlinks that are created as 'directories' need their + // directory target to exist for it to count as a directory symlink. + return; + } + + T target = targetExists ? GetExistingItem() : GetMissingItem(); + + // When the target exists, we want to verify that its times don't change. + // If the target doesn't exist, we just run the symlink tests, otherwise + // we run it in an Assert.All so it is easy to tell if it failed by not + // setting it on the symlink, or if it failed by setting it on the target. + // The symlink test is [0] of the Assert.All, and the target check is [1]. + + void RunSymlinkTestPart() + { + T item = CreateSymlinkToItem(target, isRelative); + SettingUpdatesPropertiesCore(item); + } + + if (!targetExists) + { + RunSymlinkTestPart(); + } + else + { + // We only use UTC times since checking the others is unnecessary. + IEnumerable timeFunctionsUtc = TimeFunctions(requiresRoundtripping: true).Where((f) => f.Kind == DateTimeKind.Utc); + DateTime[] initialTimes = timeFunctionsUtc.Select((funcs) => funcs.Getter(target)).ToArray(); + + Assert.All(new Action[] { RunSymlinkTestPart, () => + { + // Ensure that we have the latest times + if (target is FileSystemInfo fsi) fsi.Refresh(); + + DateTime[] updatedTimes = timeFunctionsUtc.Select((funcs) => funcs.Getter(target)).ToArray(); + Assert.Equal(initialTimes, updatedTimes); + } }, (action) => action()); + } + } + [Fact] [PlatformSpecific(~TestPlatforms.Browser)] // Browser is excluded as it doesn't support symlinks public void SettingUpdatesPropertiesOnSymlink() @@ -97,25 +141,16 @@ public void SettingUpdatesPropertiesOnSymlink() // to follow the symlink to completion and set the time on that entry // instead (eg. the setattrlist will do this without the flag set). // It is also the same case on unix, with the utimensat function. - T item = CreateSymlinkToItem(GetExistingItem(), false); - SettingUpdatesPropertiesCore(item); + SettingUpdatesPropertiesOnSymlinkCore(false, true); } [Fact] [PlatformSpecific(~TestPlatforms.Browser)] - public void SettingUpdatesPropertiesOnNonexistentSymlink() + public void SettingUpdatesPropertiesOnSymlinkWithNonExistentTarget() { - if (IsDirectory && !OperatingSystem.IsWindows()) - { - // On Unix, symlinks that are created as 'directories' need their - // directory target to exist for it to count as a directory symlink. - return; - } - // Same as the above SettingUpdatesPropertiesOnSymlink test, // except that the target of the symlink doesn't exist. - T item = CreateSymlinkToItem(GetMissingItem(), false); - SettingUpdatesPropertiesCore(item); + SettingUpdatesPropertiesOnSymlinkCore(false, false); } [Fact] @@ -124,25 +159,16 @@ public void SettingUpdatesPropertiesOnRelativeSymlink() { // Same as the SettingUpdatesPropertiesOnSymlink function, // except that the symlink target is relative rather than absolute. - T item = CreateSymlinkToItem(GetExistingItem(), true); - SettingUpdatesPropertiesCore(item); + SettingUpdatesPropertiesOnSymlinkCore(true, true); } [Fact] [PlatformSpecific(~TestPlatforms.Browser)] - public void SettingUpdatesPropertiesOnRelativeNonexistentSymlink() + public void SettingUpdatesPropertiesOnRelativeSymlinkWithNonExistingTarget() { - if (IsDirectory && !OperatingSystem.IsWindows()) - { - // On Unix, symlinks that are created as 'directories' need their - // directory target to exist for it to count as a directory symlink. - return; - } - - // Same as the SettingUpdatesPropertiesOnNonexistentSymlink function, + // Same as the SettingUpdatesPropertiesOnSymlinkWithNonExistentTarget function, // except that the symlink target is relative rather than absolute. - T item = CreateSymlinkToItem(GetMissingItem(), true); - SettingUpdatesPropertiesCore(item); + SettingUpdatesPropertiesOnSymlinkCore(true, false); } [Fact] From 1b7b8688995002412c24b3610790ae8d250b9b14 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Thu, 28 Oct 2021 16:10:10 +1100 Subject: [PATCH 12/19] Inline RunSymlinkTestPart 'function' Inline RunSymlinkTestPart 'function' so that the code can be reordered so the access time test can be valid. --- .../tests/Base/BaseGetSetTimes.cs | 19 ++++++++++++------- 1 file changed, 12 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 92b856510e020f..2749e431c00f57 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -104,23 +104,28 @@ private void SettingUpdatesPropertiesOnSymlinkCore(bool isRelative, bool targetE // setting it on the symlink, or if it failed by setting it on the target. // The symlink test is [0] of the Assert.All, and the target check is [1]. - void RunSymlinkTestPart() + if (!targetExists) { T item = CreateSymlinkToItem(target, isRelative); SettingUpdatesPropertiesCore(item); } - - if (!targetExists) - { - RunSymlinkTestPart(); - } else { + // Create the symlink first, since it may change the access time of the target + // to check if it's a file or folder, and whether it exists. + T item = CreateSymlinkToItem(target, isRelative); + + // Ensure that we have the latest times + if (target is FileSystemInfo fsi) fsi.Refresh(); + // We only use UTC times since checking the others is unnecessary. IEnumerable timeFunctionsUtc = TimeFunctions(requiresRoundtripping: true).Where((f) => f.Kind == DateTimeKind.Utc); DateTime[] initialTimes = timeFunctionsUtc.Select((funcs) => funcs.Getter(target)).ToArray(); - Assert.All(new Action[] { RunSymlinkTestPart, () => + Assert.All(new Action[] { () => + { + SettingUpdatesPropertiesCore(item); + }, () => { // Ensure that we have the latest times if (target is FileSystemInfo fsi) fsi.Refresh(); From b8460fff919c6f9f4cd7c131ddba0f8bb0fd2783 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Thu, 28 Oct 2021 16:15:09 +1100 Subject: [PATCH 13/19] Share CreateSymlinkToItem call in tests and update comment for clarity --- .../tests/Base/BaseGetSetTimes.cs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index 2749e431c00f57..e0eeb17ac70408 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -104,18 +104,11 @@ private void SettingUpdatesPropertiesOnSymlinkCore(bool isRelative, bool targetE // setting it on the symlink, or if it failed by setting it on the target. // The symlink test is [0] of the Assert.All, and the target check is [1]. - if (!targetExists) - { - T item = CreateSymlinkToItem(target, isRelative); - SettingUpdatesPropertiesCore(item); - } + T item = CreateSymlinkToItem(target, isRelative); + if (!targetExists) SettingUpdatesPropertiesCore(item); else { - // Create the symlink first, since it may change the access time of the target - // to check if it's a file or folder, and whether it exists. - T item = CreateSymlinkToItem(target, isRelative); - - // Ensure that we have the latest times + // Ensure that we have the latest times (access time could be changed by creating the symlink) if (target is FileSystemInfo fsi) fsi.Refresh(); // We only use UTC times since checking the others is unnecessary. From 9bf86db0bb04b594531b6b41f4ccd779102ead7c Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Fri, 29 Oct 2021 18:56:30 +1100 Subject: [PATCH 14/19] Update symlink time tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Make SettingUpdatesPropertiesOnSymlink a theory • Remove special case for Unix due to https://github.com/dotnet/runtime/pull/52639#discussion_r739009259 (will revert if fails) • Rename isRelative to targetIsRelative for clarity --- .../tests/Base/BaseGetSetTimes.cs | 64 +++++-------------- 1 file changed, 16 insertions(+), 48 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index e0eeb17ac70408..b34a933b761b37 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -87,14 +87,22 @@ public void SettingUpdatesProperties() SettingUpdatesPropertiesCore(item); } - private void SettingUpdatesPropertiesOnSymlinkCore(bool isRelative, bool targetExists) + [Theory] + [PlatformSpecific(~TestPlatforms.Browser)] // Browser is excluded as it doesn't support symlinks + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public void SettingUpdatesPropertiesOnSymlink(bool targetIsRelative, bool targetExists) { - if (!targetExists && IsDirectory && !OperatingSystem.IsWindows()) - { - // On Unix, symlinks that are created as 'directories' need their - // directory target to exist for it to count as a directory symlink. - return; - } + // This test is in this class since it needs all of the time functions. + // 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). + // It is also the same case on unix, with the utimensat function. + // It is a theory since there are variants which have a relative target + // or absolute target, and whether the target exists or not. T target = targetExists ? GetExistingItem() : GetMissingItem(); @@ -104,7 +112,7 @@ private void SettingUpdatesPropertiesOnSymlinkCore(bool isRelative, bool targetE // setting it on the symlink, or if it failed by setting it on the target. // The symlink test is [0] of the Assert.All, and the target check is [1]. - T item = CreateSymlinkToItem(target, isRelative); + T item = CreateSymlinkToItem(target, targetIsRelative); if (!targetExists) SettingUpdatesPropertiesCore(item); else { @@ -129,46 +137,6 @@ private void SettingUpdatesPropertiesOnSymlinkCore(bool isRelative, bool targetE } } - [Fact] - [PlatformSpecific(~TestPlatforms.Browser)] // Browser is excluded as it doesn't support symlinks - public void SettingUpdatesPropertiesOnSymlink() - { - // This test is in this class since it needs all of the time functions. - // 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). - // It is also the same case on unix, with the utimensat function. - SettingUpdatesPropertiesOnSymlinkCore(false, true); - } - - [Fact] - [PlatformSpecific(~TestPlatforms.Browser)] - public void SettingUpdatesPropertiesOnSymlinkWithNonExistentTarget() - { - // Same as the above SettingUpdatesPropertiesOnSymlink test, - // except that the target of the symlink doesn't exist. - SettingUpdatesPropertiesOnSymlinkCore(false, false); - } - - [Fact] - [PlatformSpecific(~TestPlatforms.Browser)] - public void SettingUpdatesPropertiesOnRelativeSymlink() - { - // Same as the SettingUpdatesPropertiesOnSymlink function, - // except that the symlink target is relative rather than absolute. - SettingUpdatesPropertiesOnSymlinkCore(true, true); - } - - [Fact] - [PlatformSpecific(~TestPlatforms.Browser)] - public void SettingUpdatesPropertiesOnRelativeSymlinkWithNonExistingTarget() - { - // Same as the SettingUpdatesPropertiesOnSymlinkWithNonExistentTarget function, - // except that the symlink target is relative rather than absolute. - SettingUpdatesPropertiesOnSymlinkCore(true, false); - } - [Fact] [PlatformSpecific(~TestPlatforms.Browser)] // Browser is excluded as there is only 1 effective time store. public void SettingUpdatesPropertiesAfterAnother() From 5713e272d2f137522c2911ef1951120f1c31a4f9 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Fri, 29 Oct 2021 19:08:14 +1100 Subject: [PATCH 15/19] Remove unnecessary Assert.All --- .../tests/Base/BaseGetSetTimes.cs | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index b34a933b761b37..851b718bbb7d09 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -107,33 +107,30 @@ public void SettingUpdatesPropertiesOnSymlink(bool targetIsRelative, bool target T target = targetExists ? GetExistingItem() : GetMissingItem(); // When the target exists, we want to verify that its times don't change. - // If the target doesn't exist, we just run the symlink tests, otherwise - // we run it in an Assert.All so it is easy to tell if it failed by not - // setting it on the symlink, or if it failed by setting it on the target. - // The symlink test is [0] of the Assert.All, and the target check is [1]. T item = CreateSymlinkToItem(target, targetIsRelative); if (!targetExists) SettingUpdatesPropertiesCore(item); else { // Ensure that we have the latest times (access time could be changed by creating the symlink) - if (target is FileSystemInfo fsi) fsi.Refresh(); + FileSystemInfo fsi = target as FileSystemInfo; + fsi?.Refresh(); // We only use UTC times since checking the others is unnecessary. IEnumerable timeFunctionsUtc = TimeFunctions(requiresRoundtripping: true).Where((f) => f.Kind == DateTimeKind.Utc); + + // Get the target's initial times DateTime[] initialTimes = timeFunctionsUtc.Select((funcs) => funcs.Getter(target)).ToArray(); - Assert.All(new Action[] { () => - { - SettingUpdatesPropertiesCore(item); - }, () => - { - // Ensure that we have the latest times - if (target is FileSystemInfo fsi) fsi.Refresh(); + // Run the test on the symlink + SettingUpdatesPropertiesCore(item); + + // Ensure that we have the latest times + fsi?.Refresh(); - DateTime[] updatedTimes = timeFunctionsUtc.Select((funcs) => funcs.Getter(target)).ToArray(); - Assert.Equal(initialTimes, updatedTimes); - } }, (action) => action()); + // Ensure the target's times haven't changed. + DateTime[] updatedTimes = timeFunctionsUtc.Select((funcs) => funcs.Getter(target)).ToArray(); + Assert.Equal(initialTimes, updatedTimes); } } From d50be1b6e1213876475839069101f6a7ba230537 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Sun, 31 Oct 2021 09:05:48 +1100 Subject: [PATCH 16/19] Changes to SettingUpdatesPropertiesOnSymlink test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Rename item field to link field • Don't use if one-liner • Use all time functions since only using UTC isn't necessary • Remove the now-defunct IsDirectory property since we aren't checking it anymore --- .../tests/Base/BaseGetSetTimes.cs | 19 +++++++++---------- .../tests/Directory/GetSetTimes.cs | 2 -- .../tests/DirectoryInfo/GetSetTimes.cs | 2 -- .../tests/File/GetSetTimes.cs | 2 -- .../tests/FileInfo/GetSetTimes.cs | 2 -- 5 files changed, 9 insertions(+), 18 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index 851b718bbb7d09..6572a99506b03e 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -25,7 +25,6 @@ public abstract class BaseGetSetTimes : FileSystemTest protected abstract T GetMissingItem(); protected abstract T CreateSymlink(string path, string pathToTarget); - protected abstract bool IsDirectory { get; } protected T CreateSymlinkToItem(T item, bool isRelative) { @@ -108,28 +107,28 @@ public void SettingUpdatesPropertiesOnSymlink(bool targetIsRelative, bool target // When the target exists, we want to verify that its times don't change. - T item = CreateSymlinkToItem(target, targetIsRelative); - if (!targetExists) SettingUpdatesPropertiesCore(item); + T link = CreateSymlinkToItem(target, targetIsRelative); + if (!targetExists) + { + SettingUpdatesPropertiesCore(link); + } else { // Ensure that we have the latest times (access time could be changed by creating the symlink) FileSystemInfo fsi = target as FileSystemInfo; fsi?.Refresh(); - // We only use UTC times since checking the others is unnecessary. - IEnumerable timeFunctionsUtc = TimeFunctions(requiresRoundtripping: true).Where((f) => f.Kind == DateTimeKind.Utc); - // Get the target's initial times - DateTime[] initialTimes = timeFunctionsUtc.Select((funcs) => funcs.Getter(target)).ToArray(); + IEnumerable timeFunctions = TimeFunctions(requiresRoundtripping: true); + DateTime[] initialTimes = timeFunctions.Select((funcs) => funcs.Getter(target)).ToArray(); - // Run the test on the symlink - SettingUpdatesPropertiesCore(item); + SettingUpdatesPropertiesCore(link); // Ensure that we have the latest times fsi?.Refresh(); // Ensure the target's times haven't changed. - DateTime[] updatedTimes = timeFunctionsUtc.Select((funcs) => funcs.Getter(target)).ToArray(); + DateTime[] updatedTimes = timeFunctions.Select((funcs) => funcs.Getter(target)).ToArray(); Assert.Equal(initialTimes, updatedTimes); } } diff --git a/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs index 50831e49640468..698fbd1d67ce08 100644 --- a/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Directory/GetSetTimes.cs @@ -11,8 +11,6 @@ public class Directory_GetSetTimes : StaticGetSetTimes protected override string CreateSymlink(string path, string pathToTarget) => Directory.CreateSymbolicLink(path, pathToTarget).FullName; - protected override bool IsDirectory => true; - public override IEnumerable TimeFunctions(bool requiresRoundtripping = false) { if (IOInputs.SupportsGettingCreationTime && (!requiresRoundtripping || IOInputs.SupportsSettingCreationTime)) diff --git a/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs index 433181ed1b9cc4..795a8750e49bf2 100644 --- a/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/DirectoryInfo/GetSetTimes.cs @@ -13,8 +13,6 @@ public class DirectoryInfo_GetSetTimes : InfoGetSetTimes protected override DirectoryInfo CreateSymlink(string path, string pathToTarget) => (DirectoryInfo)Directory.CreateSymbolicLink(path, pathToTarget); - protected override bool IsDirectory => true; - protected override string GetItemPath(DirectoryInfo item) => item.FullName; protected override void InvokeCreate(DirectoryInfo item) => item.Create(); diff --git a/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs index 6e8f0d96c074db..50dcf0990a0eb6 100644 --- a/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/File/GetSetTimes.cs @@ -24,8 +24,6 @@ protected override string GetExistingItem() protected override string CreateSymlink(string path, string pathToTarget) => File.CreateSymbolicLink(path, pathToTarget).FullName; - protected override bool IsDirectory => false; - [Fact] [PlatformSpecific(TestPlatforms.Linux)] public void BirthTimeIsNotNewerThanLowestOfAccessModifiedTimes() diff --git a/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs index b2efd8f8ef5b79..0559edc669e5d0 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileInfo/GetSetTimes.cs @@ -19,8 +19,6 @@ protected override FileInfo GetExistingItem() protected override FileInfo CreateSymlink(string path, string pathToTarget) => (FileInfo)File.CreateSymbolicLink(path, pathToTarget); - protected override bool IsDirectory => false; - private static bool HasNonZeroNanoseconds(DateTime dt) => dt.Ticks % 10 != 0; public FileInfo GetNonZeroMilliseconds() From a894d875707a4850d496226a7b871c67ef4de46c Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Sun, 31 Oct 2021 19:47:47 +1100 Subject: [PATCH 17/19] Remove unnecessary fsi.Refresh() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Remove unnecessary fsi.Refresh() since atime is only updated when reading a file --- .../System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index 6572a99506b03e..ee275f2b66728b 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -114,18 +114,14 @@ public void SettingUpdatesPropertiesOnSymlink(bool targetIsRelative, bool target } else { - // Ensure that we have the latest times (access time could be changed by creating the symlink) - FileSystemInfo fsi = target as FileSystemInfo; - fsi?.Refresh(); - // Get the target's initial times IEnumerable timeFunctions = TimeFunctions(requiresRoundtripping: true); DateTime[] initialTimes = timeFunctions.Select((funcs) => funcs.Getter(target)).ToArray(); SettingUpdatesPropertiesCore(link); - // Ensure that we have the latest times - fsi?.Refresh(); + // Ensure that we have the latest times. + if (target is FileSystemInfo fsi) fsi.Refresh(); // Ensure the target's times haven't changed. DateTime[] updatedTimes = timeFunctions.Select((funcs) => funcs.Getter(target)).ToArray(); From d8bff217d3282b32875d0966fd64e3af9618b98d Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Fri, 5 Nov 2021 16:13:43 +1100 Subject: [PATCH 18/19] Updates to test and pal_time.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Remove targetIsRelative cases • Multi-line if statement • Combine HAVE_LUTIMES and #else conditions to allow more code charing --- .../Native/Unix/System.Native/pal_time.c | 16 +++++------- .../tests/Base/BaseGetSetTimes.cs | 26 +++++++++---------- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_time.c b/src/libraries/Native/Unix/System.Native/pal_time.c index a84aa529ba30b8..3c6394604fa1e2 100644 --- a/src/libraries/Native/Unix/System.Native/pal_time.c +++ b/src/libraries/Native/Unix/System.Native/pal_time.c @@ -34,14 +34,6 @@ 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))); -#elif HAVE_LUTIMES - struct timeval updatedTimes[2]; - updatedTimes[0].tv_sec = (long)times[0].tv_sec; - updatedTimes[0].tv_usec = (int)times[0].tv_nsec / 1000; - - updatedTimes[1].tv_sec = (long)times[1].tv_sec; - updatedTimes[1].tv_usec = (int)times[1].tv_nsec / 1000; - while (CheckInterrupted(result = lutimes(path, updatedTimes))); #else struct timeval updatedTimes[2]; updatedTimes[0].tv_sec = (long)times[0].tv_sec; @@ -49,7 +41,13 @@ int32_t SystemNative_UTimensat(const char* path, TimeSpec* times) updatedTimes[1].tv_sec = (long)times[1].tv_sec; updatedTimes[1].tv_usec = (int)times[1].tv_nsec / 1000; - while (CheckInterrupted(result = utimes(path, updatedTimes))); + while (CheckInterrupted(result = +#if HAVE_LUTIMES + lutimes +#else + utimes +#endif + (path, updatedTimes))); #endif return result; diff --git a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs index ee275f2b66728b..57a63f1c1cca28 100644 --- a/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs +++ b/src/libraries/System.IO.FileSystem/tests/Base/BaseGetSetTimes.cs @@ -26,13 +26,11 @@ public abstract class BaseGetSetTimes : FileSystemTest protected abstract T CreateSymlink(string path, string pathToTarget); - protected T CreateSymlinkToItem(T item, bool isRelative) + protected T CreateSymlinkToItem(T item) { - // Creates a Symlink to 'item' (may or may not exist) that is optionally - // a relative path rather than an absolute path. + // Creates a Symlink to 'item' (target may or may not exist) string itemPath = GetItemPath(item); - string target = isRelative ? Path.GetFileName(itemPath) : itemPath; - return CreateSymlink(itemPath + ".link", target); + return CreateSymlink(path: itemPath + ".link", pathToTarget: itemPath); } protected abstract string GetItemPath(T item); @@ -88,11 +86,9 @@ public void SettingUpdatesProperties() [Theory] [PlatformSpecific(~TestPlatforms.Browser)] // Browser is excluded as it doesn't support symlinks - [InlineData(false, false)] - [InlineData(false, true)] - [InlineData(true, false)] - [InlineData(true, true)] - public void SettingUpdatesPropertiesOnSymlink(bool targetIsRelative, bool targetExists) + [InlineData(false)] + [InlineData(true)] + public void SettingUpdatesPropertiesOnSymlink(bool targetExists) { // This test is in this class since it needs all of the time functions. // This test makes sure that the times are set on the symlink itself. @@ -100,14 +96,13 @@ public void SettingUpdatesPropertiesOnSymlink(bool targetIsRelative, bool target // to follow the symlink to completion and set the time on that entry // instead (eg. the setattrlist will do this without the flag set). // It is also the same case on unix, with the utimensat function. - // It is a theory since there are variants which have a relative target - // or absolute target, and whether the target exists or not. + // It is a theory since we test both the target existing and missing. T target = targetExists ? GetExistingItem() : GetMissingItem(); // When the target exists, we want to verify that its times don't change. - T link = CreateSymlinkToItem(target, targetIsRelative); + T link = CreateSymlinkToItem(target); if (!targetExists) { SettingUpdatesPropertiesCore(link); @@ -121,7 +116,10 @@ public void SettingUpdatesPropertiesOnSymlink(bool targetIsRelative, bool target SettingUpdatesPropertiesCore(link); // Ensure that we have the latest times. - if (target is FileSystemInfo fsi) fsi.Refresh(); + if (target is FileSystemInfo fsi) + { + fsi.Refresh(); + } // Ensure the target's times haven't changed. DateTime[] updatedTimes = timeFunctions.Select((funcs) => funcs.Getter(target)).ToArray(); From 569a24f1a5555e03e56ce8b59c7f2d838677cc83 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Fri, 5 Nov 2021 16:15:47 +1100 Subject: [PATCH 19/19] Remove trailing space --- src/libraries/Native/Unix/System.Native/pal_time.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_time.c b/src/libraries/Native/Unix/System.Native/pal_time.c index 3c6394604fa1e2..4f9b5326cb73c3 100644 --- a/src/libraries/Native/Unix/System.Native/pal_time.c +++ b/src/libraries/Native/Unix/System.Native/pal_time.c @@ -41,7 +41,7 @@ int32_t SystemNative_UTimensat(const char* path, TimeSpec* times) updatedTimes[1].tv_sec = (long)times[1].tv_sec; updatedTimes[1].tv_usec = (int)times[1].tv_nsec / 1000; - while (CheckInterrupted(result = + while (CheckInterrupted(result = #if HAVE_LUTIMES lutimes #else