Merge pull request #2928 from abertelrud/eng/add-workspace-callbacks

Add Workspace callbacks for when each package is loaded

Author: abertelrud

Sources/SPMTestSupport/TestWorkspace.swift

@@ -750,4 +750,12 @@ public final class TestWorkspaceDelegate: WorkspaceDelegate {
     public func willResolveDependencies(reason: WorkspaceResolveReason) {
         events.append("will resolve dependencies")
     }
+    
+    public func willLoadManifest(packagePath: AbsolutePath, url: String, version: Version?, packageKind: PackageReference.Kind) {
+        events.append("will load manifest for \(packageKind) package: \(url)")
+    }
+    
+    public func didLoadManifest(packagePath: AbsolutePath, url: String, version: Version?, packageKind: PackageReference.Kind, manifest: Manifest?, diagnostics: [Diagnostic]) {
+        events.append("did load manifest for \(packageKind) package: \(url)")
+    }
 }

Sources/Workspace/Workspace.swift

@@ -1,7 +1,7 @@
 /*
  This source file is part of the Swift.org open source project
 
- Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
+ Copyright (c) 2014 - 2020 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
@@ -38,6 +38,12 @@ public enum WorkspaceResolveReason: Equatable {
 /// The delegate interface used by the workspace to report status information.
 public protocol WorkspaceDelegate: class {
 
+    /// The workspace is about to load a package manifest (which might be in the cache, or might need to be parsed). Note that this does not include speculative loading of manifests that may occr during dependency resolution; rather, it includes only the final manifest loading that happens after a particular package version has been checked out into a working directory.
+    func willLoadManifest(packagePath: AbsolutePath, url: String, version: Version?, packageKind: PackageReference.Kind)
+    
+    /// The workspace has loaded a package manifest, either successfully or not. The manifest is nil if an error occurs, in which case there will also be at least one error in the list of diagnostics (there may be warnings even if a manifest is loaded successfully).
+    func didLoadManifest(packagePath: AbsolutePath, url: String, version: Version?, packageKind: PackageReference.Kind, manifest: Manifest?, diagnostics: [Diagnostic])
+    
     /// The workspace has started fetching this repository.
     func fetchingWillBegin(repository: String)
 
@@ -53,10 +59,22 @@ public protocol WorkspaceDelegate: class {
     /// The workspace has finished updating and all the dependencies are already up-to-date.
     func dependenciesUpToDate()
 
-    /// The workspace has started cloning this repository.
+    /// The workspace is about to clone a repository from the local cache to a working directory.
+    func willClone(repository url: String, to path: AbsolutePath)
+
+    /// The workspace has cloned a repository from the local cache to a working directory. The error indicates whether the operation failed or succeeded.
+    func didClone(repository url: String, to path: AbsolutePath, error: Diagnostic?)
+    
+    /// The workspace has started cloning this repository. This callback is marginally deprecated in favor of the willClone/didClone pair.
     func cloning(repository: String)
 
-    /// The workspace is checking out a repository.
+    /// The workspace is about to check out a particular revision of a working directory.
+    func willCheckOut(repository url: String, revision: String, at path: AbsolutePath)
+
+    /// The workspace has checked out a particular revision of a working directory. The error indicates whether the operation failed or succeeded.
+    func didCheckOut(repository url: String, revision: String, at path: AbsolutePath, error: Diagnostic?)
+
+    /// The workspace is checking out a repository. This callback is marginally deprecated in favor of the willCheckOut/didCheckOut pair.
     func checkingOut(repository: String, atReference reference: String, to path: AbsolutePath)
 
     /// The workspace is removing this repository because it is no longer needed.
@@ -78,6 +96,17 @@ public protocol WorkspaceDelegate: class {
 }
 
 public extension WorkspaceDelegate {
+    func willLoadManifest(packagePath: AbsolutePath, url: String, version: Version?, packageKind: PackageReference.Kind) {}
+    func didLoadManifest(packagePath: AbsolutePath, url: String, version: Version?, packageKind: PackageReference.Kind, manifest: Manifest?, diagnostics: [Diagnostic]) {}
+    func willClone(repository url: String, to path: AbsolutePath) {
+        cloning(repository: url)
+    }
+    func didClone(repository url: String, to path: AbsolutePath, error: Diagnostic?) {}
+    func cloning(repository: String) {}
+    func willCheckOut(repository url: String, revision: String, at path: AbsolutePath) {
+        checkingOut(repository: url, atReference: revision, to: path)
+    }
+    func didCheckOut(repository url: String, revision: String, at path: AbsolutePath, error: Diagnostic?) {}
     func checkingOut(repository: String, atReference: String, to path: AbsolutePath) {}
     func repositoryWillUpdate(_ repository: String) {}
     func repositoryDidUpdate(_ repository: String) {}
@@ -1255,7 +1284,10 @@ extension Workspace {
         packageKind: PackageReference.Kind,
         diagnostics: DiagnosticsEngine
     ) -> Manifest? {
-        return diagnostics.with(location: PackageLocation.Local(packagePath: packagePath)) { diagnostics in
+        // Load the manifest, bracketed by the calls to the delegate callbacks. The delegate callback is only passed any diagnostics emited during the parsing of the manifest, but they are also forwarded up to the caller.
+        delegate?.willLoadManifest(packagePath: packagePath, url: url, version: version, packageKind: packageKind)
+        let manifestDiagnostics = DiagnosticsEngine(handlers: [{diagnostics.emit($0)}])
+        let manifest: Manifest? = diagnostics.with(location: PackageLocation.Local(packagePath: packagePath)) { diagnostics in
             return diagnostics.wrap {
                 // Load the tools version for the package.
                 let toolsVersion = try toolsVersionLoader.load(
@@ -1277,6 +1309,8 @@ extension Workspace {
                 )
             }
         }
+        delegate?.didLoadManifest(packagePath: packagePath, url: url, version: version, packageKind: packageKind, manifest: manifest, diagnostics: manifestDiagnostics.diagnostics)
+        return manifest
     }
 
     fileprivate func updateBinaryArtifacts(
@@ -2220,8 +2254,9 @@ extension Workspace {
         try fileSystem.removeFileTree(path)
 
         // Inform the delegate that we're starting cloning.
-        delegate?.cloning(repository: handle.repository.url)
+        delegate?.willClone(repository: handle.repository.url, to: path)
         try handle.cloneCheckout(to: path, editable: false)
+        delegate?.didClone(repository: handle.repository.url, to: path, error: nil)
 
         return path
     }
@@ -2247,7 +2282,7 @@ extension Workspace {
         let workingRepo = try repositoryManager.provider.openCheckout(at: path)
 
         // Inform the delegate.
-        delegate?.checkingOut(repository: package.repository.url, atReference: checkoutState.description, to: path)
+        delegate?.willCheckOut(repository: package.repository.url, revision: checkoutState.description, at: path)
 
         // Do mutable-immutable dance because checkout operation modifies the disk state.
         try fileSystem.chmod(.userWritable, path: path, options: [.recursive, .onlyFiles])
@@ -2261,6 +2296,8 @@ extension Workspace {
             checkoutState: checkoutState))
         try state.saveState()
 
+        delegate?.didCheckOut(repository: package.repository.url, revision: checkoutState.description, at: path, error: nil)
+
         return path
     }
 

Tests/WorkspaceTests/WorkspaceTests.swift

@@ -1,7 +1,7 @@
 /*
  This source file is part of the Swift.org open source project
 
- Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
+ Copyright (c) 2014 - 2020 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
@@ -90,6 +90,18 @@ final class WorkspaceTests: XCTestCase {
             result.check(dependency: "quix", at: .checkout(.version("1.2.0")))
         }
 
+        // Check the load-package callbacks.
+        XCTAssertMatch(workspace.delegate.events, [
+            .equal("will load manifest for root package: /tmp/ws/roots/Foo"),
+            .equal("did load manifest for root package: /tmp/ws/roots/Foo"),
+        ])
+        XCTAssertMatch(workspace.delegate.events, [
+            .equal("will load manifest for remote package: /tmp/ws/pkgs/Quix"),
+            .equal("did load manifest for remote package: /tmp/ws/pkgs/Quix"),
+            .equal("will load manifest for remote package: /tmp/ws/pkgs/Baz"),
+            .equal("did load manifest for remote package: /tmp/ws/pkgs/Baz")
+        ])
+
         // Close and reopen workspace.
         workspace.closeWorkspace()
         workspace.checkManagedDependencies() { result in
@@ -2720,7 +2732,7 @@ final class WorkspaceTests: XCTestCase {
         // Check we don't have updating Foo event.
         workspace.checkUpdate(roots: ["Root"]) { diagnostics in
             XCTAssertNoDiagnostics(diagnostics)
-            XCTAssertEqual(workspace.delegate.events, ["Everything is already up-to-date"])
+            XCTAssertMatch(workspace.delegate.events, ["Everything is already up-to-date"])
         }
     }