Skip to content

Make Resolved* in PackageGraph value types #7160

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

Merged
merged 29 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
7430175
Filter out duplicate package manifest conditions
MaxDesiatov Nov 22, 2023
6fd1606
Add doc comment to the new type
MaxDesiatov Nov 22, 2023
456959e
Fix tests regression
MaxDesiatov Nov 22, 2023
420f01a
Remove custom `ResolvedTarget.Dependency` conformance
MaxDesiatov Nov 22, 2023
76121f4
Roll back to arrays and reduce the diff
MaxDesiatov Dec 2, 2023
4bd4bb4
Update comments and reduce the diff
MaxDesiatov Dec 2, 2023
d6c0c8a
Reduce the diff
MaxDesiatov Dec 2, 2023
f3004a7
Reduce the diff
MaxDesiatov Dec 2, 2023
a6760d9
Reduce the diff to track the regression
MaxDesiatov Dec 2, 2023
f8aee43
[NFC] Remove `SupportedPlatforms`, add `PlatformVersionProvider`
MaxDesiatov Dec 3, 2023
1dee112
Use value types for storage in `Resolved*` types of `PackageGraph`
MaxDesiatov Dec 2, 2023
086d0f9
Make `Resolved*` in `PackageGraph` value types
MaxDesiatov Dec 2, 2023
27d4f68
Minor cleanups
MaxDesiatov Dec 2, 2023
b6c2698
Fix build errors
MaxDesiatov Dec 3, 2023
9387af3
Merge branch 'main' into maxd/platform-version-provider
MaxDesiatov Dec 5, 2023
b777e12
Rename `getDerived` to `getSupportedPlatform`
MaxDesiatov Dec 5, 2023
fcdaea7
Merge branch 'maxd/platform-version-provider' of github.com:apple/swi…
MaxDesiatov Dec 5, 2023
cce1f12
Fix build errors after merge
MaxDesiatov Dec 5, 2023
afee8e6
Address PR feedback
MaxDesiatov Dec 5, 2023
6dab954
Address PR feedback
MaxDesiatov Dec 5, 2023
477935a
Sort cases in `PlatformRegistry`, use `init` in `PackageBuilder`
MaxDesiatov Dec 6, 2023
5b6670e
Merge branch 'main' into maxd/platform-version-provider
MaxDesiatov Dec 7, 2023
57ff0a4
Merge branch 'maxd/platform-version-provider' of github.com:apple/swi…
MaxDesiatov Dec 7, 2023
19627d6
Merge branch 'main' of github.com:apple/swift-package-manager into ma…
MaxDesiatov Dec 13, 2023
5ecf03d
clean up formatting
MaxDesiatov Dec 14, 2023
c6fcbea
Add dependency equality test in `TargetTests`
MaxDesiatov Dec 14, 2023
ad5792c
Fix target dependencies equality
MaxDesiatov Dec 14, 2023
02d5dfb
Use `topologicalSort` on `Identifiable`, fix `PackageGraphPerfTests`
MaxDesiatov Dec 14, 2023
c3ab4f5
Fix formatting
MaxDesiatov Dec 14, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import Basics
import PackageLoading
import PackageModel
import class PackageGraph.ResolvedTarget
import struct PackageGraph.ResolvedTarget
import struct SPMBuildCore.BuildParameters
import struct SPMBuildCore.BuildToolPluginInvocationResult
import struct SPMBuildCore.PrebuildCommandResult
Expand All @@ -26,9 +26,7 @@ public final class ClangTargetBuildDescription {
public let target: ResolvedTarget

/// The underlying clang target.
public var clangTarget: ClangTarget {
target.underlyingTarget as! ClangTarget
}
public let clangTarget: ClangTarget

/// The tools version of the package that declared the target. This can
/// can be used to conditionalize semantically significant changes in how
Expand All @@ -45,7 +43,7 @@ public final class ClangTargetBuildDescription {

/// The list of all resource files in the target, including the derived ones.
public var resources: [Resource] {
self.target.underlyingTarget.resources + self.pluginDerivedResources
self.target.underlying.resources + self.pluginDerivedResources
}

/// Path to the bundle generated for this module (if any).
Expand All @@ -54,7 +52,7 @@ public final class ClangTargetBuildDescription {
return .none
}

if let bundleName = target.underlyingTarget.potentialBundleName {
if let bundleName = target.underlying.potentialBundleName {
return self.buildParameters.bundlePath(named: bundleName)
} else {
return .none
Expand Down Expand Up @@ -119,10 +117,11 @@ public final class ClangTargetBuildDescription {
fileSystem: FileSystem,
observabilityScope: ObservabilityScope
) throws {
guard target.underlyingTarget is ClangTarget else {
guard let clangTarget = target.underlying as? ClangTarget else {
throw InternalError("underlying target type mismatch \(target)")
}

self.clangTarget = clangTarget
self.fileSystem = fileSystem
self.target = target
self.toolsVersion = toolsVersion
Expand Down
2 changes: 1 addition & 1 deletion Sources/Build/BuildDescription/PluginDescription.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public final class PluginDescription: Codable {
testDiscoveryTarget: Bool = false,
fileSystem: FileSystem
) throws {
guard target.underlyingTarget is PluginTarget else {
guard target.underlying is PluginTarget else {
throw InternalError("underlying target type mismatch \(target)")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ public final class ProductBuildDescription: SPMBuildCore.ProductBuildDescription
// Support for linking tests against executables is conditional on the tools
// version of the package that defines the executable product.
let executableTarget = try product.executableTarget
if let target = executableTarget.underlyingTarget as? SwiftTarget,
if let target = executableTarget.underlying as? SwiftTarget,
self.toolsVersion >= .v5_5,
self.buildParameters.driverParameters.canRenameEntrypointFunctionName,
target.supportsTestableExecutablesFeature
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ struct SharedTargetBuildDescription {
additionalFileRules: additionalFileRules,
defaultLocalization: target.defaultLocalization,
targetName: target.name,
targetPath: target.underlyingTarget.path,
targetPath: target.underlying.path,
observabilityScope: observabilityScope
)
let pluginDerivedResources = derivedResources
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ public final class SwiftTargetBuildDescription {
/// The target described by this target.
public let target: ResolvedTarget

private let swiftTarget: SwiftTarget

/// The tools version of the package that declared the target. This can
/// can be used to conditionalize semantically significant changes in how
/// a target is built.
Expand All @@ -57,7 +59,7 @@ public final class SwiftTargetBuildDescription {

/// Path to the bundle generated for this module (if any).
var bundlePath: AbsolutePath? {
if let bundleName = target.underlyingTarget.potentialBundleName, needsResourceBundle {
if let bundleName = target.underlying.potentialBundleName, needsResourceBundle {
return self.buildParameters.bundlePath(named: bundleName)
} else {
return .none
Expand All @@ -83,7 +85,7 @@ public final class SwiftTargetBuildDescription {

/// The list of all resource files in the target, including the derived ones.
public var resources: [Resource] {
self.target.underlyingTarget.resources + self.pluginDerivedResources
self.target.underlying.resources + self.pluginDerivedResources
}

/// The objects in this target, containing either machine code or bitcode
Expand Down Expand Up @@ -139,7 +141,7 @@ public final class SwiftTargetBuildDescription {

/// The swift version for this target.
var swiftVersion: SwiftLanguageVersion {
(self.target.underlyingTarget as! SwiftTarget).swiftVersion
self.swiftTarget.swiftVersion
}

/// Describes the purpose of a test target, including any special roles such as containing a list of discovered
Expand Down Expand Up @@ -257,10 +259,11 @@ public final class SwiftTargetBuildDescription {
fileSystem: FileSystem,
observabilityScope: ObservabilityScope
) throws {
guard target.underlyingTarget is SwiftTarget else {
guard let swiftTarget = target.underlying as? SwiftTarget else {
throw InternalError("underlying target type mismatch \(target)")
}

self.swiftTarget = swiftTarget
self.package = package
self.target = target
self.toolsVersion = toolsVersion
Expand Down Expand Up @@ -514,7 +517,7 @@ public final class SwiftTargetBuildDescription {
// when we link the executable, we will ask the linker to rename the entry point
// symbol to just `_main` again (or if the linker doesn't support it, we'll
// generate a source containing a redirect).
if (self.target.underlyingTarget as? SwiftTarget)?.supportsTestableExecutablesFeature == true
if (self.target.underlying as? SwiftTarget)?.supportsTestableExecutablesFeature == true
&& !self.isTestTarget && self.toolsVersion >= .v5_5
{
// We only do this if the linker supports it, as indicated by whether we
Expand Down
4 changes: 2 additions & 2 deletions Sources/Build/BuildDescription/TargetBuildDescription.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@
//===----------------------------------------------------------------------===//

import Basics
import struct SPMBuildCore.BuildParameters
import class PackageGraph.ResolvedTarget
import struct PackageGraph.ResolvedTarget
import struct PackageModel.Resource
import struct SPMBuildCore.BuildToolPluginInvocationResult
import struct SPMBuildCore.BuildParameters

public enum BuildDescriptionError: Swift.Error {
case requestedFileNotPartOfTarget(targetName: String, requestedFilePath: AbsolutePath)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import struct LLBuildManifest.Node
import struct Basics.AbsolutePath
import struct Basics.InternalError
import class PackageGraph.ResolvedTarget
import struct PackageGraph.ResolvedTarget
import PackageModel

extension LLBuildManifestBuilder {
Expand Down
12 changes: 6 additions & 6 deletions Sources/Build/BuildManifest/LLBuildManifestBuilder+Swift.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import struct Basics.TSCAbsolutePath
import struct LLBuildManifest.Node
import struct LLBuildManifest.LLBuildManifest
import struct SPMBuildCore.BuildParameters
import class PackageGraph.ResolvedTarget
import struct PackageGraph.ResolvedTarget
import protocol TSCBasic.FileSystem
import enum TSCBasic.ProcessEnv
import func TSCBasic.topologicalSort
Expand Down Expand Up @@ -212,8 +212,8 @@ extension LLBuildManifestBuilder {
// product into its constituent targets.
continue
}
guard target.underlyingTarget.type != .systemModule,
target.underlyingTarget.type != .binary
guard target.underlying.type != .systemModule,
target.underlying.type != .binary
else {
// Much like non-Swift targets, system modules will consist of a modulemap
// somewhere in the filesystem, with the path to that module being either
Expand Down Expand Up @@ -416,11 +416,11 @@ extension LLBuildManifestBuilder {

func addStaticTargetInputs(_ target: ResolvedTarget) throws {
// Ignore C Modules.
if target.underlyingTarget is SystemLibraryTarget { return }
if target.underlying is SystemLibraryTarget { return }
// Ignore Binary Modules.
if target.underlyingTarget is BinaryTarget { return }
if target.underlying is BinaryTarget { return }
// Ignore Plugin Targets.
if target.underlyingTarget is PluginTarget { return }
if target.underlying is PluginTarget { return }

// Depend on the binary for executable targets.
if target.type == .executable {
Expand Down
2 changes: 1 addition & 1 deletion Sources/Build/BuildOperation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
for package in graph.rootPackages where package.manifest.toolsVersion >= .v5_3 {
for target in package.targets {
// Get the set of unhandled files in targets.
var unhandledFiles = Set(target.underlyingTarget.others)
var unhandledFiles = Set(target.underlying.others)
if unhandledFiles.isEmpty { continue }

// Subtract out any that were inputs to any commands generated by plugins.
Expand Down
2 changes: 1 addition & 1 deletion Sources/Build/BuildPlan/BuildPlan+Clang.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ extension BuildPlan {
let dependencies = try clangTarget.target.recursiveDependencies(satisfying: clangTarget.buildEnvironment)

for case .target(let dependency, _) in dependencies {
switch dependency.underlyingTarget {
switch dependency.underlying {
case is SwiftTarget:
if case let .swift(dependencyTargetDescription)? = targetMap[dependency] {
if let moduleMap = dependencyTargetDescription.moduleMap {
Expand Down
20 changes: 10 additions & 10 deletions Sources/Build/BuildPlan/BuildPlan+Product.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
import struct Basics.AbsolutePath
import struct Basics.Triple
import struct Basics.InternalError
import class PackageGraph.ResolvedProduct
import class PackageGraph.ResolvedTarget
import struct PackageGraph.ResolvedProduct
import struct PackageGraph.ResolvedTarget
import class PackageModel.BinaryTarget
import class PackageModel.ClangTarget
import class PackageModel.Target
Expand All @@ -32,7 +32,7 @@ extension BuildPlan {

// Add flags for system targets.
for systemModule in dependencies.systemModules {
guard case let target as SystemLibraryTarget = systemModule.underlyingTarget else {
guard case let target as SystemLibraryTarget = systemModule.underlying else {
throw InternalError("This should not be possible.")
}
// Add pkgConfig libs arguments.
Expand All @@ -53,7 +53,7 @@ extension BuildPlan {
// Link C++ if needed.
// Note: This will come from build settings in future.
for target in dependencies.staticTargets {
if case let target as ClangTarget = target.underlyingTarget, target.isCXX {
if case let target as ClangTarget = target.underlying, target.isCXX {
let triple = buildProduct.buildParameters.triple
if triple.isDarwin() {
buildProduct.additionalFlags += ["-lc++"]
Expand All @@ -67,7 +67,7 @@ extension BuildPlan {
}

for target in dependencies.staticTargets {
switch target.underlyingTarget {
switch target.underlying {
case is SwiftTarget:
// Swift targets are guaranteed to have a corresponding Swift description.
guard case .swift(let description) = targetMap[target] else {
Expand Down Expand Up @@ -131,7 +131,7 @@ extension BuildPlan {
// For test targets, we need to consider the first level of transitive dependencies since the first level is always test targets.
let topLevelDependencies: [PackageModel.Target]
if product.type == .test {
topLevelDependencies = product.targets.flatMap { $0.underlyingTarget.dependencies }.compactMap {
topLevelDependencies = product.targets.flatMap { $0.underlying.dependencies }.compactMap {
switch $0 {
case .product:
return nil
Expand All @@ -149,7 +149,7 @@ extension BuildPlan {
switch dependency {
// Include all the dependencies of a target.
case .target(let target, _):
let isTopLevel = topLevelDependencies.contains(target.underlyingTarget) || product.targets.contains(target)
let isTopLevel = topLevelDependencies.contains(target.underlying) || product.targets.contains(target)
let topLevelIsMacro = isTopLevel && product.type == .macro
let topLevelIsPlugin = isTopLevel && product.type == .plugin
let topLevelIsTest = isTopLevel && product.type == .test
Expand Down Expand Up @@ -200,9 +200,9 @@ extension BuildPlan {
case .executable, .snippet, .macro:
if product.targets.contains(target) {
staticTargets.append(target)
} else if product.type == .test && (target.underlyingTarget as? SwiftTarget)?.supportsTestableExecutablesFeature == true {
} else if product.type == .test && (target.underlying as? SwiftTarget)?.supportsTestableExecutablesFeature == true {
// Only "top-level" targets should really be considered here, not transitive ones.
let isTopLevel = topLevelDependencies.contains(target.underlyingTarget) || product.targets.contains(target)
let isTopLevel = topLevelDependencies.contains(target.underlying) || product.targets.contains(target)
if let toolsVersion = graph.package(for: product)?.manifest.toolsVersion, toolsVersion >= .v5_5, isTopLevel {
staticTargets.append(target)
}
Expand All @@ -220,7 +220,7 @@ extension BuildPlan {
systemModules.append(target)
// Add binary to binary paths set.
case .binary:
guard let binaryTarget = target.underlyingTarget as? BinaryTarget else {
guard let binaryTarget = target.underlying as? BinaryTarget else {
throw InternalError("invalid binary target '\(target.name)'")
}
switch binaryTarget.kind {
Expand Down
2 changes: 1 addition & 1 deletion Sources/Build/BuildPlan/BuildPlan+Swift.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ extension BuildPlan {
// depends on.
let environment = swiftTarget.buildParameters.buildEnvironment
for case .target(let dependency, _) in try swiftTarget.target.recursiveDependencies(satisfying: environment) {
switch dependency.underlyingTarget {
switch dependency.underlying {
case let underlyingTarget as ClangTarget where underlyingTarget.type == .library:
guard case let .clang(target)? = targetMap[dependency] else {
throw InternalError("unexpected clang target \(underlyingTarget)")
Expand Down
Loading