Merge branch 'move-walk-to-filesystem' of https://github.com/aciidb0mb3r/swift-package-manager

Author: ddunbar

Sources/Basic/FileSystem.swift

@@ -96,6 +96,9 @@ public protocol FileSystem {
     /// Check whether the given path is accessible and a file.
     func isFile(_ path: AbsolutePath) -> Bool
 
+    /// Check whether the given path is accessible and is a symbolic link.
+    func isSymlink(_ path: AbsolutePath) -> Bool
+
     /// Get the contents of the given directory, in an undefined order.
     //
     // FIXME: Actual file system interfaces will allow more efficient access to
@@ -145,6 +148,10 @@ private class LocalFileSystem: FileSystem {
     func isFile(_ path: AbsolutePath) -> Bool {
         return Basic.isFile(path)
     }
+
+    func isSymlink(_ path: AbsolutePath) -> Bool {
+        return Basic.isSymlink(path)
+    }
 
     func getDirectoryContents(_ path: AbsolutePath) throws -> [String] {
         guard let dir = libc.opendir(path.asString) else {
@@ -353,6 +360,12 @@ public class InMemoryFileSystem: FileSystem {
             return false
         }
     }
+
+    public func isSymlink(_ path: AbsolutePath) -> Bool {
+        // FIXME: Always return false until in memory implementation
+        // gets symbolic link semantics.
+        return false
+    }
 
     public func getDirectoryContents(_ path: AbsolutePath) throws -> [String] {
         guard let node = try getNode(path) else {
@@ -502,6 +515,10 @@ public struct RerootedFileSystemView: FileSystem {
         return underlyingFileSystem.isFile(formUnderlyingPath(path))
     }
 
+    public func isSymlink(_ path: AbsolutePath) -> Bool {
+        return underlyingFileSystem.isSymlink(formUnderlyingPath(path))
+    }
+
     public func getDirectoryContents(_ path: AbsolutePath) throws -> [String] {
         return try underlyingFileSystem.getDirectoryContents(formUnderlyingPath(path))
     }

Sources/Basic/PathShims.swift

@@ -134,17 +134,13 @@ public func unlink(_ path: AbsolutePath) throws {
  - Warning: directories that cannot be entered due to permission problems
  are silently ignored. So keep that in mind.
 
- - Warning: If path doesn’t exist or cannot be entered this generator will
- be empty. It is up to you to check `path` is valid before using this
- function.
- 
  - Warning: Symbolic links that point to directories are *not* followed.
 
  - Note: setting recursively to `false` still causes the generator to feed
  you the directory; just not its contents.
  */
-public func walk(_ path: AbsolutePath, recursively: Bool = true) -> RecursibleDirectoryContentsGenerator {
-    return RecursibleDirectoryContentsGenerator(path: path, recursionFilter: { _ in recursively })
+public func walk(_ path: AbsolutePath, fileSystem: FileSystem = localFileSystem, recursively: Bool = true) throws -> RecursibleDirectoryContentsGenerator {
+    return try RecursibleDirectoryContentsGenerator(path: path, fileSystem: fileSystem, recursionFilter: { _ in recursively })
 }
 
 /**
@@ -155,86 +151,52 @@ public func walk(_ path: AbsolutePath, recursively: Bool = true) -> RecursibleDi
  - Warning: directories that cannot be entered due to permissions problems
  are silently ignored. So keep that in mind.
 
- - Warning: If path doesn’t exist or cannot be entered this generator will
- be empty. It is up to you to check `path` is valid before using this
- function.
- 
  - Warning: Symbolic links that point to directories are *not* followed.
 
  - Note: returning `false` from `recursing` still produces that directory
  from the generator; just not its contents.
  */
-public func walk(_ path: AbsolutePath, recursing: (AbsolutePath) -> Bool) -> RecursibleDirectoryContentsGenerator {
-    return RecursibleDirectoryContentsGenerator(path: path, recursionFilter: recursing)
-}
-
-/**
- A generator for a single directory’s contents
- */
-private class DirectoryContentsGenerator: IteratorProtocol {
-    private let dirptr: DirHandle?
-    fileprivate let path: AbsolutePath
-    
-    fileprivate init(path: AbsolutePath) {
-        dirptr = libc.opendir(path.asString)
-        self.path = path
-    }
-    
-    deinit {
-        if let openeddir = dirptr { closedir(openeddir) }
-    }
-    
-    func next() -> dirent? {
-        guard let validdir = dirptr else { return nil }  // yuck, silently ignoring the error to maintain this pattern
-        
-        while true {
-            var entry = dirent()
-            var result: UnsafeMutablePointer<dirent>? = nil
-            guard readdir_r(validdir, &entry, &result) == 0 else { continue }
-            guard result != nil else { return nil }
-            
-            switch (entry.d_name.0, entry.d_name.1, entry.d_name.2) {
-            case (46, 0, _):   // "."
-                continue
-            case (46, 46, 0):  // ".."
-                continue
-            default:
-                return entry
-            }
-        }
-    }
+public func walk(_ path: AbsolutePath, fileSystem: FileSystem = localFileSystem, recursing: (AbsolutePath) -> Bool) throws -> RecursibleDirectoryContentsGenerator {
+    return try RecursibleDirectoryContentsGenerator(path: path, fileSystem: fileSystem, recursionFilter: recursing)
 }
 
 /**
  Produced by `walk`.
  */
 public class RecursibleDirectoryContentsGenerator: IteratorProtocol, Sequence {
-    private var current: DirectoryContentsGenerator
+    private var current: (path: AbsolutePath, iterator: IndexingIterator<[String]>)
     private var towalk = [AbsolutePath]()
+
     private let shouldRecurse: (AbsolutePath) -> Bool
+    private let fileSystem: FileSystem
 
-    private init(path: AbsolutePath, recursionFilter: (AbsolutePath) -> Bool) {
-        current = DirectoryContentsGenerator(path: path)
+    private init(path: AbsolutePath, fileSystem: FileSystem, recursionFilter: (AbsolutePath) -> Bool) throws {
+        self.fileSystem = fileSystem
+        // FIXME: getDirectoryContents should have an iterator version.
+        current = (path, try fileSystem.getDirectoryContents(path).makeIterator())
         shouldRecurse = recursionFilter
     }
-    
+
     public func next() -> AbsolutePath? {
         outer: while true {
-            guard let entry = current.next() else {
+            guard let entry = current.iterator.next() else {
                 while !towalk.isEmpty {
+                    // FIXME: This looks inefficient.
                     let path = towalk.removeFirst()
                     guard shouldRecurse(path) else { continue }
-                    current = DirectoryContentsGenerator(path: path)
+                    // Ignore if we can't get content for this path.
+                    guard let current = try? fileSystem.getDirectoryContents(path).makeIterator() else { continue }
+                    self.current = (path, current)
                     continue outer
                 }
                 return nil
             }
-            let name = entry.name ?? ""
-            let path = current.path.appending(component: name)
-            if isDirectory(path) && !isSymlink(path) {
+
+            let path = current.path.appending(component: entry)
+            if fileSystem.isDirectory(path) && !fileSystem.isSymlink(path) {
                 towalk.append(path)
             }
-            return current.path.appending(component: name)
+            return path
         }
     }
 }

Sources/Commands/SwiftTestTool.swift

@@ -187,7 +187,7 @@ public struct SwiftTestTool: SwiftTool {
         if maybePath.asString.exists {
             possibleTestPath = maybePath
         } else {
-            let possiblePaths = walk(opts.path.build).filter {
+            let possiblePaths = try walk(opts.path.build).filter {
                 $0.basename != "Package.xctest" &&   // this was our hardcoded name, may still exist if no clean
                 $0.suffix == ".xctest"
             }

Sources/PackageLoading/PackageBuilder.swift

@@ -200,7 +200,7 @@ public struct PackageBuilder {
     }
 
     func sourceRoot() throws -> AbsolutePath {
-        let viableRoots = walk(packagePath, recursively: false).filter { entry in
+        let viableRoots = try walk(packagePath, recursively: false).filter { entry in
             switch entry.basename.lowercased() {
             case "sources", "source", "src", "srcs":
                 return entry.asString.isDirectory && !excludedPaths.contains(entry)
@@ -235,16 +235,16 @@ public struct PackageBuilder {
         let srcroot = try sourceRoot()
 
         if srcroot != packagePath {
-            let invalidRootFiles = walk(packagePath, recursively: false).filter(isValidSource)
+            let invalidRootFiles = try walk(packagePath, recursively: false).filter(isValidSource)
             guard invalidRootFiles.isEmpty else {
                 throw ModuleError.invalidLayout(.invalidLayout(invalidRootFiles.map{ $0.asString }))
             }
         }
 
-        let maybeModules = walk(srcroot, recursively: false).filter(shouldConsiderDirectory)
+        let maybeModules = try walk(srcroot, recursively: false).filter(shouldConsiderDirectory)
 
         if maybeModules.count == 1 && maybeModules[0] != srcroot {
-            let invalidModuleFiles = walk(srcroot, recursively: false).filter(isValidSource)
+            let invalidModuleFiles = try walk(srcroot, recursively: false).filter(isValidSource)
             guard invalidModuleFiles.isEmpty else {
                 throw ModuleError.invalidLayout(.invalidLayout(invalidModuleFiles.map{ $0.asString }))
             }
@@ -313,7 +313,7 @@ public struct PackageBuilder {
     }
 
     private func modulify(_ path: AbsolutePath, name: String, isTest: Bool) throws -> Module {
-        let walked = walk(path, recursing: shouldConsiderDirectory).map{ $0 }
+        let walked = try walk(path, recursing: shouldConsiderDirectory).map{ $0 }
 
         let cSources = walked.filter{ isValidSource($0, validExtensions: SupportedLanguageExtension.cFamilyExtensions) }
         let swiftSources = walked.filter{ isValidSource($0, validExtensions: SupportedLanguageExtension.swiftExtensions) }
@@ -401,7 +401,7 @@ public struct PackageBuilder {
         let testsPath = packagePath.appending("Tests")
 
         // Don't try to walk Tests if it is in excludes.
-        if testsPath.asString.isDirectory && excludedPaths.contains(testsPath) { return [] }
+        guard testsPath.asString.isDirectory && !excludedPaths.contains(testsPath) else { return [] }
 
         // Create the test modules
         let testModules = try walk(testsPath, recursively: false).filter(shouldConsiderDirectory).flatMap { dir in

Sources/Xcodeproj/generate().swift

@@ -138,16 +138,16 @@ func open(_ path: AbsolutePath, body: ((String) -> Void) throws -> Void) throws
 /// Finds directories that will be added as blue folder
 /// Excludes hidden directories and Xcode projects and directories that contains source code
 func findDirectoryReferences(path: AbsolutePath) throws -> [AbsolutePath] {
-    let rootDirectories = walk(path, recursively: false)
+    let rootDirectories = try walk(path, recursively: false)
     let rootDirectoriesToConsider = rootDirectories.filter {
         if $0.suffix == ".xcodeproj" { return false }
         if $0.suffix == ".playground" { return false }
         if $0.basename.hasPrefix(".") { return false }
         return $0.asString.isDirectory
     }
 
-    let filteredDirectories = rootDirectoriesToConsider.filter {
-        let directoriesWithSources = walk($0).filter {
+    let filteredDirectories = try rootDirectoriesToConsider.filter {
+        let directoriesWithSources = try walk($0).filter {
             guard let fileExt = $0.asString.fileExt else { return false }
             return SupportedLanguageExtension.validExtensions.contains(fileExt)
         }

Tests/Basic/FileSystemTests.swift

@@ -42,6 +42,15 @@ class FileSystemTests: XCTestCase {
         XCTAssertTrue(fs.isFile(file.path))
         XCTAssertFalse(fs.isDirectory(file.path))
         XCTAssertFalse(fs.isFile("/does-not-exist"))
+        XCTAssertFalse(fs.isSymlink("/does-not-exist"))
+
+        // isSymlink()
+        let tempDir = try! TemporaryDirectory(removeTreeOnDeinit: true)
+        let sym = tempDir.path.appending("hello")
+        try! symlink(sym, pointingAt: file.path)
+        XCTAssertTrue(fs.isSymlink(sym))
+        XCTAssertTrue(fs.isFile(sym))
+        XCTAssertFalse(fs.isDirectory(sym))
 
         // isDirectory()
         XCTAssert(fs.isDirectory("/"))
@@ -146,6 +155,9 @@ class FileSystemTests: XCTestCase {
         // isFile()
         XCTAssert(!fs.isFile("/does-not-exist"))
 
+        // isSymlink()
+        XCTAssert(!fs.isSymlink("/does-not-exist"))
+
         // getDirectoryContents()
         XCTAssertThrows(FileSystemError.noEntry) {
             _ = try fs.getDirectoryContents("/does-not-exist")
@@ -206,6 +218,7 @@ class FileSystemTests: XCTestCase {
         try! fs.writeFileContents(filePath, bytes: "Hello, world!")
         XCTAssert(fs.exists(filePath))
         XCTAssertTrue(fs.isFile(filePath))
+        XCTAssertFalse(fs.isSymlink(filePath))
         XCTAssert(!fs.isDirectory(filePath))
         XCTAssertEqual(try! fs.readFileContents(filePath), "Hello, world!")
 

Tests/Basic/PathShimTests.swift

@@ -102,7 +102,7 @@ class WalkTests : XCTestCase {
             AbsolutePath("/bin"),
             AbsolutePath("/sbin")
         ]
-        for x in walk("/", recursively: false) {
+        for x in try! walk("/", recursively: false) {
             if let i = expected.index(of: x) {
                 expected.remove(at: i)
             }
@@ -117,7 +117,7 @@ class WalkTests : XCTestCase {
             root.appending(component: "Build"),
             root.appending(component: "Utility")
         ]
-        for x in walk(root) {
+        for x in try! walk(root) {
             if let i = expected.index(of: x) {
                 expected.remove(at: i)
             }
@@ -138,7 +138,7 @@ class WalkTests : XCTestCase {
         XCTAssertEqual(resolveSymlinks(tmpDirPath.appending("foo/symlink")), tmpDirPath.appending("bar"))
         XCTAssertTrue(resolveSymlinks(tmpDirPath.appending("foo/symlink/baz")).asString.isDirectory)
 
-        let results = walk(tmpDirPath.appending("foo")).map{ $0 }
+        let results = try! walk(tmpDirPath.appending("foo")).map{ $0 }
 
         XCTAssertEqual(results, [tmpDirPath.appending("foo/symlink")])
     }
@@ -154,7 +154,7 @@ class WalkTests : XCTestCase {
 
         XCTAssertTrue(tmpDirPath.appending("symlink").asString.isSymlink)
 
-        let results = walk(tmpDirPath.appending("symlink")).map{ $0 }.sorted()
+        let results = try! walk(tmpDirPath.appending("symlink")).map{ $0 }.sorted()
 
         // we recurse a symlink to a directory, so this should work,
         // but `abc` should not show because `baz` is a symlink too