Merge pull request #683 from aciidb0mb3r/workspace-edit

[Workspace] Add edit(dependency:at:) method

Author: aciidgh

Sources/Commands/Workspace.swift

@@ -22,6 +22,9 @@ public enum WorkspaceOperationError: Swift.Error {
 
     /// The repository has uncommited changes.
     case hasUncommitedChanges(repo: AbsolutePath)
+
+    /// The dependency is already in edit mode.
+    case dependencyAlreadyInEditMode
 }
 
 /// The delegate interface used by the workspace to report status information.
@@ -79,7 +82,7 @@ public class Workspace {
     ///
     /// Each dependency will have a checkout containing the sources at a
     /// particular revision, and may have an associated version.
-    public struct ManagedDependency {
+    public class ManagedDependency {
         /// The specifier for the dependency.
         public let repository: RepositorySpecifier
 
@@ -89,20 +92,45 @@ public class Workspace {
         /// The current version of the dependency, if known.
         public let currentVersion: Version?
 
+        /// The dependency is in editable state i.e. user is expected to modify the sources of the dependency.
+        /// The version of the dependency will not be considered during dependency resolution.
+        var isInEditableState: Bool {
+            return basedOn != nil
+        }
+
+        /// A dependency which in editable state is based on a dependency from which it edited from.
+        /// This information is useful so it can be restored when users unedit a package.
+        let basedOn: ManagedDependency?
+
         /// The current revision of the dependency.
         ///
         /// This should always be a revision corresponding to the version in the
         /// repository, but in certain circumstances it may not be the *current*
         /// one (e.g., if this data is accessed with a different version of the
         /// package manager, which would cause an alternate version to be
         /// resolved).
-        public let currentRevision: Revision
+        public let currentRevision: Revision?
 
         fileprivate init(repository: RepositorySpecifier, subpath: RelativePath, currentVersion: Version?, currentRevision: Revision) {
             self.repository = repository
             self.subpath = subpath
             self.currentVersion = currentVersion
             self.currentRevision = currentRevision
+            self.basedOn = nil
+        }
+
+        private init(basedOn dependency: ManagedDependency, subpath: RelativePath) {
+            assert(!dependency.isInEditableState)
+            self.basedOn = dependency
+            self.repository = dependency.repository
+            self.subpath = subpath
+            self.currentRevision = nil
+            self.currentVersion = nil
+        }
+
+        /// Create an editable managed dependency based on a dependency which was *not* in edit state.
+        func makingEditable(subpath: RelativePath) -> ManagedDependency {
+            return ManagedDependency(basedOn: self, subpath: subpath)
         }
 
         // MARK: Persistence
@@ -113,41 +141,45 @@ public class Workspace {
                   case let .string(repositoryURL)? = contents["repositoryURL"],
                   case let .string(subpathString)? = contents["subpath"],
                   let currentVersionData = contents["currentVersion"],
-                  case let .string(currentRevisionString)? = contents["currentRevision"] else {
-                return nil
-            }
-            let currentVersion: Version?
-            switch currentVersionData {
-            case .null:
-                currentVersion = nil
-            case .string(let string):
-                currentVersion = Version(string)
-                if currentVersion == nil {
-                    return nil
-                }
-            default:
+                  let basedOnData = contents["basedOn"],
+                  let currentRevisionString = contents["currentRevision"] else {
                 return nil
             }
             self.repository = RepositorySpecifier(url: repositoryURL)
             self.subpath = RelativePath(subpathString)
-            self.currentVersion = currentVersion
-            self.currentRevision = Revision(identifier: currentRevisionString)
+            self.currentVersion = ManagedDependency.optionalStringTransformer(currentVersionData, transformer: Version.init)
+            self.currentRevision = ManagedDependency.optionalStringTransformer(currentRevisionString, transformer: Revision.init(identifier:))
+            self.basedOn = ManagedDependency(json: basedOnData) ?? nil
         }
 
         fileprivate func toJSON() -> JSON {
-            let currentVersionData: JSON
-            if let currentVersion = self.currentVersion {
-                currentVersionData = .string(String(describing: currentVersion))
-            } else {
-                currentVersionData = .null
-            }
             return .dictionary([
                     "repositoryURL": .string(repository.url),
                     "subpath": .string(subpath.asString),
-                    "currentVersion": currentVersionData,
-                    "currentRevision": .string(currentRevision.identifier),
+                    "currentVersion": ManagedDependency.optionalJSONTransformer(currentVersion) { .string(String(describing: $0)) },
+                    "currentRevision": ManagedDependency.optionalJSONTransformer(currentRevision) { .string($0.identifier) },
+                    "basedOn": basedOn?.toJSON() ?? .null,
                 ])
         }
+
+        // FIXME: Move these to JSON.
+        private static func optionalStringTransformer<T>(_ value: JSON, transformer: (String) -> T?) -> T? {
+            switch value {
+            case .null:
+                return nil
+            case .string(let string):
+                return transformer(string)
+            default:
+                return nil
+            }
+        }
+
+        private static func optionalJSONTransformer<T>(_ value: T?, transformer: (T) -> JSON) -> JSON {
+            guard let value = value else {
+                return .null
+            }
+            return transformer(value)
+        }
     }
 
     /// A struct representing all the current manifests (root + external) in a package graph.
@@ -199,6 +231,9 @@ public class Workspace {
     /// The path for working repository clones (checkouts).
     let checkoutsPath: AbsolutePath
 
+    /// The path where packages which are put in edit mode are checked out.
+    let editablesPath: AbsolutePath
+
     /// The manifest loader to use.
     let manifestLoader: ManifestLoaderProtocol
 
@@ -209,7 +244,7 @@ public class Workspace {
     private let containerProvider: RepositoryPackageContainerProvider
 
     /// The current state of managed dependencies.
-    private var dependencyMap: [RepositorySpecifier: ManagedDependency]
+    private(set) var dependencyMap: [RepositorySpecifier: ManagedDependency]
 
     /// The known set of dependencies.
     public var dependencies: AnySequence<ManagedDependency> {
@@ -225,17 +260,20 @@ public class Workspace {
     /// - Parameters:
     ///   - path: The path of the root package.
     ///   - dataPath: The path for the workspace data files, if explicitly provided.
+    ///   - editablesPath: The path where editable packages should be placed, if explicitly provided.
     ///   - manifestLoader: The manifest loader.
     /// - Throws: If the state was present, but could not be loaded.
     public init(
         rootPackage path: AbsolutePath,
         dataPath: AbsolutePath? = nil,
+        editablesPath: AbsolutePath? = nil,
         manifestLoader: ManifestLoaderProtocol,
         delegate: WorkspaceDelegate
     ) throws {
         self.delegate = delegate
         self.rootPackagePath = path
         self.dataPath = dataPath ?? path.appending(component: ".build")
+        self.editablesPath = editablesPath ?? path.appending(component: "Packages")
         self.manifestLoader = manifestLoader
 
         let repositoriesPath = self.dataPath.appending(component: "repositories")
@@ -286,6 +324,30 @@ public class Workspace {
         try removeFileTree(dataPath)
     }
 
+    /// Puts a dependency in edit mode creating a checkout in editables directory.
+    func edit(dependency: ManagedDependency, at revision: Revision, packageName: String) throws {
+        // Ensure that the dependency is not already in edit mode.
+        guard !dependency.isInEditableState else {
+            throw WorkspaceOperationError.dependencyAlreadyInEditMode
+        }
+
+        // Compute new path for the dependency.
+        let path = editablesPath.appending(component: packageName)
+
+        let handle = repositoryManager.lookup(repository: dependency.repository)
+        // We should already have the handle if we're editing a dependency.
+        assert(handle.isAvailable)
+
+        try handle.cloneCheckout(to: path, editable: true)
+        let workingRepo = try repositoryManager.provider.openCheckout(at: path)
+        try workingRepo.checkout(revision: revision)
+
+        // Change its stated to edited.
+        dependencyMap[dependency.repository] = dependency.makingEditable(subpath: path.relative(to: editablesPath))
+        // Save the state.
+        try saveState()
+    }
+
     // MARK: Low-level Operations
 
     /// Fetch a given `repository` and create a local checkout for it.
@@ -537,8 +599,11 @@ public class Workspace {
             let specifier = RepositorySpecifier(url: externalManifest.url)
             let managedDependency = dependencyMap[specifier]!
 
-            // If we know the manifest is at a particular version, use that.
-            if let version = managedDependency.currentVersion {
+            if managedDependency.isInEditableState {
+                // FIXME: We need a way to state that we don't want any constaints on this dependency.
+                fatalError("FIXME: Unimplemented.")
+            } else if let version = managedDependency.currentVersion {
+                // If we know the manifest is at a particular version, use that.
                 // FIXME: This is broken, successor isn't correct and should be eliminated.
                 constraints.append(RepositoryPackageConstraint(container: specifier, versionRequirement: .range(version..<version.successor())))
             } else {

Sources/SourceControl/GitRepository.swift

@@ -257,7 +257,7 @@ public class GitRepository: Repository, WorkingCheckout {
     /// Gets the current list of remotes of the repository.
     ///
     /// - Returns: An array of tuple containing name and url of the remote.
-    func remotes() throws -> [(name: String, url: String)] {
+    public func remotes() throws -> [(name: String, url: String)] {
         return try queue.sync {
             // Get the remote names.
             let remoteNamesOutput = try Git.runPopen([Git.tool, "-C", path.asString, "remote"]).chomp()

Tests/CommandsTests/WorkspaceTests.swift

@@ -372,8 +372,71 @@ final class WorkspaceTests: XCTestCase {
         }
     }
 
+    func testEditDependency() throws {
+        mktmpdir { path in
+            let manifestGraph = try MockManifestGraph(at: path,
+                rootDeps: [
+                    MockDependency("A", version: Version(1, 0, 0)..<Version(1, .max, .max)),
+                ],
+                packages: [
+                    MockPackage("A", version: v1),
+                    MockPackage("A", version: nil), // To load the edited package manifest.
+                ]
+            )
+            // Create the workspace.
+            let workspace = try Workspace(rootPackage: path, manifestLoader: manifestGraph.manifestLoader, delegate: TestWorkspaceDelegate())
+            // Load the package graph.
+            let graph = try workspace.loadPackageGraph()
+            // Sanity checks.
+            XCTAssertEqual(graph.packages.count, 2)
+            XCTAssertEqual(graph.packages.map{ $0.name }.sorted(), ["A", "Root"])
+
+            let manifests = try workspace.loadDependencyManifests()
+            guard let aManifest = manifests.lookup("A") else {
+                return XCTFail("Expected manifest for package A not found")
+            }
+
+            func getDependency(_ manifest: Manifest) -> Workspace.ManagedDependency {
+                return workspace.dependencyMap[RepositorySpecifier(url: manifest.url)]!
+            }
+
+            // Get the dependency for package A.
+            let dependency = getDependency(aManifest)
+            // It should not be in edit mode.
+            XCTAssert(!dependency.isInEditableState)
+            // Put the dependency in edit mode at its current revision.
+            try workspace.edit(dependency: dependency, at: dependency.currentRevision!, packageName: aManifest.name)
+
+            let editedDependency = getDependency(aManifest)
+            // It should be in edit mode.
+            XCTAssert(editedDependency.isInEditableState)
+            // Check the based on data.
+            XCTAssertEqual(editedDependency.basedOn?.subpath, dependency.subpath)
+            XCTAssertEqual(editedDependency.basedOn?.currentVersion, dependency.currentVersion)
+            XCTAssertEqual(editedDependency.basedOn?.currentRevision, dependency.currentRevision)
+
+            // Get the repo from edits path.
+            let editRepo = GitRepository(path: workspace.editablesPath.appending(editedDependency.subpath))
+            // Ensure that the editable checkout's remote points to the original repo path.
+            XCTAssertEqual(try editRepo.remotes()[0].url, manifestGraph.repo("A").url)
+
+            do {
+                try workspace.edit(dependency: editedDependency, at: dependency.currentRevision!, packageName: aManifest.name)
+                XCTFail("Unexpected success, \(editedDependency) is already in edit mode")
+            } catch WorkspaceOperationError.dependencyAlreadyInEditMode {}
+
+            do {
+                // Reopen workspace and check if we maintained the state.
+                let workspace = try Workspace(rootPackage: path, manifestLoader: manifestGraph.manifestLoader, delegate: TestWorkspaceDelegate())
+                let dependency = workspace.dependencyMap[RepositorySpecifier(url: aManifest.url)]!
+                XCTAssert(dependency.isInEditableState)
+            }
+        }
+    }
+
     static var allTests = [
         ("testBasics", testBasics),
+        ("testEditDependency", testEditDependency),
         ("testDependencyManifestLoading", testDependencyManifestLoading),
         ("testPackageGraphLoadingBasics", testPackageGraphLoadingBasics),
         ("testPackageGraphLoadingWithCloning", testPackageGraphLoadingWithCloning),