From afc23241288876ae62b827fc22e621a025c178b1 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 3 Sep 2020 10:19:01 -0700 Subject: [PATCH] Revert #40641 Reverts "Ensure FileStatus and FileSystemEntry Hidden and ReadOnly attributes are retrieved the same way" (commit 6ed1e41e590c41862b993f99c8f401c1935a523a) which introduced a perf regression. --- .../IO/Enumeration/FileSystemEntry.Unix.cs | 38 ++++++----- .../src/System/IO/FileStatus.Unix.cs | 64 ++++++------------- .../tests/Enumeration/AttributeTests.cs | 60 ++--------------- .../tests/Enumeration/SkipAttributeTests.cs | 35 +++------- 4 files changed, 53 insertions(+), 144 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 2880aaa5df8e1..78390ccdec28c 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 @@ -48,33 +48,36 @@ namespace System.IO.Enumeration // 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 statInfo) >= 0) + && Interop.Sys.Stat(entry.FullPath, out Interop.Sys.FileStatus targetStatus) >= 0) { // Symlink or unknown: Stat to it to see if we can resolve it to a directory. - isDirectory = FileStatus.IsDirectory(statInfo); + isDirectory = (targetStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; } - - // Same idea as the directory check, just repeated for (and tweaked due to the nature of) symlinks. - int resultLStat = Interop.Sys.LStat(entry.FullPath, out Interop.Sys.FileStatus lstatInfo); - - bool isReadOnly = resultLStat >= 0 && FileStatus.IsReadOnly(lstatInfo); - + // 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) { isSymlink = true; } - else if (resultLStat >= 0 && directoryEntry.InodeType == Interop.Sys.NodeType.DT_UNKNOWN) + else if ((directoryEntry.InodeType == Interop.Sys.NodeType.DT_UNKNOWN) + && (Interop.Sys.LStat(entry.FullPath, out Interop.Sys.FileStatus linkTargetStatus) >= 0)) { - isSymlink = FileStatus.IsSymLink(lstatInfo); + isSymlink = (linkTargetStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK; } - // If the filename starts with a period or has UF_HIDDEN flag set, it's hidden. - bool isHidden = directoryEntry.Name[0] == '.' || (resultLStat >= 0 && FileStatus.IsHidden(lstatInfo)); - entry._status = default; FileStatus.Initialize(ref entry._status, isDirectory); - FileAttributes attributes = FileStatus.GetAttributes(isReadOnly, isSymlink, isDirectory, isHidden); + FileAttributes attributes = default; + if (isSymlink) + attributes |= FileAttributes.ReparsePoint; + if (isDirectory) + attributes |= FileAttributes.Directory; + if (directoryEntry.Name[0] == '.') + attributes |= FileAttributes.Hidden; + + if (attributes == default) + attributes = FileAttributes.Normal; entry._initialAttributes = attributes; return attributes; @@ -140,12 +143,7 @@ public FileAttributes Attributes public DateTimeOffset LastAccessTimeUtc => _status.GetLastAccessTime(FullPath, continueOnError: true); public DateTimeOffset LastWriteTimeUtc => _status.GetLastWriteTime(FullPath, continueOnError: true); public bool IsDirectory => _status.InitiallyDirectory; - /// - /// Returns if the file is hidden; otherwise. - /// In Linux and OSX, a file can be marked hidden if the filename is prepended with a dot. - /// In Windows and OSX, a file can be hidden if the special hidden attribute is set. For example, via the enum flag. - /// - public bool IsHidden => _directoryEntry.Name[0] == '.' || (_initialAttributes & FileAttributes.Hidden) != 0; + public bool IsHidden => _directoryEntry.Name[0] == '.'; 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 3b638448ac3e6..e32d3a1c54dfd 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 @@ -39,23 +39,19 @@ internal struct FileStatus internal bool IsReadOnly(ReadOnlySpan path, bool continueOnError = false) { EnsureStatInitialized(path, continueOnError); - return IsReadOnly(_fileStatus); - } - - internal static bool IsReadOnly(Interop.Sys.FileStatus fileStatus) - { #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 (fileStatus.Uid == Interop.Sys.GetEUid()) + + if (_fileStatus.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 (_fileStatus.Gid == Interop.Sys.GetEGid()) { // User belongs to a group that effectively owns the file readBit = Interop.Sys.Permissions.S_IRGRP; @@ -69,57 +65,37 @@ internal static bool IsReadOnly(Interop.Sys.FileStatus fileStatus) } #endif - return (fileStatus.Mode & (int)readBit) != 0 && // has read permission - (fileStatus.Mode & (int)writeBit) == 0; // but not write permission + return ((_fileStatus.Mode & (int)readBit) != 0 && // has read permission + (_fileStatus.Mode & (int)writeBit) == 0); // but not write permission } - internal static bool IsDirectory(Interop.Sys.FileStatus fileStatus) + public FileAttributes GetAttributes(ReadOnlySpan path, ReadOnlySpan fileName) { - return (fileStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; - } + // IMPORTANT: Attribute logic must match the logic in FileSystemEntry - internal static bool IsHidden(Interop.Sys.FileStatus fileStatus) - { - return (fileStatus.UserFlags & (uint)Interop.Sys.UserFlags.UF_HIDDEN) == (uint)Interop.Sys.UserFlags.UF_HIDDEN; - } + EnsureStatInitialized(path); - internal static bool IsSymLink(Interop.Sys.FileStatus fileStatus) - { - return (fileStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK; - } + if (!_exists) + return (FileAttributes)(-1); - internal static FileAttributes GetAttributes(bool isReadOnly, bool isSymlink, bool isDirectory, bool isHidden) - { FileAttributes attributes = default; - if (isReadOnly) + if (IsReadOnly(path)) attributes |= FileAttributes.ReadOnly; - if (isSymlink) + + if ((_fileStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFLNK) attributes |= FileAttributes.ReparsePoint; - if (isDirectory) + + if (_isDirectory) attributes |= FileAttributes.Directory; - if (isHidden) + + // 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; } - public FileAttributes GetAttributes(ReadOnlySpan path, ReadOnlySpan fileName) - { - // IMPORTANT: Attribute logic must match the logic in FileSystemEntry - - EnsureStatInitialized(path); - - if (!_exists) - return (FileAttributes)(-1); - - return GetAttributes( - IsReadOnly(path), - IsSymLink(_fileStatus), - _isDirectory, - (fileName.Length > 0 && fileName[0] == '.') || IsHidden(_fileStatus)); - } - public void SetAttributes(string path, FileAttributes attributes) { // Validate that only flags from the attribute are being provided. This is an @@ -324,13 +300,13 @@ public void Refresh(ReadOnlySpan path) _exists = true; // IMPORTANT: Is directory logic must match the logic in FileSystemEntry - _isDirectory = IsDirectory(_fileStatus); + _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 = IsDirectory(targetStatus); + _isDirectory = (targetStatus.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR; } _fileStatusInitialized = 0; diff --git a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs index 3974b95659077..3736c0669cf78 100644 --- a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs +++ b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs @@ -113,42 +113,18 @@ public void DirectoryAttributesAreExpected() } [Fact] - [PlatformSpecific(TestPlatforms.Windows | TestPlatforms.OSX)] - public void IsHiddenAttribute_Windows_OSX() + public void IsHiddenAttribute() { - // 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())); - FileInfo fileTwo = new FileInfo(Path.Combine(testDirectory.FullName, prefix + GetTestFileName())); + + // Put a period in front to make it hidden on Unix + FileInfo fileTwo = new FileInfo(Path.Combine(testDirectory.FullName, "." + GetTestFileName())); fileOne.Create().Dispose(); fileTwo.Create().Dispose(); - - if (useHiddenFlag) - { - fileTwo.Attributes |= FileAttributes.Hidden; - } - - FileInfo fileCheck = new FileInfo(fileTwo.FullName); - Assert.Equal(fileTwo.Attributes, fileCheck.Attributes); + if (PlatformDetection.IsWindows) + fileTwo.Attributes = fileTwo.Attributes | FileAttributes.Hidden; IEnumerable enumerable = new FileSystemEnumerable( testDirectory.FullName, @@ -160,29 +136,5 @@ private void IsHiddenAttributeInternal(bool useDotPrefix, bool useHiddenFlag) 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); - } } } diff --git a/src/libraries/System.IO.FileSystem/tests/Enumeration/SkipAttributeTests.cs b/src/libraries/System.IO.FileSystem/tests/Enumeration/SkipAttributeTests.cs index cb79200bc1780..3a22c61b6f77e 100644 --- a/src/libraries/System.IO.FileSystem/tests/Enumeration/SkipAttributeTests.cs +++ b/src/libraries/System.IO.FileSystem/tests/Enumeration/SkipAttributeTests.cs @@ -21,42 +21,25 @@ protected virtual string[] GetPaths(string directory, EnumerationOptions options } [Fact] - [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) + public void SkippingHiddenFiles() { DirectoryInfo testDirectory = Directory.CreateDirectory(GetTestFilePath()); DirectoryInfo testSubdirectory = Directory.CreateDirectory(Path.Combine(testDirectory.FullName, GetTestFileName())); - FileInfo fileOne = new FileInfo(Path.Combine(testDirectory.FullName, GetTestFileName())); - FileInfo fileThree = 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())); + // Put a period in front to make it hidden on Unix + FileInfo fileTwo = 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())); fileOne.Create().Dispose(); fileTwo.Create().Dispose(); + if (PlatformDetection.IsWindows) + fileTwo.Attributes = fileTwo.Attributes | FileAttributes.Hidden; fileThree.Create().Dispose(); fileFour.Create().Dispose(); - - if (useHiddenFlag) - { - fileTwo.Attributes |= FileAttributes.Hidden; - fileFour.Attributes |= FileAttributes.Hidden; - } + if (PlatformDetection.IsWindows) + fileFour.Attributes = fileTwo.Attributes | FileAttributes.Hidden; // Default EnumerationOptions is to skip hidden string[] paths = GetPaths(testDirectory.FullName, new EnumerationOptions());