Port to dune 2.0 by emillon · Pull Request #1196 · ocaml-ppx/ocamlformat

emillon · 2019-12-18T15:34:42Z

This ports to (lang dune 2.0) and uses all niceties from the new dune lang.

(with-accepted-exit-codes) & (with-stdin-from) (these replace all our uses of (system))
long-form target inference thanks to @NathanReb (this means that we can use file names directly without having to declare (targets) and refer to them as %{targets}

Note that this bumps the minimum dune version to 2.0.0. This shouldn't be too much of a problem since very few packages have a < 2.0.0 constraint on opam-repository (and almost all of them have a fixed version - the compatibility is pretty good).

Let me know what you think.

jberdine · 2019-12-18T16:26:49Z

I would suggest to test carefully whether the behavior described in ocaml/dune#2958 affects tests, etc. here.

gpetiot · 2019-12-18T16:27:40Z

I think it's worth adding a line in the changelog that the required dune version is now >= 2.0.0, otherwise the changes in dune files look good.

Julow

Nice. I agree this should be in the changelog.
Compatibility is not necessarily a concern as upgrading OCamlformat itself is involved and I don't expect users to upgrade it as soon as possible.

emillon · 2019-12-19T11:28:31Z

I agree with the changelog. We can wait a bit until the dune 2 issues are identified.

XVilka · 2020-01-16T05:11:19Z

There might be a problem with Dune >= 2.0 migration - some popular packages still depend on the jbuilder (jbuilder.transition) that is incompatible with the newer dune, so you wouldn't be able to install the ocamlformat in the opam switches with these packages installed:

[new release] atdgen-codec-runtime, atd, atdgen-runtime, atds, atdj and atdgen (2.1.0) ocaml/opam-repository#15444
Make a new release (due to jbuilder obsolescence) djs55/ocaml-sha#43
probably some more, these I just met myself with opam list --installed --depends jbuilder.transition

P.S. You might want to rebase your PR.

jberdine · 2020-01-16T10:50:14Z

Since jbuilder and dune 2 can be co-installed, aren't those packages that depend on jbuilder.transition instead of jbuilder just broken?

XVilka · 2020-01-16T11:00:24Z

@jberdine I am not sure, opam doesn't allow me to upgrade those precisely because of that dependency on jbuilder:

Everything as up-to-date as possible (run with --verbose to show unavailable upgrades).

The following packages are not being upgraded because the new versions conflict with other installed packages:
  - dune.2.1.2
    ∗ jbuilder.transition is installed and requires dune < 2.0
  - dune-configurator.2.1.2

emillon · 2020-01-16T13:55:42Z

You can still fix these packages, or use an older version of ocamlformat.

The current situation with dependencies is not ideal: since at the moment you have to build ocamlformat in the same switch as your sources, it also induces a constraint on base and other ocamlformat dependencies. But that's not strictly required: a binary is just a binary, it does not depend on dune or base at runtime. It will be a nicer solution if we can fix that at the opam level.

jberdine · 2020-01-16T14:38:34Z

at the moment you have to build ocamlformat in the same switch as your sources

There must be some context I'm missing. I almost never build ocamlformat from the same switch as used for the code it formats.

emillon · 2020-01-16T15:11:05Z

I mean if you rely on opam to install ocamlformat as test-dep or something like that. I agree with you that it's best to do it in a separate switch (that's effectively what ocaml-ci is doing), but there's no way to tell opam to do that for now.

emillon · 2020-04-15T12:06:24Z

I just rebased this; not sure what's going on with the ocp-indent tests. (I tried to force >= 1.8.1 in case of a different version but that did not fix the issue)

Julow

There seems to be a problem with dune build @fmt. It seems that test/.ocamlformat_ignore wasn't useful and formatting of tests were disabled by something else. Could it be that test/passing/dune-project was overriding (using fmt) (by not specifying it) from ./dune-project ?

The reason test/.ocamlformat_ignore is not working is that it should be called test/.ocamlformat-ignore.

test/passing/gen/gen.ml

emillon · 2020-04-20T09:24:12Z

Formatting rules were not set up by default as the project was a 1.x. In a 2.x project it's necessary to disable them using (formatting disabled). I fixed this part.

emillon · 2020-04-20T12:11:39Z

and this is working. nice catch, thanks!

gpetiot

Looks good, thanks!

Julow

After that, I think we can merge !

dune

emillon requested review from Julow and gpetiot December 18, 2019 15:34

facebook-github-bot added the CLA Signed label Dec 18, 2019

Julow approved these changes Dec 19, 2019

View reviewed changes

emillon mentioned this pull request Jan 8, 2020

dune upgrade for lang 2.0 ocaml/dune#3011

Closed

emillon force-pushed the dune2 branch 3 times, most recently from f877114 to c1b175c Compare April 15, 2020 12:05

Julow reviewed Apr 15, 2020

View reviewed changes

test/passing/gen/gen.ml Outdated Show resolved Hide resolved

emillon force-pushed the dune2 branch from c1b175c to 0fcc5ca Compare April 20, 2020 09:23

emillon force-pushed the dune2 branch 2 times, most recently from 55bf97b to e8abe72 Compare April 20, 2020 12:07

gpetiot approved these changes Apr 20, 2020

View reviewed changes

Julow reviewed Apr 20, 2020

View reviewed changes

dune Outdated Show resolved Hide resolved

emillon mentioned this pull request Apr 24, 2020

Fix dune 2.5 warning relative to menhir 2.1 #1353

Merged

emillon force-pushed the dune2 branch from e8abe72 to 4b14658 Compare April 24, 2020 12:00

emillon force-pushed the dune2 branch 2 times, most recently from 34ed060 to b6d2192 Compare May 12, 2020 08:50

emillon added 5 commits May 13, 2020 12:11

Use (lang dune 2.2)

8a0dd92

Use with-accepted-exit-codes

b07373a

read lines in generator

8f57fa9

use with-stdin-from

efc36e7

Use long-form target inference

821c23f

emillon added the no changelog set this to bypass the CI check for changelog entries label May 13, 2020

emillon force-pushed the dune2 branch from b6d2192 to 821c23f Compare May 13, 2020 10:12

emillon merged commit 693fcd4 into ocaml-ppx:master May 13, 2020

emillon deleted the dune2 branch May 13, 2020 10:14

gpetiot mentioned this pull request May 13, 2020

Restore previous functionality for pre-post extension points #1342

Merged

Conversation

emillon commented Dec 18, 2019

Uh oh!

jberdine commented Dec 18, 2019 via email

Uh oh!

gpetiot commented Dec 18, 2019

Uh oh!

Julow left a comment

Choose a reason for hiding this comment

Uh oh!

emillon commented Dec 19, 2019

Uh oh!

XVilka commented Jan 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jberdine commented Jan 16, 2020

Uh oh!

XVilka commented Jan 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

emillon commented Jan 16, 2020

Uh oh!

jberdine commented Jan 16, 2020 via email

Uh oh!

emillon commented Jan 16, 2020

Uh oh!

emillon commented Apr 15, 2020

Uh oh!

Julow left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

emillon commented Apr 20, 2020

Uh oh!

emillon commented Apr 20, 2020

Uh oh!

gpetiot left a comment

Choose a reason for hiding this comment

Uh oh!

Julow left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

XVilka commented Jan 16, 2020 •

edited

Loading

XVilka commented Jan 16, 2020 •

edited

Loading