-
Notifications
You must be signed in to change notification settings - Fork 143
Prevent a manual technology root from overriding the module node with the same name #770
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
Prevent a manual technology root from overriding the module node with the same name #770
Conversation
@d-ronnqvist could you complete the |
# Conflicts: # Sources/SwiftDocC/Infrastructure/Link Resolution/PathHierarchy+Find.swift
@swift-ci please test |
Added |
@@ -352,7 +352,7 @@ struct PathHierarchy { | |||
newNode.identifier = newReference | |||
self.lookup[newReference] = newNode | |||
|
|||
modules[name] = newNode | |||
modules.append(newNode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @d-ronnqvist. It seems that this is what fixes the bug, and the rest is refactoring as a consequence of changing modules from a dictionary to an array.
However, I'm confused about some other changes that don't appear to have any impact on resolving the bug. I'll add a comment to each of the things I believe are unnecessary in this PR, so you can explain me why these changes were made.
@@ -174,7 +174,7 @@ extension PathHierarchyBasedLinkResolver { | |||
} | |||
|
|||
return SerializableLinkResolutionInformation( | |||
version: .init(major: 0, minor: 0, patch: 1), // This is still in development | |||
version: .init(major: 0, minor: 1, patch: 0), // This is still in development |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How this is related to the crash?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a consequence of changing the modules from a dictionary to an array which also changed the file format of the serialized path hierarchy. Since that file format change isn't backwards compatible I bumped the version number.
@swift-ci please test |
Bug/issue #, if applicable: rdar://119389007
Summary
Fix a crash where if a developer added a technology root with the same file name as a module, the technology root would replace the module node and effectively drop all the symbols in the project.
Dependencies
None.
Testing
In any project with documentation for a module; add a manual
@TechnologyRoot
and name the file the same as the module name (with a ".md" extension).Build documentation for the project. DocC shouldn't crash but may still run into #593
Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/test
script and it succeeded