From 5e96eedd78579dab3336d302b27bf6edc5305188 Mon Sep 17 00:00:00 2001 From: Brentley Jones Date: Tue, 17 Jul 2018 15:37:40 -0500 Subject: [PATCH 1/4] Add support for Copy Files build phase --- CHANGELOG.md | 2 +- Docs/ProjectSpec.md | 25 +++++- Sources/ProjectSpec/SpecParsingError.swift | 3 + Sources/ProjectSpec/TargetSource.swift | 84 +++++++++++++++++-- Sources/XcodeGenKit/PBXProjGenerator.swift | 34 +++++++- Sources/XcodeGenKit/SourceGenerator.swift | 14 ++-- Tests/Fixtures/TestProject/CopyFiles/Empty.h | 0 .../Project.xcodeproj/project.pbxproj | 22 +++++ Tests/Fixtures/TestProject/project.yml | 5 ++ 9 files changed, 169 insertions(+), 20 deletions(-) create mode 100644 Tests/Fixtures/TestProject/CopyFiles/Empty.h diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c6a63ff..bf03ef40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ - Added `targetTemplates` [355](https://github.com/yonaskolb/XcodeGen/pull/355) @yonaskolb - Added `aggregateTargets` [354](https://github.com/yonaskolb/XcodeGen/pull/354) @yonaskolb - Added `options.groupSortPosition` [356](https://github.com/yonaskolb/XcodeGen/pull/356) @yonaskolb - +- Added ability to specify `copyFiles` build phase for sources [345](https://github.com/yonaskolb/XcodeGen/pull/345) @brentleyjones #### Fixed - Sort files using localizedStandardCompare [341](https://github.com/yonaskolb/XcodeGen/pull/341) @rohitpal440 diff --git a/Docs/ProjectSpec.md b/Docs/ProjectSpec.md index a12bec8f..255a511d 100644 --- a/Docs/ProjectSpec.md +++ b/Docs/ProjectSpec.md @@ -260,21 +260,33 @@ A source can be provided via a string (the path) or an object of the form: - `sources` - Compile Sources phase - `resources` - Copy Bundle Resources phase - `headers` - Headers Phase - - `none` - Will not be added to any build phases + - `copyFiles` - Copy Files Phase. Must be specified as an object with the following fields: + - [x] **destination**: **String** - Destination of the Copy Files phase. This can be one of the following values: + - `absolutePath` + - `productsDirectory` + - `wrapper` + - `executables` + - `resources` + - `javaResources` + - `frameworks` + - `sharedFrameworks` + - `sharedSupport` + - `plugins` + - [ ] **subpath**: **String** - The path inside of the destination to copy the files. - [ ] **type**: **String**: This can be one of the following values - `file`: a file reference with a parent group will be created (Default for files or directories with extensions) - `group`: a group with all it's containing files. (Default for directories without extensions) - `folder`: a folder reference. -- [ ] **headerVisibility**: **String** The visibility of any headers. This defaults to `public`, but can be either: +- [ ] **headerVisibility**: **String** - The visibility of any headers. This defaults to `public`, but can be either: - `public` - `private` - `project` ```yaml targets: - MyTarget + MyTarget: sources: MyTargetSource - MyOtherTarget + MyOtherTarget: sources: - MyOtherTargetSource1 - path: MyOtherTargetSource2 @@ -288,6 +300,11 @@ targets: - "-Wextra" - path: MyOtherTargetSource3 compilerFlags: "-Werror -Wextra" + - path: ModuleMaps + buildPhase: + copyFiles: + destination: productsDirectory + subpath: include/$(PRODUCT_NAME) - path: Resources type: folder ``` diff --git a/Sources/ProjectSpec/SpecParsingError.swift b/Sources/ProjectSpec/SpecParsingError.swift index ff09260e..3e0e5a6b 100644 --- a/Sources/ProjectSpec/SpecParsingError.swift +++ b/Sources/ProjectSpec/SpecParsingError.swift @@ -5,6 +5,7 @@ public enum SpecParsingError: Error, CustomStringConvertible { case unknownTargetPlatform(String) case invalidDependency([String: Any]) case unknownSourceBuildPhase(String) + case invalidSourceCopyFilesPhase case invalidVersion(String) public var description: String { @@ -17,6 +18,8 @@ public enum SpecParsingError: Error, CustomStringConvertible { return "Unknown Target dependency: \(dependency)" case let .unknownSourceBuildPhase(buildPhase): return "Unknown Source Build Phase: \(buildPhase)" + case .invalidSourceCopyFilesPhase: + return "copyFiles Build Phase without a copyFiles section" case let .invalidVersion(version): return "Invalid version: \(version)" } diff --git a/Sources/ProjectSpec/TargetSource.swift b/Sources/ProjectSpec/TargetSource.swift index 5fd73a5e..8eb2724f 100644 --- a/Sources/ProjectSpec/TargetSource.swift +++ b/Sources/ProjectSpec/TargetSource.swift @@ -28,17 +28,59 @@ public struct TargetSource: Equatable { } } - public enum BuildPhase: String { + public enum BuildPhase: Equatable { case sources case headers case resources + case copyFiles(CopyFilesSettings) case none + // Not currently exposed as selectable options, but used internally + case frameworks + case runScript + case carbonResources + + public struct CopyFilesSettings: Equatable, Hashable { + public enum Destination: String { + case absolutePath + case productsDirectory + case wrapper + case executables + case resources + case javaResources + case frameworks + case sharedFrameworks + case sharedSupport + case plugins + + public var destination: xcproj.PBXCopyFilesBuildPhase.SubFolder? { + switch self { + case .absolutePath: return .absolutePath + case .productsDirectory: return .productsDirectory + case .wrapper: return .wrapper + case .executables: return .executables + case .resources: return .resources + case .javaResources: return .javaResources + case .frameworks: return .frameworks + case .sharedFrameworks: return .sharedFrameworks + case .sharedSupport: return .sharedSupport + case .plugins: return .plugins + } + } + } + + public var destination: Destination + public var subpath: String + } public var buildPhase: xcproj.BuildPhase? { switch self { case .sources: return .sources case .headers: return .headers case .resources: return .resources + case .copyFiles: return .copyFiles + case .frameworks: return .frameworks + case .runScript: return .runScript + case .carbonResources: return .carbonResources case .none: return nil } } @@ -100,12 +142,42 @@ extension TargetSource: JSONObjectConvertible { excludes = jsonDictionary.json(atKeyPath: "excludes") ?? [] type = jsonDictionary.json(atKeyPath: "type") optional = jsonDictionary.json(atKeyPath: "optional") ?? false + if let string: String = jsonDictionary.json(atKeyPath: "buildPhase") { - if let buildPhase = BuildPhase(rawValue: string) { - self.buildPhase = buildPhase - } else { - throw SpecParsingError.unknownSourceBuildPhase(string) - } + buildPhase = try BuildPhase(string: string) + } else if let dict: JSONDictionary = jsonDictionary.json(atKeyPath: "buildPhase") { + buildPhase = try BuildPhase(jsonDictionary: dict) } } } + +extension TargetSource.BuildPhase { + + public init(string: String) throws { + switch string { + case "sources": self = .sources + case "headers": self = .headers + case "resources": self = .resources + case "copyFiles": + throw SpecParsingError.invalidSourceCopyFilesPhase + case "none": self = .none + default: + throw SpecParsingError.unknownSourceBuildPhase(string) + } + } +} + +extension TargetSource.BuildPhase: JSONObjectConvertible { + + public init(jsonDictionary: JSONDictionary) throws { + self = .copyFiles(try jsonDictionary.json(atKeyPath: "copyFiles")) + } +} + +extension TargetSource.BuildPhase.CopyFilesSettings: JSONObjectConvertible { + + public init(jsonDictionary: JSONDictionary) throws { + destination = try jsonDictionary.json(atKeyPath: "destination") + subpath = jsonDictionary.json(atKeyPath: "subpath") ?? "" + } +} diff --git a/Sources/XcodeGenKit/PBXProjGenerator.swift b/Sources/XcodeGenKit/PBXProjGenerator.swift index cd253624..7b5b2f10 100644 --- a/Sources/XcodeGenKit/PBXProjGenerator.swift +++ b/Sources/XcodeGenKit/PBXProjGenerator.swift @@ -567,9 +567,8 @@ public class PBXProjGenerator { let fileReference = targetFileReferences[target.name] var buildPhases: [String] = [] - func getBuildFilesForPhase(_ buildPhase: BuildPhase) -> [String] { + func getBuildFilesForSourceFiles(_ sourceFiles: [SourceFile]) -> [String] { let files = sourceFiles - .filter { $0.buildPhase == buildPhase } .reduce(into: [SourceFile]()) { output, sourceFile in if !output.contains(where: { $0.fileReference == sourceFile.fileReference }) { output.append(sourceFile) @@ -579,6 +578,21 @@ public class PBXProjGenerator { return files.map { createObject(id: $0.fileReference + target.name, $0.buildFile) } .map { $0.reference } } + + func getBuildFilesForPhase(_ buildPhase: BuildPhase) -> [String] { + let filteredSourceFiles = sourceFiles + .filter { $0.buildPhase?.buildPhase == buildPhase } + return getBuildFilesForSourceFiles(filteredSourceFiles) + } + + func getBuildFilesForCopyFilesPhases() -> [TargetSource.BuildPhase.CopyFilesSettings: [String]] { + var sourceFilesByCopyFiles: [TargetSource.BuildPhase.CopyFilesSettings: [SourceFile]] = [:] + for sourceFile in sourceFiles { + guard case let .copyFiles(copyFilesSettings)? = sourceFile.buildPhase else { continue } + sourceFilesByCopyFiles[copyFilesSettings, default: []].append(sourceFile) + } + return sourceFilesByCopyFiles.mapValues { getBuildFilesForSourceFiles($0) } + } buildPhases += try target.prebuildScripts.map { try generateBuildScript(targetName: target.name, buildScript: $0) } @@ -591,6 +605,22 @@ public class PBXProjGenerator { let resourcesBuildPhase = createObject(id: target.name, PBXResourcesBuildPhase(files: resourcesBuildPhaseFiles)) buildPhases.append(resourcesBuildPhase.reference) } + + let copyFilesBuildPhasesFiles = getBuildFilesForCopyFilesPhases() + if !copyFilesBuildPhasesFiles.isEmpty { + for (copyFiles, buildPhaseFiles) in copyFilesBuildPhasesFiles { + let copyFilesBuildPhase = createObject( + id: "copy files" + copyFiles.destination.rawValue + copyFiles.subpath + target.name, + PBXCopyFilesBuildPhase( + dstPath: copyFiles.subpath, + dstSubfolderSpec: copyFiles.destination.destination, + files: buildPhaseFiles + ) + ) + + buildPhases.append(copyFilesBuildPhase.reference) + } + } let headersBuildPhaseFiles = getBuildFilesForPhase(.headers) if !headersBuildPhaseFiles.isEmpty && (target.type == .framework || target.type == .dynamicLibrary) { diff --git a/Sources/XcodeGenKit/SourceGenerator.swift b/Sources/XcodeGenKit/SourceGenerator.swift index 1165e17a..ba9b81c1 100644 --- a/Sources/XcodeGenKit/SourceGenerator.swift +++ b/Sources/XcodeGenKit/SourceGenerator.swift @@ -7,7 +7,7 @@ struct SourceFile { let path: Path let fileReference: String let buildFile: PBXBuildFile - let buildPhase: BuildPhase? + let buildPhase: TargetSource.BuildPhase? } class SourceGenerator { @@ -52,15 +52,15 @@ class SourceGenerator { _ = try getSourceFiles(targetSource: TargetSource(path: path), path: fullPath) } - func generateSourceFile(targetSource: TargetSource, path: Path, buildPhase: BuildPhase? = nil) -> SourceFile { + func generateSourceFile(targetSource: TargetSource, path: Path, buildPhase: TargetSource.BuildPhase? = nil) -> SourceFile { let fileReference = fileReferencesByPath[path.string.lowercased()]! var settings: [String: Any] = [:] - let chosenBuildPhase: BuildPhase? + let chosenBuildPhase: TargetSource.BuildPhase? if let buildPhase = buildPhase { chosenBuildPhase = buildPhase } else if let buildPhase = targetSource.buildPhase { - chosenBuildPhase = buildPhase.buildPhase + chosenBuildPhase = buildPhase } else { chosenBuildPhase = getDefaultBuildPhase(for: path) } @@ -156,7 +156,7 @@ class SourceGenerator { } /// returns a default build phase for a given path. This is based off the filename - private func getDefaultBuildPhase(for path: Path) -> BuildPhase? { + private func getDefaultBuildPhase(for path: Path) -> TargetSource.BuildPhase? { if path.lastComponent == "Info.plist" { return nil } @@ -424,9 +424,9 @@ class SourceGenerator { rootGroups.insert(fileReference) } - let buildPhase: BuildPhase? + let buildPhase: TargetSource.BuildPhase? if let targetBuildPhase = targetSource.buildPhase { - buildPhase = targetBuildPhase.buildPhase + buildPhase = targetBuildPhase } else { buildPhase = .resources } diff --git a/Tests/Fixtures/TestProject/CopyFiles/Empty.h b/Tests/Fixtures/TestProject/CopyFiles/Empty.h new file mode 100644 index 00000000..e69de29b diff --git a/Tests/Fixtures/TestProject/Project.xcodeproj/project.pbxproj b/Tests/Fixtures/TestProject/Project.xcodeproj/project.pbxproj index e4992247..2cdbf0a7 100644 --- a/Tests/Fixtures/TestProject/Project.xcodeproj/project.pbxproj +++ b/Tests/Fixtures/TestProject/Project.xcodeproj/project.pbxproj @@ -77,6 +77,7 @@ BF_734036107922 /* MyFramework.h in Headers */ = {isa = PBXBuildFile; fileRef = FR_183521624014 /* MyFramework.h */; settings = {ATTRIBUTES = (Public, ); }; }; BF_747443236192 /* App_watchOS.app in Embed Watch Content */ = {isa = PBXBuildFile; fileRef = FR_324671077936 /* App_watchOS.app */; settings = {ATTRIBUTES = (RemoveHeadersOnCopy, ); }; }; BF_757906110813 = {isa = PBXBuildFile; fileRef = FR_662315837182 /* Framework.framework */; }; + BF_807556340853 /* Empty.h in CopyFiles */ = {isa = PBXBuildFile; fileRef = FR_837374194182 /* Empty.h */; }; BF_813358525536 /* MyFramework.h in Headers */ = {isa = PBXBuildFile; fileRef = FR_183521624014 /* MyFramework.h */; settings = {ATTRIBUTES = (Public, ); }; }; BF_828878846239 /* MainInterface.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = VG_201160695646 /* MainInterface.storyboard */; }; BF_830383951771 /* NotificationController.swift in Sources */ = {isa = PBXBuildFile; fileRef = FR_304712043717 /* NotificationController.swift */; }; @@ -237,6 +238,16 @@ name = "Embed Frameworks"; runOnlyForDeploymentPostprocessing = 0; }; + CFBP_7191905390 /* CopyFiles */ = { + isa = PBXCopyFilesBuildPhase; + buildActionMask = 2147483647; + dstPath = "include/$(PRODUCT_NAME)"; + dstSubfolderSpec = 16; + files = ( + BF_807556340853 /* Empty.h in CopyFiles */, + ); + runOnlyForDeploymentPostprocessing = 0; + }; /* End PBXCopyFilesBuildPhase section */ /* Begin PBXFileReference section */ @@ -299,6 +310,7 @@ FR_815403394914 /* Headers */ = {isa = PBXFileReference; lastKnownFileType = folder; path = Headers; sourceTree = SOURCE_ROOT; }; FR_825232110500 /* App_iOS.app */ = {isa = PBXFileReference; includeInIndex = 0; lastKnownFileType = wrapper.application; path = App_iOS.app; sourceTree = BUILT_PRODUCTS_DIR; }; FR_830053537293 /* Assets.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = Assets.xcassets; sourceTree = ""; }; + FR_837374194182 /* Empty.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = Empty.h; sourceTree = ""; }; FR_854336462818 /* AppDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppDelegate.swift; sourceTree = ""; }; FR_868653349092 /* Assets.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = Assets.xcassets; sourceTree = ""; }; FR_935153865209 /* iMessageApp.app */ = {isa = PBXFileReference; includeInIndex = 0; lastKnownFileType = wrapper.application; path = iMessageApp.app; sourceTree = BUILT_PRODUCTS_DIR; }; @@ -388,6 +400,14 @@ path = "iMessage MessagesExtension"; sourceTree = ""; }; + G_1885780145626 /* CopyFiles */ = { + isa = PBXGroup; + children = ( + FR_837374194182 /* Empty.h */, + ); + path = CopyFiles; + sourceTree = ""; + }; G_1952740716080 /* Frameworks */ = { isa = PBXGroup; children = ( @@ -561,6 +581,7 @@ G_3246710779368 /* App_watchOS */, G_5070234922517 /* App_watchOS Extension */, G_8340618952527 /* Configs */, + G_1885780145626 /* CopyFiles */, G_3234630030493 /* FileGroup */, G_4661500274312 /* Framework */, G_8268950006174 /* iMessage */, @@ -850,6 +871,7 @@ buildPhases = ( SBP_82523211050 /* Sources */, RBP_82523211050 /* Resources */, + CFBP_7191905390 /* CopyFiles */, FBP_82523211050 /* Frameworks */, CFBP_6493932244 /* Embed Frameworks */, CFBP_2836118931 /* Embed Watch Content */, diff --git a/Tests/Fixtures/TestProject/project.yml b/Tests/Fixtures/TestProject/project.yml index 1da27be4..2b68ea95 100644 --- a/Tests/Fixtures/TestProject/project.yml +++ b/Tests/Fixtures/TestProject/project.yml @@ -47,6 +47,11 @@ targets: - path: Mintfile type: file buildPhase: none + - path: CopyFiles + buildPhase: + copyFiles: + destination: productsDirectory + subpath: include/$(PRODUCT_NAME) settings: INFOPLIST_FILE: App_iOS/Info.plist PRODUCT_BUNDLE_IDENTIFIER: com.project.app From d1c3ac10d0031edf987ad738abb8f4ad95ef86da Mon Sep 17 00:00:00 2001 From: Brentley Jones Date: Mon, 30 Jul 2018 08:13:29 -0500 Subject: [PATCH 2/4] Refactor `SpecParsingError.unknownSourceBuildPhase` Consolidate `.unknownSourceBuildPhase` and `.invalidSourceCopyFilesPhase` into one. --- Sources/ProjectSpec/SpecParsingError.swift | 9 +++------ Sources/ProjectSpec/TargetSource.swift | 4 ++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/Sources/ProjectSpec/SpecParsingError.swift b/Sources/ProjectSpec/SpecParsingError.swift index 3e0e5a6b..5d48d4d0 100644 --- a/Sources/ProjectSpec/SpecParsingError.swift +++ b/Sources/ProjectSpec/SpecParsingError.swift @@ -4,8 +4,7 @@ public enum SpecParsingError: Error, CustomStringConvertible { case unknownTargetType(String) case unknownTargetPlatform(String) case invalidDependency([String: Any]) - case unknownSourceBuildPhase(String) - case invalidSourceCopyFilesPhase + case invalidSourceBuildPhase(String) case invalidVersion(String) public var description: String { @@ -16,10 +15,8 @@ public enum SpecParsingError: Error, CustomStringConvertible { return "Unknown Target platform: \(platform)" case let .invalidDependency(dependency): return "Unknown Target dependency: \(dependency)" - case let .unknownSourceBuildPhase(buildPhase): - return "Unknown Source Build Phase: \(buildPhase)" - case .invalidSourceCopyFilesPhase: - return "copyFiles Build Phase without a copyFiles section" + case let .invalidSourceBuildPhase(error): + return "Invalid Source Build Phase: \(error)" case let .invalidVersion(version): return "Invalid version: \(version)" } diff --git a/Sources/ProjectSpec/TargetSource.swift b/Sources/ProjectSpec/TargetSource.swift index 8eb2724f..221cd7b9 100644 --- a/Sources/ProjectSpec/TargetSource.swift +++ b/Sources/ProjectSpec/TargetSource.swift @@ -159,10 +159,10 @@ extension TargetSource.BuildPhase { case "headers": self = .headers case "resources": self = .resources case "copyFiles": - throw SpecParsingError.invalidSourceCopyFilesPhase + throw SpecParsingError.invalidSourceBuildPhase("copyFiles must specify directory") case "none": self = .none default: - throw SpecParsingError.unknownSourceBuildPhase(string) + throw SpecParsingError.invalidSourceBuildPhase("Unknown build phase \(string.quoted)") } } } From 0090d75246bc0b18f6b6515b33ecc4db173b2ef0 Mon Sep 17 00:00:00 2001 From: Brentley Jones Date: Tue, 31 Jul 2018 07:33:26 -0500 Subject: [PATCH 3/4] Add back `none` build phase to ProjectSpec.md --- Docs/ProjectSpec.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Docs/ProjectSpec.md b/Docs/ProjectSpec.md index 255a511d..f03fef1c 100644 --- a/Docs/ProjectSpec.md +++ b/Docs/ProjectSpec.md @@ -273,6 +273,7 @@ A source can be provided via a string (the path) or an object of the form: - `sharedSupport` - `plugins` - [ ] **subpath**: **String** - The path inside of the destination to copy the files. + - `none` - Will not be added to any build phases - [ ] **type**: **String**: This can be one of the following values - `file`: a file reference with a parent group will be created (Default for files or directories with extensions) - `group`: a group with all it's containing files. (Default for directories without extensions) From b44dc46244579e87a83887496ddee44d37b598d0 Mon Sep 17 00:00:00 2001 From: Brentley Jones Date: Tue, 31 Jul 2018 07:36:49 -0500 Subject: [PATCH 4/4] Cleanup `SpecParsingError.invalidSourceBuildPhase` --- Sources/ProjectSpec/TargetSource.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/ProjectSpec/TargetSource.swift b/Sources/ProjectSpec/TargetSource.swift index 221cd7b9..83a5cdf0 100644 --- a/Sources/ProjectSpec/TargetSource.swift +++ b/Sources/ProjectSpec/TargetSource.swift @@ -159,10 +159,10 @@ extension TargetSource.BuildPhase { case "headers": self = .headers case "resources": self = .resources case "copyFiles": - throw SpecParsingError.invalidSourceBuildPhase("copyFiles must specify directory") + throw SpecParsingError.invalidSourceBuildPhase("copyFiles must specify a \"destination\" and optional \"subpath\"") case "none": self = .none default: - throw SpecParsingError.invalidSourceBuildPhase("Unknown build phase \(string.quoted)") + throw SpecParsingError.invalidSourceBuildPhase(string.quoted) } } }