Repository navigation
Conversation
pmattos
requested review from
bkhouri,
bripeticca,
cmcgee1024,
daveinglis,
daveyc123,
dschaefer2,
jakepetroules,
owenv,
plemarquand and
rconnell9
as code owners
August 20, 2026 23:39
pmattos
changed the title
plemarquand
reviewed
Aug 21, 2026
| projectDir: projectDir, | ||
| name: name, | ||
| targetBuildSettings: self.package.underlying.packageBaseBuildSettings, | ||
| developmentRegion: self.package.manifest.defaultLocalization |
Contributor
There was a problem hiding this comment.
developmentRegion wasn't being set before, and looks like it's serialized. Is this going to affect anything by being added?
Contributor
Author
There was a problem hiding this comment.
I don't think so. The placeholder project now carries a development region where it previously carried nil. A placeholder has a single aggregate target with no product, no build phases and no files, so there should be nothing for a base localization to affect.
Contributor
Author
|
@swift-ci test |
plemarquand
approved these changes
Aug 21, 2026
…lder buildPlaceholderPIF(id:path:projectDir:name:) only used the builder instance to read the package's base build settings and to store the resulting project. That meant a placeholder PIF could only be built once a PackagePIFBuilder existed, which requires a resolved package and a modules graph — yet a placeholder is wanted precisely when a package failed to load, that is, when those inputs are missing and no builder can be constructed. It also made the placeholder builder untestable without a full package graph. Split it into the same three layers buildEmptyPIF already uses: a type-level buildPlaceholderPIF(id:path:projectDir:name:targetBuildSettings:developmentRegion:) that needs no instance and returns both the project and the placeholder module, a buildPlaceholderPIF(package:) convenience, and the existing instance method kept with its current signature as a thin wrapper, so callers are unaffected. The settings parameter is named targetBuildSettings because they are applied to the aggregate target's build configurations rather than to the project's. The placeholder project now also sets developmentRegion, which it was alone in omitting. Adds PIFPlaceholderTests, which the refactor is what makes possible: the type-level entry point needs no package graph to exercise. rdar://185457772
PackagePIFBuilder is final, so class func and static func are behaviorally identical here: nothing can override them either way. class func nonetheless suggests overridability the type has renounced, and it was out of step with the rest of the project — before this change SwiftPM had exactly four non-override class func declarations, all of them in this file, against 1049 static func declarations elsewhere. Applies to both the empty and placeholder PIF builders. rdar://185457772
pmattos
force-pushed
the
pmattos/static-pif-placeholder
branch
from
September 3, 2026 16:41
7898fe4 to
0eb681e
Compare
Contributor
Author
|
@swift-ci test |
Contributor
Author
|
@swift-ci test Windows |
Contributor
Author
|
@swift-ci test |
Contributor
Author
|
@swift-ci test |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Make placeholder PIF construction available on the
PackagePIFBuildertype.Motivation:
buildPlaceholderPIF(id:path:projectDir:name:)was an instance method, but it only usedselfin two places: to read the package's base build settings, and to store the resulting project in_pifProject. Everything else was constructed from its arguments.That coupling had two costs:
PackagePIFBuilderhad been constructed, which requires a resolved package and a modules graph.An additional motivation is to follow the same three-layer shape
buildEmptyPIFalready uses:.Modifications:
Add two helper class functions to build the placeholder PIF.
The instance method keeps its signature and becomes a thin wrapper instead.
I also added some needed unit tests to validate this feature.
Finally, a minor cleanup for the related
buildEmptyPIFoperation.Result:
A placeholder PIF can now be built without a
PackagePIFBuilderinstance, which makes it usable for a package that failed to load.Development region. Behavior through the existing instance method is unchanged, with one exception: the placeholder project now carries a development region where it previously carried
nil. A placeholder has a single aggregate target with no product, no build phases and no files, so there should be nothing for a base localization to affect.Note that
buildPlaceholderPIFhas no callers within SwiftPM — it is API for clients embeddingSwiftBuildSupport, like Xcode.Fixes rdar://185457772