Begin including SPI documentation in local builds, and fix all newly-revealed DocC issues #295
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,8 +43,7 @@ git checkout -b release/x.y.z | |
| The package manifest files (Package.swift _and_ [email protected]) must | ||
| be updated so that the release can be used as a package dependency: | ||
|
|
||
| 1. Change the `isBuildingForDistribution` constant from `false` to `true`. | ||
| 1. Open the "Documentation/Testing.docc/TemporaryGettingStarted.md" file and | ||
| update the line: | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,15 +65,7 @@ let package = Package( | |
| .product(name: "SwiftSyntaxMacros", package: "swift-syntax"), | ||
| .product(name: "SwiftCompilerPlugin", package: "swift-syntax"), | ||
| ], | ||
| swiftSettings: .packageSettings | ||
| ), | ||
|
|
||
| // "Support" targets: These contain C family code and are used exclusively | ||
|
|
@@ -110,12 +102,20 @@ package.targets.append(contentsOf: [ | |
| ]) | ||
| #endif | ||
|
|
||
| /// Whether this package is building for distribution. | ||
| /// | ||
| /// This can be used to conditionalize certain settings which are not intended | ||
| /// for use when building the package for distribution to end users. | ||
| /// Non-distribution (i.e. development-only) builds may enable things | ||
| /// like stricter compiler features which are unsupported or inappropriate for | ||
| /// client builds. | ||
| let isBuildingForDistribution = false | ||
|
There was a problem hiding this comment. What if instead we checked for an environment variable and built SPI when it's set? let buildSPIDocumentation: Bool = {
if let boolValue = Context.environment["SWT_SPI_DOCUMENTATION_ENABLED"].flatMap(Bool.init) {
return boolValue
} else if let intValue = Context.environment["SWT_SPI_DOCUMENTATION_ENABLED"].flatMap(UInt64.init) {
return intValue != 0
}
return false // disabled by default
}()(That's a slightly simplified version of what our own There was a problem hiding this comment. We could do this, although a workflow I commonly use for viewing docs is the "Product > Build Documentation" command in recent versions of Xcode, and I was hoping for that to get this flag too. I don't know of a way to apply an env var in that scenario (when using the GUI Xcode app, not its xcodebuild CLI). There was a problem hiding this comment. The constant I added here would still be needed, or at least it would still be helpful, even if we conditionalized the SPI docs flag based on an env var, because the constant is what (now) decides whether all the unsafeFlags get included There was a problem hiding this comment. Honestly we can also just drop the unsafe flags outright. There was a problem hiding this comment.
We can ask the DocC folks if they can help with this. There was a problem hiding this comment.
+1 for some similar scenario I experienced. And I believe it is a Xcode/SwiftPM issue instead of a DocC issue. Xcode GUI only gives us the ability to set env value for executable target when they are running. I could not find a easy solution to setup the env value I use in my Package.swift file. Currently I just use a small macOS app I wrote to trigger Otherwise we'll have to quit Xcode and manually export the env value in shell and then open Xcode in the shell to make Package.swift access the env we set. 
As a person who maintains some Swift packages and likes to add environment variable condition config in package, it is really troublesome at present if we do not use any other 3rd party tools. |
||
|
|
||
| extension Array where Element == PackageDescription.SwiftSetting { | ||
| /// Settings intended to be applied to every Swift target in this package. | ||
| /// Analogous to project-level build settings in an Xcode project. | ||
| static var packageSettings: Self { | ||
| var settings = availabilityMacroSettings + [ | ||
| .enableExperimentalFeature("StrictConcurrency"), | ||
| .enableUpcomingFeature("ExistentialAny"), | ||
|
|
||
|
|
@@ -124,6 +124,12 @@ extension Array where Element == PackageDescription.SwiftSetting { | |
|
|
||
| .define("SWT_TARGET_OS_APPLE", .when(platforms: [.macOS, .iOS, .macCatalyst, .watchOS, .tvOS, .visionOS])), | ||
| ] | ||
|
|
||
| if !isBuildingForDistribution { | ||
| settings.append(contentsOf: developmentOnlySettings) | ||
| } | ||
|
|
||
| return settings | ||
| } | ||
|
|
||
| /// Settings which define commonly-used OS availability macros. | ||
|
|
@@ -142,6 +148,15 @@ extension Array where Element == PackageDescription.SwiftSetting { | |
| .enableExperimentalFeature("AvailabilityMacro=_distantFuture:macOS 99.0, iOS 99.0, watchOS 99.0, tvOS 99.0"), | ||
| ] | ||
| } | ||
|
|
||
| /// Settings which are useful during development, but should not be included | ||
| /// when building for distribution. | ||
| private static var developmentOnlySettings: Self { | ||
| [ | ||
| .unsafeFlags(["-require-explicit-sendable"]), | ||
| .unsafeFlags(["-include-spi-symbols"]), | ||
| ] | ||
| } | ||
| } | ||
|
|
||
| extension Array where Element == PackageDescription.CXXSetting { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even though I still like the idea of this setting, in practice it is not giving us any benefit or protection since currently it must be commented-out for distribution builds (which are the usage scenario it was meant for). Having it here causes us an additional manual step each time we cut a release, and there's little practical value in conditionalizing it to only non-distribution builds IMO.