fix for Issue/104 #106
Closed
fix for Issue/104 #106
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is non-breaking change that adds a new public method: read_files - a file-oriented entry point to the front end. This takes a list of target DSDL files allowing the user to maintain an explicit list instead of depending on globular filesystem discovery. Furthermore this method returns a set which is the transitive closure of types depended on by the target list of types. This allows consumers to track dependencies and compiler back ends to generate .d files and otherwise support incremental builds. The new method may increase performance for systems with large pools of messages when generating code for a small sub-set as it only considers the target and dependencies of the target when parsing dsdl files.
Co-authored-by: Pavel Kirienko <[email protected]>
Co-authored-by: Pavel Kirienko <[email protected]>
Co-authored-by: Pavel Kirienko <[email protected]>
This changes our logic for detecting duplicate definitions on case-sensitive file-systems to only operate on files of interest and to fix this detection logic per issue OpenCyphal#104. If globular searches find duplicates we do not waste time detecting this until/unless these duplicates are actually considered when parsing target definitions.
emrainey
reviewed
Jun 28, 2024
Comment on lines
+1187
to
+1196
| "greenland/colliding/IceBerg.1.0.dsdl", | ||
| dedent( | ||
| """ | ||
| @sealed | ||
| """ | ||
| ), | ||
| ) | ||
|
|
||
| wrkspc.new( | ||
| "greenland/COLLIDING/IceBerg.1.0.dsdl", |
There was a problem hiding this comment.
These two should collide as we can't trust capitalization of the File system?
There was a problem hiding this comment.
This goes to stability. The assertion is the outputs of pydsdl should be the same for a given set of inputs if run on a case-sensitive or case-insensitive filesystem. Because these are two different files on one and the same file on another we prevent it to avoid this ambiguity.
|
I assume this is superseded by #107; please reopen if I got it wrong |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes #104 and improves our duplicate detection logic to be more sustainable by only looking at files that are of interest to the target files.