swiftlang / swift-docc-plugin

Swift Package Manager command plugin for Swift-DocC
https://swiftpackageindex.com/swiftlang/swift-docc-plugin/documentation/swiftdoccplugin
Apache License 2.0
326 stars 52 forks source link

Add initial experimental support for combined documentation for multiple targets #84

Closed d-ronnqvist closed 3 months ago

d-ronnqvist commented 5 months ago

Bug/issue #, if applicable: rdar://116698361

Summary

This adds an experimental --enable-experimental-combined-documentation flag to the DocC plugin which does two things

For 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:

.package(url: "https://github.com/d-ronnqvist/swift-docc-plugin", branch: "multi-target-documentation"),

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 message

error: Unsupported use of '--enable-experimental-combined-documentation'. DocC version at '/path/to/current/docc' doesn't support combined documentation.

Building 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 the generate-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.

d-ronnqvist commented 5 months ago

@swift-ci please test

d-ronnqvist commented 5 months ago

@swift-ci please test

d-ronnqvist commented 5 months ago

@swift-ci please test

d-ronnqvist commented 5 months ago

@swift-ci please test macOS

sofiaromorales commented 3 months ago

Thanks for this work, David!

Some comments:

  1. If we emit a single combined documentation, also emitting the docs separately seems unnecessary. Something to consider is to not output the individual documentation archives if the --enable-experimental-combined-documentation was passed.

  2. Would make sense to emit the combined documentations grouped by dependencies? Since this produces a single doccarchive my first reaction was to think that I could link between them even if they don't depend on each other.

  3. As a future work would be nice to synthesize a top level page for all the targets.

d-ronnqvist commented 3 months ago
  1. If we emit a single combined documentation, also emitting the docs separately seems unnecessary. Something to consider is to not output the individual documentation archives if the --enable-experimental-combined-documentation was passed.

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 --output-dir command line option that more people will encounter now that there's a reason to build multiple target's documentation in one invocation. Adding this makes the PR a bit bigger but I you prefer, I can push those commits to this PR instead of holding them for a follow-up PR.

  1. Would make sense to emit the combined documentations grouped by dependencies? Since this produces a single doccarchive my first reaction was to think that I could link between them even if they don't depend on each other.

I don't understand what change this question is asking for.

  1. As a future work would be nice to synthesize a top level page for all the targets.

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.

d-ronnqvist commented 3 months ago

@swift-ci please test

d-ronnqvist commented 3 months ago

I am concerned about the correctness of the error catching and cancellation logic.

I've fixed the but with catching errors from the individual build tasks.

Additionally I would prefer if we used swift concurrency as this is new code.

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 Operation and OperationQueue from Foundation gives us a long-lived and well tested implementation of this for free.

d-ronnqvist commented 3 months ago

@swift-ci please test

d-ronnqvist commented 3 months ago

@swift-ci please test

d-ronnqvist commented 3 months ago

@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.

d-ronnqvist commented 3 months ago

@swift-ci please test