Merge pull request #705 from aciidb0mb3r/add-branch-edit

[Workspace] Add option to checkout to a new branch when editing

Author: aciidgh

Sources/Commands/Workspace.swift

@@ -31,6 +31,9 @@ public enum WorkspaceOperationError: Swift.Error {
 
     /// The dependency is not in edit mode.
     case dependencyNotInEditMode
+
+    /// The branch already exists in repository.
+    case branchAlreadyExists
 }
 
 /// The delegate interface used by the workspace to report status information.
@@ -331,7 +334,15 @@ public class Workspace {
     }
 
     /// Puts a dependency in edit mode creating a checkout in editables directory.
-    func edit(dependency: ManagedDependency, at revision: Revision, packageName: String) throws {
+    ///
+    /// - Parameters:
+    ///     - dependency: The dependency to put in edit mode.
+    ///     - revision:   The revision at which the dependency checked out to.
+    ///     - packageName: The name of the package corresponding to the dependency. This is used for the checkout directory name.
+    ///     - checkoutBranch: If provided, a new branch with this name will be created from the revision provided.
+    ///
+    /// - throws: WorkspaceOperationError
+    func edit(dependency: ManagedDependency, at revision: Revision, packageName: String, checkoutBranch: String? = nil) throws {
         // Ensure that the dependency is not already in edit mode.
         guard !dependency.isInEditableState else {
             throw WorkspaceOperationError.dependencyAlreadyInEditMode
@@ -344,9 +355,21 @@ public class Workspace {
         // We should already have the handle if we're editing a dependency.
         assert(handle.isAvailable)
 
+        // If a branch is provided, make sure it isn't already present in the repository.
+        if let branch = checkoutBranch {
+            let repo = try handle.open()
+            guard !repo.exists(revision: Revision(identifier: branch)) else {
+                throw WorkspaceOperationError.branchAlreadyExists
+            }
+        }
+
         try handle.cloneCheckout(to: path, editable: true)
         let workingRepo = try repositoryManager.provider.openCheckout(at: path)
         try workingRepo.checkout(revision: revision)
+        // Checkout to the new branch if provided.
+        if let branch = checkoutBranch {
+            try workingRepo.checkout(newBranch: branch)
+        }
 
         // Change its stated to edited.
         dependencyMap[dependency.repository] = dependency.makingEditable(subpath: path.relative(to: editablesPath))

Tests/CommandsTests/WorkspaceTests.swift

@@ -420,6 +420,12 @@ final class WorkspaceTests: XCTestCase {
             let editRepo = GitRepository(path: editRepoPath)
             // Ensure that the editable checkout's remote points to the original repo path.
             XCTAssertEqual(try editRepo.remotes()[0].url, manifestGraph.repo("A").url)
+            // Check revision and head.
+            XCTAssertEqual(try editRepo.getCurrentRevision(), dependency.currentRevision!)
+            // FIXME: Current checkout behavior seems wrong, it just resets and doesn't leave checkout to a detached head.
+          #if false
+            XCTAssertEqual(try popen([Git.tool, "-C", editRepoPath.asString, "rev-parse", "--abbrev-ref", "HEAD"]).chomp(), "HEAD")
+          #endif
 
             do {
                 try workspace.edit(dependency: editedDependency, at: dependency.currentRevision!, packageName: aManifest.name)
@@ -441,6 +447,50 @@ final class WorkspaceTests: XCTestCase {
         }
     }
 
+    func testEditDependencyOnNewBranch() 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()
+            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)
+            // Put the dependency in edit mode at its current revision on a new branch.
+            try workspace.edit(dependency: dependency, at: dependency.currentRevision!, packageName: aManifest.name, checkoutBranch: "BugFix")
+            let editedDependency = getDependency(aManifest)
+            XCTAssert(editedDependency.isInEditableState)
+
+            let editRepoPath = workspace.editablesPath.appending(editedDependency.subpath)
+            let editRepo = GitRepository(path: editRepoPath)
+            XCTAssertEqual(try editRepo.getCurrentRevision(), dependency.currentRevision!)
+            XCTAssertEqual(try popen([Git.tool, "-C", editRepoPath.asString, "rev-parse", "--abbrev-ref", "HEAD"]).chomp(), "BugFix")
+            // Unedit it.
+            try workspace.unedit(dependency: editedDependency, forceRemove: false)
+            XCTAssertEqual(getDependency(aManifest).isInEditableState, false)
+
+            do {
+                try workspace.edit(dependency: dependency, at: dependency.currentRevision!, packageName: aManifest.name, checkoutBranch: "master")
+                XCTFail("Unexpected edit success")
+            } catch WorkspaceOperationError.branchAlreadyExists {}
+        }
+    }
+
     func testUneditDependency() throws {
         mktmpdir { path in
             let manifestGraph = try MockManifestGraph(at: path,
@@ -506,6 +556,7 @@ final class WorkspaceTests: XCTestCase {
     static var allTests = [
         ("testBasics", testBasics),
         ("testEditDependency", testEditDependency),
+        ("testEditDependencyOnNewBranch", testEditDependencyOnNewBranch),
         ("testDependencyManifestLoading", testDependencyManifestLoading),
         ("testPackageGraphLoadingBasics", testPackageGraphLoadingBasics),
         ("testPackageGraphLoadingWithCloning", testPackageGraphLoadingWithCloning),