Skip to content

Validate traits in target build setting conditions on manifest load#9980

Open
bripeticca wants to merge 4 commits into
swiftlang:mainfrom
bripeticca:traits/validate-traits-on-target-build-settings-conditions
Open

Validate traits in target build setting conditions on manifest load#9980
bripeticca wants to merge 4 commits into
swiftlang:mainfrom
bripeticca:traits/validate-traits-on-target-build-settings-conditions

Conversation

@bripeticca
Copy link
Copy Markdown
Contributor

@bripeticca bripeticca commented Apr 22, 2026

Traits for a target's build setting conditions were not being validated upon loading the manifest; while the trait enablement for these conditions is checked later on (post-resolution) and is not actually validated here either, we should really be validating the traits referenced in the target build setting conditions immediately after loading the manifest itself.

@bripeticca
Copy link
Copy Markdown
Contributor Author

@swift-ci test

@bripeticca bripeticca changed the title [WIP] Validate traits in target build setting conditions on manifest load Validate traits in target build setting conditions on manifest load Apr 22, 2026
@bripeticca bripeticca marked this pull request as ready for review April 22, 2026 20:53
@bkhouri
Copy link
Copy Markdown
Contributor

bkhouri commented Apr 28, 2026

@swift-ci test self hosted windows

let observability = ObservabilitySystem.makeForTesting()
let (_, validationDiagnostics) = try await loadAndValidateManifest(content, observabilityScope: observability.topScope)
XCTAssertNoDiagnostics(observability.diagnostics)
let firstDiagnostic = try XCTUnwrap(validationDiagnostics[0])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity, why don't we emit the diagnostics via the observability diagnostics property?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question -- it seems like there is a pre-existing practice involved with the Manifest validation step here (see ManifestLoader+Validation.swift), so I continued to follow that for this change since I make the checks for these traits within those same validation methods.

Comment thread Tests/PackageLoadingTests/TraitLoadingTests.swift Outdated
@bripeticca
Copy link
Copy Markdown
Contributor Author

@swift-ci test

@bripeticca
Copy link
Copy Markdown
Contributor Author

@swift-ci test windows

@bripeticca
Copy link
Copy Markdown
Contributor Author

@swift-ci smoke test macOS

@bripeticca
Copy link
Copy Markdown
Contributor Author

@swift-ci test macOS

@bripeticca
Copy link
Copy Markdown
Contributor Author

@swift-ci test

@bripeticca
Copy link
Copy Markdown
Contributor Author

@swift-ci test windows

2 similar comments
@bripeticca
Copy link
Copy Markdown
Contributor Author

@swift-ci test windows

@bripeticca
Copy link
Copy Markdown
Contributor Author

@swift-ci test windows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants