Skip to content

Commit

Permalink
Fix issue where frameworks with MACH_O_TYPE:staticlib were being embe…
Browse files Browse the repository at this point in the history
…dded (#1003)
  • Loading branch information
mrabiciu committed Jan 14, 2021
1 parent 0eeb636 commit 6b7ab91
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -17,6 +17,7 @@
- Fixed error message output for `minimumXcodeGenVersion`. [#967](https://github.com/yonaskolb/XcodeGen/pull/967) @joshwalker
- Remove force-unwrapping causing crash for `LegacyTarget`s [#982](https://github.com/yonaskolb/XcodeGen/pull/982) @jcolicchio
- Fixed a race condition in an internal JSON decoder, which would occasionally fail with an error like `Parsing project spec failed: Error Domain=Unspecified error Code=0`. [#995](https://github.com/yonaskolb/XcodeGen/pull/995) @elliottwilliams
- Fixed issue where frameworks with `MACH_O_TYPE: staticlib` were being incorrectly embedded. [#1003](https://github.com/yonaskolb/XcodeGen/pull/1003) @mrabiciu

#### Internal
- Updated to Yams 4.0.0 [#984](https://github.com/yonaskolb/XcodeGen/pull/984) @swiftty
Expand Down
19 changes: 15 additions & 4 deletions Sources/ProjectSpec/Linkage.swift
Expand Up @@ -7,10 +7,10 @@ public enum Linkage {
case none
}

extension PBXProductType {
extension Target {

public var defaultLinkage: Linkage {
switch self {
switch type {
case .none,
.appExtension,
.application,
Expand All @@ -36,12 +36,23 @@ extension PBXProductType {
.xpcService:
return .none
case .framework, .xcFramework:
// TODO: This should check `MACH_O_TYPE` in case this is a "Static Framework"
return .dynamic
// Check the MACH_O_TYPE for "Static Framework"
if settings.buildSettings.machOType == "staticlib" {
return .static
} else {
return .dynamic
}
case .dynamicLibrary:
return .dynamic
case .staticLibrary, .staticFramework:
return .static
}
}
}

private extension BuildSettings {

var machOType: String? {
self["MACH_O_TYPE"] as? String
}
}
15 changes: 7 additions & 8 deletions Sources/ProjectSpec/XCProjExtensions.swift
Expand Up @@ -54,17 +54,16 @@ extension PBXProductType {
}

/// Function to determine when a dependendency should be embedded into the target
public func shouldEmbed(_ dependencyType: PBXProductType) -> Bool {
switch dependencyType {
case .staticLibrary, .staticFramework:
// Some dependendencies should not be embed, independently of the target type
public func shouldEmbed(_ dependencyTarget: Target) -> Bool {
switch dependencyTarget.defaultLinkage {
case .static:
// Static dependencies should never embed
return false

default:
case .dynamic, .none:
if isApp {
// If target is an app, all dependencies should be embed (except for the ones mentioned above)
// If target is an app, all dependencies should be embed (unless they're static)
return true
} else if isTest, [.framework, .bundle].contains(dependencyType) {
} else if isTest, [.framework, .bundle].contains(dependencyTarget.type) {
// If target is test, some dependencies should be embed (depending on their type)
return true
} else {
Expand Down
4 changes: 2 additions & 2 deletions Sources/XcodeGenKit/PBXProjGenerator.swift
Expand Up @@ -690,7 +690,7 @@ public class PBXProjGenerator {
}

func processTargetDependency(_ dependency: Dependency, dependencyTarget: Target, embedFileReference: PBXFileElement?) {
let dependencyLinkage = dependencyTarget.type.defaultLinkage
let dependencyLinkage = dependencyTarget.defaultLinkage
let link = dependency.link ??
((dependencyLinkage == .dynamic && target.type != .staticLibrary) ||
(dependencyLinkage == .static && target.type.isExecutable))
Expand All @@ -707,7 +707,7 @@ public class PBXProjGenerator {
}
}

let embed = dependency.embed ?? target.type.shouldEmbed(dependencyTarget.type)
let embed = dependency.embed ?? target.type.shouldEmbed(dependencyTarget)
if embed {
let embedFile = addObject(
PBXBuildFile(
Expand Down
96 changes: 96 additions & 0 deletions Tests/XcodeGenKitTests/ProjectGeneratorTests.swift
Expand Up @@ -786,6 +786,102 @@ class ProjectGeneratorTests: XCTestCase {
try expect(copyFilesPhases.count) == expectedCopyFilesPhasesCount
}
}

$0.it("ensures static frameworks are not embedded by default") {

let app = Target(
name: "App",
type: .application,
platform: .iOS,
dependencies: [
Dependency(type: .target, reference: "DynamicFramework"),
Dependency(type: .target, reference: "DynamicFrameworkNotEmbedded", embed: false),
Dependency(type: .target, reference: "StaticFramework"),
Dependency(type: .target, reference: "StaticFrameworkExplicitlyEmbedded", embed: true),
Dependency(type: .target, reference: "StaticFramework2"),
Dependency(type: .target, reference: "StaticFramework2ExplicitlyEmbedded", embed: true),
Dependency(type: .target, reference: "StaticLibrary"),
]
)

let targets = [
app,
Target(
name: "DynamicFramework",
type: .framework,
platform: .iOS
),
Target(
name: "DynamicFrameworkNotEmbedded",
type: .framework,
platform: .iOS
),
Target(
name: "StaticFramework",
type: .framework,
platform: .iOS,
settings: Settings(buildSettings: ["MACH_O_TYPE": "staticlib"])
),
Target(
name: "StaticFrameworkExplicitlyEmbedded",
type: .framework,
platform: .iOS,
settings: Settings(buildSettings: ["MACH_O_TYPE": "staticlib"])
),
Target(
name: "StaticFramework2",
type: .staticFramework,
platform: .iOS
),
Target(
name: "StaticFramework2ExplicitlyEmbedded",
type: .staticFramework,
platform: .iOS
),
Target(
name: "StaticLibrary",
type: .staticLibrary,
platform: .iOS
),
]

let expectedLinkedFiles = Set([
"DynamicFramework.framework",
"DynamicFrameworkNotEmbedded.framework",
"StaticFramework.framework",
"StaticFrameworkExplicitlyEmbedded.framework",
"StaticFramework2.framework",
"StaticFramework2ExplicitlyEmbedded.framework",
"libStaticLibrary.a",
])

let expectedEmbeddedFrameworks = Set([
"DynamicFramework.framework",
"StaticFrameworkExplicitlyEmbedded.framework",
"StaticFramework2ExplicitlyEmbedded.framework"
])

let project = Project(
name: "test",
targets: targets
)
let pbxProject = try project.generatePbxProj()

let appTarget = try unwrap(pbxProject.nativeTargets.first(where: { $0.name == app.name }))
let buildPhases = appTarget.buildPhases
let frameworkPhases = pbxProject.frameworksBuildPhases.filter { buildPhases.contains($0) }
let copyFilesPhases = pbxProject.copyFilesBuildPhases.filter { buildPhases.contains($0) }
let embedFrameworkPhase = copyFilesPhases.first { $0.dstSubfolderSpec == .frameworks }

// Ensure all targets are linked
let linkFrameworks = (frameworkPhases[0].files ?? []).compactMap { $0.file?.nameOrPath }
let linkPackages = (frameworkPhases[0].files ?? []).compactMap { $0.product?.productName }
try expect(Set(linkFrameworks + linkPackages)) == expectedLinkedFiles

// Ensure only dynamic frameworks are embedded (unless there's an explicit override)
let embeddedFrameworks = Set((embedFrameworkPhase?.files ?? []).compactMap { $0.file?.nameOrPath })
try expect(embeddedFrameworks) == expectedEmbeddedFrameworks
}

$0.it("copies files only on install in the Embed Frameworks step") {
let app = Target(
Expand Down

0 comments on commit 6b7ab91

Please sign in to comment.