Skip to content
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

Enable external link support by default #1118

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 32 additions & 34 deletions Sources/SwiftDocC/Infrastructure/ConvertActionConverter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ package enum ConvertActionConverter {

// Arrays to gather additional metadata if `emitDigest` is `true`.
var indexingRecords = [IndexingRecord]()
var linkSummaries = [LinkDestinationSummary]()
var assets = [RenderReferenceType : [RenderReference]]()
var coverageInfo = [CoverageDataEntry]()
let coverageFilterClosure = documentationCoverageOptions.generateFilterClosure()
Expand All @@ -104,6 +103,22 @@ package enum ConvertActionConverter {
let resultsSyncQueue = DispatchQueue(label: "Convert Serial Queue", qos: .unspecified, attributes: [])
let resultsGroup = DispatchGroup()

let linkHierarchySerializationProblems = Synchronized<[Problem]>([])
if FeatureFlags.current.isLinkHierarchySerializationEnabled {
resultsGroup.async(queue: resultsSyncQueue) {
signposter.withIntervalSignpost("Serialize link hierarchy", id: signposter.makeSignpostID()) {
do {
let serializableLinkInformation = try context.linkResolver.localResolver.prepareForSerialization(bundleID: bundle.id)
try outputConsumer.consume(linkResolutionInformation: serializableLinkInformation)
} catch {
linkHierarchySerializationProblems.sync {
recordProblem(from: error, in: &$0, withIdentifier: "link-resolver")
}
}
}
}
}

let renderSignpostHandle = signposter.beginInterval("Render", id: signposter.makeSignpostID(), "Render \(context.knownPages.count) pages")

var conversionProblems: [Problem] = context.knownPages.concurrentPerform { identifier, results in
Expand Down Expand Up @@ -142,16 +157,18 @@ package enum ConvertActionConverter {
let nodeLinkSummaries = entity.externallyLinkableElementSummaries(context: context, renderNode: renderNode, includeTaskGroups: true)
let nodeIndexingRecords = try renderNode.indexingRecords(onPage: identifier)

for linkSummary in nodeLinkSummaries {
try outputConsumer.consumeIncremental(linkableElementSummary: linkSummary)
}
resultsGroup.async(queue: resultsSyncQueue) {
assets.merge(renderNode.assetReferences, uniquingKeysWith: +)
linkSummaries.append(contentsOf: nodeLinkSummaries)
indexingRecords.append(contentsOf: nodeIndexingRecords)
}
} else if FeatureFlags.current.isExperimentalLinkHierarchySerializationEnabled {
} else if FeatureFlags.current.isLinkHierarchySerializationEnabled {
let nodeLinkSummaries = entity.externallyLinkableElementSummaries(context: context, renderNode: renderNode, includeTaskGroups: false)

resultsGroup.async(queue: resultsSyncQueue) {
linkSummaries.append(contentsOf: nodeLinkSummaries)
for linkSummary in nodeLinkSummaries {
try outputConsumer.consumeIncremental(linkableElementSummary: linkSummary)
}
}
} catch {
Expand All @@ -163,44 +180,25 @@ package enum ConvertActionConverter {
// Wait for any concurrent updates to complete.
resultsGroup.wait()

conversionProblems += linkHierarchySerializationProblems.sync { $0 }

signposter.endInterval("Render", renderSignpostHandle)

guard !Task.isCancelled else { return [] }

// Write various metadata
if emitDigest {
if emitDigest || FeatureFlags.current.isLinkHierarchySerializationEnabled {
signposter.withIntervalSignpost("Emit digest", id: signposter.makeSignpostID()) {
do {
try outputConsumer.consume(linkableElementSummaries: linkSummaries)
try outputConsumer.consume(indexingRecords: indexingRecords)
try outputConsumer.consume(assets: assets)
} catch {
recordProblem(from: error, in: &conversionProblems, withIdentifier: "metadata")
}
}
}

if FeatureFlags.current.isExperimentalLinkHierarchySerializationEnabled {
signposter.withIntervalSignpost("Serialize link hierarchy", id: signposter.makeSignpostID()) {
do {
let serializableLinkInformation = try context.linkResolver.localResolver.prepareForSerialization(bundleID: bundle.id)
try outputConsumer.consume(linkResolutionInformation: serializableLinkInformation)

if !emitDigest {
try outputConsumer.consume(linkableElementSummaries: linkSummaries)
try outputConsumer.finishedConsumingLinkElementSummaries()
if emitDigest {
// Only emit the other digest files if `--emit-digest` is passed
try outputConsumer.consume(indexingRecords: indexingRecords)
try outputConsumer.consume(assets: assets)
try outputConsumer.consume(problems: context.problems + conversionProblems)
}
} catch {
recordProblem(from: error, in: &conversionProblems, withIdentifier: "link-resolver")
}
}
}

if emitDigest {
signposter.withIntervalSignpost("Emit digest", id: signposter.makeSignpostID()) {
do {
try outputConsumer.consume(problems: context.problems + conversionProblems)
} catch {
recordProblem(from: error, in: &conversionProblems, withIdentifier: "problems")
recordProblem(from: error, in: &conversionProblems, withIdentifier: "metadata")
}
}
}
Expand Down
12 changes: 12 additions & 0 deletions Sources/SwiftDocC/Infrastructure/ConvertOutputConsumer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ public protocol ConvertOutputConsumer {
func consume(assetsInBundle bundle: DocumentationBundle) throws

/// Consumes the linkable element summaries produced during a conversion.
/// > Warning: This method might be called concurrently.
func consumeIncremental(linkableElementSummary: LinkDestinationSummary) throws
/// Consumes the linkable element summaries produced during a conversion.
func finishedConsumingLinkElementSummaries() throws

@available(*, deprecated, renamed: "consume(linkableElementSummary:)", message: "Use 'consume(linkableElementSummary:)' instead. This deprecated API will be removed after 6.2 is released")
func consume(linkableElementSummaries: [LinkDestinationSummary]) throws

/// Consumes the indexing records produced during a conversion.
Expand Down Expand Up @@ -58,3 +64,9 @@ public extension ConvertOutputConsumer {
func consume(buildMetadata: BuildMetadata) throws {}
func consume(linkResolutionInformation: SerializableLinkResolutionInformation) throws {}
}

// Default implementations to avoid a source breaking change from introducing new protocol requirements
public extension ConvertOutputConsumer {
func consume(linkableElementSummary: LinkDestinationSummary) throws {}
func finishedConsumingLinkElementSummaries() throws {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ extension PathHierarchy {
}
let topLevelNames = Set(modules.map(\.name) + (onlyFindSymbols ? [] : [articlesContainer.name, tutorialContainer.name]))

if isAbsolute, FeatureFlags.current.isExperimentalLinkHierarchySerializationEnabled {
if isAbsolute, FeatureFlags.current.isLinkHierarchySerializationEnabled {
throw Error.moduleNotFound(
pathPrefix: pathForError(of: rawPath, droppingLast: remaining.count),
remaining: Array(remaining),
Expand Down
10 changes: 8 additions & 2 deletions Sources/SwiftDocC/Utility/FeatureFlags.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,14 @@ public struct FeatureFlags: Codable {
/// Whether or not experimental support for device frames on images and video is enabled.
public var isExperimentalDeviceFrameSupportEnabled = false

/// Whether or not experimental support for emitting a serialized version of the local link resolution information is enabled.
public var isExperimentalLinkHierarchySerializationEnabled = false
/// Whether or not support for emitting a serialized version of the local link resolution information is enabled.
public var isLinkHierarchySerializationEnabled = true

@available(*, deprecated, renamed: "isLinkHierarchySerializationEnabled", message: "Use 'isLinkHierarchySerializationEnabled' instead. This deprecated API will be removed after 6.2 is released")
public var isExperimentalLinkHierarchySerializationEnabled: Bool {
get { isLinkHierarchySerializationEnabled }
set { isLinkHierarchySerializationEnabled = newValue }
}

/// Whether or not experimental support for combining overloaded symbol pages is enabled.
public var isExperimentalOverloadedSymbolPresentationEnabled = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,33 @@ struct ConvertFileWritingConsumer: ConvertOutputConsumer {
}
}

private var linkableElementsData = Synchronized(Data())

/// Consumes one linkable element summary produced during a conversion.
func consumeIncremental(linkableElementSummary: LinkDestinationSummary) throws {
let data = try encode(linkableElementSummary)
linkableElementsData.sync {
if !$0.isEmpty {
$0.append(Data(",".utf8))
}
$0.append(data)
}
}

/// Finishes consuming the linkable element summaries produced during a conversion.
func finishedConsumingLinkElementSummaries() throws {
let linkableElementsURL = targetFolder.appendingPathComponent(Self.linkableEntitiesFileName, isDirectory: false)
let data = linkableElementsData.sync { accumulatedData in
var data = Data()
swap(&data, &accumulatedData)
data.insert(UTF8.CodeUnit(ascii: "["), at: 0)
data.append(UTF8.CodeUnit(ascii: "]"))

return data
}
try fileManager.createFile(at: linkableElementsURL, contents: data)
}

func consume(linkableElementSummaries summaries: [LinkDestinationSummary]) throws {
let linkableElementsURL = targetFolder.appendingPathComponent(Self.linkableEntitiesFileName, isDirectory: false)
let data = try encode(summaries)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ extension ConvertAction {
let outOfProcessResolver: OutOfProcessReferenceResolver?

FeatureFlags.current.isExperimentalDeviceFrameSupportEnabled = convert.enableExperimentalDeviceFrameSupport
FeatureFlags.current.isExperimentalLinkHierarchySerializationEnabled = convert.enableExperimentalLinkHierarchySerialization
FeatureFlags.current.isLinkHierarchySerializationEnabled = convert.enableLinkHierarchySerialization
FeatureFlags.current.isExperimentalOverloadedSymbolPresentationEnabled = convert.enableExperimentalOverloadedSymbolPresentation
FeatureFlags.current.isExperimentalMentionedInEnabled = convert.enableExperimentalMentionedIn
FeatureFlags.current.isParametersAndReturnsValidationEnabled = convert.enableParametersAndReturnsValidation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -486,13 +486,19 @@ extension Docc {
var allowArbitraryCatalogDirectories = false

@Flag(
name: .customLong("enable-experimental-external-link-support"),
name: .customLong("external-link-support"),
inversion: .prefixedEnableDisable,
help: ArgumentHelp("Support external links to this documentation output.", discussion: """
Write additional link metadata files to the output directory to support resolving documentation links to the documentation in that output directory.
""")
)
var enableLinkHierarchySerialization = true

// This flag only exist to allow developers to pass the previous '--enable-experimental-...' flag without errors.
@Flag(name: .customLong("enable-experimental-external-link-support"), help: .hidden)
@available(*, deprecated, message: "This deprecated API will be removed after 6.2 is released")
var enableExperimentalLinkHierarchySerialization = false

@Flag(help: .hidden)
var experimentalModifyCatalogWithGeneratedCuration = false

Expand Down Expand Up @@ -536,6 +542,7 @@ extension Docc {
Convert.warnAboutDeprecatedOptionIfNeeded("enable-experimental-json-index", message: "This flag has no effect. The JSON render is emitted by default.")
Convert.warnAboutDeprecatedOptionIfNeeded("experimental-parse-doxygen-commands", message: "This flag has no effect. Doxygen support is enabled by default.")
Convert.warnAboutDeprecatedOptionIfNeeded("enable-experimental-parameters-and-returns-validation", message: "This flag has no effect. Parameter and return value validation is enabled by default.")
Convert.warnAboutDeprecatedOptionIfNeeded("enable-experimental-external-link-support", message: "This flag has no effect. External link support is enabled by default.")
Convert.warnAboutDeprecatedOptionIfNeeded("index", message: "Use '--emit-lmdb-index' indead.")
emitLMDBIndex = emitLMDBIndex
}
Expand Down Expand Up @@ -575,9 +582,15 @@ extension Docc {
}

/// A user-provided value that is true if the user enables experimental serialization of the local link resolution information.
public var enableLinkHierarchySerialization: Bool {
get { featureFlags.enableLinkHierarchySerialization }
set { featureFlags.enableLinkHierarchySerialization = newValue }
}

@available(*, deprecated, renamed: "enableLinkHierarchySerialization", message: "Use 'enableLinkHierarchySerialization' instead. This deprecated API will be removed after 6.2 is released")
public var enableExperimentalLinkHierarchySerialization: Bool {
get { featureFlags.enableExperimentalLinkHierarchySerialization }
set { featureFlags.enableExperimentalLinkHierarchySerialization = newValue }
get { enableLinkHierarchySerialization }
set { enableLinkHierarchySerialization = newValue }
}

/// A user-provided value that is true if the user wants to in-place modify the provided documentation catalog to write generated curation to documentation extension files.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ class DocumentationConverterTests: XCTestCase {
func consume(problems: [Problem]) throws { }
func consume(assetsInBundle bundle: DocumentationBundle) throws {}
func consume(linkableElementSummaries: [LinkDestinationSummary]) throws {}
func consumeIncremental(linkableElementSummary: LinkDestinationSummary) throws {}
func finishedConsumingLinkElementSummaries() throws {}
func consume(indexingRecords: [IndexingRecord]) throws {}
func consume(assets: [RenderReferenceType: [RenderReference]]) throws {}
func consume(benchmarks: Benchmark) throws {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5268,8 +5268,6 @@ let expected = """
}

func testResolveExternalLinkFromTechnologyRoot() throws {
enableFeatureFlag(\.isExperimentalLinkHierarchySerializationEnabled)

let externalModuleName = "ExternalModuleName"

func makeExternalDependencyFiles() throws -> (SerializableLinkResolutionInformation, [LinkDestinationSummary]) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@ import SwiftDocCTestUtilities

class ExternalPathHierarchyResolverTests: XCTestCase {

override func setUp() {
super.setUp()
enableFeatureFlag(\.isExperimentalLinkHierarchySerializationEnabled)
}

// These tests resolve absolute symbol links in both a local and external context to verify that external links work the same local links.

func testUnambiguousAbsolutePaths() throws {
Expand Down
4 changes: 0 additions & 4 deletions Tests/SwiftDocCTests/Infrastructure/PathHierarchyTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,6 @@ class PathHierarchyTests: XCTestCase {
}

func testAmbiguousPaths() throws {
enableFeatureFlag(\.isExperimentalLinkHierarchySerializationEnabled)

let (_, context) = try testBundleAndContext(named: "MixedLanguageFrameworkWithLanguageRefinements")
let tree = context.linkResolver.localResolver.pathHierarchy

Expand Down Expand Up @@ -3552,8 +3550,6 @@ class PathHierarchyTests: XCTestCase {
}

func testResolveExternalLinkFromTechnologyRoot() throws {
enableFeatureFlag(\.isExperimentalLinkHierarchySerializationEnabled)

let catalog = Folder(name: "unit-test.docc", content: [
TextFile(name: "Root.md", utf8Content: """
# Some root page
Expand Down
2 changes: 2 additions & 0 deletions Tests/SwiftDocCTests/TestRenderNodeOutputConsumer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ class TestRenderNodeOutputConsumer: ConvertOutputConsumer {
func consume(problems: [Problem]) throws { }
func consume(assetsInBundle bundle: DocumentationBundle) throws { }
func consume(linkableElementSummaries: [LinkDestinationSummary]) throws { }
func consumeIncremental(linkableElementSummary: LinkDestinationSummary) throws { }
func finishedConsumingLinkElementSummaries() throws { }
func consume(indexingRecords: [IndexingRecord]) throws { }
func consume(assets: [RenderReferenceType: [RenderReference]]) throws { }
func consume(benchmarks: Benchmark) throws { }
Expand Down
Loading