Skip to content

Extensions versioning#3270

Merged
voodoos merged 25 commits intoocaml:masterfrom
voodoos:extensions-versionning
Mar 24, 2020
Merged

Extensions versioning#3270
voodoos merged 25 commits intoocaml:masterfrom
voodoos:extensions-versionning

Conversation

@voodoos
Copy link
Copy Markdown
Collaborator

@voodoos voodoos commented Mar 16, 2020 •
edited
Loading

Presentation

This PR implements a new versioning system for syntax extensions.
When reading the dune-project file an error will be raised if an extension requires a minimal version of the dune language which is superior to the declared version of the dune language.

Fixes #2957

Changes

  • Extensions should now define explicitly the minimal dune lang version for each one of their minor versions. For example:
  Dune_lang.Syntax.create ~name:"fmt"
    ~desc:"integration with automatic formatters"
    [ ((1, 0), `Since (1, 4))
    ; ((1, 1), `Since (1, 7))
    ; ((1, 2), `Since (1, 11)) ]

  • When an extension is used without being explicitly declared the latest version supported under the actual dune lang version will be added automatically to the project file.

  • If a mismatch is found the minimal dune lang version needed is shown along with the versions of the extensions available under the current dune lang version.

  • To make it less a breaking change, only a warning is emitted in some cases:

Extension version Dune lang version Result
Exists in a greater dune lang >= 2.5 Error
Exists in a greater dune lang < 2.5 Warning
Does not exists at all >= 2.5 Error
Does not exists at all < 2.5 Error

Notes

  • Some fatal uses of the Syntax.since function are now useless.
  • The version of the dune lang itself is handled by the same mechanism as the extensions versions. It may be beneficial to better separate the two as their semantics have now diverged a little. For example the greatest supported version of the dune lang is always the greatest one while the greatest supported version of an extension depends on the active dune lang version.
  • It is a breaking change: misconfigured projects won't build anymore.

Todo

  • Make it a warning for older versions of dune lang and an error for >=2.5

@voodoos voodoos requested a review from a user March 16, 2020 16:53
@voodoos voodoos requested a review from rgrinberg as a code owner March 16, 2020 16:53
@voodoos voodoos force-pushed the extensions-versionning branch from f8e47f1 to 05add08 Compare March 16, 2020 17:01
@voodoos
Copy link
Copy Markdown
Collaborator Author

voodoos commented Mar 17, 2020 •
edited
Loading

I just realized with the CI crashing that this may be too much of a breaking change ?

For example the CI (which apparently use the PR's dune version to build the opam switch) fails because ppxlib uses Cinaps 1.0 with a dune-lang < 1.11.

I see three options:

  1. Make it a warning with a flag to turn it into an error
  2. Make it a warning for older versions of dune lang and an error for >=2.5
  3. ⚔️ Start a crusade to correct all the wrong dune projects of opam

And my choice would be option 2.

@ghost
Copy link
Copy Markdown

ghost commented Mar 17, 2020

Options 2 seems best to me as well.

One thing we could do to help though is scan the dune-universe for invalid dune-project files and ping the corresponding authors/projects.

ghost
ghost reviewed Mar 17, 2020
View reviewed changes
ghost
ghost reviewed Mar 17, 2020
View reviewed changes
ghost
ghost reviewed Mar 17, 2020
View reviewed changes
ghost
ghost reviewed Mar 17, 2020
View reviewed changes
ghost
ghost reviewed Mar 17, 2020
View reviewed changes
ghost
ghost suggested changes Mar 17, 2020
View reviewed changes
ghost
ghost reviewed Mar 17, 2020
View reviewed changes
rgrinberg
rgrinberg reviewed Mar 17, 2020
View reviewed changes
ghost
ghost approved these changes Mar 17, 2020
View reviewed changes
Copy link
Copy Markdown

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Looks good!

@voodoos
Copy link
Copy Markdown
Collaborator Author

voodoos commented Mar 17, 2020 •
edited
Loading

I implemented Option 2 for backward compatibility. The current policy is the following:

Extension version Dune lang version Result
Exists in a greater dune lang >= 2.5 Error
Exists in a greater dune lang < 2.5 Warning
Does not exists at all >= 2.5 Error
Does not exists at all < 2.5 Error
  • It is probably better to wait for the CI to finish at least once.
  • I will also add some tests for the different warning/error cases (but we won't be able to test the first case for now...).

voodoos and others added 15 commits March 17, 2020 16:58
@voodoos
Add tests
c07cbd8 
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@voodoos
Implement new versioning method
6e89fdb 
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@voodoos
Switch menhir to new system and fix the tests
260174e 
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@voodoos
Move cinaps to new versionning
5438b43 
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@voodoos
Move action_plugin to new versioning
055eae4 
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@voodoos
move fmmt to new versioning. Update now failing tests
6422b69 
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@voodoos
Move mdx to new versioning and correct tests
6097372 
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@voodoos
Move coq and bootstrap to new versioning
6d03bcf 
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@voodoos
Move library_variants to new versioning and correct the tests
e5f239e 
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@voodoos
Move building_ocaml_compiler to new versioning and correct test
2fa957f 
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@voodoos
Remove support for old versioning system
c8e5403 
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@jeremiedimino @voodoos
doc
287e790 
Signed-off-by: Jeremie Dimino <jeremie@dimino.org>
@voodoos
More explicit labeled arguments
2db337d 
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@voodoos
Clearer folding on versions in make
d34b43a 
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@voodoos
Better error messages
325decd 
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@ghost
Copy link
Copy Markdown

ghost commented Mar 17, 2020

You can bump the version of the dune language to 2.5 in this PR. This way we can test it now. We usually bump it as soon as required after a release.

voodoos and others added 3 commits March 17, 2020 16:59
@voodoos
Use Option.some_if
f628537 
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@jeremiedimino @voodoos
Add a comment in supperted_ranges
62fdc3c 
Signed-off-by: Jeremie Dimino <jeremie@dimino.org>
@voodoos
Warning if <=2.5
6c8fac5 
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@voodoos voodoos force-pushed the extensions-versionning branch from f1213ed to 6c8fac5 Compare March 17, 2020 16:02
@voodoos
Copy link
Copy Markdown
Collaborator Author

voodoos commented Mar 17, 2020 •
edited
Loading

You can bump the version of the dune language to 2.5 in this PR. This way we can test it now. We usually bump it as soon as required after a release.

However it is not sufficient to enable us to test the first case: for that we need an extension with a newer version introduced in 2.5 or later. Is there one already ?

voodoos added 2 commits March 17, 2020 17:23
@voodoos
Bump version number to 2.5
2189684 
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@voodoos
Add test and todo related to 2.5
c94b4d2 
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@voodoos voodoos requested review from emillon and nojb as code owners March 17, 2020 16:30
@ghost
Copy link
Copy Markdown

ghost commented Mar 17, 2020

There isn't indeed. Let's just rely on code review for the first case then.

BTW, reading this part of the code again, I just remembered that you can use the ~is_error argument of User_warning.emit for things that are sometimes warnings and sometimes errors.

voodoos added 2 commits March 17, 2020 17:52
@voodoos
Correct coq's tests
b27c28a 
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@voodoos
Make use of warnings is_error label
f93a0a2 
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@ejgallego ejgallego mentioned this pull request Mar 20, 2020
Merged
@ghost
Copy link
Copy Markdown

ghost commented Mar 24, 2020

@voodoos this PR looks ready to me, you should feel free to merge it.

voodoos and others added 3 commits March 24, 2020 16:05
@voodoos
Merge branch 'master' into extensions-versionning
5f9e8ae
@voodoos
Change CHANGES
d1ecb37 
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@voodoos
Merge branch 'extensions-versionning' of github.com:voodoos/dune into…
0508d3c 
… extensions-versionning

Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@voodoos voodoos merged commit 1825241 into ocaml:master Mar 24, 2020
stephanieyou pushed a commit to stephanieyou/dune that referenced this pull request Apr 7, 2020
@voodoos @jeremiedimino @stephanieyou
Extensions versioning (ocaml#3270)
fe092c6 
Improve and check extensions versioning.

Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>

Co-authored-by: Jeremie Dimino <jeremie@dimino.org>
@rgrinberg rgrinberg mentioned this pull request Apr 10, 2020
Merged
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Apr 10, 2020
@rgrinberg
[new release] dune-build-info, dune, dune-configurator, dune-action-p…
7424c3d 
…lugin, dune-private-libs and dune-glob (2.5.0)

CHANGES:

- Add a `--release` option meaning the same as `-p` but without the
  package filtering. This is useful for custom `dune` invocation in opam
  files where we don't want `-p` (ocaml/dune#3260, @diml)

- Fix a bug introduced in 2.4.0 causing `.bc` programs to be built
  with `-custom` by default (ocaml/dune#3269, fixes ocaml/dune#3262, @diml)

- Allow contexts to be defined with local switches in workspace files (ocaml/dune#3265,
  fix ocaml/dune#3264, @rgrinberg)

- Delay expansion errors until the rule is used to build something (ocaml/dune#3261, fix
  ocaml/dune#3252, @rgrinberg, @diml)

- [coq] Support for theory dependencies and compositional builds using
  new field `(theories ...)` (ocaml/dune#2053, @ejgallego, @rgrinberg)

- From now on, each version of a syntax extension must be explicitely tied to a
  minimum version of the dune language. Inconsistent versions in a
  `dune-project` will trigger a warning for version <=2.4 and an error for
  versions >2.4 of the dune language. (ocaml/dune#3270, fixes ocaml/dune#2957, @voodoos)

- [coq] Bump coq lang version to 0.2. New coq features presented this release
  require this version of the coq lang. (ocaml/dune#3283, @ejgallego)

- Prevent installation of public executables disabled using the `enabled_if` field.
  Installation will now simply skip such executables instead of raising an
  error. (ocaml/dune#3195, @voodoos)

- `dune upgrade` will now try to upgrade projects using versions <2.0 to version
  2.0 of the dune language. (ocaml/dune#3174, @voodoos)

- Add a `top` command to integrate dune with any toplevel, not just
  utop. It is meant to be used with the new `#use_output` directive of
  OCaml 4.11 (ocaml/dune#2952, @mbernat, @diml)

- Allow per-package `version` in generated `opam` files (ocaml/dune#3287, @toots)

- [coq] Introduce the `coq.extraction` stanza. It can be used to extract OCaml
  sources (ocaml/dune#3299, fixes ocaml/dune#2178, @rgrinberg)

- Load ppx rewriters in dune utop and add pps field to toplevel stanza. Ppx
  extensions will now be usable in the toplevel
  (ocaml/dune#3266, fixes ocaml/dune#346, @stephanieyou)

- Add a `(subdir ..)` stanza to allow evaluating stanzas in sub directories.
  (ocaml/dune#3268, @rgrinberg)

- Fix a bug preventing one from running inline tests in multiple modes
  (ocaml/dune#3352, @diml)

- Allow the use of the `%{profile}` variable in the `enabled_if` field of the
  library stanza. (ocaml/dune#3344, @mrmr1993)

- Allow the use of `%{ocaml_version}` variable in `enabled_if` field of the
  library stanza. (ocaml/dune#3339, @voodoos)

- Fix dune build freezing on MacOS when cache is enabled. (ocaml/dune#3249, fixes #ocaml/dune#2973,
  @artempyanykh)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rgrinberg rgrinberg rgrinberg approved these changes

@emillon emillon Awaiting requested review from emillon

@nojb nojb Awaiting requested review from nojb

+1 more reviewer
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Extensions should check for the lowest version of dune they are available in

3 participants

@voodoos @rgrinberg @jeremiedimino