Skip to content

Update the DocC documentation script so that it works with the changes to the merge command #830

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.

Sign up for GitHub

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

Merged
merged 11 commits into from
Mar 15, 2024
Merged

Update the DocC documentation script so that it works with the changes to the merge command #830

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
623ba6a
Update script to remove output dir before merging documentation
d-ronnqvist Feb 16, 2024
0413b67
Ensure that the combined archive has a data directory
d-ronnqvist Feb 19, 2024
66e4229
Merge branch 'main' into merge-command-2
d-ronnqvist Feb 26, 2024
b08c28a
Merge branch 'main' into merge-command-2
d-ronnqvist Mar 4, 2024
60a271c
Add check that either all archives support static hosting or none do
d-ronnqvist Mar 4, 2024
a94358e
Support merging archives that don't have static hosting files
d-ronnqvist Mar 5, 2024
8c0c18d
Raise error in TestFileSystem if intermediate directories are missing
d-ronnqvist Mar 6, 2024
fe0620c
Fix tests that used TestFileSystem but created real temp directories
d-ronnqvist Mar 6, 2024
8fb03d2
Only copy static hosting files if input archives has them
d-ronnqvist Mar 6, 2024
a215ecc
Merge branch 'main' into merge-command-2
d-ronnqvist Mar 6, 2024
56378d8
Merge branch 'main' into merge-command-2
d-ronnqvist Mar 15, 2024
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
33 changes: 22 additions & 11 deletions Sources/SwiftDocCTestUtilities/TestFileSystem.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ public class TestFileSystem: FileManagerProtocol, DocumentationWorkspaceDataProv

// Default system paths
files["/"] = Self.folderFixtureData
files["/tmp"] = Self.folderFixtureData

// Import given folders
try updateDocumentationBundles(withFolders: folders)
Expand Down Expand Up @@ -121,7 +122,7 @@ public class TestFileSystem: FileManagerProtocol, DocumentationWorkspaceDataProv
defer { filesLock.unlock() }

guard let file = files[url.path] else {
throw CocoaError.error(.fileReadNoSuchFile)
throw makeFileNotFoundError(url)
}
return file
}
Expand Down Expand Up @@ -190,7 +191,9 @@ public class TestFileSystem: FileManagerProtocol, DocumentationWorkspaceDataProv

filesLock.lock()
defer { filesLock.unlock() }


try ensureParentDirectoryExists(for: dstURL)

let srcPath = srcURL.path
let dstPath = dstURL.path

Expand Down Expand Up @@ -222,13 +225,12 @@ public class TestFileSystem: FileManagerProtocol, DocumentationWorkspaceDataProv
filesLock.lock()
defer { filesLock.unlock() }

let parent = URL(fileURLWithPath: path).deletingLastPathComponent()
let url = URL(fileURLWithPath: path)
let parent = url.deletingLastPathComponent()
if parent.pathComponents.count > 1 {
// If it's not the root folder, check if parents exist
if createIntermediates == false {
guard files.keys.contains(parent.path) else {
throw CocoaError.error(.fileReadNoSuchFile)
}
try ensureParentDirectoryExists(for: url)
} else {
// Create missing parent directories
try createDirectory(atPath: parent.path, withIntermediateDirectories: true)
Expand Down Expand Up @@ -266,16 +268,14 @@ public class TestFileSystem: FileManagerProtocol, DocumentationWorkspaceDataProv
}
}

public func createFile(at: URL, contents: Data) throws {
public func createFile(at url: URL, contents: Data) throws {
filesLock.lock()
defer { filesLock.unlock() }

guard files.keys.contains(at.deletingLastPathComponent().path) else {
throw NSError(domain: NSCocoaErrorDomain, code: NSFileNoSuchFileError, userInfo: [NSFilePathErrorKey: at.path])
}
try ensureParentDirectoryExists(for: url)

if !disableWriting {
files[at.path] = contents
files[url.path] = contents
}
}

Expand Down Expand Up @@ -377,6 +377,17 @@ public class TestFileSystem: FileManagerProtocol, DocumentationWorkspaceDataProv
}
return allSubpaths
}

private func ensureParentDirectoryExists(for url: URL) throws {
let parentURL = url.deletingLastPathComponent()
guard directoryExists(atPath: parentURL.path) else {
throw makeFileNotFoundError(parentURL)
}
}

private func makeFileNotFoundError(_ url: URL) -> Error {
return CocoaError(.fileReadNoSuchFile, userInfo: [NSFilePathErrorKey: url.path])
}
}

private extension File {
Expand Down
58 changes: 51 additions & 7 deletions Sources/SwiftDocCUtilities/Action/Actions/MergeAction.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ struct MergeAction: Action {

try validateThatOutputIsEmpty()
try validateThatArchivesHaveDisjointData()
let supportsStaticHosting = try validateThatAllArchivesOrNoArchivesSupportStaticHosting()

let targetURL = try Self.createUniqueDirectory(inside: fileManager.uniqueTemporaryDirectory(), template: firstArchive, fileManager: fileManager)
defer {
Expand All @@ -40,18 +41,19 @@ struct MergeAction: Action {
}
var combinedJSONIndex = try JSONDecoder().decode(RenderIndex.self, from: jsonIndexData)

// Ensure that the destination has a data directory in case the first archive didn't have any pages.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we can move this createDirectory call outside of the loop.

When I run a test merge command on two archives, I get an error:

swift run docc merge /path/to/SlothCreator.doccarchive /path/to/SymbolKit.doccarchive --output-path=/path/to/output.doccarchive

Error: The file “symbolkit” doesn’t exist.

The problem is the call to copyItem for this file: /path/to/SymbolKit.doccarchive/documentation/symbolkit... trying to copy to /var/folders/some/temp/path/documentation/symbolkit. The previous version of this code called createDirectory inside the loop, insuring that each target directory, such as .../documentation/symbolkit, exists before we try to copy files to it. By moving the call to createDirectory up outside the loop and only calling it once for the data directory, we aren't guaranteed that each target directory will exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you tests that again or provide steps to reproduce this? I tried this on commit 66e4229 (the main merge from yesterday) with combinations of 1, 2, 3, and 4 archives with and without and explicit output path and I never encountered that error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same two archives (SlothCreator and SymbolKit) in the same order also works for me.

$ find output.doccarchive/data -maxdepth 2

output.doccarchive/data
output.doccarchive/data/documentation
output.doccarchive/data/documentation/symbolkit.json
output.doccarchive/data/documentation/symbolkit
output.doccarchive/data/documentation/slothcreator
output.doccarchive/data/documentation/slothcreator.json
output.doccarchive/data/tutorials
output.doccarchive/data/tutorials/slothcreator
output.doccarchive/data/tutorials/slothcreator.json

try? fileManager.createDirectory(at: targetURL.appendingPathComponent("data", isDirectory: true), withIntermediateDirectories: false, attributes: nil)

let directoriesToCopy = ["data/documentation", "data/tutorials", "images", "videos", "downloads"] + (supportsStaticHosting ? ["documentation", "tutorials"] : [])
for archive in archives.dropFirst() {
for directoryToCopy in ["data/documentation", "data/tutorials", "documentation", "tutorials", "images", "videos", "downloads"] {
for directoryToCopy in directoriesToCopy {
let fromDirectory = archive.appendingPathComponent(directoryToCopy, isDirectory: true)
let toDirectory = targetURL.appendingPathComponent(directoryToCopy, isDirectory: true)

var mkdir_p = false
// Ensure that the destination directory exist in case the first archive didn't have that kind of pages.
// This is necessary when merging a reference-only archive with a tutorial-only archive.
try? fileManager.createDirectory(at: toDirectory, withIntermediateDirectories: false, attributes: nil)
for from in (try? fileManager.contentsOfDirectory(at: fromDirectory, includingPropertiesForKeys: nil, options: .skipsHiddenFiles)) ?? [] {
// Create the full path to the destination directory if necessary, once for each directory to copy
if !mkdir_p {
try fileManager.createDirectory(at: toDirectory, withIntermediateDirectories: true, attributes: nil)
mkdir_p = true
}
// Copy each file or subdirectory
try fileManager.copyItem(at: from, to: toDirectory.appendingPathComponent(from.lastPathComponent))
}
Expand All @@ -75,6 +77,7 @@ struct MergeAction: Action {
return ActionResult(didEncounterError: false, outputs: [outputURL])
}

/// Validate that the different archives don't have overlapping data.
private func validateThatArchivesHaveDisjointData() throws {
// Check that the archives don't have overlapping data
typealias ArchivesByDirectoryName = [String: [String: Set<String>]]
Expand Down Expand Up @@ -131,6 +134,7 @@ struct MergeAction: Action {
}
}

/// Validate that the output directory is empty.
private func validateThatOutputIsEmpty() throws {
guard fileManager.directoryExists(atPath: outputURL.path) else {
return
Expand Down Expand Up @@ -162,4 +166,44 @@ struct MergeAction: Action {
throw NonEmptyOutputError(existingContents: existingContents, fileManager: fileManager)
}
}

/// Validate that either all archives support static hosting or that no archives support static hosting.
/// - Returns: `true` if all archives support static hosting; `false` otherwise.
private func validateThatAllArchivesOrNoArchivesSupportStaticHosting() throws -> Bool {
let nonEmptyArchives = archives.filter {
fileManager.directoryExists(atPath: $0.appendingPathComponent("data").path)
}

let archivesWithStaticHostingSupport = nonEmptyArchives.filter {
return fileManager.directoryExists(atPath: $0.appendingPathComponent("documentation").path)
|| fileManager.directoryExists(atPath: $0.appendingPathComponent("tutorials").path)
}

guard archivesWithStaticHostingSupport.count == nonEmptyArchives.count // All archives support static hosting
|| archivesWithStaticHostingSupport.count == 0 // No archives support static hosting
else {
struct DifferentStaticHostingSupportError: DescribedError {
var withSupport: Set<String>
var withoutSupport: Set<String>

var errorDescription: String {
"""
Different static hosting support in different archives.

\(withSupport.sorted().joined(separator: ", ")) support\(withSupport.count == 1 ? "s" : "") static hosting \
but \(withoutSupport.sorted().joined(separator: ", ")) do\(withoutSupport.count == 1 ? "es" : "")n't.
"""
}
}
let allArchiveNames = Set(nonEmptyArchives.map(\.lastPathComponent))
let archiveNamesWithStaticHostingSupport = Set(archivesWithStaticHostingSupport.map(\.lastPathComponent))

throw DifferentStaticHostingSupportError(
withSupport: archiveNamesWithStaticHostingSupport,
withoutSupport: allArchiveNames.subtracting(archiveNamesWithStaticHostingSupport)
)
}

return !archivesWithStaticHostingSupport.isEmpty
}
}
Loading