MPR#7710: ocamldep -sort: treat cyclic dependencies as an error#1578
Merged
xavierleroy merged 1 commit intoocaml:trunkfrom Feb 11, 2018
Merged
MPR#7710: ocamldep -sort: treat cyclic dependencies as an error#1578xavierleroy merged 1 commit intoocaml:trunkfrom
xavierleroy merged 1 commit intoocaml:trunkfrom
Conversation
Octachron
approved these changes
Jan 22, 2018
Member
Octachron
left a comment
There was a problem hiding this comment.
I have a small question, otherwise it is a clearly correct fix.
| if !worklist <> [] then begin | ||
| Format.fprintf Format.err_formatter | ||
| "@[Warning: cycle in dependencies. End of list is not sorted.@]@."; | ||
| "@[Error: cycle in dependencies. End of list is not sorted.@]@."; |
Member
There was a problem hiding this comment.
A very minor question: should this be @{<error>Error@} (with a Misc.Color.setup !Clflags.color before) to align the printing of this error with the ones printed by Location.report_exception (in case of parsing errors) ?
Contributor
Author
There was a problem hiding this comment.
You're probably right, but I'm not familiar with the error coloring code. I'll merge as it, but feel free to change the error message later if you wish. ( I'm not even sure a GPR is needed for that, as it would not change functionality.)
Before, cyclic dependencies were reported as a warning, and ocamldep -sort would exit with code 0. Now, the message says "error" and the exit code is nonzero.
EmileTrotignon
pushed a commit
to EmileTrotignon/ocaml
that referenced
this pull request
Jan 12, 2024
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Before, cyclic dependencies were reported as a warning, and ocamldep -sort would exit with code 0.
Now, the message says "error" and the exit code is nonzero.
The original behavior might have been intended as a "Hail Mary", i.e. "produce some ordering and with much luck it will pass linking", taking advantage of the fact that not all dependencies spotted by ocamldep are linking dependencies. I don't think this should be relied on.