From aaae772d0ef014e5a99fd5f0f44b3fbd04f23961 Mon Sep 17 00:00:00 2001 From: Tom Quist Date: Sun, 17 Mar 2019 14:20:01 +0100 Subject: [PATCH] Add ability to define `templateAttributes` within a target This allows parameterizing templates. Also change placeholder syntax to `${placeholderName}` also for existing placeholders `$target_name`and `$platform` and generate warnings when using the old placeholder syntax. --- CHANGELOG.md | 4 + Docs/ProjectSpec.md | 18 ++- Sources/ProjectSpec/SpecValidation.swift | 1 + Sources/ProjectSpec/SpecValidationError.swift | 7 + Sources/ProjectSpec/Target.swift | 11 +- Sources/XcodeGenCLI/GenerateCommand.swift | 13 ++ Sources/XcodeGenKit/SpecLoader.swift | 37 +++++ Tests/XcodeGenKitTests/SpecLoadingTests.swift | 126 ++++++++++++++++-- 8 files changed, 197 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66da48bb..67f90965 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ - Added ability to automatically include Carthage related dependencies via `includeRelated: true` [#506](https://github.com/yonaskolb/XcodeGen/pull/506) @rpassis - Added ability to define a per-platform `deploymentTarget` for Multi-Platform targets. [#510](https://github.com/yonaskolb/XcodeGen/pull/510) @ainopara - Added support for nested target templates [#534](https://github.com/yonaskolb/XcodeGen/pull/534) @tomquist +- Added ability to define `templateAttributes` within a target to be able to parameterize templates. [#533](https://github.com/yonaskolb/XcodeGen/pull/533) @tomquist + +#### Changed +- **DEPRECATION**: Placeholders `$target_name` and `$platform` have been deprecated in favour of `${target_name}` and `${platform}`. Support for the old placeholders will be removed in a future version [#533](https://github.com/yonaskolb/XcodeGen/pull/533) @tomquist #### Fixed - Sources outside a project spec's directory will be correctly referenced as relative paths in the project file. [#524](https://github.com/yonaskolb/XcodeGen/pull/524) diff --git a/Docs/ProjectSpec.md b/Docs/ProjectSpec.md index 40168376..3e271088 100644 --- a/Docs/ProjectSpec.md +++ b/Docs/ProjectSpec.md @@ -203,7 +203,8 @@ Settings are merged in the following order: groups, base, configs. - `CFBundleVersion` - `CFBundlePackageType` - [ ] **entitlements**: **[Plist](#plist)** - If defined this will generate and write a `.entitlements` file, and use it by setting `CODE_SIGN_ENTITLEMENTS` build setting for every configuration. All properties must be provided -- [ ] **templates**: **[String]** - A list of [Target Templates](#target-template) referenced by name that will be merged with the target in order. Any instances of `$target_name` within these templates will be replaced with the target name. +- [ ] **templates**: **[String]** - A list of [Target Templates](#target-template) referenced by name that will be merged with the target in order. Any instances of `${target_name}` within these templates will be replaced with the target name. +- [ ] **templateAttributes**: **[String: String]** - A list of attributes where each instance of `${attributeName}` within the templates listed in `templates` will be replaced with the value specified. - [ ] **transitivelyLinkDependencies**: **Bool** - If this is not specified the value from the project set in [Options](#options)`.transitivelyLinkDependencies` will be used. - [ ] **directlyEmbedCarthageDependencies**: **Bool** - If this is `true` Carthage dependencies will be embedded using an `Embed Frameworks` build phase instead of the `copy-frameworks` script. Defaults to `true` for all targets except iOS/tvOS/watchOS Applications. - [ ] **requiresObjCLinking**: **Bool** - If this is `true` any targets that link to this target will have `-ObjC` added to their `OTHER_LDFLAGS`. This is required if a static library has any catagories or extensions on Objective-C code. See [this guide](https://pewpewthespells.com/blog/objc_linker_flags.html#objc) for more details. Defaults to `true` if `type` is `library.static`. If you are 100% sure you don't have catagories or extensions on Objective-C code (pure Swift with no use of Foundation/UIKit) you can set this to `false`, otherwise it's best to leave it alone. @@ -257,9 +258,9 @@ This will provide default build settings for a certain platform. It can be any o You can also specify an array of platforms. This will generate a target for each platform. If `deploymenTarget` is specified for a multi platform target, it can have different values per platform similar to how it's defined in [Options](#options). See below for an example. -If you reference the string `$platform` anywhere within the target spec, that will be replaced with the platform. +If you reference the string `${platform}` anywhere within the target spec, that will be replaced with the platform. -The generated targets by default will have a suffix of `_$platform` applied, you can change this by specifying a `platformSuffix` or `platformPrefix`. +The generated targets by default will have a suffix of `_${platform}` applied, you can change this by specifying a `platformSuffix` or `platformPrefix`. If no `PRODUCT_NAME` build setting is specified for a target, this will be set to the target name, so that this target can be imported under a single name. @@ -276,9 +277,9 @@ targets: base: INFOPLIST_FILE: MyApp/Info.plist PRODUCT_BUNDLE_IDENTIFIER: com.myapp - MY_SETTING: platform $platform + MY_SETTING: platform ${platform} groups: - - $platform + - ${platform} ``` The above will generate 2 targets named `MyFramework_iOS` and `MyFramework_tvOS`, with all the relevant platform build settings. They will both have a `PRODUCT_NAME` of `MyFramework` @@ -583,7 +584,8 @@ This is used to override settings or run build scripts in specific targets ## Target Template This is a template that can be referenced from a normal target using the `templates` property. The properties of this template are the same as a [Target](#target)]. -Any instances of `$target_name` within each template will be replaced by the final target name which references the template. +Any instances of `${target_name}` within each template will be replaced by the final target name which references the template. +Any attributes defined within a targets `templateAttributes` will be used to replace any attribute references in the template using the syntax `${attribute_name}`. ```yaml @@ -591,6 +593,8 @@ targets: MyFramework: templates: - Framework + templateAttributes: + frameworkName: AwesomeFramework sources: - SomeSources targetTemplates: @@ -598,7 +602,7 @@ targetTemplates: platform: iOS type: framework sources: - - $target_name/Sources + - ${frameworkName}/${target_name} ``` ## Scheme diff --git a/Sources/ProjectSpec/SpecValidation.swift b/Sources/ProjectSpec/SpecValidation.swift index ab7c94c3..9c4a2d06 100644 --- a/Sources/ProjectSpec/SpecValidation.swift +++ b/Sources/ProjectSpec/SpecValidation.swift @@ -1,4 +1,5 @@ import Foundation +import JSONUtilities import PathKit extension Project { diff --git a/Sources/ProjectSpec/SpecValidationError.swift b/Sources/ProjectSpec/SpecValidationError.swift index 6c0acb7f..d1a7a0b0 100644 --- a/Sources/ProjectSpec/SpecValidationError.swift +++ b/Sources/ProjectSpec/SpecValidationError.swift @@ -4,6 +4,10 @@ public struct SpecValidationError: Error, CustomStringConvertible { public var errors: [ValidationError] + public init(errors: [ValidationError]) { + self.errors = errors + } + public enum ValidationError: Error, CustomStringConvertible { case invalidXcodeGenVersion(minimumVersion: Version, version: Version) case invalidSDKDependency(target: String, dependency: String) @@ -23,6 +27,7 @@ public struct SpecValidationError: Error, CustomStringConvertible { case missingConfigForTargetScheme(target: String, configType: ConfigType) case missingDefaultConfig(configName: String) case invalidPerConfigSettings + case deprecatedUsageOfPlaceholder(placeholderName: String) public var description: String { switch self { @@ -62,6 +67,8 @@ public struct SpecValidationError: Error, CustomStringConvertible { return "Default configuration \(name) doesn't exist" case .invalidPerConfigSettings: return "Settings that are for a specific config must go in \"configs\". \"base\" can be used for common settings" + case let .deprecatedUsageOfPlaceholder(placeholderName: placeholderName): + return "Usage of $\(placeholderName) is deprecated and will stop working in an upcoming version. Use ${\(placeholderName)} instead." } } } diff --git a/Sources/ProjectSpec/Target.swift b/Sources/ProjectSpec/Target.swift index 67b578ff..b7eb1c00 100644 --- a/Sources/ProjectSpec/Target.swift +++ b/Sources/ProjectSpec/Target.swift @@ -168,7 +168,13 @@ extension Target { } } target = target.merged(onto: mergedDictionary) - target = target.replaceString("$target_name", with: targetName) + target = target.replaceString("$target_name", with: targetName) // Will be removed in upcoming version + target = target.replaceString("${target_name}", with: targetName) + if let templateAttributes = target["templateAttributes"] as? [String: String] { + for (templateAttribute, value) in templateAttributes { + target = target.replaceString("${\(templateAttribute)}", with: value) + } + } } targetsDictionary[targetName] = target } @@ -192,7 +198,8 @@ extension Target { for platform in platforms { var platformTarget = target - platformTarget = platformTarget.replaceString("$platform", with: platform) + platformTarget = platformTarget.replaceString("$platform", with: platform) // Will be removed in upcoming version + platformTarget = platformTarget.replaceString("${platform}", with: platform) platformTarget["platform"] = platform let platformSuffix = platformTarget["platformSuffix"] as? String ?? "_\(platform)" diff --git a/Sources/XcodeGenCLI/GenerateCommand.swift b/Sources/XcodeGenCLI/GenerateCommand.swift index 10afaea0..43932818 100644 --- a/Sources/XcodeGenCLI/GenerateCommand.swift +++ b/Sources/XcodeGenCLI/GenerateCommand.swift @@ -64,6 +64,13 @@ class GenerateCommand: Command { throw GenerationError.projectSpecParsingError(error) } + // validate project dictionary + do { + try specLoader.validateProjectDictionaryWarnings() + } catch { + warning("\(error)") + } + let projectPath = projectDirectory + "\(project.name).xcodeproj" let cacheFilePath = self.cacheFilePath.value ?? @@ -143,6 +150,12 @@ class GenerateCommand: Command { } } + func warning(_ string: String) { + if !quiet.value { + stdout.print(string.yellow) + } + } + func success(_ string: String) { if !quiet.value { stdout.print(string.green) diff --git a/Sources/XcodeGenKit/SpecLoader.swift b/Sources/XcodeGenKit/SpecLoader.swift index 35514f97..cb34eb75 100644 --- a/Sources/XcodeGenKit/SpecLoader.swift +++ b/Sources/XcodeGenKit/SpecLoader.swift @@ -26,6 +26,10 @@ public class SpecLoader { return project } + public func validateProjectDictionaryWarnings() throws { + try projectDictionary?.validateWarnings() + } + public func generateCacheFile() throws -> CacheFile? { guard let projectDictionary = projectDictionary, let project = project else { @@ -38,3 +42,36 @@ public class SpecLoader { ) } } + +private extension Dictionary where Key == String, Value: Any { + + func validateWarnings() throws { + var errors: [SpecValidationError.ValidationError] = [] + if hasValueContaining("$target_name") { + errors.append(.deprecatedUsageOfPlaceholder(placeholderName: "target_name")) + } + if hasValueContaining("$platform") { + errors.append(.deprecatedUsageOfPlaceholder(placeholderName: "platform")) + } + if !errors.isEmpty { + throw SpecValidationError(errors: errors) + } + } + + func hasValueContaining(_ needle: String) -> Bool { + return values.contains { value in + switch value { + case let dictionary as JSONDictionary: + return dictionary.hasValueContaining(needle) + case let string as String: + return string.contains(needle) + case let array as [JSONDictionary]: + return array.contains { $0.hasValueContaining(needle) } + case let array as [String]: + return array.contains { $0.contains(needle) } + default: + return false + } + } + } +} diff --git a/Tests/XcodeGenKitTests/SpecLoadingTests.swift b/Tests/XcodeGenKitTests/SpecLoadingTests.swift index 569eaaf0..f9252455 100644 --- a/Tests/XcodeGenKitTests/SpecLoadingTests.swift +++ b/Tests/XcodeGenKitTests/SpecLoadingTests.swift @@ -5,6 +5,7 @@ import Spectre import XcodeGenKit import xcodeproj import XCTest +import Yams class SpecLoadingTests: XCTestCase { @@ -205,6 +206,79 @@ class SpecLoadingTests: XCTestCase { } } + func testSpecWarningValidation() { + describe { + var path: Path! + $0.before { + path = Path(components: [NSTemporaryDirectory(), "\(NSUUID().uuidString).yaml"]) + + } + $0.after { + try? FileManager.default.removeItem(atPath: path.string) + } + $0.it("fails validating warnings for deprecated placeholder usage") { + let dictionary: [String: Any] = [ + "name": "TestSpecWarningValidation", + "templates": [ + "Framework": [ + "type": "framework", + "sources": ["$target_name/$platform/Sources"], + ], + ], + "targets": [ + "Framework": [ + "type": "framework", + "platform": "iOS", + "templates": ["Framework"], + ], + ], + ] + try? Yams.dump(object: dictionary).write(toFile: path.string, atomically: true, encoding: .utf8) + let specLoader = SpecLoader(version: "1.1.0") + do { + _ = try specLoader.loadProject(path: path) + } catch { + throw failure("\(error)") + } + try expectError(SpecValidationError(errors: [ + .deprecatedUsageOfPlaceholder(placeholderName: "target_name"), + .deprecatedUsageOfPlaceholder(placeholderName: "platform"), + ]), { try specLoader.validateProjectDictionaryWarnings() }) + } + + $0.it("successfully validates warnings for new placeholder usage") { + let dictionary: [String: Any] = [ + "name": "TestSpecWarningValidation", + "templates": [ + "Framework": [ + "type": "framework", + "sources": ["${target_name}/${platform}/Sources"], + ], + ], + "targets": [ + "Framework": [ + "type": "framework", + "platform": "iOS", + "templates": ["Framework"], + ], + ], + ] + try? Yams.dump(object: dictionary).write(toFile: path.string, atomically: true, encoding: .utf8) + let specLoader = SpecLoader(version: "1.1.0") + do { + _ = try specLoader.loadProject(path: path) + } catch { + throw failure("\(error)") + } + do { + try specLoader.validateProjectDictionaryWarnings() + } catch { + throw failure("Expected to not throw a validation error. Got: \(error)") + } + } + } + } + func testProjectSpecParser() { let validTarget: [String: Any] = ["type": "application", "platform": "iOS"] let invalid = "invalid" @@ -315,7 +389,7 @@ class SpecLoadingTests: XCTestCase { "platform": ["iOS", "tvOS"], "deploymentTarget": ["iOS": 9.0, "tvOS": "10.0"], "type": "framework", - "sources": ["Framework", "Framework $platform"], + "sources": ["Framework", "Framework ${platform}"], "settings": ["SETTING": "value_$platform"], ] @@ -340,6 +414,9 @@ class SpecLoadingTests: XCTestCase { "deploymentTarget": "1.2.0", "sources": ["targetSource"], "templates": ["temp2", "temp"], + "templateAttributes": [ + "source": "replacedSource" + ] ] let project = try getProjectSpec([ @@ -353,7 +430,11 @@ class SpecLoadingTests: XCTestCase { "type": "framework", "platform": "tvOS", "deploymentTarget": "1.1.0", - "configFiles": ["debug": "Configs/$target_name/debug.xcconfig"], + "configFiles": [ + "debug": "Configs/$target_name/debug.xcconfig", + "release": "Configs/${target_name}/release.xcconfig" + ], + "sources": ["${source}"] ], ], ]) @@ -362,8 +443,9 @@ class SpecLoadingTests: XCTestCase { try expect(target.type) == .framework // uses value try expect(target.platform) == .iOS // uses latest value try expect(target.deploymentTarget) == Version("1.2.0") // keeps value - try expect(target.sources) == ["templateSource", "targetSource"] // merges array in order + try expect(target.sources) == ["replacedSource", "templateSource", "targetSource"] // merges array in order try expect(target.configFiles["debug"]) == "Configs/Framework/debug.xcconfig" // replaces $target_name + try expect(target.configFiles["release"]) == "Configs/Framework/release.xcconfig" // replaces ${target_name} } $0.it("parses nested target templates") { @@ -411,6 +493,11 @@ class SpecLoadingTests: XCTestCase { "platform": "iOS", "templates": ["temp"], "sources": ["target"], + "templateAttributes": [ + "temp": "temp-by-target", + "a": "a-by-target", + "b": "b-by-target" // This should win over attributes defined in template "temp" + ] ] let project = try getProjectSpec([ @@ -418,33 +505,50 @@ class SpecLoadingTests: XCTestCase { "targetTemplates": [ "temp": [ "templates": ["a", "d"], - "sources": ["temp"], + "sources": ["temp", "${temp}"], + "templateAttributes": [ + "b": "b-by-temp", + "c": "c-by-temp", + "d": "d-by-temp" + ] ], "a": [ "templates": ["b", "c"], - "sources": ["a"], + "sources": ["a", "${a}"], + "templateAttributes": [ + "c": "c-by-a" + ] ], "b": [ - "sources": ["b"], + "sources": ["b", "${b}"], ], "c": [ - "sources": ["c"], + "sources": ["c", "${c}"], ], "d": [ - "sources": ["d"], + "sources": ["d", "${d}"], "templates": ["e"], + "templateAttributes": [ + "e": "e-by-d" + ] ], "e": [ - "sources": ["e"], + "sources": ["e", "${e}"], ], ], - ]) + ]) let target = project.targets.first! try expect(target.type) == .framework // uses value of last nested template try expect(target.platform) == .iOS // uses latest value - try expect(target.sources) == ["b", "c", "a", "e", "d", "temp", "target"] // merges array in order + try expect(target.sources) == ["b", "b-by-target", + "c", "c-by-temp", + "a", "a-by-target", + "e", "e-by-d", + "d", "d-by-temp", + "temp", "temp-by-target", + "target"] // merges array in order } $0.it("parses nested target templates with cycle") {