ocamltest: do not compare binaries produced by ocamlopt.opt and ocamlopt.byte#9677
Merged
nojb merged 4 commits intoocaml:trunkfrom Jun 23, 2020
Merged
Conversation
d7a3e96 to
d76571a
Compare
d76571a to
005b784
Compare
Contributor
Author
|
cc @shindere |
gasche
approved these changes
Jun 23, 2020
Member
gasche
left a comment
There was a problem hiding this comment.
I believe the implementation is correct, and it simplifies the source. I will wait for a couple day and merge if there are no more comments.
Contributor
|
Gabriel Scherer (2020/06/23 05:48 -0700):
@gasche approved this pull request.
Please give me time to review, I'll comment to let know when done.
|
shindere
approved these changes
Jun 23, 2020
Contributor
shindere
left a comment
There was a problem hiding this comment.
LGTM, thanks and sorry about the delay!
Contributor
Author
Thanks! Do you think this needs a Changes entry? |
Contributor
|
Nicolás Ojeda Bär (2020/06/23 07:51 -0700):
> LGTM, thanks and sorry about the delay!
Thanks! Do you think this needs a Changes entry?
I kinda wondered and thought we don't need one but no strong opinion on
it.
|
Contributor
Author
|
Thanks, merged! |
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
3 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.
The mismatch of binaries produced by
ocamlopt.byteandocamlopt.opthas been a source of failures inocamltestfor a while. These failures are not so easy to investigate and take up valuable developer time. Moreover, the consensus is that it is not really a bug if these binaries differ, as there are multiple reasons for this, including sharing of data structures (see #8853) and dependency on the system toolchain (see #9469).As suggested by @xavierleroy, this PR disables the comparison of the binaries generated by
ocamlopt.byteandocamlopt.optin all cases.I renamed the
ocamltestactioncompare-native-programsintocompare-binary-filesinstead of removing it altogether, as it is still used by one test (reproducibility/cmis_on_file_system.ml).