Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't embed frameworks with MACH_O_TYPE=staticlib #1003

Merged
merged 2 commits into from Jan 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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