Fix framework abi provider (#11)

* Loading the abi file from the correct place * Better error handling + tests * Clarified comment --------- Co-authored-by: Alex Guretzki <[email protected]>
Adyen · Sep 9, 2024 · 6ece36d · 6ece36d
1 parent abf7e46
commit 6ece36d
Show file tree

Hide file tree

Showing 8 changed files with 69 additions and 16 deletions.
diff --git a/Sources/Helpers/XcodeTools.swift b/Sources/Helpers/XcodeTools.swift
@@ -22,9 +22,14 @@ struct XcodeTools {
     }
 
     private let shell: ShellHandling
+    private let fileHandler: FileHandling
 
-    init(shell: ShellHandling = Shell()) {
+    init(
+        shell: ShellHandling = Shell(),
+        fileHandler: FileHandling = FileManager.default
+    ) {
         self.shell = shell
+        self.fileHandler = fileHandler
     }
 
     func loadPackageDescription(
@@ -60,7 +65,11 @@ struct XcodeTools {
         // print("👾 \(command.joined(separator: " "))")
         let result = shell.execute(command.joined(separator: " "))
 
-        if result.range(of: "xcodebuild: error:") != nil || result.range(of: "BUILD FAILED") != nil {
+        if 
+            !fileHandler.fileExists(atPath: "\(projectDirectoryPath)/\(Constants.derivedDataPath)") ||
+            result.range(of: "xcodebuild: error:") != nil ||
+            result.range(of: "BUILD FAILED") != nil
+        {
             print(result)
             throw XcodeToolsError(
                 errorDescription: "💥 Building project failed",

diff --git a/Sources/Pipeline/Modules/ABIGenerator/ABIGenerator.swift b/Sources/Pipeline/Modules/ABIGenerator/ABIGenerator.swift
@@ -20,7 +20,7 @@ struct ABIGenerator: ABIGenerating {
         logger: Logging?
     ) {
         self.shell = shell
-        self.xcodeTools = XcodeTools(shell: shell)
+        self.xcodeTools = XcodeTools(shell: shell, fileHandler: fileHandler)
         self.packageFileHelper = .init(fileHandler: fileHandler, xcodeTools: xcodeTools)
         self.fileHandler = fileHandler
         self.logger = logger

diff --git a/Sources/Pipeline/Modules/ABIGenerator/ProjectABIProvider.swift b/Sources/Pipeline/Modules/ABIGenerator/ProjectABIProvider.swift
@@ -19,26 +19,32 @@ struct ProjectABIProvider: ABIGenerating {
         description: String
     ) throws -> [ABIGeneratorOutput] {
 
+        // TODO: For binary frameworks:
+        // Instead of using the abi.json - use the .swiftinterface file instead (Parsable with SwiftSyntax)
+        // The .swiftinterface file also does exist for SwiftPackages with binary targets
+        // (for non-binary Swift Packages we would still parse the abi.json)
+
         guard let scheme else {
             assertionFailure("ProjectABIProvider needs a scheme to be passed to \(#function)")
             return []
         }
 
-        logger?.log("📋 Locating ABI file for `\(description)`", from: String(describing: Self.self))
+        logger?.log("📋 Locating ABI file for `\(scheme)` in `\(description)`", from: String(describing: Self.self))
 
-        let swiftModulePaths = shell.execute("cd '\(projectDirectory)'; find . -type d -name '\(scheme).swiftmodule'")
+        let swiftModulePaths = shell.execute("cd '\(projectDirectory.path())'; find . -type d -name '\(scheme).swiftmodule'")
             .components(separatedBy: .newlines)
             .map { URL(filePath: $0) }
 
-        guard let swiftModuleDirectory = swiftModulePaths.first else {
+        guard let swiftModulePath = swiftModulePaths.first?.path() else {
             throw FileHandlerError.pathDoesNotExist(path: "find . -type d -name '\(scheme).swiftmodule'")
         }
 
+        let swiftModuleDirectory = projectDirectory.appending(path: swiftModulePath)
         let swiftModuleDirectoryContent = try fileHandler.contentsOfDirectory(atPath: swiftModuleDirectory.path())
         guard let abiJsonFilePath = swiftModuleDirectoryContent.first(where: {
-            $0.hasSuffix("abi.json")
+            $0.hasSuffix(".abi.json")
         }) else {
-            throw FileHandlerError.pathDoesNotExist(path: swiftModuleDirectory.appending(path: ".abi.json").path())
+            throw FileHandlerError.pathDoesNotExist(path: swiftModuleDirectory.appending(path: "[MODULE_NAME].abi.json").path())
         }
 
         logger?.debug("- `\(abiJsonFilePath)`", from: String(describing: Self.self))

diff --git a/Sources/Pipeline/Modules/ProjectBuilder.swift b/Sources/Pipeline/Modules/ProjectBuilder.swift
@@ -76,7 +76,10 @@ private extension ProjectBuilder {
             logger: logger
         )
 
-        let xcodeTools = XcodeTools(shell: shell)
+        let xcodeTools = XcodeTools(
+            shell: shell,
+            fileHandler: fileHandler
+        )
 
         try Self.buildProject(
             projectDirectoryPath: sourceWorkingDirectoryPath,

diff --git a/Sources/public-api-diff.swift b/Sources/public-api-diff.swift
@@ -21,7 +21,7 @@ struct PublicApiDiff: AsyncParsableCommand {
     @Option(help: "Where to output the result (File path)")
     public var output: String?
 
-    @Option(help: "Which scheme to build")
+    @Option(help: "Which scheme to build (Needed when comparing 2 swift frameworks)")
     public var scheme: String?
 
     public func run() async throws {

diff --git a/Tests/UnitTests/ABIGeneratorTests.swift b/Tests/UnitTests/ABIGeneratorTests.swift
@@ -52,7 +52,7 @@ class ABIGeneratorTests: XCTestCase {
 
         let scheme = "Scheme"
         let pathToSwiftModule = "path/to/\(scheme).swiftmodule"
-        let expectedAbiJsonUrl = URL(filePath: "\(pathToSwiftModule)/abi.json")
+        let expectedAbiJsonUrl = URL(filePath: "projectDir/\(pathToSwiftModule)/.abi.json")
 
         var shell = MockShell()
         shell.handleExecute = { _ in
@@ -61,16 +61,16 @@ class ABIGeneratorTests: XCTestCase {
 
         var fileHandler = MockFileHandler()
         fileHandler.handleContentsOfDirectory = { _ in
-            ["abi.json"]
+            [".abi.json"]
         }
 
         var logger = MockLogger()
         logger.handleLog = { message, subsystem in
-            XCTAssertEqual(message, "📋 Locating ABI file for `description`")
+            XCTAssertEqual(message, "📋 Locating ABI file for `Scheme` in `description`")
             XCTAssertEqual(subsystem, "ProjectABIProvider")
         }
         logger.handleDebug = { message, subsystem in
-            XCTAssertEqual(message, "- `abi.json`")
+            XCTAssertEqual(message, "- `.abi.json`")
             XCTAssertEqual(subsystem, "ProjectABIProvider")
         }
 
@@ -80,7 +80,12 @@ class ABIGeneratorTests: XCTestCase {
             logger: logger
         )
 
-        let output = try abiGenerator.generate(for: URL(filePath: "projectDir"), scheme: scheme, description: "description")
+        let output = try abiGenerator.generate(
+            for: URL(filePath: "projectDir"),
+            scheme: scheme,
+            description: "description"
+        )
+
         let expectedOutput: [ABIGeneratorOutput] = [.init(
             targetName: scheme,
             abiJsonFileUrl: expectedAbiJsonUrl

diff --git a/Tests/UnitTests/ProjectBuilderTests.swift b/Tests/UnitTests/ProjectBuilderTests.swift
@@ -41,6 +41,10 @@ class ProjectBuilderTests: XCTestCase {
             XCTAssertEqual(path, "\(baseWorkingDirectoryPath)/\(randomString)/Package.swift")
             return true
         }
+        fileHandler.handleFileExists = { path in
+            XCTAssertEqual(path, "\(baseWorkingDirectoryPath)/\(randomString)/.build")
+            return true
+        }
         var shell = MockShell()
         shell.handleExecute = { command in
             if command == "cd \(baseWorkingDirectoryPath)/\(randomString); swift package describe --type json" {

diff --git a/Tests/UnitTests/XcodeToolsTests.swift b/Tests/UnitTests/XcodeToolsTests.swift
@@ -39,7 +39,33 @@ class XcodeToolsTests: XCTestCase {
             return ""
         }
 
-        let xcodeTools = XcodeTools(shell: mockShell)
+        let fileHandler = MockFileHandler(handleFileExists: { filePath in
+            XCTAssertEqual(filePath, "\(projectDirectoryPath)/.build")
+            return true
+        })
+
+        let xcodeTools = XcodeTools(shell: mockShell, fileHandler: fileHandler)
         try xcodeTools.build(projectDirectoryPath: projectDirectoryPath, scheme: allTargetsLibraryName, isPackage: true)
     }
+
+    func test_build_failing() throws {
+        let projectDirectoryPath = UUID().uuidString
+        let allTargetsLibraryName = UUID().uuidString
+
+        let mockShell = MockShell { _ in return "" }
+
+        let fileHandler = MockFileHandler(handleFileExists: { filePath in
+            XCTAssertEqual(filePath, "\(projectDirectoryPath)/.build")
+            return false
+        })
+
+        let xcodeTools = XcodeTools(shell: mockShell, fileHandler: fileHandler)
+        do {
+            try xcodeTools.build(projectDirectoryPath: projectDirectoryPath, scheme: allTargetsLibraryName, isPackage: true)
+            XCTFail("Build should have failed")
+        } catch {
+            let xcodeToolsError = try XCTUnwrap(error as? XcodeToolsError)
+            XCTAssertEqual(xcodeToolsError.errorDescription, "💥 Building project failed")
+        }
+    }
 }