Preserve target order

Author: mirkokg

CHANGELOG.md

@@ -2,8 +2,8 @@
 
 ## Next Version
 
-### Added
-- Added `targetOrdering` option to control the order of targets in the generated project (sidebar in Xcode, `xcodebuild -list` output). Unlisted targets fall back to the existing alphabetical order, so behavior is unchanged when the option is not set. @mirkokg
+### Changed
+- Targets in the generated project now follow the declaration order from the YAML spec (Xcode sidebar, `xcodebuild -list` output). Previously they were always sorted alphabetically. JSON specs and projects built programmatically continue to sort alphabetically, since key order isn't recoverable. @mirkokg
 
 ## 2.45.4
 

Docs/ProjectSpec.md

@@ -151,7 +151,6 @@ Note that target names can also be changed by adding a `name` property to a targ
   - `top` - at the top, before files
   - `bottom` (default) - at the bottom, after other files
 - [ ] **groupOrdering**: **[[GroupOrdering]](#groupOrdering)** - An order of groups.
-- [ ] **targetOrdering**: **[String]** - An ordered list of target names. Listed targets appear in this order in the generated project (sidebar in Xcode, `xcodebuild -list` output); unlisted targets follow, sorted alphabetically. If a name in this list doesn't match any target or aggregate target, a validation error is raised. Defaults to an empty list, in which case all targets are sorted alphabetically.
 - [ ] **transitivelyLinkDependencies**: **Bool** - If this is `true` then targets will link to the dependencies of their target dependencies. If a target should embed its dependencies, such as application and test bundles, it will embed these transitive dependencies as well. Some complex setups might want to set this to `false` and explicitly specify dependencies at every level. Targets can override this with [Target](#target).transitivelyLinkDependencies. Defaults to `false`.
 - [ ] **generateEmptyDirectories**: **Bool** - If this is `true` then empty directories will be added to project too else will be missed. Defaults to `false`.
 - [ ] **findCarthageFrameworks**: **Bool** - When this is set to `true`, all the individual frameworks for Carthage framework dependencies will automatically be found. This property can be overridden individually for each carthage dependency - for more details see See **findFrameworks** in the [Dependency](#dependency) section. Defaults to `false`.

Sources/ProjectSpec/Project.swift

@@ -164,12 +164,19 @@ extension Project {
     }
 
     public init(spec: SpecFile) throws {
-        try self.init(basePath: spec.basePath, jsonDictionary: spec.resolvedDictionary())
+        try self.init(
+            basePath: spec.basePath,
+            jsonDictionary: spec.resolvedDictionary(),
+            targetDeclarationOrder: spec.resolvedTargetDeclarationOrder()
+        )
     }
 
-    public init(basePath: Path = "", jsonDictionary: JSONDictionary) throws {
+    public init(basePath: Path = "", jsonDictionary: JSONDictionary, targetDeclarationOrder: [String] = []) throws {
         self.basePath = basePath
 
+        let rawTargets = jsonDictionary["targets"] as? JSONDictionary ?? [:]
+        let expandedTargetOrder = Project.expandedTargetOrder(targetDeclarationOrder, rawTargets: rawTargets)
+
         let jsonDictionary = Project.resolveProject(jsonDictionary: jsonDictionary)
         let buildSettingsParser = BuildSettingsParser(jsonDictionary: jsonDictionary)
 
@@ -181,7 +188,16 @@ extension Project {
         let configs: [String: String] = jsonDictionary.json(atKeyPath: "configs") ?? [:]
         self.configs = configs.isEmpty ? Config.defaultConfigs :
             configs.map { Config(name: $0, type: ConfigType(rawValue: $1)) }.sorted { $0.name < $1.name }
-        targets = try jsonDictionary.json(atKeyPath: "targets", parallel: true).sorted { $0.name < $1.name }
+        let parsedTargets: [Target] = try jsonDictionary.json(atKeyPath: "targets", parallel: true)
+        let targetOrderIndex = Dictionary(uniqueKeysWithValues: expandedTargetOrder.enumerated().map { ($1, $0) })
+        targets = parsedTargets.sorted { lhs, rhs in
+            let lhsIndex = targetOrderIndex[lhs.name] ?? Int.max
+            let rhsIndex = targetOrderIndex[rhs.name] ?? Int.max
+            if lhsIndex != rhsIndex {
+                return lhsIndex < rhsIndex
+            }
+            return lhs.name < rhs.name
+        }
         aggregateTargets = try jsonDictionary.json(atKeyPath: "aggregateTargets").sorted { $0.name < $1.name }
         projectReferences = try jsonDictionary.json(atKeyPath: "projectReferences").sorted { $0.name < $1.name }
         schemes = try jsonDictionary.json(atKeyPath: "schemes")
@@ -218,6 +234,24 @@ extension Project {
         projectReferencesMap = Dictionary(uniqueKeysWithValues: projectReferences.map { ($0.name, $0) })
     }
 
+    static func expandedTargetOrder(_ declarationOrder: [String], rawTargets: JSONDictionary) -> [String] {
+        var result: [String] = []
+        for key in declarationOrder {
+            guard let target = rawTargets[key] as? JSONDictionary else { continue }
+            if let platforms = target["platform"] as? [String] {
+                for platform in platforms {
+                    let prefix = target["platformPrefix"] as? String ?? ""
+                    let suffix = target["platformSuffix"] as? String ?? "_\(platform)"
+                    result.append(prefix + key + suffix)
+                }
+            } else {
+                let resolvedName = target["name"] as? String ?? key
+                result.append(resolvedName)
+            }
+        }
+        return result
+    }
+
     static func resolveProject(jsonDictionary: JSONDictionary) -> JSONDictionary {
         var jsonDictionary = jsonDictionary
 

Sources/ProjectSpec/SpecFile.swift

@@ -9,6 +9,7 @@ public struct SpecFile {
     public let basePath: Path
     public let jsonDictionary: JSONDictionary
     public let subSpecs: [SpecFile]
+    public let targetDeclarationOrder: [String]
 
     /// The relative path to use when resolving paths in the json dictionary. Is an empty path when
     /// included with relativePaths disabled.
@@ -68,12 +69,13 @@ public struct SpecFile {
     }
 
     /// Memberwise initializer for SpecFile
-    public init(filePath: Path, jsonDictionary: JSONDictionary, basePath: Path = "", relativePath: Path = "", subSpecs: [SpecFile] = []) {
+    public init(filePath: Path, jsonDictionary: JSONDictionary, basePath: Path = "", relativePath: Path = "", subSpecs: [SpecFile] = [], targetDeclarationOrder: [String] = []) {
         self.basePath = basePath
         self.relativePath = relativePath
         self.jsonDictionary = jsonDictionary
         self.subSpecs = subSpecs
         self.filePath = filePath
+        self.targetDeclarationOrder = targetDeclarationOrder
     }
 
     private init(include: Include, basePath: Path, relativePath: Path, cachedSpecFiles: inout [Path: SpecFile], variables: [String: String]) throws {
@@ -91,6 +93,7 @@ public struct SpecFile {
         }
 
         let jsonDictionary = try SpecFile.loadDictionary(path: path).expand(variables: variables)
+        let targetDeclarationOrder = try SpecFile.loadTargetDeclarationOrder(path: path)
 
         let includes = Include.parse(json: jsonDictionary["include"])
         let subSpecs: [SpecFile] = try includes
@@ -99,10 +102,17 @@ public struct SpecFile {
                 return try SpecFile(include: include, basePath: basePath, relativePath: relativePath, cachedSpecFiles: &cachedSpecFiles, variables: variables)
             }
 
-        self.init(filePath: filePath, jsonDictionary: jsonDictionary, basePath: basePath, relativePath: relativePath, subSpecs: subSpecs)
+        self.init(filePath: filePath, jsonDictionary: jsonDictionary, basePath: basePath, relativePath: relativePath, subSpecs: subSpecs, targetDeclarationOrder: targetDeclarationOrder)
         cachedSpecFiles[path] = self
     }
 
+    static func loadTargetDeclarationOrder(path: Path) throws -> [String] {
+        if path.extension?.lowercased() == "json" {
+            return []
+        }
+        return try loadOrderedTargetNames(path: path)
+    }
+
     static func loadDictionary(path: Path) throws -> JSONDictionary {
         // Depending on the extension we will either load the file as YAML or JSON
         if path.extension?.lowercased() == "json" {
@@ -121,6 +131,14 @@ public struct SpecFile {
         resolvedDictionaryWithUniqueTargets()
     }
 
+    public func resolvedTargetDeclarationOrder() -> [String] {
+        var cachedSpecFiles: [Path: SpecFile] = [:]
+        let resolvedSpec = resolvingPaths(cachedSpecFiles: &cachedSpecFiles)
+
+        var mergedSpecPaths = Set<Path>()
+        return resolvedSpec.mergedTargetDeclarationOrder(set: &mergedSpecPaths)
+    }
+
     private func resolvedDictionaryWithUniqueTargets() -> JSONDictionary {
         var cachedSpecFiles: [Path: SpecFile] = [:]
         let resolvedSpec = resolvingPaths(cachedSpecFiles: &cachedSpecFiles)
@@ -140,6 +158,28 @@ public struct SpecFile {
                 .reduce([:]) { $1.merged(onto: $0) })
     }
 
+    private func mergedTargetDeclarationOrder(set mergedSpecPaths: inout Set<Path>) -> [String] {
+        let path = basePath + filePath
+
+        guard mergedSpecPaths.insert(path).inserted else { return [] }
+
+        var order: [String] = []
+        var seen = Set<String>()
+        for subSpec in subSpecs {
+            for name in subSpec.mergedTargetDeclarationOrder(set: &mergedSpecPaths) {
+                if seen.insert(name).inserted {
+                    order.append(name)
+                }
+            }
+        }
+        for name in targetDeclarationOrder {
+            if seen.insert(name).inserted {
+                order.append(name)
+            }
+        }
+        return order
+    }
+
     private func resolvingPaths(cachedSpecFiles: inout [Path: SpecFile], relativeTo basePath: Path = Path()) -> SpecFile {
         let path = basePath + filePath
         if let cachedSpecFile = cachedSpecFiles[path] {
@@ -157,7 +197,8 @@ public struct SpecFile {
             jsonDictionary: jsonDictionary,
             basePath: self.basePath,
             relativePath: self.relativePath,
-            subSpecs: subSpecs.map { $0.resolvingPaths(cachedSpecFiles: &cachedSpecFiles, relativeTo: relativePath) }
+            subSpecs: subSpecs.map { $0.resolvingPaths(cachedSpecFiles: &cachedSpecFiles, relativeTo: relativePath) },
+            targetDeclarationOrder: targetDeclarationOrder
         )
         cachedSpecFiles[path] = specFile
         return specFile

Sources/ProjectSpec/SpecLoader.swift

@@ -19,7 +19,11 @@ public class SpecLoader {
         let projectRoot = projectRoot?.absolute()
         let spec = try SpecFile(path: path, projectRoot: projectRoot, variables: variables)
         let resolvedDictionary = spec.resolvedDictionary()
-        let project = try Project(basePath: projectRoot ?? spec.basePath, jsonDictionary: resolvedDictionary)
+        let project = try Project(
+            basePath: projectRoot ?? spec.basePath,
+            jsonDictionary: resolvedDictionary,
+            targetDeclarationOrder: spec.resolvedTargetDeclarationOrder()
+        )
 
         self.project = project
         projectDictionary = resolvedDictionary

Sources/ProjectSpec/SpecOptions.swift

@@ -30,7 +30,6 @@ public struct SpecOptions: Equatable {
     public var transitivelyLinkDependencies: Bool
     public var groupSortPosition: GroupSortPosition
     public var groupOrdering: [GroupOrdering]
-    public var targetOrdering: [String]
     public var fileTypes: [String: FileType]
     public var generateEmptyDirectories: Bool
     public var findCarthageFrameworks: Bool
@@ -97,7 +96,6 @@ public struct SpecOptions: Equatable {
         transitivelyLinkDependencies: Bool = transitivelyLinkDependenciesDefault,
         groupSortPosition: GroupSortPosition = groupSortPositionDefault,
         groupOrdering: [GroupOrdering] = [],
-        targetOrdering: [String] = [],
         fileTypes: [String: FileType] = [:],
         generateEmptyDirectories: Bool = generateEmptyDirectoriesDefault,
         findCarthageFrameworks: Bool = findCarthageFrameworksDefault,
@@ -126,7 +124,6 @@ public struct SpecOptions: Equatable {
         self.transitivelyLinkDependencies = transitivelyLinkDependencies
         self.groupSortPosition = groupSortPosition
         self.groupOrdering = groupOrdering
-        self.targetOrdering = targetOrdering
         self.fileTypes = fileTypes
         self.generateEmptyDirectories = generateEmptyDirectories
         self.findCarthageFrameworks = findCarthageFrameworks
@@ -163,7 +160,6 @@ extension SpecOptions: JSONObjectConvertible {
         transitivelyLinkDependencies = jsonDictionary.json(atKeyPath: "transitivelyLinkDependencies") ?? SpecOptions.transitivelyLinkDependenciesDefault
         groupSortPosition = jsonDictionary.json(atKeyPath: "groupSortPosition") ?? SpecOptions.groupSortPositionDefault
         groupOrdering = jsonDictionary.json(atKeyPath: "groupOrdering") ?? []
-        targetOrdering = jsonDictionary.json(atKeyPath: "targetOrdering") ?? []
         generateEmptyDirectories = jsonDictionary.json(atKeyPath: "generateEmptyDirectories") ?? SpecOptions.generateEmptyDirectoriesDefault
         findCarthageFrameworks = jsonDictionary.json(atKeyPath: "findCarthageFrameworks") ?? SpecOptions.findCarthageFrameworksDefault
         localPackagesGroup = jsonDictionary.json(atKeyPath: "localPackagesGroup")
@@ -222,9 +218,6 @@ extension SpecOptions: JSONEncodable {
         if schemePathPrefix != SpecOptions.schemePathPrefixDefault {
             dict["schemePathPrefix"] = schemePathPrefix
         }
-        if !targetOrdering.isEmpty {
-            dict["targetOrdering"] = targetOrdering
-        }
 
         return dict
     }

Sources/ProjectSpec/SpecValidation.swift

@@ -77,19 +77,6 @@ extension Project {
             }
         }
 
-        if !options.targetOrdering.isEmpty {
-            let knownTargetNames = Set(projectTargets.map { $0.name })
-            var seen = Set<String>()
-            for name in options.targetOrdering {
-                if !seen.insert(name).inserted {
-                    errors.append(.duplicateTargetOrderingEntry(name))
-                }
-                if !knownTargetNames.contains(name) {
-                    errors.append(.invalidTargetOrderingReference(name))
-                }
-            }
-        }
-
         for settings in settingGroups.values {
             errors += validateSettings(settings)
         }

Sources/ProjectSpec/SpecValidationError.swift

@@ -43,8 +43,6 @@ public struct SpecValidationError: Error, CustomStringConvertible {
         case duplicateDependencies(target: String, dependencyReference: String)
         case invalidPluginPackageReference(plugin: String, package: String)
         case emptySourcePath(target: String)
-        case invalidTargetOrderingReference(String)
-        case duplicateTargetOrderingEntry(String)
 
         public var description: String {
             switch self {
@@ -114,10 +112,6 @@ public struct SpecValidationError: Error, CustomStringConvertible {
                 return "Plugin \(plugin) has invalid package reference \(package)"
             case let .emptySourcePath(target):
                 return "Target \(target.quoted) has an empty source path entry"
-            case let .invalidTargetOrderingReference(name):
-                return "Target ordering includes \(name.quoted) which is not a known target or aggregate target"
-            case let .duplicateTargetOrderingEntry(name):
-                return "Target ordering contains \(name.quoted) more than once"
             }
         }
     }

Sources/ProjectSpec/Yaml.swift

@@ -17,6 +17,19 @@ public func loadYamlDictionary(path: Path) throws -> [String: Any] {
     return yaml as? [String: Any] ?? [:]
 }
 
+public func loadOrderedTargetNames(path: Path) throws -> [String] {
+    guard path.extension?.lowercased() != "json" else { return [] }
+    let string: String = try path.read()
+    guard !string.isEmpty else { return [] }
+    guard let node = try Yams.compose(yaml: string),
+          let rootMapping = node.mapping,
+          let targetsNode = rootMapping["targets"],
+          let targetsMapping = targetsNode.mapping else {
+        return []
+    }
+    return targetsMapping.compactMap { key, _ in key.string }
+}
+
 public func dumpYamlDictionary(_ dictionary: [String: Any], path: Path) throws {
     let uncluttered = (dictionary as [String: Any?]).removingEmptyArraysDictionariesAndNils()
     let string: String = try Yams.dump(object: uncluttered)

Sources/XcodeGenKit/PBXProjGenerator.swift

@@ -322,16 +322,9 @@ public class PBXProjGenerator {
         pbxProject.remotePackages = packageReferences.sorted { $0.key < $1.key }.map { $1 }
         pbxProject.localPackages = localPackageReferences.sorted { $0.key < $1.key }.map { $1 }
 
-        let allTargets: [PBXTarget] = targetObjects.valueArray + targetAggregateObjects.valueArray
-        let targetOrdering = project.options.targetOrdering
-        pbxProject.targets = allTargets.sorted { lhs, rhs in
-            let lhsIndex = targetOrdering.firstIndex(of: lhs.name) ?? Int.max
-            let rhsIndex = targetOrdering.firstIndex(of: rhs.name) ?? Int.max
-            if lhsIndex != rhsIndex {
-                return lhsIndex < rhsIndex
-            }
-            return lhs.name < rhs.name
-        }
+        let orderedNativeTargets: [PBXTarget] = project.targets.compactMap { targetObjects[$0.name] }
+        let orderedAggregateTargets: [PBXTarget] = project.aggregateTargets.compactMap { targetAggregateObjects[$0.name] }
+        pbxProject.targets = orderedNativeTargets + orderedAggregateTargets
         pbxProject.attributes = projectAttributes
         pbxProject.targetAttributes = generateTargetAttributes()
         return pbxProj