Skip to content
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

Fix compilation in --dev mode with 4.08 #2296

Merged
1 commit merged into from Jun 19, 2019
Merged

Fix compilation in --dev mode with 4.08 #2296

1 commit merged into from Jun 19, 2019

Conversation

ghost
Copy link

@ghost ghost commented Jun 19, 2019

A few things that we use are deprecated in 4.08:

  • Pervasives
  • the tag functions of the Format module

Since we are currently keeping compatibility with OCaml >= 4.02, i simply disabled warning 3 for these. I also disabled the new warning 66 in dev mode, since it is causing a lot of code in Dune, and I expect in many projects to break. This warning is for unused open!, which we often use to keep open Stdune or similar even when they are not used.

BTW, testing 4.08 in the CI is going to be annoying as a lot of compiler messages changed in 4.08. I.e. the captured output will be different between < 4.08 and >= 4.08 :(

Signed-off-by: Jeremie Dimino <jeremie@dimino.org>
@ghost ghost requested review from emillon and rgrinberg as code owners June 19, 2019 09:29
src/scheme.mli Outdated
| Empty
(** [Empty] is a scheme that has no rules *)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler now complains if the comment is before the constructor, so I moved them all after

@ghost ghost requested review from ejgallego and fpottier as code owners June 19, 2019 09:34
@ghost ghost merged commit 0bc1464 into ocaml:master Jun 19, 2019
@ejgallego
Copy link
Collaborator

Sorry folks I hickjack this to mention that for a while dune has also been broken in OCaml master [we witness in Coq's CI]

Do you see value in adding a trunk job to Dune's CI? We could mark it allow_failure, so the biggest downside would be an increase on CI time.

I am almost done with other stuff so I shall shortly have time to see / debug the trunk problem more in-depth.

@rgrinberg
Copy link
Member

Yeah, it's fine to have it I think. It would be useful to know when dune starts failing on trunk as soon as possible.

@ejgallego
Copy link
Collaborator

Ok I will take care of #2298 next week if still open.

mlasson pushed a commit to mlasson/dune that referenced this pull request Jul 19, 2019
Signed-off-by: Jeremie Dimino <jeremie@dimino.org>
@bobot
Copy link
Collaborator

bobot commented Jul 29, 2019

BTW, testing 4.08 in the CI is going to be annoying as a lot of compiler messages changed in 4.08. I.e. the captured output will be different between < 4.08 and >= 4.08 :(

We should deactivate output of ocaml command printing for our tests. The message of error is never, I believe, useful.

@ghost
Copy link
Author

ghost commented Jul 30, 2019

Makes sense. I didn't know it was possible at the time.

@bobot
Copy link
Collaborator

bobot commented Jul 30, 2019

I was thinking something directly inside dune, to always hide the output of ocaml commands in our test. I don't know if ocaml has a toggle for the printing of the source.

@bobot
Copy link
Collaborator

bobot commented Jul 30, 2019

But, you are right, -error-style short exists. However I believe that the message could also change, so we should remove all the output.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants