Skip to content

Bring back dune.1.0.0 compatibility#35

Merged
dra27 merged 2 commits intoocaml:masterfrom
kit-ty-kate:dune-1.0-compat
Dec 17, 2020
Merged

Bring back dune.1.0.0 compatibility#35
dra27 merged 2 commits intoocaml:masterfrom
kit-ty-kate:dune-1.0-compat

Conversation

@kit-ty-kate
Copy link
Copy Markdown
Member

@kit-ty-kate kit-ty-kate commented Nov 20, 2020

We're seeing failures on libraries when using dune 1.x. Notably in dune-release.1.2.0:


#=== ERROR while compiling dune-release.1.2.0 =================================#
# context              2.1.0~beta4 | linux/x86_64 | ocaml-base-compiler.4.11.1 | file:///home/opam/opam-repository
# path                 ~/.opam/4.11.1/.opam-switch/build/dune-release.1.2.0
# command              ~/.opam/4.11.1/bin/dune build -p dune-release -j 55
# exit-code            1
# env-file             ~/.opam/log/dune-release-7-420871.env
# output-file          ~/.opam/log/dune-release-7-420871.out
### output ###
#     ocamlopt bin/main.exe (exit 2)
# (cd _build/default && /home/opam/.opam/4.11.1/bin/ocamlopt.opt -w -40 -g -o bin/main.exe -I /home/opam/.opam/4.11.1/lib/astring -I /home/opam/.opam/4.11.1/lib/bos -I /home/opam/.opam/4.11.1/lib/cmdliner -I /home/opam/.opam/4.11.1/lib/fmt -I /home/opam/.opam/4.11.1/lib/fpath -I /home/opam/.opam/4.11.1/lib/logs -I /home/opam/.opam/4.11.1/lib/ocamlgraph -I /home/opam/.opam/4.11.1/lib/opam-core -I /home/opam/.opam/4.11.1/lib/opam-file-format -I /home/opam/.opam/4.11.1/lib/opam-format -I /home/opam/.opam/4.11.1/lib/opam-repository -I /home/opam/.opam/4.11.1/lib/opam-state -I /home/opam/.opam/4.11.1/lib/re -I /home/opam/.opam/4.11.1/lib/result -I /home/opam/.opam/4.11.1/lib/rresult -I /home/opam/.opam/4.11.1/lib/seq -I /home/opam/.opam/4.11.1/lib/stdlib-shims -I lib /home/opam/.opam/4.11.1/lib/stdlib-shims/stdlib_shims.cmxa /home/opam/.opam/4.11.1/lib/fmt/fmt.cmxa /home/opam/.opam/4.11.1/lib/result/result.cmxa /home/opam/.opam/4.11.1/lib/rresult/rresult.cmxa /home/opam/.opam/4.11.1/lib/astring/astring.cmxa /home/opam/.opam/4.11.1/lib/fpath/fpath.cmxa /home/opam/.opam/4.11.1/lib/ocaml/unix.cmxa /home/opam/.opam/4.11.1/lib/logs/logs.cmxa /home/opam/.opam/4.11.1/lib/bos/bos.cmxa /home/opam/.opam/4.11.1/lib/ocaml/bigarray.cmxa /home/opam/.opam/4.11.1/lib/ocamlgraph/graph.cmxa /home/opam/.opam/4.11.1/lib/re/re.cmxa /home/opam/.opam/4.11.1/lib/opam-core/opam_core.cmxa /home/opam/.opam/4.11.1/lib/opam-file-format/opam-file-format.cmxa /home/opam/.opam/4.11.1/lib/opam-format/opam_format.cmxa /home/opam/.opam/4.11.1/lib/fmt/fmt_tty.cmxa /home/opam/.opam/4.11.1/lib/logs/logs_fmt.cmxa /home/opam/.opam/4.11.1/lib/bos/bos_setup.cmxa lib/dune_release.cmxa /home/opam/.opam/4.11.1/lib/opam-repository/opam_repository.cmxa /home/opam/.opam/4.11.1/lib/opam-state/opam_state.cmxa /home/opam/.opam/4.11.1/lib/cmdliner/cmdliner.cmxa /home/opam/.opam/4.11.1/lib/logs/logs_cli.cmxa /home/opam/.opam/4.11.1/lib/fmt/fmt_cli.cmxa bin/.main.eobjs/cli.cmx bin/.main.eobjs/bistro.cmx bin/.main.eobjs/distrib.cmx bin/.main.eobjs/help.cmx bin/.main.eobjs/lint.cmx bin/.main.eobjs/opam.cmx bin/.main.eobjs/publish.cmx bin/.main.eobjs/tag.cmx bin/.main.eobjs/main.cmx)
# File "_none_", line 1:
# Error: No implementations provided for the following modules:
#          OpamParserTypes referenced from /home/opam/.opam/4.11.1/lib/opam-file-format/opam-file-format.cmxa(OpamParser)

It fails with dune.1.6.2 and 1.9.3 but not with dune 2.0 (since it recompiles with dune itself)

This brings back the dune 1.0 compatibility so that everytime a package tried to compile opam-file-format using dune, opam-file-format itself will be compiled using dune.
Currently the non-dune builds seems to not install the interface-only module: OpamParserTypes and dune doesn't seem to like that.

@dra27
Copy link
Copy Markdown
Member

dra27 commented Nov 20, 2020

The linker error is because of the new module FullPos - so OpamParserTypes is not actually an mli-only module. We get away with it in the dune build because it compiles opamParser.ml with -no-alias-deps.

I think it should be consequence-free simply to rename opamParserTypes.mli to opamParserTypes.ml (and obviously update src/dune) since any users will be linking to the .cma/.cmxa and won't notice the change.

dra27
dra27 reviewed Nov 20, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@dra27 dra27 left a comment

Choose a reason for hiding this comment

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

Simplification possible to have sample-opam but use it by the current name internally.

@rjbou rjbou added this to the 2.1.2 milestone Nov 27, 2020
@dra27 dra27 mentioned this pull request Dec 6, 2020
Closed
@dra27
Copy link
Copy Markdown
Member

dra27 commented Dec 17, 2020

Ah, I have just looked at this again in the context of #36 and also realise that this reverts a change in #31. In #23, I added dune as a general depopt but in #31 @rjbou tightened that to 2.0.0. There's no comment there - can you remember why that was, Raja? I'm wondering if it's because only Dune 2 and greater generate dune-package files, so we didn't need to recompile for older Dunes?

This isn't strictly speaking "bringing back" Dune 1.0 compatibility, it's wallpapering over the fault in the make-based build system.

@dra27
Copy link
Copy Markdown
Member

dra27 commented Dec 17, 2020

Or at least, to clarify, the changes to the opam file aren't restoring compatibility for Dune 1.x - the changes to tests/sample.opam do!

@rjbou
Copy link
Copy Markdown
Contributor

rjbou commented Dec 17, 2020

At the beginning, dune was restricted to > 1.11.4, which is the same than restricting to > 2.0.0 (as 2.0 is not available).
I should have commented why the restriction, what I remember is that I've tested with several dune version, and the good ones were > 1.11.4 (because of sample? because of package in the test dune file? because of something else that also has been fixed is the meanwhile?).
The restriction is no more needed now, it's compiling & testing correctly.

@dra27
Copy link
Copy Markdown
Member

dra27 commented Dec 17, 2020

Thanks, @rjbou - I wonder if it was the test, in that case! In which case, let's merge this and I'll separately work on the problem with the Makefile version.

@dra27 dra27 merged commit b2b6f89 into ocaml:master Dec 17, 2020
@dra27
Copy link
Copy Markdown
Member

dra27 commented Dec 17, 2020

Thanks, @kit-ty-kate!

@rjbou
Copy link
Copy Markdown
Contributor

rjbou commented Dec 17, 2020

Thanks @kit-ty-kate & @dra27!

@kit-ty-kate kit-ty-kate deleted the dune-1.0-compat branch December 18, 2020 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dra27 dra27 dra27 left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2.1.2

Development

Successfully merging this pull request may close these issues.

3 participants

@kit-ty-kate @dra27 @rjbou