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

Extensions versioning #3270

Merged
merged 25 commits into from
Mar 24, 2020
Merged

Extensions versioning #3270

merged 25 commits into from
Mar 24, 2020

Conversation

voodoos
Copy link
Collaborator

@voodoos voodoos commented Mar 16, 2020

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
Collaborator Author

voodoos commented Mar 17, 2020

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

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.

src/dune_lang/syntax.mli Outdated Show resolved Hide resolved
src/dune_lang/syntax.ml Outdated Show resolved Hide resolved
src/dune_lang/syntax.ml Outdated Show resolved Hide resolved
src/dune_lang/syntax.ml Outdated Show resolved Hide resolved
src/dune_lang/syntax.ml Outdated Show resolved Hide resolved
src/dune_lang/syntax.ml Outdated Show resolved Hide resolved
Copy link

@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
Collaborator Author

voodoos commented Mar 17, 2020

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
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
Signed-off-by: Jeremie Dimino <jeremie@dimino.org>
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@ghost
Copy link

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
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
Signed-off-by: Jeremie Dimino <jeremie@dimino.org>
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
Collaborator Author

voodoos 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.

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
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
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

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
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
@ghost
Copy link

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
Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>
… 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
Improve and check extensions versioning.

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

Co-authored-by: Jeremie Dimino <jeremie@dimino.org>
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Apr 10, 2020
…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
Labels
None yet
Projects
None yet
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