ocaml / omd

extensible Markdown library and tool in "pure OCaml"
ISC License
156 stars 45 forks source link

Port 1.3 to dune #273

Closed tmattio closed 2 years ago

tmattio commented 2 years ago

This PR backports the switch of the build system from oasis to dune.

This is part of the OCaml release readiness effort for OCaml 5. In particular, omd is a dependency of ocaml-lsp-server, so the build failure of omd on OCaml 5 prevents us from using LSP there.

Omd 1.3.1 builds with Oasis, which isn't compatible with OCaml 5. Seeing that the project is deprecated in the OCaml Platform, I am not sure we'll be making it compatible. Instead, it seems sensible to backport Dune support to Omd to allow it to build on OCaml 5.

Some remarks on the PR:

nojb commented 2 years ago

Wouldn't it be better to put energy into making a release of the current code, rather than marching on with the antediluvian 1.3.1 release?

tmattio commented 2 years ago

I certainly wouldn't mind a stable release of Omd 2, but I think we'd still want to make omd 1.3.1 compatible with OCaml 5. Here's a list of reverse dependencies:

I doubt we'll want to migrate all of these in the context of OCaml 5 release readiness.

It also seems like https://github.com/ocaml/omd/pull/269 is going to be necessary to cut a stable release. It's still in draft though, and I wouldn't want to rush the release of omd: we'd need the Platform tools to be compatible with OCaml 5 before alpha1.

shonfeder commented 2 years ago

I agree that we should support 1.3. We still are working to achieve conformity to the spec, and the current state of 2.0 doesn't begin to approach the extensibility or features supplied by 1.3 afaik.

We could push a stable release of 2.0 even without fixing the bugs that cause us to deviate from the spec, but the maintenance burden to migrate all the dependencies would be very significant.

sonologico commented 2 years ago

Agree on supporting 1.3 for now. I don't think we should do much more than this PR though.

shonfeder commented 2 years ago

I'm getting a test failure:

$ dune test -f
list + code + space + header: FAILURE
   input = "- A\n- B\n\n    ```\n    code\n    ```\n\n# header"
expected = "(Ulp(Li (Paragraph(Text \"A\")))(Li (Paragraph(Text \"B\"))(Code_block code)))(NL)(H1(Text \"header\"))"
  result = "(Ulp(Li (Paragraph(Text \"A\")))(Li (Paragraph(Text \"B\"))(Code_block code)))(NL)(NL)(H1(Text \"header\"))"
36 tests passed, 1 test failed.

I'll investigate, but first I'm gonna add CI to this PR if that's ok, @tmattio. Since we're essentially committing to longer term support for 1.3 with this, I'd like to ensure we have a CI loop to protect.

shonfeder commented 2 years ago

Hitting build failures on Ocaml 4.04 which I haven't been able to work out tonight:


(cd _build/default && /home/runner/work/omd/omd/_opam/bin/ocamlc.opt -w @1..3@5..28@30..39@43@46..47@49..57@61..62-40 -strict-sequence -strict-formats -short-paths -keep-locs -g -o tests/test_spec.bc src/omd.cma tests/.test_spec.eobjs/byte/dune__exe__Test_spec.cmo)
File "_none_", line 1:
Error: Error while linking src/omd.cma(Omd_lexer):
The external function `caml_ba_dim_1' is not available
File "tests/dune", line 2, characters 7-16:
2 |  (name test_spec)
           ^^^^^^^^^
(cd _build/default && /home/runner/work/omd/omd/_opam/bin/ocamlopt.opt -w @1..3@5..28@30..39@43@46..47@49..57@61..62-40 -strict-sequence -strict-formats -short-paths -keep-locs -g -o tests/test_spec.exe src/omd.cmxa tests/.test_spec.eobjs/native/dune__exe__Test_spec.cmx)
/usr/bin/ld: src/omd.a(omd_lexer.o): in function `camlOmd_lexer__get_1636':
/home/runner/work/omd/omd/_build/default/src/omd_lexer.ml:339: undefined reference to `caml_ba_get_1'
/usr/bin/ld: src/omd.a(omd_lexer.o): in function `camlOmd_lexer__sub_1638':
/home/runner/work/omd/omd/_build/default/src/omd_lexer.ml:346: undefined reference to `caml_ba_get_1'
collect2: error: ld returned 1 exit status
File "caml_startup", line 1:
Error: Error during linking
File "tests/cow/dune", line 2, characters 7-11:
2 |  (name test)
           ^^^^
(cd _build/default && /home/runner/work/omd/omd/_opam/bin/ocamlopt.opt -w @1..3@5..28@30..39@43@46..47@49..57@61..62-40 -strict-sequence -strict-formats -short-paths -keep-locs -g -o tests/cow/test.exe src/omd.cmxa tests/cow/.test.eobjs/native/dune__exe__Test.cmx)
/usr/bin/ld: src/omd.a(omd_lexer.o): in function `camlOmd_lexer__get_1636':
/home/runner/work/omd/omd/_build/default/src/omd_lexer.ml:339: undefined reference to `caml_ba_get_1'
/usr/bin/ld: src/omd.a(omd_lexer.o): in function `camlOmd_lexer__sub_1638':
/home/runner/work/omd/omd/_build/default/src/omd_lexer.ml:346: undefined reference to `caml_ba_get_1'
collect2: error: ld returned 1 exit status
File "caml_startup", line 1:
Error: Error during linking
File "src/dune", line 10, characters 7-15:
10 |  (name omd_main)
            ^^^^^^^^
(cd _build/default && /home/runner/work/omd/omd/_opam/bin/ocamlopt.opt -w @1..3@5..28@30..39@43@46..47@49..57@61..62-40 -strict-sequence -strict-formats -short-paths -keep-locs -g -o src/omd_main.exe src/omd.cmxa src/.omd_main.eobjs/native/dune__exe__Omd_main.cmx)
/usr/bin/ld: src/omd.a(omd_lexer.o): in function `camlOmd_lexer__get_1636':
/home/runner/work/omd/omd/_build/default/src/omd_lexer.ml:339: undefined reference to `caml_ba_get_1'
/usr/bin/ld: src/omd.a(omd_lexer.o): in function `camlOmd_lexer__sub_1638':
/home/runner/work/omd/omd/_build/default/src/omd_lexer.ml:346: undefined reference to `caml_ba_get_1'
collect2: error: ld returned 1 exit status
File "caml_startup", line 1:
Error: Error during linking
Error: Process completed with exit code 1.

If anyone knows the issue here, help much welcome, otherwise I'll poke around on it over the next few days when I have time