From a50c0fcc78f7c1c1f750271f0ae6802618bb0747 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 00:01:32 -0800 Subject: [PATCH 01/31] New hidden and readonly tests --- .../tests/Enumeration/AttributeTests.cs | 62 ++++++++++++++++--- .../tests/Enumeration/SkipAttributeTests.cs | 37 ++++++++--- 2 files changed, 82 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs index 3736c0669cf78..6f9fac890bc02 100644 --- a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs +++ b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs @@ -113,18 +113,42 @@ public void DirectoryAttributesAreExpected() } [Fact] - public void IsHiddenAttribute() + [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.OSX)] + public void IsHiddenAttribute_Windows_OSX() { + // Put a period in front to make it hidden on Unix + IsHiddenAttributeInternal(useDotPrefix: false, useHiddenFlag: true); + + } + + + [Fact] + [PlatformSpecific(TestPlatforms.AnyUnix)] + public void IsHiddenAttribute_Unix() + { + // Windows and MacOS hide a file by setting the hidden attribute + IsHiddenAttributeInternal(useDotPrefix: true, useHiddenFlag: false); + } + + private void IsHiddenAttributeInternal(bool useDotPrefix, bool useHiddenFlag) + { + string prefix = useDotPrefix ? "." : ""; + DirectoryInfo testDirectory = Directory.CreateDirectory(GetTestFilePath()); - FileInfo fileOne = new FileInfo(Path.Combine(testDirectory.FullName, GetTestFileName())); - // Put a period in front to make it hidden on Unix - FileInfo fileTwo = new FileInfo(Path.Combine(testDirectory.FullName, "." + GetTestFileName())); + FileInfo fileOne = new FileInfo(Path.Combine(testDirectory.FullName, GetTestFileName())); + FileInfo fileTwo = new FileInfo(Path.Combine(testDirectory.FullName, prefix + GetTestFileName())); fileOne.Create().Dispose(); fileTwo.Create().Dispose(); - if (PlatformDetection.IsWindows) - fileTwo.Attributes = fileTwo.Attributes | FileAttributes.Hidden; + + if (useHiddenFlag) + { + fileTwo.Attributes |= FileAttributes.Hidden; + } + + FileInfo fileCheck = new FileInfo(fileTwo.FullName); + Assert.Equal(fileTwo.Attributes, fileCheck.Attributes); IEnumerable enumerable = new FileSystemEnumerable( testDirectory.FullName, @@ -136,5 +160,29 @@ public void IsHiddenAttribute() Assert.Equal(new string[] { fileTwo.FullName }, enumerable); } + + [Fact] + public void IsReadOnlyAttribute() + { + DirectoryInfo testDirectory = Directory.CreateDirectory(GetTestFilePath()); + + FileInfo fileOne = new FileInfo(Path.Combine(testDirectory.FullName, GetTestFileName())); + FileInfo fileTwo = new FileInfo(Path.Combine(testDirectory.FullName, GetTestFileName())); + + fileOne.Create().Dispose(); + fileTwo.Create().Dispose(); + + fileTwo.Attributes |= FileAttributes.ReadOnly; + + IEnumerable enumerable = new FileSystemEnumerable( + testDirectory.FullName, + (ref FileSystemEntry entry) => entry.ToFullPath(), + new EnumerationOptions() { AttributesToSkip = 0 }) + { + ShouldIncludePredicate = (ref FileSystemEntry entry) => (entry.Attributes & FileAttributes.ReadOnly) != 0 + }; + + Assert.Equal(new string[] { fileTwo.FullName }, enumerable); + } } -} +} \ No newline at end of file diff --git a/src/libraries/System.IO.FileSystem/tests/Enumeration/SkipAttributeTests.cs b/src/libraries/System.IO.FileSystem/tests/Enumeration/SkipAttributeTests.cs index 3a22c61b6f77e..19c40e9c6a0b5 100644 --- a/src/libraries/System.IO.FileSystem/tests/Enumeration/SkipAttributeTests.cs +++ b/src/libraries/System.IO.FileSystem/tests/Enumeration/SkipAttributeTests.cs @@ -21,25 +21,42 @@ protected virtual string[] GetPaths(string directory, EnumerationOptions options } [Fact] - public void SkippingHiddenFiles() + [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.OSX)] + public void SkippingHiddenFiles_Windows_OSX() + { + SkippingHiddenFilesInternal(useDotPrefix: false, useHiddenFlag: true); + } + + [Fact] + [PlatformSpecific(TestPlatforms.AnyUnix)] + public void SkippingHiddenFiles_Unix() + { + SkippingHiddenFilesInternal(useDotPrefix: true, useHiddenFlag: false); + } + + private void SkippingHiddenFilesInternal(bool useDotPrefix, bool useHiddenFlag) { DirectoryInfo testDirectory = Directory.CreateDirectory(GetTestFilePath()); DirectoryInfo testSubdirectory = Directory.CreateDirectory(Path.Combine(testDirectory.FullName, GetTestFileName())); - FileInfo fileOne = new FileInfo(Path.Combine(testDirectory.FullName, GetTestFileName())); - // Put a period in front to make it hidden on Unix - FileInfo fileTwo = new FileInfo(Path.Combine(testDirectory.FullName, "." + GetTestFileName())); + FileInfo fileOne = new FileInfo(Path.Combine(testDirectory.FullName, GetTestFileName())); FileInfo fileThree = new FileInfo(Path.Combine(testSubdirectory.FullName, GetTestFileName())); - FileInfo fileFour = new FileInfo(Path.Combine(testSubdirectory.FullName, "." + GetTestFileName())); + + // Put a period in front of files two and four to make them hidden on Unix + string prefix = useDotPrefix ? "." : ""; + FileInfo fileTwo = new FileInfo(Path.Combine(testDirectory.FullName, prefix + GetTestFileName())); + FileInfo fileFour = new FileInfo(Path.Combine(testSubdirectory.FullName, prefix + GetTestFileName())); fileOne.Create().Dispose(); fileTwo.Create().Dispose(); - if (PlatformDetection.IsWindows) - fileTwo.Attributes = fileTwo.Attributes | FileAttributes.Hidden; fileThree.Create().Dispose(); fileFour.Create().Dispose(); - if (PlatformDetection.IsWindows) - fileFour.Attributes = fileTwo.Attributes | FileAttributes.Hidden; + + if (useHiddenFlag) + { + fileTwo.Attributes |= FileAttributes.Hidden; + fileFour.Attributes |= FileAttributes.Hidden; + } // Default EnumerationOptions is to skip hidden string[] paths = GetPaths(testDirectory.FullName, new EnumerationOptions()); @@ -103,4 +120,4 @@ protected override string[] GetPaths(string directory, EnumerationOptions option return new DirectoryInfo(directory).GetFiles("*", options).Select(i => i.FullName).ToArray(); } } -} +} \ No newline at end of file From b8f69b724a9251346380201a727ff70b44fd7a93 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 13:45:27 -0800 Subject: [PATCH 02/31] FileStatus.Unix.cs public->internal --- .../System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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..1dcf75e81c812 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 @@ -69,7 +69,7 @@ internal bool IsReadOnly(ReadOnlySpan path, bool continueOnError = false) (_fileStatus.Mode & (int)writeBit) == 0); // but not write permission } - public FileAttributes GetAttributes(ReadOnlySpan path, ReadOnlySpan fileName) + internal FileAttributes GetAttributes(ReadOnlySpan path, ReadOnlySpan fileName) { // IMPORTANT: Attribute logic must match the logic in FileSystemEntry @@ -96,7 +96,7 @@ public FileAttributes GetAttributes(ReadOnlySpan path, ReadOnlySpan return attributes != default ? attributes : FileAttributes.Normal; } - public void SetAttributes(string path, FileAttributes attributes) + internal void SetAttributes(string path, FileAttributes attributes) { // Validate that only flags from the attribute are being provided. This is an // approximation for the validation done by the Win32 function. @@ -271,7 +271,7 @@ internal long GetLength(ReadOnlySpan path, bool continueOnError = false) return _fileStatus.Size; } - public void Refresh(ReadOnlySpan path) + internal void Refresh(ReadOnlySpan path) { // This should not throw, instead we store the result so that we can throw it // when someone actually accesses a property. From 84433ddd59c3a94ca602912dd4a568020c949c84 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 13:59:56 -0800 Subject: [PATCH 03/31] FileStatus.Unix.cs sort members --- .../src/System/IO/FileStatus.Unix.cs | 346 +++++++++--------- 1 file changed, 172 insertions(+), 174 deletions(-) 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 1dcf75e81c812..e9c40d91c7699 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 @@ -9,6 +9,9 @@ internal struct FileStatus { private const int NanosecondsPerTick = 100; + // Exists as of the last refresh + private bool _exists; + // The last cached stat information about the file private Interop.Sys.FileStatus _fileStatus; @@ -16,25 +19,103 @@ internal struct FileStatus // errors, or the errno error code. private int _fileStatusInitialized; + // Is a directory as of the last refresh + internal bool _isDirectory; + // We track intent of creation to know whether or not we want to (1) create a // DirectoryInfo around this status struct or (2) actually are part of a DirectoryInfo. internal bool InitiallyDirectory { get; private set; } - // Is a directory as of the last refresh - internal bool _isDirectory; + internal void EnsureStatInitialized(ReadOnlySpan path, bool continueOnError = false) + { + if (_fileStatusInitialized == -1) + { + Refresh(path); + } - // Exists as of the last refresh - private bool _exists; + if (_fileStatusInitialized != 0 && !continueOnError) + { + int errno = _fileStatusInitialized; + _fileStatusInitialized = -1; + throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(errno), new string(path)); + } + } - internal static void Initialize( - ref FileStatus status, - bool isDirectory) + internal static void Initialize(ref FileStatus status, bool isDirectory) { status.InitiallyDirectory = isDirectory; status._fileStatusInitialized = -1; } - internal void Invalidate() => _fileStatusInitialized = -1; + internal FileAttributes GetAttributes(ReadOnlySpan path, ReadOnlySpan fileName) + { + // IMPORTANT: Attribute logic must match the logic in FileSystemEntry + + EnsureStatInitialized(path); + + if (!_exists) + return (FileAttributes)(-1); + + FileAttributes attributes = default; + + if (IsReadOnly(path)) + attributes |= FileAttributes.ReadOnly; + + if ((_fileStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK) + attributes |= FileAttributes.ReparsePoint; + + if (_isDirectory) + attributes |= FileAttributes.Directory; + + // If the filename starts with a period or has UF_HIDDEN flag set, it's hidden. + if (fileName.Length > 0 && (fileName[0] == '.' || (_fileStatus.UserFlags & (uint)Interop.Sys.UserFlags.UF_HIDDEN) == (uint)Interop.Sys.UserFlags.UF_HIDDEN)) + attributes |= FileAttributes.Hidden; + + return attributes != default ? attributes : FileAttributes.Normal; + } + + internal DateTimeOffset GetCreationTime(ReadOnlySpan path, bool continueOnError = false) + { + EnsureStatInitialized(path, continueOnError); + if (!_exists) + return DateTimeOffset.FromFileTime(0); + + if ((_fileStatus.Flags & Interop.Sys.FileStatusFlags.HasBirthTime) != 0) + return UnixTimeToDateTimeOffset(_fileStatus.BirthTime, _fileStatus.BirthTimeNsec); + + // fall back to the oldest time we have in between change and modify time + if (_fileStatus.MTime < _fileStatus.CTime || + (_fileStatus.MTime == _fileStatus.CTime && _fileStatus.MTimeNsec < _fileStatus.CTimeNsec)) + return UnixTimeToDateTimeOffset(_fileStatus.MTime, _fileStatus.MTimeNsec); + + return UnixTimeToDateTimeOffset(_fileStatus.CTime, _fileStatus.CTimeNsec); + } + + + internal bool GetExists(ReadOnlySpan path) + { + if (_fileStatusInitialized == -1) + Refresh(path); + + return _exists && InitiallyDirectory == _isDirectory; + } + + internal DateTimeOffset GetLastAccessTime(ReadOnlySpan path, bool continueOnError = false) + { + EnsureStatInitialized(path, continueOnError); + if (!_exists) + return DateTimeOffset.FromFileTime(0); + return UnixTimeToDateTimeOffset(_fileStatus.ATime, _fileStatus.ATimeNsec); + } + + internal long GetLength(ReadOnlySpan path, bool continueOnError = false) + { + EnsureStatInitialized(path, continueOnError); + return _fileStatus.Size; + } + + internal void Invalidate() => + _fileStatusInitialized = -1; internal bool IsReadOnly(ReadOnlySpan path, bool continueOnError = false) { @@ -69,31 +150,91 @@ internal bool IsReadOnly(ReadOnlySpan path, bool continueOnError = false) (_fileStatus.Mode & (int)writeBit) == 0); // but not write permission } - internal FileAttributes GetAttributes(ReadOnlySpan path, ReadOnlySpan fileName) + internal void Refresh(ReadOnlySpan path) { - // IMPORTANT: Attribute logic must match the logic in FileSystemEntry + // This should not throw, instead we store the result so that we can throw it + // when someone actually accesses a property. - EnsureStatInitialized(path); + // Use lstat to get the details on the object, without following symlinks. + // If it is a symlink, then subsequently get details on the target of the symlink, + // storing those results separately. We only report failure if the initial + // lstat fails, as a broken symlink should still report info on exists, attributes, etc. + _isDirectory = false; + path = Path.TrimEndingDirectorySeparator(path); - if (!_exists) - return (FileAttributes)(-1); + int result = Interop.Sys.LStat(path, out _fileStatus); + if (result < 0) + { + Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); - FileAttributes attributes = default; + // This should never set the error if the file can't be found. + // (see the Windows refresh passing returnErrorOnNotFound: false). + if (errorInfo.Error == Interop.Error.ENOENT + || errorInfo.Error == Interop.Error.ENOTDIR) + { + _fileStatusInitialized = 0; + _exists = false; + } + else + { + _fileStatusInitialized = errorInfo.RawErrno; + } + return; + } - if (IsReadOnly(path)) - attributes |= FileAttributes.ReadOnly; + _exists = true; - if ((_fileStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK) - attributes |= FileAttributes.ReparsePoint; + // IMPORTANT: Is directory logic must match the logic in FileSystemEntry + _isDirectory = (_fileStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; - if (_isDirectory) - attributes |= FileAttributes.Directory; + // If we're a symlink, attempt to check the target to see if it is a directory + if ((_fileStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK && + Interop.Sys.Stat(path, out Interop.Sys.FileStatus targetStatus) >= 0) + { + _isDirectory = (targetStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; + } - // If the filename starts with a period or has UF_HIDDEN flag set, it's hidden. - if (fileName.Length > 0 && (fileName[0] == '.' || (_fileStatus.UserFlags & (uint)Interop.Sys.UserFlags.UF_HIDDEN) == (uint)Interop.Sys.UserFlags.UF_HIDDEN)) - attributes |= FileAttributes.Hidden; + _fileStatusInitialized = 0; + } - return attributes != default ? attributes : FileAttributes.Normal; + private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime) + { + // force a refresh so that we have an up-to-date times for values not being overwritten + _fileStatusInitialized = -1; + EnsureStatInitialized(path); + + // we use utimes()/utimensat() to set the accessTime and writeTime + Interop.Sys.TimeSpec* buf = stackalloc Interop.Sys.TimeSpec[2]; + + long seconds = time.ToUnixTimeSeconds(); + + const long TicksPerMillisecond = 10000; + const long TicksPerSecond = TicksPerMillisecond * 1000; + long nanoseconds = (time.UtcDateTime.Ticks - DateTimeOffset.UnixEpoch.Ticks - seconds * TicksPerSecond) * NanosecondsPerTick; + +#if TARGET_BROWSER + buf[0].TvSec = seconds; + buf[0].TvNsec = nanoseconds; + buf[1].TvSec = seconds; + buf[1].TvNsec = nanoseconds; +#else + if (isAccessTime) + { + buf[0].TvSec = seconds; + buf[0].TvNsec = nanoseconds; + buf[1].TvSec = _fileStatus.MTime; + buf[1].TvNsec = _fileStatus.MTimeNsec; + } + else + { + buf[0].TvSec = _fileStatus.ATime; + buf[0].TvNsec = _fileStatus.ATimeNsec; + buf[1].TvSec = seconds; + buf[1].TvNsec = nanoseconds; + } +#endif + Interop.CheckIo(Interop.Sys.UTimensat(path, buf), path, InitiallyDirectory); + _fileStatusInitialized = -1; } internal void SetAttributes(string path, FileAttributes attributes) @@ -161,33 +302,7 @@ internal void SetAttributes(string path, FileAttributes attributes) _fileStatusInitialized = -1; } - internal bool GetExists(ReadOnlySpan path) - { - if (_fileStatusInitialized == -1) - Refresh(path); - - return _exists && InitiallyDirectory == _isDirectory; - } - - internal DateTimeOffset GetCreationTime(ReadOnlySpan path, bool continueOnError = false) - { - EnsureStatInitialized(path, continueOnError); - if (!_exists) - return DateTimeOffset.FromFileTime(0); - - if ((_fileStatus.Flags & Interop.Sys.FileStatusFlags.HasBirthTime) != 0) - return UnixTimeToDateTimeOffset(_fileStatus.BirthTime, _fileStatus.BirthTimeNsec); - - // fall back to the oldest time we have in between change and modify time - if (_fileStatus.MTime < _fileStatus.CTime || - (_fileStatus.MTime == _fileStatus.CTime && _fileStatus.MTimeNsec < _fileStatus.CTimeNsec)) - return UnixTimeToDateTimeOffset(_fileStatus.MTime, _fileStatus.MTimeNsec); - - return UnixTimeToDateTimeOffset(_fileStatus.CTime, _fileStatus.CTimeNsec); - } - - internal void SetCreationTime(string path, DateTimeOffset time) - { + 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 @@ -198,17 +313,9 @@ internal void SetCreationTime(string path, DateTimeOffset time) // 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); - if (!_exists) - return DateTimeOffset.FromFileTime(0); - return UnixTimeToDateTimeOffset(_fileStatus.ATime, _fileStatus.ATimeNsec); - } - - internal void SetLastAccessTime(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) { @@ -218,119 +325,10 @@ 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(); - } - - private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime) - { - // force a refresh so that we have an up-to-date times for values not being overwritten - _fileStatusInitialized = -1; - EnsureStatInitialized(path); - - // we use utimes()/utimensat() to set the accessTime and writeTime - Interop.Sys.TimeSpec* buf = stackalloc Interop.Sys.TimeSpec[2]; - - long seconds = time.ToUnixTimeSeconds(); - - const long TicksPerMillisecond = 10000; - const long TicksPerSecond = TicksPerMillisecond * 1000; - long nanoseconds = (time.UtcDateTime.Ticks - DateTimeOffset.UnixEpoch.Ticks - seconds * TicksPerSecond) * NanosecondsPerTick; - -#if TARGET_BROWSER - buf[0].TvSec = seconds; - buf[0].TvNsec = nanoseconds; - buf[1].TvSec = seconds; - buf[1].TvNsec = nanoseconds; -#else - if (isAccessTime) - { - buf[0].TvSec = seconds; - buf[0].TvNsec = nanoseconds; - buf[1].TvSec = _fileStatus.MTime; - buf[1].TvNsec = _fileStatus.MTimeNsec; - } - else - { - buf[0].TvSec = _fileStatus.ATime; - buf[0].TvNsec = _fileStatus.ATimeNsec; - buf[1].TvSec = seconds; - buf[1].TvNsec = nanoseconds; - } -#endif - Interop.CheckIo(Interop.Sys.UTimensat(path, buf), path, InitiallyDirectory); - _fileStatusInitialized = -1; - } - - internal long GetLength(ReadOnlySpan path, bool continueOnError = false) - { - EnsureStatInitialized(path, continueOnError); - return _fileStatus.Size; - } - - internal void Refresh(ReadOnlySpan path) - { - // This should not throw, instead we store the result so that we can throw it - // when someone actually accesses a property. - - // Use lstat to get the details on the object, without following symlinks. - // If it is a symlink, then subsequently get details on the target of the symlink, - // storing those results separately. We only report failure if the initial - // lstat fails, as a broken symlink should still report info on exists, attributes, etc. - _isDirectory = false; - path = Path.TrimEndingDirectorySeparator(path); - - int result = Interop.Sys.LStat(path, out _fileStatus); - if (result < 0) - { - Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); - - // This should never set the error if the file can't be found. - // (see the Windows refresh passing returnErrorOnNotFound: false). - if (errorInfo.Error == Interop.Error.ENOENT - || errorInfo.Error == Interop.Error.ENOTDIR) - { - _fileStatusInitialized = 0; - _exists = false; - } - else - { - _fileStatusInitialized = errorInfo.RawErrno; - } - return; - } - - _exists = true; - - // IMPORTANT: Is directory logic must match the logic in FileSystemEntry - _isDirectory = (_fileStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; - - // If we're a symlink, attempt to check the target to see if it is a directory - if ((_fileStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK && - Interop.Sys.Stat(path, out Interop.Sys.FileStatus targetStatus) >= 0) - { - _isDirectory = (targetStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; - } - - _fileStatusInitialized = 0; - } - - internal void EnsureStatInitialized(ReadOnlySpan path, bool continueOnError = false) - { - if (_fileStatusInitialized == -1) - { - Refresh(path); - } + internal void SetLastWriteTime(string path, DateTimeOffset time) => + SetAccessOrWriteTime(path, time, isAccessTime: false); - if (_fileStatusInitialized != 0 && !continueOnError) - { - int errno = _fileStatusInitialized; - _fileStatusInitialized = -1; - throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(errno), new string(path)); - } - } + private DateTimeOffset UnixTimeToDateTimeOffset(long seconds, long nanoseconds) => + DateTimeOffset.FromUnixTimeSeconds(seconds).AddTicks(nanoseconds / NanosecondsPerTick).ToLocalTime(); } } From 5070cde3c8f2175bab958918f9b1ff88ca2416ae Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 14:08:43 -0800 Subject: [PATCH 04/31] FileStatus.Unix.cs brackets --- .../src/System/IO/FileStatus.Unix.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) 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 e9c40d91c7699..37b826970132e 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 @@ -54,22 +54,32 @@ internal FileAttributes GetAttributes(ReadOnlySpan path, ReadOnlySpan 0 && (fileName[0] == '.' || (_fileStatus.UserFlags & (uint)Interop.Sys.UserFlags.UF_HIDDEN) == (uint)Interop.Sys.UserFlags.UF_HIDDEN)) + { attributes |= FileAttributes.Hidden; + } return attributes != default ? attributes : FileAttributes.Normal; } @@ -78,15 +88,21 @@ internal DateTimeOffset GetCreationTime(ReadOnlySpan path, bool continueOn { EnsureStatInitialized(path, continueOnError); if (!_exists) + { return DateTimeOffset.FromFileTime(0); + } if ((_fileStatus.Flags & Interop.Sys.FileStatusFlags.HasBirthTime) != 0) + { return UnixTimeToDateTimeOffset(_fileStatus.BirthTime, _fileStatus.BirthTimeNsec); + } // fall back to the oldest time we have in between change and modify time if (_fileStatus.MTime < _fileStatus.CTime || (_fileStatus.MTime == _fileStatus.CTime && _fileStatus.MTimeNsec < _fileStatus.CTimeNsec)) + { return UnixTimeToDateTimeOffset(_fileStatus.MTime, _fileStatus.MTimeNsec); + } return UnixTimeToDateTimeOffset(_fileStatus.CTime, _fileStatus.CTimeNsec); } @@ -95,7 +111,9 @@ internal DateTimeOffset GetCreationTime(ReadOnlySpan path, bool continueOn internal bool GetExists(ReadOnlySpan path) { if (_fileStatusInitialized == -1) + { Refresh(path); + } return _exists && InitiallyDirectory == _isDirectory; } @@ -104,7 +122,9 @@ internal DateTimeOffset GetLastAccessTime(ReadOnlySpan path, bool continue { EnsureStatInitialized(path, continueOnError); if (!_exists) + { return DateTimeOffset.FromFileTime(0); + } return UnixTimeToDateTimeOffset(_fileStatus.ATime, _fileStatus.ATimeNsec); } @@ -257,7 +277,9 @@ internal void SetAttributes(string path, FileAttributes attributes) EnsureStatInitialized(path); if (!_exists) + { FileSystemInfo.ThrowNotFound(path); + } if (Interop.Sys.CanSetHiddenFlag) { @@ -321,7 +343,9 @@ internal DateTimeOffset GetLastWriteTime(ReadOnlySpan path, bool continueO { EnsureStatInitialized(path, continueOnError); if (!_exists) + { return DateTimeOffset.FromFileTime(0); + } return UnixTimeToDateTimeOffset(_fileStatus.MTime, _fileStatus.MTimeNsec); } From 06863682c73f53b7a0749a8880d20e8b51a1256d Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 14:19:06 -0800 Subject: [PATCH 05/31] Add VerifyStatCall method --- .../src/System/IO/FileStatus.Unix.cs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) 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 37b826970132e..ef44771e5c9fb 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 @@ -352,6 +352,29 @@ internal DateTimeOffset GetLastWriteTime(ReadOnlySpan path, bool continueO internal void SetLastWriteTime(string path, DateTimeOffset time) => SetAccessOrWriteTime(path, time, isAccessTime: false); + // Receives the return value of a stat or lstat call. + // If the call is unsuccessful, returns a positive number representing the last error info. + // If the call is successful, returns 0. + private int VerifyStatCall(int returnValue) + { + // stat and lstat return -1 on error, 0 on success + if (returnValue < 0) + { + Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); + + // This should never set the error if the file can't be found. + // (see the Windows refresh passing returnErrorOnNotFound: false). + if (errorInfo.Error != Interop.Error.ENOENT && // A component of the path does not exist, or path is an empty string + errorInfo.Error != Interop.Error.ENOTDIR) // A component of the path prefix of path is not a directory + { + // Expect a positive integer + return errorInfo.RawErrno; + } + } + + return 0; + } + private DateTimeOffset UnixTimeToDateTimeOffset(long seconds, long nanoseconds) => DateTimeOffset.FromUnixTimeSeconds(seconds).AddTicks(nanoseconds / NanosecondsPerTick).ToLocalTime(); } From 5e5932b4fd8fa4891510f937876d882490520ef4 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 14:22:17 -0800 Subject: [PATCH 06/31] Use VerifyStatCall with existing FileStatus Lstat call --- .../src/System/IO/FileStatus.Unix.cs | 25 +++++-------------- 1 file changed, 6 insertions(+), 19 deletions(-) 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 ef44771e5c9fb..59d200225ecdf 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 @@ -175,30 +175,17 @@ internal void Refresh(ReadOnlySpan path) // This should not throw, instead we store the result so that we can throw it // when someone actually accesses a property. + _isDirectory = false; + path = Path.TrimEndingDirectorySeparator(path); + // Use lstat to get the details on the object, without following symlinks. // If it is a symlink, then subsequently get details on the target of the symlink, // storing those results separately. We only report failure if the initial // lstat fails, as a broken symlink should still report info on exists, attributes, etc. - _isDirectory = false; - path = Path.TrimEndingDirectorySeparator(path); - - int result = Interop.Sys.LStat(path, out _fileStatus); - if (result < 0) + _fileStatusInitialized = VerifyStatCall(Interop.Sys.LStat(path, out _fileStatus)); + if (_fileStatusInitialized != 0) { - Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); - - // This should never set the error if the file can't be found. - // (see the Windows refresh passing returnErrorOnNotFound: false). - if (errorInfo.Error == Interop.Error.ENOENT - || errorInfo.Error == Interop.Error.ENOTDIR) - { - _fileStatusInitialized = 0; - _exists = false; - } - else - { - _fileStatusInitialized = errorInfo.RawErrno; - } + _exists = false; return; } From a866e739ca55ae5ef6711e919c766c7d26d96613 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 14:23:53 -0800 Subject: [PATCH 07/31] FileStatus.Unix.cs sort Initialize method --- .../src/System/IO/FileStatus.Unix.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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 59d200225ecdf..2d67d020202f9 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 @@ -41,12 +41,6 @@ internal void EnsureStatInitialized(ReadOnlySpan path, bool continueOnErro } } - internal static void Initialize(ref FileStatus status, bool isDirectory) - { - status.InitiallyDirectory = isDirectory; - status._fileStatusInitialized = -1; - } - internal FileAttributes GetAttributes(ReadOnlySpan path, ReadOnlySpan fileName) { // IMPORTANT: Attribute logic must match the logic in FileSystemEntry @@ -134,6 +128,12 @@ internal long GetLength(ReadOnlySpan path, bool continueOnError = false) return _fileStatus.Size; } + internal static void Initialize(ref FileStatus status, bool isDirectory) + { + status.InitiallyDirectory = isDirectory; + status._fileStatusInitialized = -1; + } + internal void Invalidate() => _fileStatusInitialized = -1; From 48d0d5a0115a33f921e175332e2ece18c369e4b5 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 14:27:28 -0800 Subject: [PATCH 08/31] Rename _fileStatus => _mainCache, _fileStatusInitialized => _initializedMainCache --- .../src/System/IO/FileStatus.Unix.cs | 91 ++++++++++--------- 1 file changed, 46 insertions(+), 45 deletions(-) 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 2d67d020202f9..63cc8025514ef 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 @@ -12,12 +12,13 @@ internal struct FileStatus // Exists as of the last refresh private bool _exists; - // The last cached stat information about the file - private Interop.Sys.FileStatus _fileStatus; + // The last cached lstat information about the file + private Interop.Sys.FileStatus _mainCache; - // -1 if _fileStatus isn't initialized, 0 if _fileStatus was initialized with no - // errors, or the errno error code. - private int _fileStatusInitialized; + // -1 if _mainCache isn't initialized - Refresh should always change this value + // 0 if _mainCache was initialized with no errors + // or the errno error code (always positive value) + private int _initializedMainCache; // Is a directory as of the last refresh internal bool _isDirectory; @@ -28,15 +29,15 @@ internal struct FileStatus internal void EnsureStatInitialized(ReadOnlySpan path, bool continueOnError = false) { - if (_fileStatusInitialized == -1) + if (_initializedMainCache == -1) { Refresh(path); } - if (_fileStatusInitialized != 0 && !continueOnError) + if (_initializedMainCache != 0 && !continueOnError) { - int errno = _fileStatusInitialized; - _fileStatusInitialized = -1; + int errno = _initializedMainCache; + _initializedMainCache = -1; throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(errno), new string(path)); } } @@ -59,7 +60,7 @@ internal FileAttributes GetAttributes(ReadOnlySpan path, ReadOnlySpan path, ReadOnlySpan 0 && (fileName[0] == '.' || (_fileStatus.UserFlags & (uint)Interop.Sys.UserFlags.UF_HIDDEN) == (uint)Interop.Sys.UserFlags.UF_HIDDEN)) + if (fileName.Length > 0 && (fileName[0] == '.' || (_mainCache.UserFlags & (uint)Interop.Sys.UserFlags.UF_HIDDEN) == (uint)Interop.Sys.UserFlags.UF_HIDDEN)) { attributes |= FileAttributes.Hidden; } @@ -86,25 +87,25 @@ internal DateTimeOffset GetCreationTime(ReadOnlySpan path, bool continueOn return DateTimeOffset.FromFileTime(0); } - if ((_fileStatus.Flags & Interop.Sys.FileStatusFlags.HasBirthTime) != 0) + if ((_mainCache.Flags & Interop.Sys.FileStatusFlags.HasBirthTime) != 0) { - return UnixTimeToDateTimeOffset(_fileStatus.BirthTime, _fileStatus.BirthTimeNsec); + return UnixTimeToDateTimeOffset(_mainCache.BirthTime, _mainCache.BirthTimeNsec); } // fall back to the oldest time we have in between change and modify time - if (_fileStatus.MTime < _fileStatus.CTime || - (_fileStatus.MTime == _fileStatus.CTime && _fileStatus.MTimeNsec < _fileStatus.CTimeNsec)) + if (_mainCache.MTime < _mainCache.CTime || + (_mainCache.MTime == _mainCache.CTime && _mainCache.MTimeNsec < _mainCache.CTimeNsec)) { - return UnixTimeToDateTimeOffset(_fileStatus.MTime, _fileStatus.MTimeNsec); + return UnixTimeToDateTimeOffset(_mainCache.MTime, _mainCache.MTimeNsec); } - return UnixTimeToDateTimeOffset(_fileStatus.CTime, _fileStatus.CTimeNsec); + return UnixTimeToDateTimeOffset(_mainCache.CTime, _mainCache.CTimeNsec); } internal bool GetExists(ReadOnlySpan path) { - if (_fileStatusInitialized == -1) + if (_initializedMainCache == -1) { Refresh(path); } @@ -119,23 +120,23 @@ internal DateTimeOffset GetLastAccessTime(ReadOnlySpan path, bool continue { return DateTimeOffset.FromFileTime(0); } - return UnixTimeToDateTimeOffset(_fileStatus.ATime, _fileStatus.ATimeNsec); + return UnixTimeToDateTimeOffset(_mainCache.ATime, _mainCache.ATimeNsec); } internal long GetLength(ReadOnlySpan path, bool continueOnError = false) { EnsureStatInitialized(path, continueOnError); - return _fileStatus.Size; + return _mainCache.Size; } internal static void Initialize(ref FileStatus status, bool isDirectory) { status.InitiallyDirectory = isDirectory; - status._fileStatusInitialized = -1; + status._initializedMainCache = -1; } internal void Invalidate() => - _fileStatusInitialized = -1; + _initializedMainCache = -1; internal bool IsReadOnly(ReadOnlySpan path, bool continueOnError = false) { @@ -146,13 +147,13 @@ internal bool IsReadOnly(ReadOnlySpan path, bool continueOnError = false) #else Interop.Sys.Permissions readBit, writeBit; - if (_fileStatus.Uid == Interop.Sys.GetEUid()) + if (_mainCache.Uid == Interop.Sys.GetEUid()) { // User effectively owns the file readBit = Interop.Sys.Permissions.S_IRUSR; writeBit = Interop.Sys.Permissions.S_IWUSR; } - else if (_fileStatus.Gid == Interop.Sys.GetEGid()) + else if (_mainCache.Gid == Interop.Sys.GetEGid()) { // User belongs to a group that effectively owns the file readBit = Interop.Sys.Permissions.S_IRGRP; @@ -166,8 +167,8 @@ internal bool IsReadOnly(ReadOnlySpan path, bool continueOnError = false) } #endif - return ((_fileStatus.Mode & (int)readBit) != 0 && // has read permission - (_fileStatus.Mode & (int)writeBit) == 0); // but not write permission + return ((_mainCache.Mode & (int)readBit) != 0 && // has read permission + (_mainCache.Mode & (int)writeBit) == 0); // but not write permission } internal void Refresh(ReadOnlySpan path) @@ -182,8 +183,8 @@ internal void Refresh(ReadOnlySpan path) // If it is a symlink, then subsequently get details on the target of the symlink, // storing those results separately. We only report failure if the initial // lstat fails, as a broken symlink should still report info on exists, attributes, etc. - _fileStatusInitialized = VerifyStatCall(Interop.Sys.LStat(path, out _fileStatus)); - if (_fileStatusInitialized != 0) + _initializedMainCache = VerifyStatCall(Interop.Sys.LStat(path, out _mainCache)); + if (_initializedMainCache != 0) { _exists = false; return; @@ -192,22 +193,22 @@ internal void Refresh(ReadOnlySpan path) _exists = true; // IMPORTANT: Is directory logic must match the logic in FileSystemEntry - _isDirectory = (_fileStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; + _isDirectory = (_mainCache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; // If we're a symlink, attempt to check the target to see if it is a directory - if ((_fileStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK && + if ((_mainCache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK && Interop.Sys.Stat(path, out Interop.Sys.FileStatus targetStatus) >= 0) { _isDirectory = (targetStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; } - _fileStatusInitialized = 0; + _initializedMainCache = 0; } private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime) { // force a refresh so that we have an up-to-date times for values not being overwritten - _fileStatusInitialized = -1; + _initializedMainCache = -1; EnsureStatInitialized(path); // we use utimes()/utimensat() to set the accessTime and writeTime @@ -229,19 +230,19 @@ private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool { buf[0].TvSec = seconds; buf[0].TvNsec = nanoseconds; - buf[1].TvSec = _fileStatus.MTime; - buf[1].TvNsec = _fileStatus.MTimeNsec; + buf[1].TvSec = _mainCache.MTime; + buf[1].TvNsec = _mainCache.MTimeNsec; } else { - buf[0].TvSec = _fileStatus.ATime; - buf[0].TvNsec = _fileStatus.ATimeNsec; + buf[0].TvSec = _mainCache.ATime; + buf[0].TvNsec = _mainCache.ATimeNsec; buf[1].TvSec = seconds; buf[1].TvNsec = nanoseconds; } #endif Interop.CheckIo(Interop.Sys.UTimensat(path, buf), path, InitiallyDirectory); - _fileStatusInitialized = -1; + _initializedMainCache = -1; } internal void SetAttributes(string path, FileAttributes attributes) @@ -272,25 +273,25 @@ internal void SetAttributes(string path, FileAttributes attributes) { if ((attributes & FileAttributes.Hidden) != 0) { - if ((_fileStatus.UserFlags & (uint)Interop.Sys.UserFlags.UF_HIDDEN) == 0) + if ((_mainCache.UserFlags & (uint)Interop.Sys.UserFlags.UF_HIDDEN) == 0) { // If Hidden flag is set and cached file status does not have the flag set then set it - Interop.CheckIo(Interop.Sys.LChflags(path, (_fileStatus.UserFlags | (uint)Interop.Sys.UserFlags.UF_HIDDEN)), path, InitiallyDirectory); + Interop.CheckIo(Interop.Sys.LChflags(path, (_mainCache.UserFlags | (uint)Interop.Sys.UserFlags.UF_HIDDEN)), path, InitiallyDirectory); } } else { - if ((_fileStatus.UserFlags & (uint)Interop.Sys.UserFlags.UF_HIDDEN) == (uint)Interop.Sys.UserFlags.UF_HIDDEN) + if ((_mainCache.UserFlags & (uint)Interop.Sys.UserFlags.UF_HIDDEN) == (uint)Interop.Sys.UserFlags.UF_HIDDEN) { // If Hidden flag is not set and cached file status does have the flag set then remove it - Interop.CheckIo(Interop.Sys.LChflags(path, (_fileStatus.UserFlags & ~(uint)Interop.Sys.UserFlags.UF_HIDDEN)), path, InitiallyDirectory); + Interop.CheckIo(Interop.Sys.LChflags(path, (_mainCache.UserFlags & ~(uint)Interop.Sys.UserFlags.UF_HIDDEN)), path, InitiallyDirectory); } } } // The only thing we can reasonably change is whether the file object is readonly by changing permissions. - int newMode = _fileStatus.Mode; + int newMode = _mainCache.Mode; if ((attributes & FileAttributes.ReadOnly) != 0) { // Take away all write permissions from user/group/everyone @@ -303,12 +304,12 @@ internal void SetAttributes(string path, FileAttributes attributes) } // Change the permissions on the file - if (newMode != _fileStatus.Mode) + if (newMode != _mainCache.Mode) { Interop.CheckIo(Interop.Sys.ChMod(path, newMode), path, InitiallyDirectory); } - _fileStatusInitialized = -1; + _initializedMainCache = -1; } internal void SetCreationTime(string path, DateTimeOffset time) => @@ -333,7 +334,7 @@ internal DateTimeOffset GetLastWriteTime(ReadOnlySpan path, bool continueO { return DateTimeOffset.FromFileTime(0); } - return UnixTimeToDateTimeOffset(_fileStatus.MTime, _fileStatus.MTimeNsec); + return UnixTimeToDateTimeOffset(_mainCache.MTime, _mainCache.MTimeNsec); } internal void SetLastWriteTime(string path, DateTimeOffset time) => From 96dd50c72c629225b441d22b9285db3360ebbda2 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 14:29:58 -0800 Subject: [PATCH 09/31] Add _secondaryCache and _initializedSecondaryCache --- .../src/System/IO/FileStatus.Unix.cs | 9 +++++++++ 1 file changed, 9 insertions(+) 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 63cc8025514ef..ed3a2856496ad 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 @@ -15,11 +15,20 @@ internal struct FileStatus // The last cached lstat information about the file private Interop.Sys.FileStatus _mainCache; + // The last cached stat information about the file + // Refresh only collects this if lstat determines the path is a symbolic link + private Interop.Sys.FileStatus _secondaryCache; + // -1 if _mainCache isn't initialized - Refresh should always change this value // 0 if _mainCache was initialized with no errors // or the errno error code (always positive value) private int _initializedMainCache; + // -1 if _secondaryCache isn't initialized - Refresh only changes this value if lstat determines the path is a symbolic link + // 0 if _secondaryCache was initialized with no errors + // or the errno error code (always positive value) + private int _initializedSecondaryCache; + // Is a directory as of the last refresh internal bool _isDirectory; From f6b35c3841a186d6420414aec1ec7bd882f501f6 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 14:33:58 -0800 Subject: [PATCH 10/31] Add IsValid. Use it where value of _initializedMainCache is checked. --- .../System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) 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 ed3a2856496ad..2496990f8316c 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 @@ -36,9 +36,13 @@ internal struct FileStatus // DirectoryInfo around this status struct or (2) actually are part of a DirectoryInfo. internal bool InitiallyDirectory { get; private set; } + private bool IsValid => + _initializedMainCache == 0 && // Should always be successfully refreshed + (_initializedSecondaryCache == -1 || _initializedSecondaryCache == 0); // Only refreshed when path is detected to be a symbolic link + internal void EnsureStatInitialized(ReadOnlySpan path, bool continueOnError = false) { - if (_initializedMainCache == -1) + if (!IsValid) { Refresh(path); } @@ -114,7 +118,7 @@ internal DateTimeOffset GetCreationTime(ReadOnlySpan path, bool continueOn internal bool GetExists(ReadOnlySpan path) { - if (_initializedMainCache == -1) + if (!IsValid) { Refresh(path); } From f8650302a737ac4d4c35dca31bf0ee917fb07c19 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 14:36:40 -0800 Subject: [PATCH 11/31] Update EnsureStatInitialized to check both _initialized* values --- .../src/System/IO/FileStatus.Unix.cs | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) 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 2496990f8316c..3faf24b3e9990 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 @@ -47,11 +47,26 @@ internal void EnsureStatInitialized(ReadOnlySpan path, bool continueOnErro Refresh(path); } - if (_initializedMainCache != 0 && !continueOnError) + if (!continueOnError) { - int errno = _initializedMainCache; - _initializedMainCache = -1; - throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(errno), new string(path)); + int errno = 0; + + // Lstat should always be initialized by Refresh + if (_initializedMainCache != 0) + { + errno = _initializedMainCache; + } + // Stat is optionally initialized when Refresh detects object is a symbolic link + else if (_initializedSecondaryCache != 0 && _initializedSecondaryCache != -1) + { + errno = _initializedSecondaryCache; + } + + if (errno != 0) + { + Invalidate(); + throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(errno), new string(path)); + } } } From 334b561dd3e0a622fdee0dbd7f556a68e3668aa3 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 14:53:17 -0800 Subject: [PATCH 12/31] Adjust VerifyStatCall to consider ENOENT, ENOTDIR as 'path not exists' cases, but not failure. Use _secondaryCache and _initializedSecondaryCache. --- .../src/System/IO/FileStatus.Unix.cs | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) 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 3faf24b3e9990..38379da36614f 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 @@ -211,26 +211,27 @@ internal void Refresh(ReadOnlySpan path) // If it is a symlink, then subsequently get details on the target of the symlink, // storing those results separately. We only report failure if the initial // lstat fails, as a broken symlink should still report info on exists, attributes, etc. - _initializedMainCache = VerifyStatCall(Interop.Sys.LStat(path, out _mainCache)); - if (_initializedMainCache != 0) + if (!VerifyStatCall(Interop.Sys.LStat(path, out _mainCache), out _initializedMainCache)) { _exists = false; return; } - _exists = true; - // IMPORTANT: Is directory logic must match the logic in FileSystemEntry _isDirectory = (_mainCache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; // If we're a symlink, attempt to check the target to see if it is a directory - if ((_mainCache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK && - Interop.Sys.Stat(path, out Interop.Sys.FileStatus targetStatus) >= 0) + if ((_mainCache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK) { - _isDirectory = (targetStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; + if (!VerifyStatCall(Interop.Sys.Stat(path, out _secondaryCache), out _initializedSecondaryCache)) + { + _exists = false; + return; + } + _isDirectory = (_secondaryCache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; } - _initializedMainCache = 0; + _exists = true; } private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime) @@ -371,8 +372,10 @@ internal DateTimeOffset GetLastWriteTime(ReadOnlySpan path, bool continueO // Receives the return value of a stat or lstat call. // If the call is unsuccessful, returns a positive number representing the last error info. // If the call is successful, returns 0. - private int VerifyStatCall(int returnValue) + private bool VerifyStatCall(int returnValue, out int initialized) { + initialized = 0; + // stat and lstat return -1 on error, 0 on success if (returnValue < 0) { @@ -384,11 +387,12 @@ private int VerifyStatCall(int returnValue) errorInfo.Error != Interop.Error.ENOTDIR) // A component of the path prefix of path is not a directory { // Expect a positive integer - return errorInfo.RawErrno; + initialized = errorInfo.RawErrno; } + return false; } - return 0; + return true; } private DateTimeOffset UnixTimeToDateTimeOffset(long seconds, long nanoseconds) => From e8e5d9b6df8e2270091850a866ca634d53104e23 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 15:14:25 -0800 Subject: [PATCH 13/31] Add and consume IsSymbolicLink and HasSymbolicLinkFlag. --- .../src/System/IO/FileStatus.Unix.cs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) 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 38379da36614f..da03994097d67 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 @@ -32,6 +32,10 @@ internal struct FileStatus // Is a directory as of the last refresh internal bool _isDirectory; + // Only call if Refresh has been successfully called at least once + private bool HasSymbolicLinkFlag => + (_mainCache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK; + // We track intent of creation to know whether or not we want to (1) create a // DirectoryInfo around this status struct or (2) actually are part of a DirectoryInfo. internal bool InitiallyDirectory { get; private set; } @@ -88,7 +92,7 @@ internal FileAttributes GetAttributes(ReadOnlySpan path, ReadOnlySpan path, bool continueOnError = false) (_mainCache.Mode & (int)writeBit) == 0); // but not write permission } + internal bool IsSymbolicLink(ReadOnlySpan path, bool continueOnError = false) + { + EnsureStatInitialized(path, continueOnError); + return HasSymbolicLinkFlag; + } + internal void Refresh(ReadOnlySpan path) { // This should not throw, instead we store the result so that we can throw it @@ -220,14 +230,15 @@ internal void Refresh(ReadOnlySpan path) // IMPORTANT: Is directory logic must match the logic in FileSystemEntry _isDirectory = (_mainCache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; - // If we're a symlink, attempt to check the target to see if it is a directory - if ((_mainCache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK) + // If lstat indicated the path is a symlink + if (HasSymbolicLinkFlag) { if (!VerifyStatCall(Interop.Sys.Stat(path, out _secondaryCache), out _initializedSecondaryCache)) { _exists = false; return; } + // attempt to check the target to see if it is a directory _isDirectory = (_secondaryCache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; } From 82c382f35dd2a585508f11b3200c206e24917d50 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 15:33:27 -0800 Subject: [PATCH 14/31] Fix Refresh stat call error verification to prevent bug in SymLinksMayExistIndependentlyOfTarget unit test (link should exist after main file deleted). --- .../System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) 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 da03994097d67..3bce8e0d79156 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 @@ -233,13 +233,11 @@ internal void Refresh(ReadOnlySpan path) // If lstat indicated the path is a symlink if (HasSymbolicLinkFlag) { - if (!VerifyStatCall(Interop.Sys.Stat(path, out _secondaryCache), out _initializedSecondaryCache)) + // attempt to check the target to see if it is a directory + if (VerifyStatCall(Interop.Sys.Stat(path, out _secondaryCache), out _initializedSecondaryCache)) { - _exists = false; - return; + _isDirectory = (_secondaryCache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; } - // attempt to check the target to see if it is a directory - _isDirectory = (_secondaryCache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; } _exists = true; From 4dbb740ecb74576d9640007a5454c8848d6c5b1e Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 15:36:48 -0800 Subject: [PATCH 15/31] Use Invalidate to reset _initialized* values --- .../System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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 3bce8e0d79156..a8bda018a3096 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 @@ -164,11 +164,14 @@ internal long GetLength(ReadOnlySpan path, bool continueOnError = false) internal static void Initialize(ref FileStatus status, bool isDirectory) { status.InitiallyDirectory = isDirectory; - status._initializedMainCache = -1; + status.Invalidate(); } - internal void Invalidate() => + internal void Invalidate() + { _initializedMainCache = -1; + _initializedSecondaryCache = -1; + } internal bool IsReadOnly(ReadOnlySpan path, bool continueOnError = false) { From b3cb947752fa4e80d635d1f5181096a3d12172b8 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 15:40:47 -0800 Subject: [PATCH 16/31] Add HasReadOnlyFlag and consume in GetAttributes and IsReadOnly --- .../src/System/IO/FileStatus.Unix.cs | 72 ++++++++++--------- 1 file changed, 40 insertions(+), 32 deletions(-) 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 a8bda018a3096..a74d4bed2ae56 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 @@ -32,6 +32,41 @@ internal struct FileStatus // Is a directory as of the last refresh internal bool _isDirectory; + // Only call if Refresh has been successfully called at least once + private bool HasReadOnlyFlag + { + get + { +#if TARGET_BROWSER + const Interop.Sys.Permissions readBit = Interop.Sys.Permissions.S_IRUSR; + const Interop.Sys.Permissions writeBit = Interop.Sys.Permissions.S_IWUSR; +#else + Interop.Sys.Permissions readBit, writeBit; + + if (_mainCache.Uid == Interop.Sys.GetEUid()) + { + // User effectively owns the file + readBit = Interop.Sys.Permissions.S_IRUSR; + writeBit = Interop.Sys.Permissions.S_IWUSR; + } + else if (_mainCache.Gid == Interop.Sys.GetEGid()) + { + // User belongs to a group that effectively owns the file + readBit = Interop.Sys.Permissions.S_IRGRP; + writeBit = Interop.Sys.Permissions.S_IWGRP; + } + else + { + // Others permissions + readBit = Interop.Sys.Permissions.S_IROTH; + writeBit = Interop.Sys.Permissions.S_IWOTH; + } + #endif + return ((_mainCache.Mode & (int)readBit) != 0 && // has read permission + (_mainCache.Mode & (int)writeBit) == 0); // but not write permission + } + } + // Only call if Refresh has been successfully called at least once private bool HasSymbolicLinkFlag => (_mainCache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK; @@ -87,7 +122,7 @@ internal FileAttributes GetAttributes(ReadOnlySpan path, ReadOnlySpan path, bool continueOnError = false) { EnsureStatInitialized(path, continueOnError); -#if TARGET_BROWSER - const Interop.Sys.Permissions readBit = Interop.Sys.Permissions.S_IRUSR; - const Interop.Sys.Permissions writeBit = Interop.Sys.Permissions.S_IWUSR; -#else - Interop.Sys.Permissions readBit, writeBit; - - if (_mainCache.Uid == Interop.Sys.GetEUid()) - { - // User effectively owns the file - readBit = Interop.Sys.Permissions.S_IRUSR; - writeBit = Interop.Sys.Permissions.S_IWUSR; - } - else if (_mainCache.Gid == Interop.Sys.GetEGid()) - { - // User belongs to a group that effectively owns the file - readBit = Interop.Sys.Permissions.S_IRGRP; - writeBit = Interop.Sys.Permissions.S_IWGRP; - } - else - { - // Others permissions - readBit = Interop.Sys.Permissions.S_IROTH; - writeBit = Interop.Sys.Permissions.S_IWOTH; - } -#endif - - return ((_mainCache.Mode & (int)readBit) != 0 && // has read permission - (_mainCache.Mode & (int)writeBit) == 0); // but not write permission + return HasReadOnlyFlag; } internal bool IsSymbolicLink(ReadOnlySpan path, bool continueOnError = false) @@ -249,7 +257,7 @@ internal void Refresh(ReadOnlySpan path) private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime) { // force a refresh so that we have an up-to-date times for values not being overwritten - _initializedMainCache = -1; + Invalidate(); EnsureStatInitialized(path); // we use utimes()/utimensat() to set the accessTime and writeTime @@ -283,7 +291,7 @@ private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool } #endif Interop.CheckIo(Interop.Sys.UTimensat(path, buf), path, InitiallyDirectory); - _initializedMainCache = -1; + Invalidate(); } internal void SetAttributes(string path, FileAttributes attributes) @@ -350,7 +358,7 @@ internal void SetAttributes(string path, FileAttributes attributes) Interop.CheckIo(Interop.Sys.ChMod(path, newMode), path, InitiallyDirectory); } - _initializedMainCache = -1; + Invalidate(); } internal void SetCreationTime(string path, DateTimeOffset time) => From a69901bedfe039ab799ec6154c82743af921fe5a Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 15:53:49 -0800 Subject: [PATCH 17/31] Add HasHiddenFlag and IsHidden and consume in SetAttributes --- .../src/System/IO/FileStatus.Unix.cs | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) 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 a74d4bed2ae56..2932453872494 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 @@ -32,6 +32,12 @@ internal struct FileStatus // Is a directory as of the last refresh internal bool _isDirectory; + // Checks if the main path (without following symbolic links) has the hidden attribute + // Only call if Refresh has been successfully called at least once + private bool HasHiddenFlag => + (_mainCache.UserFlags & (uint)Interop.Sys.UserFlags.UF_HIDDEN) == (uint)Interop.Sys.UserFlags.UF_HIDDEN; + + // Checks if the main path (without following symbolic links) has the read-only attribute // Only call if Refresh has been successfully called at least once private bool HasReadOnlyFlag { @@ -67,6 +73,7 @@ private bool HasReadOnlyFlag } } + // Checks if the main path is a symbolic link // Only call if Refresh has been successfully called at least once private bool HasSymbolicLinkFlag => (_mainCache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK; @@ -138,7 +145,7 @@ internal FileAttributes GetAttributes(ReadOnlySpan path, ReadOnlySpan 0 && (fileName[0] == '.' || (_mainCache.UserFlags & (uint)Interop.Sys.UserFlags.UF_HIDDEN) == (uint)Interop.Sys.UserFlags.UF_HIDDEN)) + if (fileName.Length > 0 && (fileName[0] == '.' || HasHiddenFlag)) { attributes |= FileAttributes.Hidden; } @@ -208,6 +215,12 @@ internal void Invalidate() _initializedSecondaryCache = -1; } + internal bool IsHidden(ReadOnlySpan path, bool continueOnError = false) + { + EnsureStatInitialized(path, continueOnError); + return HasHiddenFlag; + } + internal bool IsReadOnly(ReadOnlySpan path, bool continueOnError = false) { EnsureStatInitialized(path, continueOnError); @@ -320,9 +333,10 @@ internal void SetAttributes(string path, FileAttributes attributes) if (Interop.Sys.CanSetHiddenFlag) { + bool isHidden = HasHiddenFlag; if ((attributes & FileAttributes.Hidden) != 0) { - if ((_mainCache.UserFlags & (uint)Interop.Sys.UserFlags.UF_HIDDEN) == 0) + if (!isHidden) { // If Hidden flag is set and cached file status does not have the flag set then set it Interop.CheckIo(Interop.Sys.LChflags(path, (_mainCache.UserFlags | (uint)Interop.Sys.UserFlags.UF_HIDDEN)), path, InitiallyDirectory); @@ -330,7 +344,7 @@ internal void SetAttributes(string path, FileAttributes attributes) } else { - if ((_mainCache.UserFlags & (uint)Interop.Sys.UserFlags.UF_HIDDEN) == (uint)Interop.Sys.UserFlags.UF_HIDDEN) + if (isHidden) { // If Hidden flag is not set and cached file status does have the flag set then remove it Interop.CheckIo(Interop.Sys.LChflags(path, (_mainCache.UserFlags & ~(uint)Interop.Sys.UserFlags.UF_HIDDEN)), path, InitiallyDirectory); From 9dc2681d3e1c33e65191d47ffdf4b58cae890cbf Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 19:37:28 -0800 Subject: [PATCH 18/31] Add IsDirectory and HasDirectoryFlag Need to preserve the _isDirectory field. The order and the place in which it acquires a value matter. It should also be set to false when EnsureStatInitialized is called. The unit test that verifies this is System.IO.Tests.DirectoryInfo_Exists.SymLinksMayExistIndependentlyOfTarget. --- .../src/System/IO/FileStatus.Unix.cs | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) 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 2932453872494..2657aafec1bc1 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 @@ -30,7 +30,7 @@ internal struct FileStatus private int _initializedSecondaryCache; // Is a directory as of the last refresh - internal bool _isDirectory; + private bool _isDirectory; // Checks if the main path (without following symbolic links) has the hidden attribute // Only call if Refresh has been successfully called at least once @@ -179,11 +179,7 @@ internal DateTimeOffset GetCreationTime(ReadOnlySpan path, bool continueOn internal bool GetExists(ReadOnlySpan path) { - if (!IsValid) - { - Refresh(path); - } - + EnsureStatInitialized(path, continueOnError: true); return _exists && InitiallyDirectory == _isDirectory; } @@ -203,6 +199,12 @@ internal long GetLength(ReadOnlySpan path, bool continueOnError = false) return _mainCache.Size; } + // Checks if the specified stat or lstat cache has the directory attribute + // Only call if Refresh has been successfully called at least once and + // you're certain the passed-in cache was successfully retrieved + private bool HasDirectoryFlag(Interop.Sys.FileStatus cache) => + (cache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; + internal static void Initialize(ref FileStatus status, bool isDirectory) { status.InitiallyDirectory = isDirectory; @@ -215,6 +217,12 @@ internal void Invalidate() _initializedSecondaryCache = -1; } + internal bool IsDirectory(ReadOnlySpan path, bool continueOnError = false) + { + EnsureStatInitialized(path, continueOnError); + return _isDirectory; // Value should be set in Refresh + } + internal bool IsHidden(ReadOnlySpan path, bool continueOnError = false) { EnsureStatInitialized(path, continueOnError); @@ -251,16 +259,13 @@ internal void Refresh(ReadOnlySpan path) return; } - // IMPORTANT: Is directory logic must match the logic in FileSystemEntry - _isDirectory = (_mainCache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; + _isDirectory = HasDirectoryFlag(_mainCache); - // If lstat indicated the path is a symlink if (HasSymbolicLinkFlag) { - // attempt to check the target to see if it is a directory if (VerifyStatCall(Interop.Sys.Stat(path, out _secondaryCache), out _initializedSecondaryCache)) { - _isDirectory = (_secondaryCache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; + _isDirectory = HasDirectoryFlag(_secondaryCache); } } From 6ba7c79c75cfe0029874c287300468c1395e51e1 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 19:45:51 -0800 Subject: [PATCH 19/31] Remove Initialize. Call Invalidate and InitiallyDirectory directly on both usage cases. --- .../src/System/IO/Enumeration/FileSystemEntry.Unix.cs | 3 ++- .../System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs | 8 +------- .../src/System/IO/FileSystemInfo.Unix.cs | 3 ++- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs index 78390ccdec28c..0d3fef9f38841 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs @@ -66,7 +66,8 @@ namespace System.IO.Enumeration } entry._status = default; - FileStatus.Initialize(ref entry._status, isDirectory); + entry._status.Invalidate(); + entry._status.InitiallyDirectory = isDirectory; FileAttributes attributes = default; if (isSymlink) 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 2657aafec1bc1..b70bbb33a8a56 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 @@ -80,7 +80,7 @@ private bool HasReadOnlyFlag // We track intent of creation to know whether or not we want to (1) create a // DirectoryInfo around this status struct or (2) actually are part of a DirectoryInfo. - internal bool InitiallyDirectory { get; private set; } + internal bool InitiallyDirectory { get; set; } private bool IsValid => _initializedMainCache == 0 && // Should always be successfully refreshed @@ -205,12 +205,6 @@ internal long GetLength(ReadOnlySpan path, bool continueOnError = false) private bool HasDirectoryFlag(Interop.Sys.FileStatus cache) => (cache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; - internal static void Initialize(ref FileStatus status, bool isDirectory) - { - status.InitiallyDirectory = isDirectory; - status.Invalidate(); - } - internal void Invalidate() { _initializedMainCache = -1; diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileSystemInfo.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileSystemInfo.Unix.cs index 24218a8e4a6a7..c908e6f05407a 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileSystemInfo.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileSystemInfo.Unix.cs @@ -11,7 +11,8 @@ public partial class FileSystemInfo protected FileSystemInfo() { - FileStatus.Initialize(ref _fileStatus, this is DirectoryInfo); + _fileStatus.Invalidate(); + _fileStatus.InitiallyDirectory = this is DirectoryInfo; } internal static unsafe FileSystemInfo Create(string fullPath, string fileName, ref FileStatus fileStatus) From 4b3aec0b5624da7a38d0127767f9923271104253 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 20:14:15 -0800 Subject: [PATCH 20/31] FileSystemEntry.Initialize should get stat info via its FileStatus instance. --- .../IO/Enumeration/FileSystemEntry.Unix.cs | 16 +++++++++------- .../src/System/IO/FileStatus.Unix.cs | 6 +++++- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs index 0d3fef9f38841..02f8199f56ed6 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs @@ -36,6 +36,10 @@ namespace System.IO.Enumeration // IMPORTANT: Attribute logic must match the logic in FileStatus + entry._status = default; + entry._status.Invalidate(); + entry._status.EnsureStatInitialized(entry.FullPath, continueOnError: true); + bool isDirectory = false; bool isSymlink = false; if (directoryEntry.InodeType == Interop.Sys.NodeType.DT_DIR) @@ -46,13 +50,13 @@ namespace System.IO.Enumeration // Some operating systems don't have the inode type in the dirent structure, // so we use DT_UNKNOWN as a sentinel value. As such, check if the dirent is a // directory. - else if ((directoryEntry.InodeType == Interop.Sys.NodeType.DT_LNK - || directoryEntry.InodeType == Interop.Sys.NodeType.DT_UNKNOWN) - && Interop.Sys.Stat(entry.FullPath, out Interop.Sys.FileStatus targetStatus) >= 0) + else if (entry._status.IsSecondaryCacheValid && // Set by EnsureStatInitialized + (directoryEntry.InodeType == Interop.Sys.NodeType.DT_LNK || directoryEntry.InodeType == Interop.Sys.NodeType.DT_UNKNOWN)) { - // Symlink or unknown: Stat to it to see if we can resolve it to a directory. - isDirectory = (targetStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; + // Symlink or unknown: Stat should tell us if we can resolve it to a directory. + isDirectory = entry._status.HasSecondaryDirectoryFlag; } + // Same idea as the directory check, just repeated for (and tweaked due to the // nature of) symlinks. if (directoryEntry.InodeType == Interop.Sys.NodeType.DT_LNK) @@ -65,8 +69,6 @@ namespace System.IO.Enumeration isSymlink = (linkTargetStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK; } - entry._status = default; - entry._status.Invalidate(); entry._status.InitiallyDirectory = isDirectory; FileAttributes attributes = default; 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 b70bbb33a8a56..65fae32887331 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 @@ -73,6 +73,8 @@ private bool HasReadOnlyFlag } } + internal bool HasSecondaryDirectoryFlag => HasDirectoryFlag(_secondaryCache); + // Checks if the main path is a symbolic link // Only call if Refresh has been successfully called at least once private bool HasSymbolicLinkFlag => @@ -82,6 +84,8 @@ private bool HasReadOnlyFlag // DirectoryInfo around this status struct or (2) actually are part of a DirectoryInfo. internal bool InitiallyDirectory { get; set; } + internal bool IsSecondaryCacheValid => _initializedSecondaryCache == 0; + private bool IsValid => _initializedMainCache == 0 && // Should always be successfully refreshed (_initializedSecondaryCache == -1 || _initializedSecondaryCache == 0); // Only refreshed when path is detected to be a symbolic link @@ -202,7 +206,7 @@ internal long GetLength(ReadOnlySpan path, bool continueOnError = false) // Checks if the specified stat or lstat cache has the directory attribute // Only call if Refresh has been successfully called at least once and // you're certain the passed-in cache was successfully retrieved - private bool HasDirectoryFlag(Interop.Sys.FileStatus cache) => + private static bool HasDirectoryFlag(Interop.Sys.FileStatus cache) => (cache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; internal void Invalidate() From 75552fe7c720366bfb4d8bf06c91e750aa4d9ab8 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 20:17:33 -0800 Subject: [PATCH 21/31] FileSystemEntry.Initialize should get lstat info via its FileStatus instance. --- .../src/System/IO/Enumeration/FileSystemEntry.Unix.cs | 11 +++++------ .../src/System/IO/FileStatus.Unix.cs | 4 +++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs index 02f8199f56ed6..45f8448f6aef0 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs @@ -41,7 +41,6 @@ namespace System.IO.Enumeration entry._status.EnsureStatInitialized(entry.FullPath, continueOnError: true); bool isDirectory = false; - bool isSymlink = false; if (directoryEntry.InodeType == Interop.Sys.NodeType.DT_DIR) { // We know it's a directory. @@ -57,16 +56,16 @@ namespace System.IO.Enumeration isDirectory = entry._status.HasSecondaryDirectoryFlag; } - // Same idea as the directory check, just repeated for (and tweaked due to the - // nature of) symlinks. + // Same idea as the directory check, just repeated for (and tweaked due to the nature of) symlinks. + bool isSymlink = false; if (directoryEntry.InodeType == Interop.Sys.NodeType.DT_LNK) { isSymlink = true; } - else if ((directoryEntry.InodeType == Interop.Sys.NodeType.DT_UNKNOWN) - && (Interop.Sys.LStat(entry.FullPath, out Interop.Sys.FileStatus linkTargetStatus) >= 0)) + else if (entry._status.IsMainCacheValid && // Set by EnsureStatInitialized + directoryEntry.InodeType == Interop.Sys.NodeType.DT_UNKNOWN) { - isSymlink = (linkTargetStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK; + isSymlink = entry._status.HasSymbolicLinkFlag; } entry._status.InitiallyDirectory = isDirectory; 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 65fae32887331..ce7c81713e2db 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 @@ -77,13 +77,15 @@ private bool HasReadOnlyFlag // Checks if the main path is a symbolic link // Only call if Refresh has been successfully called at least once - private bool HasSymbolicLinkFlag => + internal bool HasSymbolicLinkFlag => (_mainCache.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK; // We track intent of creation to know whether or not we want to (1) create a // DirectoryInfo around this status struct or (2) actually are part of a DirectoryInfo. internal bool InitiallyDirectory { get; set; } + internal bool IsMainCacheValid => _initializedMainCache == 0; + internal bool IsSecondaryCacheValid => _initializedSecondaryCache == 0; private bool IsValid => From 62b77b8a8ff9e982dab933088a99156c59b7754e Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 20:27:04 -0800 Subject: [PATCH 22/31] Combine FileSystemEntry.IsHidden dot check with flag check. --- .../System/IO/Enumeration/FileSystemEntry.Unix.cs | 14 ++++++++++++-- .../src/System/IO/FileStatus.Unix.cs | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs index 45f8448f6aef0..8973f9e50b9a1 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs @@ -72,14 +72,22 @@ namespace System.IO.Enumeration FileAttributes attributes = default; if (isSymlink) + { attributes |= FileAttributes.ReparsePoint; + } if (isDirectory) + { attributes |= FileAttributes.Directory; - if (directoryEntry.Name[0] == '.') + } + if (entry.IsHidden) + { attributes |= FileAttributes.Hidden; + } if (attributes == default) + { attributes = FileAttributes.Normal; + } entry._initialAttributes = attributes; return attributes; @@ -145,7 +153,9 @@ public FileAttributes Attributes public DateTimeOffset LastAccessTimeUtc => _status.GetLastAccessTime(FullPath, continueOnError: true); public DateTimeOffset LastWriteTimeUtc => _status.GetLastWriteTime(FullPath, continueOnError: true); public bool IsDirectory => _status.InitiallyDirectory; - public bool IsHidden => _directoryEntry.Name[0] == '.'; + public bool IsHidden => + (_directoryEntry.NameLength > 0 && _directoryEntry.Name[0] == '.') || + (_status.IsMainCacheValid && _status.HasHiddenFlag); public FileSystemInfo ToFileSystemInfo() { 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 ce7c81713e2db..56fcda9f96b28 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 @@ -34,7 +34,7 @@ internal struct FileStatus // Checks if the main path (without following symbolic links) has the hidden attribute // Only call if Refresh has been successfully called at least once - private bool HasHiddenFlag => + internal bool HasHiddenFlag => (_mainCache.UserFlags & (uint)Interop.Sys.UserFlags.UF_HIDDEN) == (uint)Interop.Sys.UserFlags.UF_HIDDEN; // Checks if the main path (without following symbolic links) has the read-only attribute From ac7062b1e948ae11562d21a159e82e421e0858c2 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 21 Jan 2021 20:34:07 -0800 Subject: [PATCH 23/31] Move internal static FileSystemInfo.Create logic into FileSystemEntry.ToFileSystemInfo, since it's the only place where it is used. Move the Debug.Assert to the top with the ROS instead of the string. --- .../System/IO/Enumeration/FileSystemEntry.Unix.cs | 11 ++++++++++- .../src/System/IO/FileSystemInfo.Unix.cs | 12 ------------ 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs index 8973f9e50b9a1..8695a3188b229 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs @@ -159,8 +159,17 @@ public FileAttributes Attributes public FileSystemInfo ToFileSystemInfo() { + Debug.Assert(!PathInternal.IsPartiallyQualified(FullPath), "FullPath should be fully qualified when constructed from directory enumeration"); + string fullPath = ToFullPath(); - return FileSystemInfo.Create(fullPath, new string(FileName), ref _status); + string fileName = new(FileName); + + FileSystemInfo info = IsDirectory + ? new DirectoryInfo(fullPath, fileName: fileName, isNormalized: true) + : new FileInfo(fullPath, fileName: fileName, isNormalized: true); + + info.Init(ref _status); + return info; } /// diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileSystemInfo.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileSystemInfo.Unix.cs index c908e6f05407a..f48c7115dcbcf 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileSystemInfo.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileSystemInfo.Unix.cs @@ -15,18 +15,6 @@ protected FileSystemInfo() _fileStatus.InitiallyDirectory = this is DirectoryInfo; } - internal static unsafe FileSystemInfo Create(string fullPath, string fileName, ref FileStatus fileStatus) - { - FileSystemInfo info = fileStatus.InitiallyDirectory - ? (FileSystemInfo)new DirectoryInfo(fullPath, fileName: fileName, isNormalized: true) - : new FileInfo(fullPath, fileName: fileName, isNormalized: true); - - Debug.Assert(!PathInternal.IsPartiallyQualified(fullPath), $"'{fullPath}' should be fully qualified when constructed from directory enumeration"); - - info.Init(ref fileStatus); - return info; - } - internal void Invalidate() => _fileStatus.Invalidate(); internal unsafe void Init(ref FileStatus fileStatus) From c4aa27fdefc40237590d8f95e6d66aca241da967 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Fri, 22 Jan 2021 11:08:06 -0800 Subject: [PATCH 24/31] In FileSystemEntry, only refresh each cache when needed. --- .../src/System/IO/Enumeration/FileSystemEntry.Unix.cs | 9 ++++----- .../src/System/IO/FileStatus.Unix.cs | 10 ++++++++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs index 8695a3188b229..52374f68b432d 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs @@ -38,7 +38,6 @@ namespace System.IO.Enumeration entry._status = default; entry._status.Invalidate(); - entry._status.EnsureStatInitialized(entry.FullPath, continueOnError: true); bool isDirectory = false; if (directoryEntry.InodeType == Interop.Sys.NodeType.DT_DIR) @@ -49,8 +48,8 @@ namespace System.IO.Enumeration // Some operating systems don't have the inode type in the dirent structure, // so we use DT_UNKNOWN as a sentinel value. As such, check if the dirent is a // directory. - else if (entry._status.IsSecondaryCacheValid && // Set by EnsureStatInitialized - (directoryEntry.InodeType == Interop.Sys.NodeType.DT_LNK || directoryEntry.InodeType == Interop.Sys.NodeType.DT_UNKNOWN)) + else if ((directoryEntry.InodeType == Interop.Sys.NodeType.DT_LNK || directoryEntry.InodeType == Interop.Sys.NodeType.DT_UNKNOWN) && + entry._status.TryRefreshMainCache(entry.FullPath)) // Only call lstat if inode is link or unknown { // Symlink or unknown: Stat should tell us if we can resolve it to a directory. isDirectory = entry._status.HasSecondaryDirectoryFlag; @@ -62,8 +61,8 @@ namespace System.IO.Enumeration { isSymlink = true; } - else if (entry._status.IsMainCacheValid && // Set by EnsureStatInitialized - directoryEntry.InodeType == Interop.Sys.NodeType.DT_UNKNOWN) + else if (directoryEntry.InodeType == Interop.Sys.NodeType.DT_UNKNOWN && + entry._status.TryRefreshSecondaryCache(entry.FullPath)) // Only call stat if inode is unknown { isSymlink = entry._status.HasSymbolicLinkFlag; } 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 56fcda9f96b28..e37caaf8ed38b 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 @@ -253,7 +253,7 @@ internal void Refresh(ReadOnlySpan path) // If it is a symlink, then subsequently get details on the target of the symlink, // storing those results separately. We only report failure if the initial // lstat fails, as a broken symlink should still report info on exists, attributes, etc. - if (!VerifyStatCall(Interop.Sys.LStat(path, out _mainCache), out _initializedMainCache)) + if (!TryRefreshMainCache(path)) { _exists = false; return; @@ -263,7 +263,7 @@ internal void Refresh(ReadOnlySpan path) if (HasSymbolicLinkFlag) { - if (VerifyStatCall(Interop.Sys.Stat(path, out _secondaryCache), out _initializedSecondaryCache)) + if (TryRefreshSecondaryCache(path)) { _isDirectory = HasDirectoryFlag(_secondaryCache); } @@ -272,6 +272,12 @@ internal void Refresh(ReadOnlySpan path) _exists = true; } + internal bool TryRefreshMainCache(ReadOnlySpan path) => + VerifyStatCall(Interop.Sys.LStat(path, out _mainCache), out _initializedMainCache); + + internal bool TryRefreshSecondaryCache(ReadOnlySpan path) => + VerifyStatCall(Interop.Sys.Stat(path, out _secondaryCache), out _initializedSecondaryCache); + private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool isAccessTime) { // force a refresh so that we have an up-to-date times for values not being overwritten From 371d1a4c01711c601f618db4fc3462d895ac9f8d Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Fri, 22 Jan 2021 11:09:14 -0800 Subject: [PATCH 25/31] Rename EnsureStatInitialized to EnsureCachesInitialized --- .../src/System/IO/FileStatus.Unix.cs | 26 +++++++++---------- .../src/System/IO/FileSystemInfo.Unix.cs | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-) 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 e37caaf8ed38b..ca00191b6e183 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 @@ -92,7 +92,7 @@ private bool HasReadOnlyFlag _initializedMainCache == 0 && // Should always be successfully refreshed (_initializedSecondaryCache == -1 || _initializedSecondaryCache == 0); // Only refreshed when path is detected to be a symbolic link - internal void EnsureStatInitialized(ReadOnlySpan path, bool continueOnError = false) + internal void EnsureCachesInitialized(ReadOnlySpan path, bool continueOnError = false) { if (!IsValid) { @@ -126,7 +126,7 @@ internal FileAttributes GetAttributes(ReadOnlySpan path, ReadOnlySpan path, ReadOnlySpan path, bool continueOnError = false) { - EnsureStatInitialized(path, continueOnError); + EnsureCachesInitialized(path, continueOnError); if (!_exists) { return DateTimeOffset.FromFileTime(0); @@ -185,13 +185,13 @@ internal DateTimeOffset GetCreationTime(ReadOnlySpan path, bool continueOn internal bool GetExists(ReadOnlySpan path) { - EnsureStatInitialized(path, continueOnError: true); + EnsureCachesInitialized(path, continueOnError: true); return _exists && InitiallyDirectory == _isDirectory; } internal DateTimeOffset GetLastAccessTime(ReadOnlySpan path, bool continueOnError = false) { - EnsureStatInitialized(path, continueOnError); + EnsureCachesInitialized(path, continueOnError); if (!_exists) { return DateTimeOffset.FromFileTime(0); @@ -201,7 +201,7 @@ internal DateTimeOffset GetLastAccessTime(ReadOnlySpan path, bool continue internal long GetLength(ReadOnlySpan path, bool continueOnError = false) { - EnsureStatInitialized(path, continueOnError); + EnsureCachesInitialized(path, continueOnError); return _mainCache.Size; } @@ -219,25 +219,25 @@ internal void Invalidate() internal bool IsDirectory(ReadOnlySpan path, bool continueOnError = false) { - EnsureStatInitialized(path, continueOnError); + EnsureCachesInitialized(path, continueOnError); return _isDirectory; // Value should be set in Refresh } internal bool IsHidden(ReadOnlySpan path, bool continueOnError = false) { - EnsureStatInitialized(path, continueOnError); + EnsureCachesInitialized(path, continueOnError); return HasHiddenFlag; } internal bool IsReadOnly(ReadOnlySpan path, bool continueOnError = false) { - EnsureStatInitialized(path, continueOnError); + EnsureCachesInitialized(path, continueOnError); return HasReadOnlyFlag; } internal bool IsSymbolicLink(ReadOnlySpan path, bool continueOnError = false) { - EnsureStatInitialized(path, continueOnError); + EnsureCachesInitialized(path, continueOnError); return HasSymbolicLinkFlag; } @@ -282,7 +282,7 @@ private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool { // force a refresh so that we have an up-to-date times for values not being overwritten Invalidate(); - EnsureStatInitialized(path); + EnsureCachesInitialized(path); // we use utimes()/utimensat() to set the accessTime and writeTime Interop.Sys.TimeSpec* buf = stackalloc Interop.Sys.TimeSpec[2]; @@ -335,7 +335,7 @@ internal void SetAttributes(string path, FileAttributes attributes) throw new ArgumentException(SR.Arg_InvalidFileAttrs, "Attributes"); } - EnsureStatInitialized(path); + EnsureCachesInitialized(path); if (!_exists) { @@ -403,7 +403,7 @@ internal void SetAttributes(string path, FileAttributes attributes) internal DateTimeOffset GetLastWriteTime(ReadOnlySpan path, bool continueOnError = false) { - EnsureStatInitialized(path, continueOnError); + EnsureCachesInitialized(path, continueOnError); if (!_exists) { return DateTimeOffset.FromFileTime(0); diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileSystemInfo.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileSystemInfo.Unix.cs index f48c7115dcbcf..7613d9a55b9df 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileSystemInfo.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileSystemInfo.Unix.cs @@ -20,7 +20,7 @@ protected FileSystemInfo() internal unsafe void Init(ref FileStatus fileStatus) { _fileStatus = fileStatus; - _fileStatus.EnsureStatInitialized(FullPath); + _fileStatus.EnsureCachesInitialized(FullPath); } public FileAttributes Attributes From f9e16194b91e350580e9fb8f4d43372968c021e2 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Fri, 22 Jan 2021 11:10:11 -0800 Subject: [PATCH 26/31] Rename Refresh to RefreshCaches --- .../System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs | 4 ++-- .../System.IO.FileSystem/src/System/IO/FileSystemInfo.Unix.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) 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 ca00191b6e183..1baffd77a670e 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 @@ -96,7 +96,7 @@ internal void EnsureCachesInitialized(ReadOnlySpan path, bool continueOnEr { if (!IsValid) { - Refresh(path); + RefreshCaches(path); } if (!continueOnError) @@ -241,7 +241,7 @@ internal bool IsSymbolicLink(ReadOnlySpan path, bool continueOnError = fal return HasSymbolicLinkFlag; } - internal void Refresh(ReadOnlySpan path) + internal void RefreshCaches(ReadOnlySpan path) { // This should not throw, instead we store the result so that we can throw it // when someone actually accesses a property. diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileSystemInfo.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileSystemInfo.Unix.cs index 7613d9a55b9df..dfeba4144a675 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileSystemInfo.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileSystemInfo.Unix.cs @@ -51,7 +51,7 @@ internal DateTimeOffset LastWriteTimeCore internal long LengthCore => _fileStatus.GetLength(FullPath); - public void Refresh() => _fileStatus.Refresh(FullPath); + public void Refresh() => _fileStatus.RefreshCaches(FullPath); internal static void ThrowNotFound(string path) { From 6c489adfb0972afae4cab2d417a2ab94d488ecfa Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Fri, 22 Jan 2021 11:18:35 -0800 Subject: [PATCH 27/31] Add ThrowOnCacheInitializationError --- .../src/System/IO/FileStatus.Unix.cs | 40 ++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) 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 1baffd77a670e..456d07e1b6d27 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 @@ -101,24 +101,7 @@ internal void EnsureCachesInitialized(ReadOnlySpan path, bool continueOnEr if (!continueOnError) { - int errno = 0; - - // Lstat should always be initialized by Refresh - if (_initializedMainCache != 0) - { - errno = _initializedMainCache; - } - // Stat is optionally initialized when Refresh detects object is a symbolic link - else if (_initializedSecondaryCache != 0 && _initializedSecondaryCache != -1) - { - errno = _initializedSecondaryCache; - } - - if (errno != 0) - { - Invalidate(); - throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(errno), new string(path)); - } + ThrowOnCacheInitializationError(path); } } @@ -271,6 +254,27 @@ internal void RefreshCaches(ReadOnlySpan path) _exists = true; } + private void ThrowOnCacheInitializationError(ReadOnlySpan path) + { + int errno = 0; + + // Lstat should always be initialized by Refresh + if (_initializedMainCache != 0) + { + errno = _initializedMainCache; + } + // Stat is optionally initialized when Refresh detects object is a symbolic link + else if (_initializedSecondaryCache != 0 && _initializedSecondaryCache != -1) + { + errno = _initializedSecondaryCache; + } + + if (errno != 0) + { + Invalidate(); + throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(errno), new string(path)); + } + } internal bool TryRefreshMainCache(ReadOnlySpan path) => VerifyStatCall(Interop.Sys.LStat(path, out _mainCache), out _initializedMainCache); From 136e2c3f37b1472dc9bd74a6b29b26213c570a8b Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Fri, 22 Jan 2021 11:23:05 -0800 Subject: [PATCH 28/31] Only refresh relevant cache for IsHidden, IsReadOnly and IsSymbolicLink --- .../src/System/IO/FileStatus.Unix.cs | 38 +++++++++++++++++-- 1 file changed, 35 insertions(+), 3 deletions(-) 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 456d07e1b6d27..84013dc771ad5 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 @@ -202,25 +202,57 @@ internal void Invalidate() internal bool IsDirectory(ReadOnlySpan path, bool continueOnError = false) { + // We first check if main path has directory flag, then follow the symbolic link in case the target has directory flag + // So we need to try to refresh both caches EnsureCachesInitialized(path, continueOnError); return _isDirectory; // Value should be set in Refresh } internal bool IsHidden(ReadOnlySpan path, bool continueOnError = false) { - EnsureCachesInitialized(path, continueOnError); + // We only check for the hidden flag in the main path without following symbolic links + if (!IsMainCacheValid) + { + if (!TryRefreshMainCache(path)) + { + if (!continueOnError) + { + ThrowOnCacheInitializationError(path); + } + } + } return HasHiddenFlag; } internal bool IsReadOnly(ReadOnlySpan path, bool continueOnError = false) { - EnsureCachesInitialized(path, continueOnError); + // We only check for the readonly flag in the main path without following symbolic links + if (!IsMainCacheValid) + { + if (!TryRefreshMainCache(path)) + { + if (!continueOnError) + { + ThrowOnCacheInitializationError(path); + } + } + } return HasReadOnlyFlag; } internal bool IsSymbolicLink(ReadOnlySpan path, bool continueOnError = false) { - EnsureCachesInitialized(path, continueOnError); + // We only check for the symbolic link flag in the main path without following symbolic links + if (!IsMainCacheValid) + { + if (!TryRefreshMainCache(path)) + { + if (!continueOnError) + { + ThrowOnCacheInitializationError(path); + } + } + } return HasSymbolicLinkFlag; } From 43476e7e49c8ba8b2986dd7716bdb45f3bbeb20a Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Fri, 22 Jan 2021 14:54:54 -0800 Subject: [PATCH 29/31] Ensure FileSystemEntry.IsHidden refreshes cache if needed. Invert FileSystemEntry.Initialize calls to refresh individual caches. --- .../src/System/IO/Enumeration/FileSystemEntry.Unix.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs index 52374f68b432d..d57576d1dea60 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs @@ -49,7 +49,7 @@ namespace System.IO.Enumeration // so we use DT_UNKNOWN as a sentinel value. As such, check if the dirent is a // directory. else if ((directoryEntry.InodeType == Interop.Sys.NodeType.DT_LNK || directoryEntry.InodeType == Interop.Sys.NodeType.DT_UNKNOWN) && - entry._status.TryRefreshMainCache(entry.FullPath)) // Only call lstat if inode is link or unknown + entry._status.TryRefreshSecondaryCache(entry.FullPath)) // Only call stat if inode is link or unknown { // Symlink or unknown: Stat should tell us if we can resolve it to a directory. isDirectory = entry._status.HasSecondaryDirectoryFlag; @@ -62,7 +62,7 @@ namespace System.IO.Enumeration isSymlink = true; } else if (directoryEntry.InodeType == Interop.Sys.NodeType.DT_UNKNOWN && - entry._status.TryRefreshSecondaryCache(entry.FullPath)) // Only call stat if inode is unknown + entry._status.TryRefreshMainCache(entry.FullPath)) // Only call lstat if inode is unknown { isSymlink = entry._status.HasSymbolicLinkFlag; } @@ -153,8 +153,7 @@ public FileAttributes Attributes public DateTimeOffset LastWriteTimeUtc => _status.GetLastWriteTime(FullPath, continueOnError: true); public bool IsDirectory => _status.InitiallyDirectory; public bool IsHidden => - (_directoryEntry.NameLength > 0 && _directoryEntry.Name[0] == '.') || - (_status.IsMainCacheValid && _status.HasHiddenFlag); + (_directoryEntry.NameLength > 0 && _directoryEntry.Name[0] == '.') || _status.IsHidden(FullPath, continueOnError: true); public FileSystemInfo ToFileSystemInfo() { From a869d2c6d6bac2c4625b1574550ca74cf6fad6f9 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Fri, 22 Jan 2021 15:43:15 -0800 Subject: [PATCH 30/31] Add FileSystemEntry.HasHiddenPrefix to check for dot in name. Do soft hidden check in Initialize to prevent perf regression. --- .../src/System/IO/Enumeration/FileSystemEntry.Unix.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs index d57576d1dea60..98fbdd704ff84 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs @@ -78,7 +78,11 @@ namespace System.IO.Enumeration { attributes |= FileAttributes.Directory; } - if (entry.IsHidden) + + // If at this point we have not collected the main cache (lstat) we don't have a hard requirement + // to hit the disk yet, so a soft check is enough to prevent perf regression. + // FileSystemEnumerator.MoveNext may hit the disk, if needed, when updating the attributes. + if (entry.HasHiddenPrefix || (entry._status.IsMainCacheValid && entry._status.HasHiddenFlag)) { attributes |= FileAttributes.Hidden; } @@ -108,6 +112,8 @@ private ReadOnlySpan FullPath } } + private bool HasHiddenPrefix => (_directoryEntry.NameLength > 0 && _directoryEntry.Name[0] == '.'); + public ReadOnlySpan FileName { get @@ -153,7 +159,7 @@ public FileAttributes Attributes public DateTimeOffset LastWriteTimeUtc => _status.GetLastWriteTime(FullPath, continueOnError: true); public bool IsDirectory => _status.InitiallyDirectory; public bool IsHidden => - (_directoryEntry.NameLength > 0 && _directoryEntry.Name[0] == '.') || _status.IsHidden(FullPath, continueOnError: true); + HasHiddenPrefix || _status.IsHidden(FullPath, continueOnError: true); // Need to hit the disk (lstat) to check for flag public FileSystemInfo ToFileSystemInfo() { From 76dd79381389cad21c915220cf3473d73f91625d Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Fri, 22 Jan 2021 15:45:52 -0800 Subject: [PATCH 31/31] Fix test comments --- .../System.IO.FileSystem/tests/Enumeration/AttributeTests.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs index 6f9fac890bc02..eba349e849a03 100644 --- a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs +++ b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs @@ -116,9 +116,8 @@ public void DirectoryAttributesAreExpected() [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.OSX)] public void IsHiddenAttribute_Windows_OSX() { - // Put a period in front to make it hidden on Unix + // Windows and MacOS hide a file by setting the hidden attribute IsHiddenAttributeInternal(useDotPrefix: false, useHiddenFlag: true); - } @@ -126,7 +125,7 @@ public void IsHiddenAttribute_Windows_OSX() [PlatformSpecific(TestPlatforms.AnyUnix)] public void IsHiddenAttribute_Unix() { - // Windows and MacOS hide a file by setting the hidden attribute + // Put a period in front to make it hidden on Unix IsHiddenAttributeInternal(useDotPrefix: true, useHiddenFlag: false); }