Revert #40641 (#41817)
authorCarlos Sanchez <1175054+carlossanlop@users.noreply.github.com>
Fri, 4 Sep 2020 00:40:36 +0000 (17:40 -0700)
committerGitHub <noreply@github.com>
Fri, 4 Sep 2020 00:40:36 +0000 (17:40 -0700)
Reverts "Ensure FileStatus and FileSystemEntry Hidden and ReadOnly attributes are retrieved the same way"
(commit 6ed1e41e590c41862b993f99c8f401c1935a523a) which introduced a perf regression.

Co-authored-by: carlossanlop <carlossanlop@users.noreply.github.com>
src/libraries/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEntry.Unix.cs
src/libraries/System.IO.FileSystem/src/System/IO/FileStatus.Unix.cs
src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs
src/libraries/System.IO.FileSystem/tests/Enumeration/SkipAttributeTests.cs

index 2880aaa..78390cc 100644 (file)
@@ -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 @@ namespace System.IO.Enumeration
         public DateTimeOffset LastAccessTimeUtc => _status.GetLastAccessTime(FullPath, continueOnError: true);
         public DateTimeOffset LastWriteTimeUtc => _status.GetLastWriteTime(FullPath, continueOnError: true);
         public bool IsDirectory => _status.InitiallyDirectory;
-        /// <summary>
-        /// Returns <see langword="true"/> if the file is hidden; <see langword="false" /> 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 <see cref="FileSystemInfo.Attributes" /> enum flag.
-        /// </summary>
-        public bool IsHidden => _directoryEntry.Name[0] == '.' || (_initialAttributes & FileAttributes.Hidden) != 0;
+        public bool IsHidden => _directoryEntry.Name[0] == '.';
 
         public FileSystemInfo ToFileSystemInfo()
         {
index 98ff781..45b0ed8 100644 (file)
@@ -39,23 +39,19 @@ namespace System.IO
         internal bool IsReadOnly(ReadOnlySpan<char> 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 @@ namespace System.IO
             }
 #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<char> path, ReadOnlySpan<char> 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<char> path, ReadOnlySpan<char> 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
@@ -330,13 +306,13 @@ namespace System.IO
             _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;
index 3974b95..3736c06 100644 (file)
@@ -113,42 +113,18 @@ namespace System.IO.Tests.Enumeration
         }
 
         [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<string> enumerable = new FileSystemEnumerable<string>(
                 testDirectory.FullName,
@@ -160,29 +136,5 @@ namespace System.IO.Tests.Enumeration
 
             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<string> enumerable = new FileSystemEnumerable<string>(
-                 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);
-        }
     }
 }
index cb79200..3a22c61 100644 (file)
@@ -21,42 +21,25 @@ namespace System.IO.Tests.Enumeration
         }
 
         [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());