Update the DocC documentation script so that it works with the changes to the merge command #830
Conversation
| @@ -40,18 +40,15 @@ 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. | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
|
@swift-ci please test |
| || fileManager.directoryExists(atPath: $0.appendingPathComponent("tutorials").path) | ||
| } | ||
|
|
||
| guard archivesWithStaticHostingSupport.count == nonEmptyArchives.count else { |
There was a problem hiding this comment.
This checks that all archives have static hosting support. But aren't you missing an OR clause to allow the scenario when none of the archives have static hosting support?
When I test merging two archives without static hosting support, I get:
Error: Different static hosting support in different archives.
support static hosting but SlothCreator.doccarchive, SymbolKit2.doccarchive don't.
There was a problem hiding this comment.
🤦 Fixed in a94358e
Also, ensure that temp directory always exists
|
@swift-ci please test |
|
@swift-ci please test |
Bug/issue #, if applicable: rdar://124270602
Summary
This small follow up PR to #821 updates the script that DocC uses to build and host its contributor documentation to work the the latest changes to the merge command and moves some directory creation code out of a loop.
Dependencies
None
Testing
Nothing in particular
Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/testscript and it succeeded[ ] Updated documentation if necessary