Revert "Embed external xcframeworks regardless of linking type (#6217)…

…" (#6237) This reverts commit 3bebe42.
tuist · Apr 26, 2024 · e04d3ce · e04d3ce
1 parent 492b350
commit e04d3ce
Show file tree

Hide file tree

Showing 18 changed files with 71 additions and 193 deletions.
diff --git a/Sources/TuistCore/Graph/GraphLoader.swift b/Sources/TuistCore/Graph/GraphLoader.swift
@@ -162,12 +162,11 @@ public final class GraphLoader: GraphLoading {
                 cache: cache
             )
 
-        case let .xcframework(frameworkPath, status, _, isExternal):
+        case let .xcframework(frameworkPath, status, _):
             return try loadXCFramework(
                 path: frameworkPath,
                 cache: cache,
-                status: status,
-                isExternal: isExternal
+                status: status
             )
 
         case let .sdk(name, status, _):
@@ -250,8 +249,7 @@ public final class GraphLoader: GraphLoading {
     private func loadXCFramework(
         path: AbsolutePath,
         cache: Cache,
-        status: FrameworkStatus,
-        isExternal: Bool
+        status: FrameworkStatus
     ) throws -> GraphDependency {
         if let loaded = cache.xcframeworks[path] {
             return loaded
@@ -268,7 +266,7 @@ public final class GraphLoader: GraphLoading {
             linking: metadata.linking,
             mergeable: metadata.mergeable,
             status: metadata.status,
-            isExternal: isExternal
+            macroPath: metadata.macroPath
         ))
         cache.add(xcframework: xcframework, at: path)
         return xcframework

diff --git a/Sources/TuistCore/Graph/GraphTraverser.swift b/Sources/TuistCore/Graph/GraphTraverser.swift
@@ -253,7 +253,7 @@ public class GraphTraverser: GraphTraversing {
         /// Precompiled frameworks
         var precompiledFrameworks = filterDependencies(
             from: .target(name: name, path: path),
-            test: { $0.isPrecompiledAndEmbeddable },
+            test: { $0.isPrecompiledDynamicAndLinkable },
             skip: or(canDependencyEmbedProducts, isDependencyPrecompiledMacro)
         )
         // Skip merged precompiled libraries from merging into the runnable binary
@@ -362,7 +362,7 @@ public class GraphTraverser: GraphTraversing {
             }
 
         let precompiledLibrariesAndFrameworks = Set(precompiled + precompiledDependencies)
-            .filter(\.isPrecompiledAndEmbeddable)
+            .filter(\.isPrecompiledDynamicAndLinkable)
             .compactMap { dependencyReference(to: $0, from: targetGraphDependency) }
 
         references.formUnion(precompiledLibrariesAndFrameworks)
@@ -525,7 +525,7 @@ public class GraphTraverser: GraphTraversing {
         let from = GraphDependency.target(name: name, path: path)
         let precompiledFrameworksPaths = filterDependencies(
             from: from,
-            test: { $0.isPrecompiledAndEmbeddable },
+            test: { $0.isPrecompiledDynamicAndLinkable },
             skip: canDependencyEmbedProducts
         )
         .lazy

diff --git a/Sources/TuistCore/NodeLoaders/XCFrameworkLoader.swift b/Sources/TuistCore/NodeLoaders/XCFrameworkLoader.swift
@@ -1,5 +1,4 @@
 import Foundation
-import Mockable
 import TSCBasic
 import TuistGraph
 import TuistSupport
@@ -24,13 +23,11 @@ enum XCFrameworkLoaderError: FatalError, Equatable {
     }
 }
 
-@Mockable
 public protocol XCFrameworkLoading {
     /// Reads an existing xcframework and returns its in-memory representation, `GraphDependency.xcframework`.
     /// - Parameter path: Path to the .xcframework.
     /// - Parameter status: `.optional` to weakly reference the .xcframework.
-    /// - Parameter isExternal: Whether the XCFramework comes from an external SPM project
-    func load(path: AbsolutePath, status: FrameworkStatus, isExternal: Bool) throws -> GraphDependency
+    func load(path: AbsolutePath, status: FrameworkStatus) throws -> GraphDependency
 }
 
 public final class XCFrameworkLoader: XCFrameworkLoading {
@@ -47,7 +44,7 @@ public final class XCFrameworkLoader: XCFrameworkLoading {
         self.xcframeworkMetadataProvider = xcframeworkMetadataProvider
     }
 
-    public func load(path: AbsolutePath, status: FrameworkStatus, isExternal: Bool) throws -> GraphDependency {
+    public func load(path: AbsolutePath, status: FrameworkStatus) throws -> GraphDependency {
         guard FileHandler.shared.exists(path) else {
             throw XCFrameworkLoaderError.xcframeworkNotFound(path)
         }
@@ -62,7 +59,7 @@ public final class XCFrameworkLoader: XCFrameworkLoading {
             linking: metadata.linking,
             mergeable: metadata.mergeable,
             status: metadata.status,
-            isExternal: isExternal
+            macroPath: metadata.macroPath
         )
         return .xcframework(xcframework)
     }

diff --git a/Sources/TuistCoreTesting/NodeLoaders/MockXCFrameworkLoader.swift b/Sources/TuistCoreTesting/NodeLoaders/MockXCFrameworkLoader.swift
@@ -0,0 +1,17 @@
+import Foundation
+import TSCBasic
+import TuistCore
+import TuistGraph
+
+public final class MockXCFrameworkLoader: XCFrameworkLoading {
+    public init() {}
+
+    var loadStub: ((AbsolutePath) throws -> GraphDependency)?
+    public func load(path: AbsolutePath, status: FrameworkStatus) throws -> GraphDependency {
+        if let loadStub {
+            return try loadStub(path)
+        } else {
+            return .testXCFramework(path: path, status: status)
+        }
+    }
+}
diff --git a/Sources/TuistGenerator/Linter/TargetLinter.swift b/Sources/TuistGenerator/Linter/TargetLinter.swift
@@ -324,7 +324,7 @@ extension TargetDependency {
             return target
         case let .framework(path, _, _):
             return path.basename
-        case let .xcframework(path, _, _, _):
+        case let .xcframework(path, _, _):
             return path.basename
         case let .library(path, _, _, _):
             return path.basename

diff --git a/Sources/TuistGraph/Graph/GraphDependency.swift b/Sources/TuistGraph/Graph/GraphDependency.swift
@@ -9,7 +9,6 @@ public enum GraphDependency: Hashable, CustomStringConvertible, Comparable, Coda
         public let linking: BinaryLinking
         public let mergeable: Bool
         public let status: FrameworkStatus
-        public let isExternal: Bool
 
         public init(
             path: AbsolutePath,
@@ -18,15 +17,14 @@ public enum GraphDependency: Hashable, CustomStringConvertible, Comparable, Coda
             linking: BinaryLinking,
             mergeable: Bool,
             status: FrameworkStatus,
-            isExternal: Bool
+            macroPath _: AbsolutePath?
         ) {
             self.path = path
             self.infoPlist = infoPlist
             self.primaryBinaryPath = primaryBinaryPath
             self.linking = linking
             self.mergeable = mergeable
             self.status = status
-            self.isExternal = isExternal
         }
 
         public var description: String {
@@ -208,13 +206,11 @@ public enum GraphDependency: Hashable, CustomStringConvertible, Comparable, Coda
         }
     }
 
-    public var isPrecompiledAndEmbeddable: Bool {
+    public var isPrecompiledDynamicAndLinkable: Bool {
         switch self {
         case .macro: return false
         case let .xcframework(xcframework):
-            return xcframework
-                .linking == .dynamic ||
-                (xcframework.isExternal && xcframework.infoPlist.libraries.first?.path.extension == "framework")
+            return xcframework.linking == .dynamic
         case let .framework(_, _, _, _, linking, _, _),
              let .library(path: _, publicHeaders: _, linking: linking, architectures: _, swiftModuleMap: _):
             return linking == .dynamic

diff --git a/Sources/TuistGraph/Models/TargetDependency.swift b/Sources/TuistGraph/Models/TargetDependency.swift
@@ -21,7 +21,7 @@ public enum TargetDependency: Equatable, Hashable, Codable {
     case target(name: String, condition: PlatformCondition? = nil)
     case project(target: String, path: AbsolutePath, condition: PlatformCondition? = nil)
     case framework(path: AbsolutePath, status: FrameworkStatus, condition: PlatformCondition? = nil)
-    case xcframework(path: AbsolutePath, status: FrameworkStatus, condition: PlatformCondition? = nil, isExternal: Bool = false)
+    case xcframework(path: AbsolutePath, status: FrameworkStatus, condition: PlatformCondition? = nil)
     case library(
         path: AbsolutePath,
         publicHeaders: AbsolutePath,
@@ -40,7 +40,7 @@ public enum TargetDependency: Equatable, Hashable, Codable {
             condition
         case .framework(path: _, status: _, condition: let condition):
             condition
-        case .xcframework(path: _, status: _, condition: let condition, _):
+        case .xcframework(path: _, status: _, condition: let condition):
             condition
         case .library(path: _, publicHeaders: _, swiftModuleMap: _, condition: let condition):
             condition
@@ -60,8 +60,8 @@ public enum TargetDependency: Equatable, Hashable, Codable {
             return .project(target: target, path: path, condition: condition)
         case .framework(path: let path, status: let status, condition: _):
             return .framework(path: path, status: status, condition: condition)
-        case .xcframework(path: let path, status: let status, condition: _, isExternal: let isExternal):
-            return .xcframework(path: path, status: status, condition: condition, isExternal: isExternal)
+        case .xcframework(path: let path, status: let status, condition: _):
+            return .xcframework(path: path, status: status, condition: condition)
         case .library(path: let path, publicHeaders: let headers, swiftModuleMap: let moduleMap, condition: _):
             return .library(path: path, publicHeaders: headers, swiftModuleMap: moduleMap, condition: condition)
         case .package(product: let product, type: let type, condition: _):

diff --git a/Sources/TuistGraphTesting/Graph/GraphDependency+TestData.swift b/Sources/TuistGraphTesting/Graph/GraphDependency+TestData.swift
@@ -39,7 +39,7 @@ extension GraphDependency {
             .appending(try! RelativePath(validating: "Test.xcframework/Test")),
         linking: BinaryLinking = .dynamic,
         status: FrameworkStatus = .required,
-        isExternal: Bool = false
+        macroPath: AbsolutePath? = nil
     ) -> GraphDependency {
         .xcframework(
             GraphDependency.XCFramework(
@@ -49,7 +49,7 @@ extension GraphDependency {
                 linking: linking,
                 mergeable: false,
                 status: status,
-                isExternal: isExternal
+                macroPath: macroPath
             )
         )
     }

diff --git a/Sources/TuistKit/Services/ProjectAutomation+ManifestMapper.swift b/Sources/TuistKit/Services/ProjectAutomation+ManifestMapper.swift
@@ -90,7 +90,7 @@ extension ProjectAutomation.Target {
                 frameworkStatus = .required
             }
             return .framework(path: path.pathString, status: frameworkStatus)
-        case let .xcframework(path, status, _, _):
+        case let .xcframework(path, status, _):
             let frameworkStatus: ProjectAutomation.FrameworkStatus
             switch status {
             case .optional:

diff --git a/Sources/TuistLoader/Loaders/ManifestModelConverter.swift b/Sources/TuistLoader/Loaders/ManifestModelConverter.swift
@@ -89,8 +89,7 @@ public final class ManifestModelConverter: ManifestModelConverting {
                 try TuistGraph.TargetDependency.from(
                     manifest: targetDependencyManifest,
                     generatorPaths: GeneratorPaths(manifestDirectory: path),
-                    externalDependencies: [:], // externalDependencies manifest can't contain other external dependencies,
-                    isExternal: true
+                    externalDependencies: [:] // externalDependencies manifest can't contain other external dependencies,
                 )
             }
         }

diff --git a/Sources/TuistLoader/Models+ManifestMappers/Project+ManifestMapper.swift b/Sources/TuistLoader/Models+ManifestMappers/Project+ManifestMapper.swift
@@ -33,8 +33,7 @@ extension TuistGraph.Project {
             try TuistGraph.Target.from(
                 manifest: $0,
                 generatorPaths: generatorPaths,
-                externalDependencies: externalDependencies,
-                isExternal: isExternal
+                externalDependencies: externalDependencies
             )
         }
 

diff --git a/Sources/TuistLoader/Models+ManifestMappers/Target+ManifestMapper.swift b/Sources/TuistLoader/Models+ManifestMappers/Target+ManifestMapper.swift
@@ -28,8 +28,7 @@ extension TuistGraph.Target {
     static func from(
         manifest: ProjectDescription.Target,
         generatorPaths: GeneratorPaths,
-        externalDependencies: [String: [TuistGraph.TargetDependency]],
-        isExternal: Bool
+        externalDependencies: [String: [TuistGraph.TargetDependency]]
     ) throws -> TuistGraph.Target {
         let name = manifest.name
         let destinations = try TuistGraph.Destination.from(destinations: manifest.destinations)
@@ -44,8 +43,7 @@ extension TuistGraph.Target {
             try TuistGraph.TargetDependency.from(
                 manifest: $0,
                 generatorPaths: generatorPaths,
-                externalDependencies: externalDependencies,
-                isExternal: isExternal
+                externalDependencies: externalDependencies
             )
         }
 

diff --git a/Sources/TuistLoader/Models+ManifestMappers/TargetDependency+ManifestMapper.swift b/Sources/TuistLoader/Models+ManifestMappers/TargetDependency+ManifestMapper.swift
@@ -29,8 +29,7 @@ extension TuistGraph.TargetDependency {
     static func from( // swiftlint:disable:this function_body_length
         manifest: ProjectDescription.TargetDependency,
         generatorPaths: GeneratorPaths,
-        externalDependencies: [String: [TuistGraph.TargetDependency]],
-        isExternal: Bool
+        externalDependencies: [String: [TuistGraph.TargetDependency]]
     ) throws -> [TuistGraph.TargetDependency] {
         switch manifest {
         case let .target(name, condition):
@@ -80,8 +79,7 @@ extension TuistGraph.TargetDependency {
                 .xcframework(
                     path: try generatorPaths.resolve(path: path),
                     status: .from(manifest: status),
-                    condition: condition?.asGraphCondition,
-                    isExternal: isExternal
+                    condition: condition?.asGraphCondition
                 ),
             ]
         case .xctest:

diff --git a/Tests/TuistAutomationAcceptanceTests/BuildAcceptanceTests.swift b/Tests/TuistAutomationAcceptanceTests/BuildAcceptanceTests.swift
@@ -120,23 +120,7 @@ final class BuildAcceptanceTestMultiplatformAppWithSDK: TuistAcceptanceTestCase
         try setUpFixture(.multiplatformAppWithSdk)
         try await run(InstallCommand.self)
         try await run(GenerateCommand.self)
-        try System.shared.runAndPrint(
-            [
-                "/usr/bin/xcrun",
-                "xcodebuild",
-                "clean",
-                "build",
-                "-scheme",
-                "App",
-                "-workspace",
-                workspacePath.pathString,
-                "-destination",
-                "platform=macOS",
-                "CODE_SIGN_IDENTITY=\"\"",
-                "CODE_SIGNING_REQUIRED=NO",
-                "CODE_SIGNING_ALLOWED=NO",
-            ]
-        )
+        try await run(BuildCommand.self, "App", "--platform", "macos")
         try await run(BuildCommand.self, "App", "--platform", "ios")
     }
 }

diff --git a/Tests/TuistCoreTests/Graph/GraphLoaderTests.swift b/Tests/TuistCoreTests/Graph/GraphLoaderTests.swift
@@ -391,7 +391,7 @@ final class GraphLoaderTests: TuistUnitTestCase {
                         linking: .dynamic,
                         mergeable: false,
                         status: .required,
-                        isExternal: false
+                        macroPath: nil
                     )
                 ),
             ]),
@@ -440,7 +440,7 @@ final class GraphLoaderTests: TuistUnitTestCase {
                         linking: .dynamic,
                         mergeable: true,
                         status: .required,
-                        isExternal: false
+                        macroPath: nil
                     )
                 ),
             ]),