Merge pull request #47 from mxcl/fix-symlink-delete

Adds `kind` fixes deleting broken symlinks

README.md

@@ -258,9 +258,15 @@ for that as the check was deemed too expensive to be worthwhile.
     equality check is required.
 * There are several symlink paths on Mac that are typically automatically
     resolved by Foundation, eg. `/private`, we attempt to do the same for
-    functions that you would expect it (notably `realpath`), but we do *not* for
-    `Path.init`, *nor* if you are joining a path that ends up being one of these
-    paths, (eg. `Path.root.join("var/private')`).
+    functions that you would expect it (notably `realpath`), we *do* the same for
+    `Path.init`, but *do not* if you are joining a path that ends up being one of
+    these paths, (eg. `Path.root.join("var/private')`).
+
+If a `Path` is a symlink but the destination of the link does not exist `exists`
+returns `false`. This seems to be the correct thing to do since symlinks are
+meant to be an abstraction for filesystems. To instead verify that there is
+no filesystem entry there at all check if `kind` is `nil`.
+
 
 ## We do not provide change directory functionality
 

Sources/Path+Attributes.swift

@@ -83,4 +83,22 @@ public extension Path {
     #endif
         return self
     }
+
+    enum Kind {
+        case file, symlink, directory
+    }
+
+    var kind: Kind? {
+        var buf = stat()
+        guard lstat(string, &buf) == 0 else {
+            return nil
+        }
+        if buf.st_mode & S_IFMT == S_IFLNK {
+            return .symlink
+        } else if buf.st_mode & S_IFMT == S_IFDIR {
+            return .directory
+        } else {
+            return .file
+        }
+    }
 }

Sources/Path+FileManager.swift

@@ -25,11 +25,11 @@ public extension Path {
      */
     @discardableResult
     func copy(to: Path, overwrite: Bool = false) throws -> Path {
-        if overwrite, to.isFile, isFile {
+        if overwrite, let tokind = to.kind, tokind != .directory, kind != .directory {
             try FileManager.default.removeItem(at: to.url)
         }
     #if os(Linux) && !swift(>=5.1) // check if fixed
-        if !overwrite, to.isFile {
+        if !overwrite, to.kind != nil {
             throw CocoaError.error(.fileWriteFileExists)
         }
     #endif
@@ -61,15 +61,15 @@ public extension Path {
      */
     @discardableResult
     func copy(into: Path, overwrite: Bool = false) throws -> Path {
-        if !into.exists {
+        if into.kind == nil {
             try into.mkdir(.p)
         }
         let rv = into/basename()
-        if overwrite, rv.isFile {
-            try rv.delete()
+        if overwrite, let kind = rv.kind, kind != .directory {
+            try FileManager.default.removeItem(at: rv.url)
         }
     #if os(Linux) && !swift(>=5.1) // check if fixed
-        if !overwrite, rv.isFile {
+        if !overwrite, rv.kind != nil {
             throw CocoaError.error(.fileWriteFileExists)
         }
     #endif
@@ -95,7 +95,7 @@ public extension Path {
      */
     @discardableResult
     func move(to: Path, overwrite: Bool = false) throws -> Path {
-        if overwrite, to.isFile {
+        if overwrite, let kind = to.kind, kind != .directory {
             try FileManager.default.removeItem(at: to.url)
         }
         try FileManager.default.moveItem(at: url, to: to.url)
@@ -119,17 +119,21 @@ public extension Path {
      */
     @discardableResult
     func move(into: Path, overwrite: Bool = false) throws -> Path {
-        if !into.exists {
+        switch into.kind {
+        case nil:
             try into.mkdir(.p)
-        } else if !into.isDirectory {
+            fallthrough
+        case .directory?:
+            let rv = into/basename()
+            if overwrite, let rvkind = rv.kind, rvkind != .directory {
+                try FileManager.default.removeItem(at: rv.url)
+            }
+            try FileManager.default.moveItem(at: url, to: rv.url)
+            return rv
+        case .file?, .symlink?:
             throw CocoaError.error(.fileWriteFileExists)
         }
-        let rv = into/basename()
-        if overwrite, rv.isFile {
-            try FileManager.default.removeItem(at: rv.url)
-        }
-        try FileManager.default.moveItem(at: url, to: rv.url)
-        return rv
+
     }
 
     /**
@@ -138,11 +142,12 @@ public extension Path {
      ∵ *Path.swift* doesn’t error if desired end result preexists.
      - Note: On UNIX will this function will succeed if the parent directory is writable and the current user has permission.
      - Note: This function will fail if the file or directory is “locked”
+     - Note: If entry is a symlink, deletes the symlink.
      - SeeAlso: `lock()`
     */
     @inlinable
     func delete() throws {
-        if exists {
+        if kind != nil {
             try FileManager.default.removeItem(at: url)
         }
     }
@@ -154,7 +159,7 @@ public extension Path {
     @inlinable
     @discardableResult
     func touch() throws -> Path {
-        if !exists {
+        if kind == nil {
             guard FileManager.default.createFile(atPath: string, contents: nil) else {
                 throw CocoaError.error(.fileWriteUnknown)
             }
@@ -228,14 +233,17 @@ public extension Path {
      */
     @discardableResult
     func symlink(into dir: Path) throws -> Path {
-        if !dir.exists {
+        switch dir.kind {
+        case nil, .symlink?:
             try dir.mkdir(.p)
-        } else if !dir.isDirectory {
+            fallthrough
+        case .directory?:
+            let dst = dir/basename()
+            try FileManager.default.createSymbolicLink(atPath: dst.string, withDestinationPath: string)
+            return dst
+        case .file?:
             throw CocoaError.error(.fileWriteFileExists)
         }
-        let dst = dir/basename()
-        try FileManager.default.createSymbolicLink(atPath: dst.string, withDestinationPath: string)
-        return dst
     }
 }
 

Sources/Path->Bool.swift

@@ -8,7 +8,10 @@ import Darwin
 public extension Path {
     //MARK: Filesystem Properties
 
-    /// Returns true if the path represents an actual filesystem entry.
+    /**
+     - Returns: `true` if the path represents an actual filesystem entry.
+     - Note: If `self` is a symlink the return value represents the destination.
+     */
     var exists: Bool {
         return FileManager.default.fileExists(atPath: string)
     }

Tests/PathTests/PathTests.swift

@@ -69,9 +69,18 @@ class PathTests: XCTestCase {
         XCTAssertEqual((Path.root/"tmp/foo/bar").relative(to: .root/"tmp/baz"), "../foo/bar")
     }
 
-    func testExists() {
+    func testExists() throws {
         XCTAssert(Path.root.exists)
         XCTAssert((Path.root/"bin").exists)
+
+        try Path.mktemp { tmpdir in
+            XCTAssertTrue(tmpdir.exists)
+            XCTAssertFalse(try tmpdir.bar.symlink(as: tmpdir.foo).exists)
+            XCTAssertTrue(tmpdir.foo.kind == .symlink)
+            XCTAssertTrue(try tmpdir.bar.touch().symlink(as: tmpdir.baz).exists)
+            XCTAssertTrue(tmpdir.bar.kind == .file)
+            XCTAssertTrue(tmpdir.kind == .directory)
+        }
     }
 
     func testIsDirectory() {
@@ -379,6 +388,21 @@ class PathTests: XCTestCase {
         #if !os(Linux)
             XCTAssertThrowsError(try tmpdir.bar3.touch().lock().delete())
         #endif
+
+            // regression test: can delete a symlink that points to a non-existent file
+            let bar5 = try tmpdir.bar4.symlink(as: tmpdir.bar5)
+            XCTAssertEqual(bar5.kind, .symlink)
+            XCTAssertFalse(bar5.exists)
+            XCTAssertNoThrow(try bar5.delete())
+            XCTAssertEqual(bar5.kind, nil)
+
+            // test that deleting a symlink *only* deletes the symlink
+            let bar7 = try tmpdir.bar6.touch().symlink(as: tmpdir.bar7)
+            XCTAssertEqual(bar7.kind, .symlink)
+            XCTAssertTrue(bar7.exists)
+            XCTAssertNoThrow(try bar7.delete())
+            XCTAssertEqual(bar7.kind, nil)
+            XCTAssertEqual(tmpdir.bar6.kind, .file)
         }
     }
 
@@ -604,4 +628,14 @@ class PathTests: XCTestCase {
         let baz: String.SubSequence? = "/a/b:1".split(separator: ":").first
         _ = baz.flatMap(Path.init)
     }
+
+    func testKind() throws {
+        try Path.mktemp { tmpdir in
+            let foo = try tmpdir.foo.touch()
+            let bar = try foo.symlink(as: tmpdir.bar)
+            XCTAssertEqual(tmpdir.kind, .directory)
+            XCTAssertEqual(foo.kind, .file)
+            XCTAssertEqual(bar.kind, .symlink)
+        }
+    }
 }

Tests/PathTests/XCTestManifests.swift

@@ -27,6 +27,7 @@ extension PathTests {
         ("testInitializerForRelativePath", testInitializerForRelativePath),
         ("testIsDirectory", testIsDirectory),
         ("testJoin", testJoin),
+        ("testKind", testKind),
         ("testLock", testLock),
         ("testMkpathIfExists", testMkpathIfExists),
         ("testMktemp", testMktemp),