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.

Sign up for GitHub

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

Jump to bottom

Refactor PBXProjGenerator and add tests around dependency order generation #978

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Refactor PBXProjGenerator and add tests around dependency order generation #978

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 77 additions & 50 deletions Sources/XcodeGenKit/PBXProjGenerator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,59 +108,20 @@ public class PBXProjGenerator {

pbxProj.rootObject = pbxProject

for target in project.targets {
let targetObject: PBXTarget

if target.isLegacy {
targetObject = PBXLegacyTarget(
name: target.name,
buildToolPath: target.legacy?.toolPath,
buildArgumentsString: target.legacy?.arguments,
passBuildSettingsInEnvironment: target.legacy?.passSettings ?? false,
buildWorkingDirectory: target.legacy?.workingDirectory,
buildPhases: []
)
} else {
targetObject = PBXNativeTarget(name: target.name, buildPhases: [])
}

targetObjects[target.name] = addObject(targetObject)

var explicitFileType: String?
var lastKnownFileType: String?
let fileType = Xcode.fileType(path: Path(target.filename))
if target.platform == .macOS || target.platform == .watchOS || target.type == .framework {
explicitFileType = fileType
} else {
lastKnownFileType = fileType
}

if !target.isLegacy {
let fileReference = addObject(
PBXFileReference(
sourceTree: .buildProductsDir,
explicitFileType: explicitFileType,
lastKnownFileType: lastKnownFileType,
path: target.filename,
includeInIndex: false
),
context: target.name
)

targetFileReferences[target.name] = fileReference
}
let generatedTargetResults = self.generateTargetInfo(targets: self.project.targets, aggregateTargets: self.project.aggregateTargets)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The motivation to move this logic to a different function was to break down the generate function into smaller functions.

And then the larger goal would be, to able to test out the generateTargetInfo function in isolation. But that requires marking the generateTargetInfo function as a public function so that it can be accessed in tests.
It seems we could make this a pure function, i.e. pass in the targets to this function, and then the function returns this struct, which we can assert on, but still need to explore how feasible it would be to write tests around this function.

So I wanted to raise this PR and gather feedback if this approach makes sense, before proceeding further.

self.targetObjects = generatedTargetResults.targetObjects.reduce(into: [:]) { (dictionary, target) in
dictionary[target.name] = target
}
self.targetAggregateObjects = generatedTargetResults.targetAggregateObjects.reduce(into: [:]) { (dictionary, target) in
dictionary[target.name] = target
}
([self.targetObjects, self.targetAggregateObjects] as [[String: PBXObject]]).flatMap{ Array($0.values) }.forEach { addObject($0) }

for target in project.aggregateTargets {

let aggregateTarget = addObject(
PBXAggregateTarget(
name: target.name,
productName: target.name
)
)
targetAggregateObjects[target.name] = aggregateTarget
self.targetFileReferences = generatedTargetResults.targetFileReferences.reduce(into: [:]) { (dictionary, targetFileReference) in
dictionary[targetFileReference.0] = targetFileReference.1
}
// file references need to be added to the target's context
generatedTargetResults.targetFileReferences.forEach { addObject($0.1, context: $0.0) }

for (name, package) in project.packages {
switch package {
Expand Down Expand Up @@ -639,6 +600,72 @@ public class PBXProjGenerator {
return pbxproj
}

struct GeneratedTargetInfo {
let targetObjects: [PBXTarget]
let targetFileReferences: [(String, PBXFileReference)]
let targetAggregateObjects: [PBXAggregateTarget]
}

func generateTargetInfo(targets: [Target], aggregateTargets: [AggregateTarget]) -> GeneratedTargetInfo {
var targetObjects: [PBXTarget] = []
var targetFileReferences: [(String, PBXFileReference)] = []
var targetAggregateObjects: [PBXAggregateTarget] = []
for target in targets {
let targetObject: PBXTarget

if target.isLegacy {
targetObject = PBXLegacyTarget(
name: target.name,
buildToolPath: target.legacy?.toolPath,
buildArgumentsString: target.legacy?.arguments,
passBuildSettingsInEnvironment: target.legacy?.passSettings ?? false,
buildWorkingDirectory: target.legacy?.workingDirectory,
buildPhases: []
)
} else {
targetObject = PBXNativeTarget(name: target.name, buildPhases: [])
}

targetObjects.append(targetObject)

var explicitFileType: String?
var lastKnownFileType: String?
let fileType: String?
if let fileExtension = Path(target.filename).extension {
fileType = Xcode.filetype(extension: fileExtension)
} else {
fileType = nil
}

if target.platform == .macOS || target.platform == .watchOS || target.type == .framework {
explicitFileType = fileType
} else {
lastKnownFileType = fileType
}

if !target.isLegacy {
let fileReference = PBXFileReference(
sourceTree: .buildProductsDir,
explicitFileType: explicitFileType,
lastKnownFileType: lastKnownFileType,
path: target.filename,
includeInIndex: false
)
targetFileReferences.append((target.name, fileReference))
}
}

for target in aggregateTargets {

let aggregateTarget = PBXAggregateTarget(
name: target.name,
productName: target.name
)
targetAggregateObjects.append(aggregateTarget)
}
return GeneratedTargetInfo(targetObjects: targetObjects, targetFileReferences: targetFileReferences, targetAggregateObjects: targetAggregateObjects)
}

func generateTarget(_ target: Target) throws {
let carthageDependencies = carthageResolver.dependencies(for: target)

Expand Down
Loading