Adopt SwiftBuild.ProjectModel API in the new PIF builder in SwiftBuildSupport #8441
Conversation
pmattos
requested review from
bnbarham,
MaxDesiatov,
jakepetroules,
xedin,
dschaefer2,
shawnhyam,
bripeticca,
plemarquand,
owenv,
bkhouri,
cmcgee1024 and
daveyc123
as code owners
pmattos
changed the title
|
This is almost ready... I just want to do another pass tomorrow ;) |
pmattos
force-pushed
the
pmattos/pif-with-project-model-API
branch
from
1be51a2 to
efeb5e2
Compare
| @@ -1641,7 +1641,7 @@ private struct PIFBuildSettingAssignment { | |||
| let platforms: [PIF.BuildSettings.Platform]? | |||
| } | |||
|
|
|||
| extension BuildSettings.AssignmentTable { | |||
| extension PackageModel.BuildSettings.AssignmentTable { | |||
| fileprivate var pifAssignments: [PIF.BuildSettings.MultipleValueSetting: [PIFBuildSettingAssignment]] { | |||
There was a problem hiding this comment.
Just fixing naming clashes.
| var packageBaseBuildSettings: ProjectModel.BuildSettings { | ||
| var settings = BuildSettings() | ||
| settings[.SDKROOT] = "auto" | ||
| settings[.SDK_VARIANT] = "auto" | ||
|
|
There was a problem hiding this comment.
This is basically how we change build settings now (i.e., using a subscript).
Brace for this exact same code everywhere...
| /// Helpers for building custom PIF targets by `PIFPackageBuilder` clients. | ||
| extension SwiftBuild.PIF.Project { | ||
| extension ProjectModel.BuildSettings { | ||
| subscript(_ setting: MultipleValueSetting, default defaultValue: [String]) -> [String] { |
There was a problem hiding this comment.
Now I'm no longer sure if this syntax sugar is such a great idea.... note that it doesn't make it explicit if the default value is prepended or appended (it's the former).
var settings = ProjectModel.BuildSettings()
settings[.PATH, default: "/bin"] + ["/foo/bar"]
// PATH is now "/bin:/foo/bar"
There was a problem hiding this comment.
This seems OK to me, I think the syntax here reads as if it implies appending
| .OTHER_SWIFT_FLAGS, | ||
| .SWIFT_ACTIVE_COMPILATION_CONDITIONS: | ||
| let multipleSetting = MultipleValueSetting(from: setting)! | ||
| self[multipleSetting, default: ["$(inherited)"]].append(contentsOf: values) |
There was a problem hiding this comment.
See my #8441 (comment).
|
|
||
| return project | ||
| } | ||
|
|
||
| public func buildPlaceholderPIF(id: String, path: String, projectDir: String, name: String) -> ModuleOrProduct { | ||
| let project = SwiftBuild.PIF.Project( | ||
| id: id, | ||
| var project = ProjectModel.Project( |
There was a problem hiding this comment.
All PIF objects are value types now!
|
@swift-ci test |
| ) -> Bool | ||
|
|
||
| /// Should we set the install path for a dynamic library/framework? | ||
| func shouldSetInstallPathForDynamicLib(productName: String) -> Bool | ||
|
|
||
| // FIXME: Let's try to replace `WritableKeyPath><_, Foo>` with `inout Foo` —— Paulo |
There was a problem hiding this comment.
Is this a note for future work in a later PR?
|
@swift-ci test |
|
@swift-ci test |
1 similar comment
|
@swift-ci test |
|
@swift-ci please test self hosted macos |
1 similar comment
|
@swift-ci please test self hosted macos |
|
@swift-ci test |
|
@swift-ci test self hosted windows |
|
@swift-ci smoke test macos |
|
@swift-ci test macOS |
Motivation:
The goal is to adopt the new
SwiftBuild.ProjectModelAPI. This new API provides a typesafer and modern way of building PIFs programmatically.This continues the work I started in PR #8405, introducing now our new PIF builder for packages.
Modifications:
Replaces all
SwiftBuild.PIF(aka,SWBProjectModel.PIF) API usage, inPackagePIFBuilder, with the newSwiftBuild.ProjectModelAPI instead.PS. I did run
swiftformatin all new/changed code, as indicated by the contributors guide.Result:
PackagePIFBuilderis now modernized... but still not actually used. This will come in my next pull request.Tracked by rdar://147526957.