-
Notifications
You must be signed in to change notification settings - Fork 56
Add initial experimental support for combined documentation for multiple targets #84
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
Add initial experimental support for combined documentation for multiple targets #84
Conversation
rdar://116698361
rdar://116698361
This flag allows the targets to link to each other and creates an additional combined archive. rdar://116698361
6a1592c
to
fd4f39b
Compare
@swift-ci please test |
@swift-ci please test |
@swift-ci please test |
@swift-ci please test macOS |
extension DocCFeatures.Feature { | ||
/// DocC supports writing diagnostic information to a JSON file, specified by `--diagnostics-output-path`. | ||
static let diagnosticsFileOutput = DocCFeatures.Feature(name: "diagnostics-file") | ||
|
||
/// DocC supports linking between documentation builds and merging archives into a combined archive. | ||
static let linkDependencies = DocCFeatures.Feature(name: "dependency") | ||
|
||
/// DocC supports grouping overloaded symbols. | ||
static let overloads = DocCFeatures.Feature(name: "overloads") | ||
} |
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.
If this is specific to the features inside the compiler why not implement this in https://github.com/swiftlang/swift-docc directly?
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.
The plugin can't import SwiftDocC so this can't be defined there.
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.
It seems off to declare this data here. If this are DocC features supported by the plugin we may want to rename this to DocCSupportedFeatures
(minor)
Sources/SwiftDocCPluginUtilities/BuildGraph/DocumentationBuildGraph.swift
Show resolved
Hide resolved
Thanks for this work, David! Some comments:
|
When we previously talked about this PR I said that I had other follow-up changes to the current state of the PR and asked if you preferred that I commit them to this PR or make a follow-up PR after this first one was merged. This is one of those changes. Those changes also relate to some confusing behavior with the
I don't understand what change this question is asking for.
This is another follow-up change that I'm already working on and have previously demoed to the team and separately to the documentation workgroup. This change however, requires some changes to DocC, so it would be a follow-up to the other follow-up changes. |
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.
🚀
@swift-ci please test |
Tests/SwiftDocCPluginUtilitiesTests/DocumentationBuildGraphTests.swift
Outdated
Show resolved
Hide resolved
Sources/SwiftDocCPluginUtilities/BuildGraph/DocumentationBuildGraph.swift
Outdated
Show resolved
Hide resolved
Sources/SwiftDocCPluginUtilities/BuildGraph/DocumentationBuildGraph.swift
Outdated
Show resolved
Hide resolved
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.
I am concerned about the correctness of the error catching and cancellation logic. Additionally I would prefer if we used swift concurrency as this is new code.
- Check for errors after queue has run the build operations - Avoid repeat-visiting targets when constructing the build graph - Update internal-only documentation comment
…wift-docc-plugin into multi-target-documentation
This feedback has been addressed.
I've fixed the but with catching errors from the individual build tasks.
I strongly disagree. Swift concurrency is not a suitable tool for this because it doesn't support defining explicit dependencies between tasks and executing them (potentially in parallel) in that dependency order. If we were to use Swift concurrency for this we would need to implement our own task scheduler and executor to ensure that dependencies tasks are run before dependants tasks and that each task is only run once. This is not only a lot of extra code to write but it's a risk of bugs. Using |
@swift-ci please test |
} | ||
} | ||
|
||
// Run all the documentation build tasks in reverse dependency order (dependencies before dependents). |
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.
// Run all the documentation build tasks in reverse dependency order (dependencies before dependents). | |
// Run all the documentation build tasks in dependency order (dependencies before dependents). |
@swift-ci please test |
@daniel-grumberg Like we talked about offline: I added this type to encapsulate running the build tasks and these tests to verify the behavior when one of the targets encounters an error during the build. |
…sitive source validation error
@swift-ci please test |
Bug/issue #, if applicable: rdar://116698361
Summary
This adds an experimental
--enable-experimental-combined-documentation
flag to the DocC plugin which does two thingsFor more information on how this behaves, see the Testing section.
It also adds updates the plugins with some new utilities to build documentation tasks in an ordered based on their target dependencies and to query the DocC executable for its supported features.
Dependencies
This requires a DocC version that supports combined documentation. For example; the version of DocC that come with the Swift 6.0 toolchain, the Swift trunk development toolchain, or the default toolchain in Xcode 16.
Testing
To test this, another package needs to depend on this branch of my fork of the Swift-DocC Plugin:
This is an additive feature, builds that don't pass the
--enable-experimental-combined-documentation
flag should continue to behave as before (with some minor improvements to the information that's printed about the generated documentation archives).Help text
When running
swift package generate-documentation --help
the plugin should only list the new--enable-experimental-combined-documentation
flag if the DocC executable in current the Swift toolchain supports combined documentation.Unsupported use
When running
swift package generate-documentation --enable-experimental-combined-documentation
with DocC from a Swift toolchain that doesn't support combined documentation, the plugin should fail with a descriptive error messageBuilding all targets
When running
swift package generate-documentation --enable-experimental-combined-documentation
without specifying any targets should build documentation for all targets that support documentation in the package—same as it does today.A target that depends on another target in the same package can write links to that other target using links that start with a "/" and the module name of the other target. For example:
``/OtherTarget/SomeSymbol/someFunction()``
.After the plugin has generated documentation for all targets, it will create one more documentation archives that combines the generated documentation for all targets from the build.
Building some targets
When running
swift package generate-documentation --enable-experimental-combined-documentation
and specifying a number of targets using--target TargetName
pairs, the plugin should build documentation for only targets—same as it does today.A target that depends on another target in the same package can only write links to that target if it is also listed as one of the
--target TargetName
pairs passed to thegenerate-documentation
command.After the plugin has generated documentation for the specified targets, it will create one more documentation archives that combines the generated documentation for only the specified targets.
Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/test
script and it succeeded