melange: set min version to 3.8 #7665

jchavarri · 2023-05-02T12:53:08Z

Discussed with @anmonteiro offline. It makes sense to have 3.8 be the first version that supports Melange. This PR raises the lower limit to match that version.

This change makes some of the logic to handle lang versions in library modes OSL to lose relevance, see promote-with-lib test.

Signed-off-by: Javier Chávarri <[email protected]>

jchavarri · 2023-05-02T12:55:40Z

test/blackbox-tests/test-cases/melange/promote-with-lib.t

-  the dune language. Please update your dune-project file to have (lang dune
-  3.8).
+              ^^^^^^^^^
+  Error: Unknown value :standard


@anmonteiro I am not sure how this was working before. But after the change to lift min supported version for Melange to 3.8, the logic to handle versioning for modes OSL never applies. Maybe we could simplify this code, so that we apply old decoder in old versions, and new decoder in new ones? Without trying to fallback, show error etc:

dune/src/dune_rules/dune_file.ml

Lines 540 to 555 in 637eb6e

(* Old behavior: if old parser succeeds, return that. Otherwise, if

parsing the ordered set language succeeds, ask the user to upgrade to

a supported version. Otherwise, fail with the first error. *)

try_

(Mode_conf.Lib.Set.decode >>| fun modes -> `Modes modes)

(fun exn ->

try_

( Mode_conf.Lib.Set.decode_osl ~stanza_loc project >>| fun modes ->

if dune_version >= expected_version then `Modes modes

else `Upgrade )

(fun _ -> raise exn))

>>| function

| `Modes modes -> modes

| `Upgrade ->

Syntax.Error.since stanza_loc Stanza.syntax expected_version

~what:"Ordered set language for modes"

Suggested simplification in 618c8ac.

How does the logic never apply? modes is allowed to be set with ordered set lang even without melange.

Signed-off-by: Javier Chávarri <[email protected]>

emillon

just to confirm, this is to be included in 3.8.0?

emillon · 2023-05-02T13:22:00Z

src/dune_rules/melange/melange_stanzas.ml

@@ -146,7 +146,7 @@ end
 let syntax =
  Dune_lang.Syntax.create ~name:Dune_project.Melange_syntax.name
    ~desc:"support for Melange compiler"
-    [ ((0, 1), `Since (3, 7)) ]
+    [ ((0, 1), `Since (3, 8)) ]


This will cause a slightly confusing message for users of melange upgrading to 3.8.0, but that's probably OK given the status of the support. (just a heads-up)

Signed-off-by: Javier Chávarri <[email protected]>

jchavarri · 2023-05-02T13:31:26Z

just to confirm, this is to be included in 3.8.0?

@emillon Yes, please.

rgrinberg

2nd commit is suspicious

jchavarri · 2023-05-03T07:21:26Z

2nd commit is suspicious

@rgrinberg It's a bit convoluted, but I think I can explain.

Originally, the code to parse modes would check the dune lang version. If it was older than 3.8, it would do the following:

dune/src/dune_rules/dune_file.ml

Lines 540 to 542 in 6954107

    
                   (* Old behavior: if old parser succeeds, return that. Otherwise, if 
        
                      parsing the ordered set language succeeds, ask the user to upgrade to 
        
                      a supported version. Otherwise, fail with the first error. *)

Note the

Otherwise, fail with the first error.

This part is important.

Now, we lifted the minimum version for melange to 3.8. The test in promote-with-lib starts with lang 3.7 and (modes :standard melange), so it will try with old parser, which fails because :standard is not supported in that version. Then it will try with new parser, and it will fail with "Version 0.1 of support for Melange compiler is not supported until version 3.8 of the dune language". Then it will show the error from the old parser "Unknown value :standard".

I don't see how we can keep the workarounds for modes OSL, they made more sense when Melange and modes OSL were introduced in different versions, but that's not the case after this PR. Do you have any ideas on how to improve this?

Signed-off-by: Javier Chávarri <[email protected]>

rgrinberg · 2023-05-03T17:19:54Z

Yes, but this feature is available without melange. E.g. what if someone writes (:standard \ native)

old

Signed-off-by: Javier Chávarri <[email protected]>

jchavarri · 2023-05-03T17:33:38Z

Yes, but this feature is available without melange. E.g. what if someone writes (:standard \ native)

True. I re-added the Modes.decode special handling, and introduced a new test outside the Melange tests to exercise the case you mentioned.

Alizter

LGTM now that it is just a version bump

rgrinberg · 2023-05-03T18:09:27Z

I think this needs a CHANGES entry

Signed-off-by: Javier Chávarri <[email protected]>

jchavarri · 2023-05-03T18:32:39Z

I think this needs a CHANGES entry

Done.

rgrinberg · 2023-05-03T20:04:45Z

And now it needs to be rebased

jchavarri · 2023-05-03T21:15:15Z

@rgrinberg I already merged main two hours ago. The latest commit from that branch (f446181) is already in this PR.

@Alizter

CHANGES: - merlin: ignore instrumentation settings for preprocessing. (ocaml/dune#7606, fixes ocaml/dune#7465, @Alizter) - Add a `coqdoc_flags` field to the `coq.theory` stanza allowing the user to pass extra arguments to `coqdoc`. (ocaml/dune#7676, fixes ocaml/dune#7954 @Alizter) - Coq language versions less 0.8 are deprecated, and will be removed in an upcoming Dune version. All users are required to migrate to `(coq lang 0.8)` which provides the right semantics for theories that have been globally installed, such as those coming from opam (@ejgallego, @Alizter) - Bump minimum version of the dune language for the melange syntax extension from 3.7 to 3.8 (ocaml/dune#7665, @jchavarri)

@rleshchinskiy

CHANGES: - Fix string quoting in the json file written by `--trace-file` (ocaml/dune#7773, @rleshchinskiy) - Read `pkg-config` arguments from the `PKG_CONFIG_ARGN` environment variable (ocaml/dune#1492, ocaml/dune#7734, @anmonteiro) - Correctly set `MANPATH` in `dune exec`. Previously, we would use the `bin/` directory of the context. (ocaml/dune#7655, @rgrinberg) - Allow overriding the `ocaml` binary with findlib configuration (ocaml/dune#7648, @rgrinberg) - merlin: ignore instrumentation settings for preprocessing. (ocaml/dune#7606, fixes ocaml/dune#7465, @Alizter) - When a rule's action is interrupted, delete any leftover directory targets. This is consistent with how we treat file targets. (ocaml/dune#7564, @rgrinberg) - Fix plugin loading with findlib. The functionality was broken in 3.7.0. (ocaml/dune#7556, @anmonteiro) - Introduce a `public_headers` field on libraries. This field is like `install_c_headers`, but it allows to choose the extension and choose the paths for the installed headers. (ocaml/dune#7512, @rgrinberg) - Load the host context `findlib.conf` when cross-compiling (ocaml/dune#7428, fixes ocaml/dune#1701, @rgrinberg, @anmonteiro) - Add a `coqdoc_flags` field to the `coq.theory` stanza allowing the user to pass extra arguments to `coqdoc`. (ocaml/dune#7676, fixes ocaml/dune#7954 @Alizter) - Resolve `ppx_runtime_libraries` in the target context when cross compiling (ocaml/dune#7450, fixes ocaml/dune#2794, @anmonteiro) - Use `$PKG_CONFIG`, when set, to find the `pkg-config` binary (ocaml/dune#7469, fixes ocaml/dune#2572, @anmonteiro) - Modules that were declared in `(modules_without_implementation)`, `(private_modules)` or `(virtual_modules)` but not declared in `(modules)` will cause Dune to emit a warning which will become an error in 3.9. (ocaml/dune#7608, fixes ocaml/dune#7026, @Alizter) - Preliminary support for Coq compiled intefaces (`.vos` files) enabled via `(mode vos)` in `coq.theory` stanzas. This can be used in combination with `dune coq top` to obtain fast re-building of dependencies (with no checking of proofs) prior to stepping into a file. (ocaml/dune#7406, @rlepigre) - Fix dune crashing on MacOS in watch mode whenever `$PATH` contains `$PWD` (ocaml/dune#7441, fixes ocaml/dune#6907, @rgrinberg) - Fix `dune install` when cross compiling (ocaml/dune#7410, fixes ocaml/dune#6191, @anmonteiro, @rizo) - Find `pps` dependencies in the host context when cross-compiling, (ocaml/dune#7410, fixes ocaml/dune#4156, @anmonteiro) - Dune in watch mode no longer builds concurrent rules in serial (ocaml/dune#7395 @rgrinberg, @jchavarri) - Dune can now detect Coq theories from outside the workspace. This allows for composition with installed theories (not necessarily installed with Dune). (ocaml/dune#7047, @Alizter, @ejgallego) - `dune coq top` now correctly respects the project root when called from a subdirectory. However, absolute filenames passed to `dune coq top` are no longer supported (due to being buggy) (ocaml/dune#7357, fixes ocaml/dune#7344, @rlepigre and @Alizter) - Added a `--no-build` option to `dune coq top` for avoiding rebuilds (ocaml/dune#7380, fixes ocaml/dune#7355, @Alizter) - RPC: Ignore SIGPIPE when clients suddenly disconnect (ocaml/dune#7299, ocaml/dune#7319, fixes ocaml/dune#6879, @rgrinberg) - Always clean up the UI on exit. (ocaml/dune#7271, fixes ocaml/dune#7142 @rgrinberg) - Bootstrap: remove reliance on shell. Previously, we'd use the shell to get the number of processors. (ocaml/dune#7274, @rgrinberg) - Bootstrap: correctly detect the number of processors by allowing `nproc` to be looked up in `$PATH` (ocaml/dune#7272, @Alizter) - Speed up file copying on macos by using `clonefile` when available (@rgrinberg, ocaml/dune#7210) - Adds support for loading plugins in toplevels (ocaml/dune#6082, fixes ocaml/dune#6081, @ivg, @richardlford) - Support commands that output 8-bit and 24-bit colors in the terminal (ocaml/dune#7188, @Alizter) - Speed up rule generation for libraries and executables with many modules (ocaml/dune#7187, @jchavarri) - Add `--watch-exclusions` to Dune build options (ocaml/dune#7216, @jonahbeckford) - Do not re-render UI on every frame if the UI doesn't change (ocaml/dune#7186, fix ocaml/dune#7184, @rgrinberg) - Make coq_db creation in scope lazy (@ejgallego, ocaml/dune#7133) - Non-user proccesses such as version control or config checking are now run silently. (ocaml/dune#6994, fixes ocaml/dune#4066, @Alizter) - Add the `--display-separate-messages` flag to separate the error messages produced by commands with a blank line. (ocaml/dune#6823, fixes ocaml/dune#6158, @esope) - Accept the Ordered Set Language for the `modes` field in `library` stanzas (ocaml/dune#6611, @anmonteiro). - dune install now respects --display quiet mode (ocaml/dune#7116, fixes ocaml/dune#4573, fixes ocaml/dune#7106, @Alizter) - Stub shared libraries (dllXXX_stubs.so) in Dune-installed libraries could not be used as dependencies of libraries in the workspace (eg when compiling to bytecode and/or Javascript). This is now fixed. (ocaml/dune#7151, @nojb) - Allow the main module of a library with `(stdlib ...)` to depend on other libraries (ocaml/dune#7154, @anmonteiro). - Bytecode executables built for JSOO are linked with `-noautolink` and no longer depend on the shared stubs of their dependent libraries (ocaml/dune#7156, @nojb) - Added a new user action `(concurrent )` which is like `(progn )` but runs the actions concurrently. (ocaml/dune#6933, @Alizter) - Allow `(stdlib ...)` to be used with `(wrapped false)` in library stanzas (ocaml/dune#7139, @anmonteiro). - Allow parallel execution of inline tests partitions (ocaml/dune#7012, @hhugo) - Support `(link_flags ...)` in `(cinaps ...)` stanza. (ocaml/dune#7423, fixes ocaml/dune#7416, @nojb) - Allow `(package ...)` in any position within `(rule ...)` stanza (ocaml/dune#7445, @Leonidas-from-XIV) - Always include `opam` files in the generated `.install` file. Previously, it would not be included whenever `(generate_opam_files true)` was set and the `.install` file wasn't yet generated. (ocaml/dune#7547, @rgrinberg) - Fix regression where Merlin was unable to handle filenames with uppercase letters under Windows. (ocaml/dune#7577, @nojb) - On nix+macos, pass `-f` to the codesign hook to avoid errors when the binary is already signed (ocaml/dune#7183, fixes ocaml/dune#6265, @greedy) - Fix bug where RPC clients built with dune-rpc-lwt would crash when closing their connection to the server (ocaml/dune#7581, @gridbugs) - Introduce mdx stanza 0.4 requiring mdx >= 2.3.0 which updates the default list of files to include `*.mld` files (ocaml/dune#7582, @Leonidas-from-XIV) - Fix RPC server on Windows (used for OCaml-LSP). (ocaml/dune#7666, @nojb) - Coq language versions less 0.8 are deprecated, and will be removed in an upcoming Dune version. All users are required to migrate to `(coq lang 0.8)` which provides the right semantics for theories that have been globally installed, such as those coming from opam (@ejgallego, @Alizter) - Bump minimum version of the dune language for the melange syntax extension from 3.7 to 3.8 (ocaml/dune#7665, @jchavarri)

melange: set min version to 3.8

637eb6e

Signed-off-by: Javier Chávarri <[email protected]>

jchavarri self-assigned this May 2, 2023

jchavarri requested review from rgrinberg and anmonteiro May 2, 2023 12:53

jchavarri added the melange label May 2, 2023

jchavarri commented May 2, 2023

View reviewed changes

melange: simplify modes handling across lang versions

618c8ac

Signed-off-by: Javier Chávarri <[email protected]>

Alizter previously approved these changes May 2, 2023

View reviewed changes

emillon reviewed May 2, 2023

View reviewed changes

fmt

fafe424

Signed-off-by: Javier Chávarri <[email protected]>

emillon added this to the 3.8.0 milestone May 2, 2023

rgrinberg requested changes May 3, 2023

View reviewed changes

jchavarri requested a review from rgrinberg May 3, 2023 07:21

Merge branch 'main' into melange-min-version-3-8

2b3bd63

Signed-off-by: Javier Chávarri <[email protected]>

test: add new lib-modes test

d2e278c

Signed-off-by: Javier Chávarri <[email protected]>

Alizter approved these changes May 3, 2023

View reviewed changes

jchavarri added 2 commits May 3, 2023 18:31

Merge branch 'main' into melange-min-version-3-8

69acf9a

Signed-off-by: Javier Chávarri <[email protected]>

melange: add changes entry

9718254

Signed-off-by: Javier Chávarri <[email protected]>

jchavarri force-pushed the melange-min-version-3-8 branch from e6725cf to 9718254 Compare May 3, 2023 18:32

rgrinberg merged commit 988b0b2 into ocaml:main May 3, 2023

jchavarri deleted the melange-min-version-3-8 branch May 3, 2023 22:41

emillon mentioned this pull request May 16, 2023

[new release] dune (15 packages) (3.8.0~alpha3) ocaml/opam-repository#23795

Closed

emillon mentioned this pull request May 23, 2023

[new release] dune (15 packages) (3.8.0) ocaml/opam-repository#23814

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

melange: set min version to 3.8 #7665

melange: set min version to 3.8 #7665

jchavarri commented May 2, 2023

jchavarri May 2, 2023

jchavarri May 2, 2023

rgrinberg May 2, 2023

emillon left a comment

emillon May 2, 2023

jchavarri commented May 2, 2023

rgrinberg left a comment

jchavarri commented May 3, 2023

rgrinberg commented May 3, 2023

jchavarri commented May 3, 2023

Alizter left a comment

rgrinberg commented May 3, 2023

jchavarri commented May 3, 2023

rgrinberg commented May 3, 2023

jchavarri commented May 3, 2023

	(* Old behavior: if old parser succeeds, return that. Otherwise, if
	parsing the ordered set language succeeds, ask the user to upgrade to
	a supported version. Otherwise, fail with the first error. *)
	try_
	(Mode_conf.Lib.Set.decode >>\| fun modes -> `Modes modes)
	(fun exn ->
	try_
	( Mode_conf.Lib.Set.decode_osl ~stanza_loc project >>\| fun modes ->
	if dune_version >= expected_version then `Modes modes
	else `Upgrade )
	(fun _ -> raise exn))
	>>\| function
	\| `Modes modes -> modes
	\| `Upgrade ->
	Syntax.Error.since stanza_loc Stanza.syntax expected_version
	~what:"Ordered set language for modes"

melange: set min version to 3.8 #7665

melange: set min version to 3.8 #7665

Conversation

jchavarri commented May 2, 2023

jchavarri May 2, 2023

Choose a reason for hiding this comment

jchavarri May 2, 2023

Choose a reason for hiding this comment

rgrinberg May 2, 2023

Choose a reason for hiding this comment

emillon left a comment

Choose a reason for hiding this comment

emillon May 2, 2023

Choose a reason for hiding this comment

jchavarri commented May 2, 2023

rgrinberg left a comment

Choose a reason for hiding this comment

jchavarri commented May 3, 2023

rgrinberg commented May 3, 2023

jchavarri commented May 3, 2023

Alizter left a comment

Choose a reason for hiding this comment

rgrinberg commented May 3, 2023

jchavarri commented May 3, 2023

rgrinberg commented May 3, 2023

jchavarri commented May 3, 2023