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

Prevent generation of redundant file elements #515

Merged
merged 4 commits into from Oct 3, 2019
Merged
Show file tree
Hide file tree
Changes from 3 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 @@ -28,6 +28,7 @@ Please, check out guidelines: https://keepachangelog.com/en/1.0.0/
- Product name linting failing when it contains variables https://github.com/tuist/tuist/pull/494 by @dcvz
- Build phases not generated in the right position https://github.com/tuist/tuist/pull/506 by @pepibumur
- Remove \$(SRCROOT) from being included in `Info.plist` path https://github.com/tuist/tuist/pull/511 by @dcvz
- Prevent generation of redundant file elements https://github.com/tuist/tuist/pull/515 by @kwridan

## 0.17.0

Expand Down
15 changes: 4 additions & 11 deletions Sources/TuistGenerator/Generator/LinkGenerator.swift
Expand Up @@ -182,7 +182,7 @@ final class LinkGenerator: LinkGenerating {
precompiledEmbedPhase.inputPaths.append(relativePath)
precompiledEmbedPhase.outputPaths.append("$(BUILT_PRODUCTS_DIR)/$(FRAMEWORKS_FOLDER_PATH)/\(path.components.last!)")

} else if case let DependencyReference.product(target) = dependency {
} else if case let DependencyReference.product(target, _) = dependency {
guard let fileRef = fileElements.product(target: target) else {
throw LinkGeneratorError.missingProduct(name: target)
}
Expand Down Expand Up @@ -283,7 +283,7 @@ final class LinkGenerator: LinkGenerating {
let buildFile = PBXBuildFile(file: fileRef)
pbxproj.add(object: buildFile)
buildPhase.files?.append(buildFile)
case let .product(target):
case let .product(target, _):
guard let fileRef = fileElements.product(target: target) else {
throw LinkGeneratorError.missingProduct(name: target)
}
Expand Down Expand Up @@ -322,14 +322,7 @@ final class LinkGenerator: LinkGenerating {
// This technique also allows resource bundles that reside in different projects to get built ahead of the
// "Copy Bundle Resources" phase.

var dependencies = [DependencyReference]()
if target.product.isStatic {
dependencies.append(contentsOf: graph.staticDependencies(path: path, name: target.name))
}

dependencies.append(contentsOf:
graph.resourceBundleDependencies(path: path, name: target.name)
.map { .product(target: $0.target.name) })
let dependencies = graph.copyProductDependencies(path: path, target: target)

if !dependencies.isEmpty {
try generateDependenciesBuildPhase(
Expand All @@ -347,7 +340,7 @@ final class LinkGenerator: LinkGenerating {
fileElements: ProjectFileElements) throws {
var files: [PBXBuildFile] = []

for case let .product(target) in dependencies.sorted() {
for case let .product(target, _) in dependencies.sorted() {
guard let fileRef = fileElements.product(target: target) else {
throw LinkGeneratorError.missingProduct(name: target)
}
Expand Down
156 changes: 64 additions & 92 deletions Sources/TuistGenerator/Generator/ProjectFileElements.swift
Expand Up @@ -33,15 +33,18 @@ class ProjectFileElements {
var sdks: [AbsolutePath: PBXFileReference] = [:]
let playgrounds: Playgrounding
let filesSortener: ProjectFilesSortening
private let system: Systeming

// MARK: - Init

init(_ elements: [AbsolutePath: PBXFileElement] = [:],
playgrounds: Playgrounding = Playgrounds(),
filesSortener: ProjectFilesSortening = ProjectFilesSortener()) {
filesSortener: ProjectFilesSortening = ProjectFilesSortener(),
system: Systeming = System()) {
self.elements = elements
self.playgrounds = playgrounds
self.filesSortener = filesSortener
self.system = system
}

func generateProjectFiles(project: Project,
Expand All @@ -50,10 +53,9 @@ class ProjectFileElements {
pbxproj: PBXProj,
sourceRootPath: AbsolutePath) throws {
var files = Set<GroupFileElement>()
var products = Set<String>()
project.targets.forEach { target in
files.formUnion(targetFiles(target: target, sourceRootPath: sourceRootPath))
products.formUnion(targetProducts(target: target))

try project.targets.forEach { target in
try files.formUnion(targetFiles(target: target, projectPath: project.path, graph: graph))
}
let projectFileElements = projectFiles(project: project)
files.formUnion(projectFileElements)
Expand All @@ -75,17 +77,15 @@ class ProjectFileElements {
pbxproj: pbxproj,
sourceRootPath: sourceRootPath)

let dependencies = graph.findAll(path: project.path)
// Products
let directProducts = project.targets.map {
DependencyReference.product(target: $0.name, productName: $0.productNameWithExtension)
}

/// Products
try generateProducts(project: project,
dependencies: dependencies,
groups: groups,
pbxproj: pbxproj)
// Dependencies
let dependencies = try graph.allDependencyReferences(for: project, system: system)

/// Dependencies
try generate(dependencies: dependencies,
path: project.path,
try generate(dependencyReferences: Set(directProducts + dependencies),
groups: groups,
pbxproj: pbxproj,
sourceRootPath: sourceRootPath,
Expand All @@ -112,13 +112,7 @@ class ProjectFileElements {
return fileElements
}

func targetProducts(target: Target) -> Set<String> {
var products: Set<String> = Set()
products.insert(target.productNameWithExtension)
return products
}

func targetFiles(target: Target, sourceRootPath _: AbsolutePath) -> Set<GroupFileElement> {
func targetFiles(target: Target, projectPath: AbsolutePath, graph: Graphing) throws -> Set<GroupFileElement> {
var files = Set<AbsolutePath>()
files.formUnion(target.sources.map { $0.path })
files.formUnion(target.coreDataModels.map { $0.path })
Expand Down Expand Up @@ -153,6 +147,20 @@ class ProjectFileElements {
isReference: $0.isReference)
})

// Local Packages
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I need to think about this further ...

The inconsistency here is that all other dependencies are resolved via allDependencyReferences whereas this one isn't included there yet.

elements.formUnion(
try graph.packages(path: projectPath, name: target.name).compactMap { node -> GroupFileElement? in
switch node.packageType {
case let .local(path: packagePath, productName: _):
return GroupFileElement(path: projectPath.appending(packagePath),
group: target.filesGroup,
isReference: true)
default:
return nil
}
}
)

return elements
}

Expand Down Expand Up @@ -184,86 +192,50 @@ class ProjectFileElements {
}
}

func generateProducts(project: Project,
dependencies: Set<GraphNode>,
groups: ProjectGroups,
pbxproj: PBXProj) throws {
try prepareProductsFileReferences(project: project, dependencies: dependencies).forEach { pair in
guard self.products[pair.targetName] == nil else { return }
pbxproj.add(object: pair.fileReference)
groups.products.children.append(pair.fileReference)
self.products[pair.targetName] = pair.fileReference
}
}

func prepareProductsFileReferences(project: Project, dependencies: Set<GraphNode>)
throws -> [(targetName: String, fileReference: PBXFileReference)] {
let targetsProducts = project.targets
.map { ($0, $0.product) }
let dependenciesProducts = dependencies
.compactMap { $0 as? TargetNode }
.map { $0.target }
.map { ($0, $0.product) }
let mergeStrategy: (Product, Product) -> Product = { first, _ in first }
let sortByName: ((Target, Product), (Target, Product)) -> Bool = { first, second in
first.0.productNameWithExtension < second.0.productNameWithExtension
}

let targetsProductsDictionary = Dictionary(targetsProducts, uniquingKeysWith: mergeStrategy)
let dependenciesProductsDictionary = Dictionary(dependenciesProducts, uniquingKeysWith: mergeStrategy)
let productsDictionary = targetsProductsDictionary.merging(dependenciesProductsDictionary,
uniquingKeysWith: mergeStrategy)
return productsDictionary
.sorted(by: sortByName)
.map { target, product in
let fileType = Xcode.filetype(extension: product.xcodeValue.fileExtension!)
return (targetName: target.name,
fileReference: PBXFileReference(sourceTree: .buildProductsDir,
explicitFileType: fileType,
path: target.productNameWithExtension,
includeInIndex: false))
}
}

func generate(dependencies: Set<GraphNode>,
path _: AbsolutePath,
func generate(dependencyReferences: Set<DependencyReference>,
groups: ProjectGroups,
pbxproj: PBXProj,
sourceRootPath: AbsolutePath,
filesGroup: ProjectGroup) throws {
let sortedDependencies = dependencies.sorted(by: { $0.path < $1.path })
try sortedDependencies.forEach { node in
switch node {
case let precompiledNode as PrecompiledNode:
let fileElement = GroupFileElement(path: precompiledNode.path,
let sortedDependencies = dependencyReferences.sorted()
try sortedDependencies.forEach { dependency in
switch dependency {
case let .absolute(dependencyPath):
let fileElement = GroupFileElement(path: dependencyPath,
group: filesGroup)
try generate(fileElement: fileElement,
groups: groups,
pbxproj: pbxproj,
sourceRootPath: sourceRootPath)
case let sdkNode as SDKNode:
generateSDKFileElement(node: sdkNode,
case let .sdk(sdkNodePath, _):
generateSDKFileElement(sdkNodePath: sdkNodePath,
toGroup: groups.frameworks,
pbxproj: pbxproj)
case let packageNode as PackageNode:
switch packageNode.packageType {
case let .local(path: packagePath, productName: _):
let fileElement = GroupFileElement(path: sourceRootPath.appending(packagePath),
group: filesGroup)
try generate(fileElement: fileElement,
groups: groups,
pbxproj: pbxproj,
sourceRootPath: sourceRootPath)
case .remote:
// Only local packages need group, remote are handled by Xcode itself
break
}
default:
return
case let .product(target: target, productName: productName):
generateProduct(targetName: target,
productName: productName,
groups: groups,
pbxproj: pbxproj)
}
}
}

private func generateProduct(targetName: String,
productName: String,
groups: ProjectGroups,
pbxproj: PBXProj) {
guard products[targetName] == nil else { return }
let fileType = RelativePath(productName).extension.flatMap { Xcode.filetype(extension: $0) }
let fileReference = PBXFileReference(sourceTree: .buildProductsDir,
explicitFileType: fileType,
path: productName,
includeInIndex: false)

pbxproj.add(object: fileReference)
groups.products.children.append(fileReference)
products[targetName] = fileReference
}

func generate(fileElement: GroupFileElement,
groups: ProjectGroups,
pbxproj: PBXProj,
Expand Down Expand Up @@ -469,20 +441,20 @@ class ProjectFileElements {
elements[fileAbsolutePath] = file
}

private func generateSDKFileElement(node: SDKNode,
private func generateSDKFileElement(sdkNodePath: AbsolutePath,
toGroup: PBXGroup,
pbxproj: PBXProj) {
guard sdks[node.path] == nil else {
guard sdks[sdkNodePath] == nil else {
return
}

addSDKElement(node: node, toGroup: toGroup, pbxproj: pbxproj)
addSDKElement(sdkNodePath: sdkNodePath, toGroup: toGroup, pbxproj: pbxproj)
}

private func addSDKElement(node: SDKNode,
private func addSDKElement(sdkNodePath: AbsolutePath,
toGroup: PBXGroup,
pbxproj: PBXProj) {
let sdkPath = node.path.relative(to: AbsolutePath("/")) // SDK paths are relative
let sdkPath = sdkNodePath.relative(to: AbsolutePath("/")) // SDK paths are relative

let lastKnownFileType = sdkPath.extension.flatMap { Xcode.filetype(extension: $0) }
let file = PBXFileReference(sourceTree: .developerDir,
Expand All @@ -491,7 +463,7 @@ class ProjectFileElements {
path: sdkPath.pathString)
pbxproj.add(object: file)
toGroup.children.append(file)
sdks[node.path] = file
sdks[sdkNodePath] = file
}

func group(path: AbsolutePath) -> PBXGroup? {
Expand Down