Support `--output-path` when building documentation for multiple targets. #89

d-ronnqvist · 2024-08-20T08:50:53Z

Bug/issue #, if applicable:

Summary

Support passing a custom --output-path value when building documentation for multiple targets.

This is technically unrelated to support for combined documentation archives, but it's more likely that developers will pass a custom output path when building combined documentation for multiple targets.

I implemented this PR in steps:

First e66ee31 added support for --output-path only when building combined documentation.
Then 39d886c extended that support to non-combined documentation builds as well.

This is technically a change in behavior—each target will output in a subdirectory of the specified output path. However, I considered the old behavior incorrect and not worth preserving because it was broken—each target wrote over the previous target's output, resulting in only one target's output.

Additionally, this PR makes the following changes

78fef3e conditionally customizes the synthesized landing page using Synthesize a minimal landing page for combined archives swift-docc#1011
3aeb675 Refines the command output to add more information to the build tasks and print the resulting archive paths the same when building one target, many targets, or a combined archive.
099fed3 Enables task parallelism when building documentation for more than one target
1df6bb8 adds two additional integration tests that verifies building combined documentation with and without a custom output path.

Dependencies

This builds on top of #87 and #88. The new changes in this PR start with e66ee31 and onwards. You can see that subset of the diff here.

Additionally, this PR conditionally uses some new functionality in swiftlang/swift-docc#1011.

Testing

There are 3 behaviors to test in this PR

building documentation for one target with a custom output path
building documentation for multiple targets with a custom output path
building combined documentation with a custom output path

Note that in all cases you need to disable package sandboxing to use a custom output path.

You can use the package plugin repo itself to test this.

Single target

Build documentation for one target with a custom output path

swift package \
  --disable-sandbox \
  generate-documentation \
  --target Snippets \
  --output-path MyOutputDir

After the build succeeds, the "MyOutputDir" directory should contain the content of the Snippets target's documentation.

Multiple targets

Build documentation for two (or more) targets with a custom output path

swift package \
  --disable-sandbox \
  generate-documentation \
  --target Snippets \
  --target SwiftDocCPlugin \
  --output-path MyOutputDir

After the build succeeds, the "MyOutputDir" directory should contain the two subdirectories named "Snippets.doccarchive" and "SwiftDocCPlugin.doccarchive" that each contain that target's documentation.

Combined targets

Build combined documentation for two (or more) targets with a custom output path

swift package \
  --disable-sandbox \
  generate-documentation \
  --target Snippets \
  --target SwiftDocCPlugin \
  --enable-experimental-combined-documentation \
  --output-path MyOutputDir

After the build succeeds, the "MyOutputDir" directory should contain the content of the combined documentation archive.

The plugin only prints the combined archives's output path. It doesn't print the individual target's output paths, even though they were built to produce the combined archive.

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

Added tests
Ran the ./bin/test script and it succeeded
~~[ ] Updated documentation if necessary~~ Not applicable.

d-ronnqvist · 2024-08-20T09:21:30Z

@swift-ci please test

daniel-grumberg · 2024-08-20T13:27:57Z

Plugins/SharedPackagePluginExtensions/PackageManager+getSymbolGraphsForDocC.swift

+/// Generating symbol graphs is the only task that can't run in parallel.
+///
+/// We serialize its execution to support build concurrency for the other tasks.
+private let symbolGraphGenerationLock = NSLock()


Why is that?

I'm not sure precisely why. I only know that if we don't serialize the calls to
PackageManager.getSymbolGraph(for:options:) then we consistently this this error, even with only two targets.

I'm suspecting that the plugin script runner doesn't expect another message before it has sent the first response, but I haven't had time to dig through that code yet.

yeah I doubt many package plugins implement parallel task running ;)

daniel-grumberg · 2024-08-20T13:33:18Z

Plugins/Swift-DocC Convert/SwiftDocCConvert.swift

@@ -42,8 +42,10 @@ import PackagePlugin
        let verbose = parsedArguments.pluginArguments.verbose
        let isCombinedDocumentationEnabled = parsedArguments.pluginArguments.enableCombinedDocumentation

+        let doccFeatures: DocCFeatures?


Suggested change

let doccFeatures: DocCFeatures?

let doccFeatures = try? DocCFeatures(doccExecutable: doccExecutableURL)

if isCombinedDocumentationEnabled && doccFeatures?.contains(.linkDependencies) == true {

// The developer uses the combined documentation plugin flag with a DocC version that doesn't support combined documentation.

Diagnostics.error("""

Unsupported use of '\(DocumentedFlag.enableCombinedDocumentation.names.preferred)'. \

DocC version at '\(doccExecutableURL.path)' doesn't support combined documentation.

""")

return

}

daniel-grumberg · 2024-08-20T13:34:33Z

Plugins/Swift-DocC Convert/SwiftDocCConvert.swift

        // An inner function that defines the work to build documentation for a given target.
        func performBuildTask(_ task: DocumentationBuildGraph<SwiftSourceModuleTarget>.Task) throws -> URL? {
            let target = task.target
-            print("Generating documentation for '\(target.name)'...")
+            print("Extracting symbol information for '\(target.name)'...")
+            let symbolGraphGenerationStartTime = DispatchTime.now()


Is there a precedent for printing timing info like this?

The existing code printed its timing info like that here. I suspect that it was trying to mimic the way that swift build prints timing:

Build of target: 'SwiftDocCPlugin' complete! (0.32s)

However, the existing code printed two "doing"-style messages ("Generating documentation for '(target.name)'..." and "Converting documentation...") but only measured the latter so I added timing for both of them so that the log would have a balanced number of "doing" and "done" messages.

daniel-grumberg · 2024-08-20T13:37:35Z

Plugins/Swift-DocC Convert/SwiftDocCConvert.swift

            .compactMap { $0 }
+            .sorted(by: { $0.lastPathComponent < $1.lastPathComponent })


Is there a particular reason this needs sorting?

Not much. I just wanted a stable sort order in the output of multiple individual targets here. However, I could move the sorting there to signal that the sorting is only for presentation purposes.

Can you move the sort to where it's used to make it clear it's not needed elsewhere?

daniel-grumberg · 2024-08-20T13:39:28Z

Sources/SwiftDocCPluginUtilities/ParsedArguments.swift

@@ -100,6 +109,7 @@ struct ParsedArguments {

        arguments.insertIfMissing(.option(DocCArguments.additionalSymbolGraphDirectory, value: symbolGraphDirectoryPath))

+        assert(arguments.extractOption(named: DocCArguments.outputPath).isEmpty,"The plugin arguments should have extracted the output path.")


I would prefer if that assertion was at the site where the option should be extracted to keep locality with the potential bug.

Also, print all archive output paths at the end instead of during build

…documentation

d-ronnqvist · 2024-08-22T14:39:20Z

@swift-ci please test

d-ronnqvist · 2024-08-22T15:37:36Z

@swift-ci please test

…ential bug.

… an archive output path

…nding pages for combined archives)

d-ronnqvist · 2024-08-23T07:38:33Z

@swift-ci please test

daniel-grumberg reviewed Aug 20, 2024

View reviewed changes

daniel-grumberg approved these changes Aug 20, 2024

View reviewed changes

d-ronnqvist added 6 commits August 22, 2024 16:38

Support using a custom --output-path when building a combined archive

7c3474c

Also, print all archive output paths at the end instead of during build

Customize the synthesized landing page when DocC supports it

f780582

Provide more information about the steps for each target's build task

7fe7b4b

Support --output-path with multiple targets, even without combined …

223d2d0

…documentation

Add integration tests for building combined documentation

8f002d5

Build documentation for different targets concurrently

29ab050

d-ronnqvist force-pushed the combined-output-path branch from 5a1607a to 67a795e Compare August 22, 2024 14:38

d-ronnqvist force-pushed the combined-output-path branch from 67a795e to 4bda3c1 Compare August 22, 2024 15:33

d-ronnqvist added 6 commits August 22, 2024 19:02

Code review feedback: move sorting archives to where it's used

fbd7407

Remove unused local variable

3f42144

Code review feedback: simplify creation of DocCFeatures

8c20998

Code review feedback: move assertion for better locality with the pot…

89a09f4

…ential bug.

Fix bug in test helper where other log output was sometimes parsed as…

0cb5275

… an archive output path

Update integration tests to reflect new DocC behavior (synthesized la…

ecb3aed

…nding pages for combined archives)

d-ronnqvist force-pushed the combined-output-path branch from 4bda3c1 to ecb3aed Compare August 23, 2024 07:38

d-ronnqvist merged commit a255bbd into swiftlang:main Aug 23, 2024
2 checks passed

d-ronnqvist deleted the combined-output-path branch August 23, 2024 07:53

d-ronnqvist mentioned this pull request Aug 23, 2024

swift-docc-plugin produces documentation for last target only #12

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support `--output-path` when building documentation for multiple targets. #89

Support `--output-path` when building documentation for multiple targets. #89

d-ronnqvist commented Aug 20, 2024 •

edited

Loading

d-ronnqvist commented Aug 20, 2024

daniel-grumberg Aug 20, 2024

d-ronnqvist Aug 20, 2024

daniel-grumberg Aug 20, 2024

daniel-grumberg Aug 20, 2024

daniel-grumberg Aug 20, 2024

d-ronnqvist Aug 20, 2024

daniel-grumberg Aug 20, 2024

d-ronnqvist Aug 20, 2024 •

edited

Loading

daniel-grumberg Aug 20, 2024

daniel-grumberg Aug 20, 2024

d-ronnqvist commented Aug 22, 2024

d-ronnqvist commented Aug 22, 2024

d-ronnqvist commented Aug 23, 2024

-        let doccFeatures: DocCFeatures?
+        let doccFeatures = try? DocCFeatures(doccExecutable: doccExecutableURL)
+        if isCombinedDocumentationEnabled && doccFeatures?.contains(.linkDependencies) == true {
+                        // The developer uses the combined documentation plugin flag with a DocC version that doesn't support combined documentation.
+                Diagnostics.error("""
+                Unsupported use of '\(DocumentedFlag.enableCombinedDocumentation.names.preferred)'. \
+                DocC version at '\(doccExecutableURL.path)' doesn't support combined documentation.
+                """)
+                return
+        }

		.compactMap { $0 }
		.sorted(by: { $0.lastPathComponent < $1.lastPathComponent })

		@@ -100,6 +109,7 @@ struct ParsedArguments {

		arguments.insertIfMissing(.option(DocCArguments.additionalSymbolGraphDirectory, value: symbolGraphDirectoryPath))

		assert(arguments.extractOption(named: DocCArguments.outputPath).isEmpty,"The plugin arguments should have extracted the output path.")

Support --output-path when building documentation for multiple targets. #89

Support --output-path when building documentation for multiple targets. #89

Conversation

d-ronnqvist commented Aug 20, 2024 • edited Loading

Summary

Dependencies

Testing

Single target

Multiple targets

Combined targets

Checklist

d-ronnqvist commented Aug 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d-ronnqvist Aug 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d-ronnqvist commented Aug 22, 2024

d-ronnqvist commented Aug 22, 2024

d-ronnqvist commented Aug 23, 2024

Support `--output-path` when building documentation for multiple targets. #89

Support `--output-path` when building documentation for multiple targets. #89

d-ronnqvist commented Aug 20, 2024 •

edited

Loading

d-ronnqvist Aug 20, 2024 •

edited

Loading