Refactored ManagedDependency to make unmanaged path an associated value of edited state

Author: mattt

Sources/SPMTestSupport/MockWorkspace.swift

@@ -437,7 +437,7 @@ public final class MockWorkspace {
         }
 
         case checkout(CheckoutState)
-        case edited(AbsolutePath?)
+        case edited(EditedState)
         case local
     }
 
@@ -476,8 +476,8 @@ public final class MockWorkspace {
                 case .branch(let branch):
                     XCTAssertEqual(dependencyCheckoutState.branch, branch, file: file, line: line)
                 }
-            case .edited(let path):
-                if dependency.state != .edited(path) {
+            case .edited(let editedState):
+                if dependency.state != .edited(editedState) {
                     XCTFail("Expected edited dependency; found '\(dependency.state)' instead", file: file, line: line)
                 }
             case .local:

Sources/Workspace/CMakeLists.txt

@@ -10,6 +10,7 @@ add_library(Workspace
   DefaultPluginScriptRunner.swift
   Destination.swift
   Diagnostics.swift
+  EditedState.swift
   Export.swift
   InitPackage.swift
   ManagedArtifact.swift

Sources/Workspace/EditedState.swift

@@ -0,0 +1,19 @@
+/*
+ This source file is part of the Swift.org open source project
+
+ Copyright (c) 2021 Apple Inc. and the Swift project authors
+ Licensed under Apache License v2.0 with Runtime Library Exception
+
+ See http://swift.org/LICENSE.txt for license information
+ See http://swift.org/CONTRIBUTORS.txt for Swift project authors
+ */
+
+import TSCBasic
+
+public enum EditedState: Equatable {
+    /// An edited dependency with a managed path relative to the editables directory.
+    case managed(RelativePath)
+
+    /// An edited dependency with an unmanaged, absolute path.
+    case unmanaged(AbsolutePath)
+}

Sources/Workspace/ManagedDependency.swift

@@ -32,7 +32,7 @@ public class ManagedDependency {
         /// If the path is non-nil, the dependency is managed by a user and is
         /// located at the path. In other words, this dependency is being used
         /// for top of the tree style development.
-        case edited(AbsolutePath?)
+        case edited(EditedState)
     }
 
     /// The package reference.
@@ -67,17 +67,6 @@ public class ManagedDependency {
     /// unedit a package.
     public internal(set) var basedOn: ManagedDependency?
 
-    public init(
-        packageRef: PackageReference,
-        subpath: RelativePath,
-        checkoutState: CheckoutState
-    ) {
-        self.package = packageRef
-        self.state = .checkout(checkoutState)
-        self.basedOn = nil
-        self.subpath = subpath
-    }
-
     /// Create a dependency present locally on the filesystem.
     public static func local(
         package: PackageReference
@@ -91,6 +80,32 @@ public class ManagedDependency {
         )
     }
 
+    /// Create an editable managed dependency based on a dependency which
+    /// was *not* in edit state.
+    public static func checkout(package: PackageReference, state: CheckoutState) -> ManagedDependency {
+        // FIXME: Temporary until subpath property is removed
+        let subpath: RelativePath
+        if let path = try? AbsolutePath(validating: package.location),
+           let lastPathComponent = path.components.last {
+            subpath = RelativePath(lastPathComponent)
+        } else {
+            subpath = RelativePath(package.identity.description)
+        }
+
+        return ManagedDependency(package: package, state: .checkout(state), subpath: subpath, basedOn: nil)
+    }
+
+    /// Create an editable managed dependency based on a dependency which
+    /// was *not* in edit state.
+    public static func edited(basedOn dependency: ManagedDependency, state: EditedState) -> ManagedDependency {
+        switch state {
+        case .managed(let path):
+            return ManagedDependency(package: dependency.package, state: .edited(state), subpath: path, basedOn: dependency)
+        case .unmanaged:
+            return ManagedDependency(package: dependency.package, state: .edited(state), subpath: dependency.subpath, basedOn: dependency)
+        }
+    }
+
     internal init(
         package: PackageReference,
         state: State,
@@ -102,28 +117,6 @@ public class ManagedDependency {
         self.basedOn = basedOn
         self.state = state
     }
-
-    private init(
-        basedOn dependency: ManagedDependency,
-        subpath: RelativePath,
-        unmanagedPath: AbsolutePath?
-    ) {
-        assert(dependency.isCheckout)
-        self.basedOn = dependency
-        self.package = dependency.package
-        self.subpath = subpath
-        self.state = .edited(unmanagedPath)
-    }
-
-    /// Create an editable managed dependency based on a dependency which
-    /// was *not* in edit state.
-    ///
-    /// - Parameters:
-    ///     - subpath: The subpath inside the editable directory.
-    ///     - unmanagedPath: A custom absolute path instead of the subpath.
-    public func editedDependency(subpath: RelativePath, unmanagedPath: AbsolutePath?) -> ManagedDependency {
-        return ManagedDependency(basedOn: self, subpath: subpath, unmanagedPath: unmanagedPath)
-    }
 }
 
 extension ManagedDependency: CustomStringConvertible {

Sources/Workspace/Workspace.swift

@@ -1070,6 +1070,8 @@ extension Workspace {
             }
         }
 
+        let editedState: EditedState
+
         // For unmanaged dependencies, create the symlink under editables dir.
         if let path = path {
             try fileSystem.createDirectory(self.location.editsDirectory)
@@ -1087,6 +1089,10 @@ extension Workspace {
                 // entry at this location.
                 try fileSystem.createSymbolicLink(symLinkPath, pointingAt: path, relative: false)
             }
+
+            editedState = .unmanaged(path)
+        } else {
+            editedState = .managed(RelativePath(packageName))
         }
 
         // Remove the existing checkout.
@@ -1097,7 +1103,7 @@ extension Workspace {
         }
 
         // Save the new state.
-        state.dependencies.add(dependency.editedDependency(subpath: RelativePath(packageName), unmanagedPath: path))
+        state.dependencies.add(.edited(basedOn: dependency, state: editedState))
         try state.saveState()
     }
 
@@ -1108,27 +1114,17 @@ extension Workspace {
         root: PackageGraphRootInput? = nil,
         diagnostics: DiagnosticsEngine
     ) throws {
-
-        // Compute if we need to force remove.
-        var forceRemove = forceRemove
-
-        switch dependency.state {
-        // If the dependency isn't in edit mode, we can't unedit it.
-        case .checkout, .local:
+        guard case .edited(let editedState) = dependency.state else {
             throw WorkspaceDiagnostics.DependencyNotInEditMode(dependencyName: dependency.package.name)
-
-        case .edited(let path):
-            if path != nil {
-                // Set force remove to true for unmanaged dependencies.  Note that
-                // this only removes the symlink under the editable directory and
-                // not the actual unmanaged package.
-                forceRemove = true
-            }
         }
 
-        // Form the edit working repo path.
         let path = self.location.editsDirectory.appending(dependency.subpath)
-        // Check for uncommited and unpushed changes if force removal is off.
+
+        var forceRemove = forceRemove
+        if case .unmanaged = editedState {
+            forceRemove = true
+        }
+
         if !forceRemove {
             let workingCopy = try repositoryManager.openWorkingCopy(at: path)
             guard !workingCopy.hasUncommittedChanges() else {
@@ -1138,6 +1134,7 @@ extension Workspace {
                 throw WorkspaceDiagnostics.UnpushedChanges(repositoryPath: path)
             }
         }
+
         // Remove the editable checkout from disk.
         if fileSystem.exists(path) {
             try fileSystem.removeFileTree(path)
@@ -1148,10 +1145,10 @@ extension Workspace {
         }
 
         if case .checkout(let checkoutState) = dependency.basedOn?.state {
-                // Restore the original checkout.
-                //
-                // The clone method will automatically update the managed dependency state.
-                _ = try clone(package: dependency.package, at: checkoutState)
+            // Restore the original checkout.
+            //
+            // The clone method will automatically update the managed dependency state.
+            _ = try clone(package: dependency.package, at: checkoutState)
         } else {
             // The original dependency was removed, update the managed dependency state.
             state.dependencies.remove(forURL: dependency.package.location)
@@ -1472,8 +1469,10 @@ extension Workspace {
         switch dependency.state {
         case .checkout:
             return self.location.repositoriesCheckoutsDirectory.appending(dependency.subpath)
-        case .edited(let path):
-            return path ?? self.location.editsDirectory.appending(dependency.subpath)
+        case .edited(.managed(let subpath)):
+            return self.location.editsDirectory.appending(subpath)
+        case .edited(.unmanaged(let path)):
+            return path
         case .local:
             return AbsolutePath(dependency.package.location)
         }
@@ -2784,10 +2783,7 @@ extension Workspace {
         try? fileSystem.chmod(.userUnWritable, path: path, options: [.recursive, .onlyFiles])
 
         // Write the state record.
-        state.dependencies.add(ManagedDependency(
-            packageRef: package,
-            subpath: path.relative(to: self.location.repositoriesCheckoutsDirectory),
-            checkoutState: checkoutState))
+        state.dependencies.add(.checkout(package: package, state: checkoutState))
         try state.saveState()
 
         delegate?.didCheckOut(repository: package.repository.url, revision: checkoutState.description, at: path, error: nil)