Port 1.3 to dune #273

tmattio · 2022-06-20T07:55:35Z

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:

it might be more invasive than it needs. In particular, I chose to delete all the files that weren't necessary, rewrite the Makefile, add dune rules for tests, etc. I'm happy to revert any of these if you'd prefer a more minimal PR.
One of the interface (src/omd_lexer_fs.mli) wasn't correct. I suspect that the file wasn't being built. This PR fixes the interface, but the correct change to make it backward compatible is to remove the file I think.

nojb · 2022-06-20T07:59:25Z

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 · 2022-06-20T08:05:18Z

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:

builder-web
camyll
cow
cowabloga
finch
hardcaml-examples
hardcaml-framework
jekyll-format
lambdoc
learn-ocaml
learn-ocaml-client
malfunction
md2mld
monorobot
ocaml-lsp-server
ocp_reveal
sail
stone

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

It also seems like #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 · 2022-06-20T15:09:09Z

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 · 2022-06-20T20:50:29Z

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

About omd_lexer_fs. I think you're right that it wasn't compiled at all.

shonfeder · 2022-06-23T01:24:38Z

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.

This test has been failing since at least tag 1.3.1, I think there just wasn't any CI to check. So I'm just updating the expected result to match.

shonfeder

Fixed up the test and added CI.

Thank you very much for the assist here, @tmattio! I'll cut a release to to the opam repo this week.

shonfeder · 2022-06-23T01:35:12Z

omd.opam

@@ -1,24 +1,32 @@
 # This file is generated by dune, edit dune-project instead


I didn't mean to commit this! Sorry :(

Seems to indicate some inconsistency in our dune builds tho? I'm not sure what accounts for that.

I think this should fix the CI failures on 4.04.2

shonfeder · 2022-06-23T02:19:31Z

src/omd_html.ml

@@ -38,7 +38,7 @@ let to_string html =
        | (a, Some v) ->
          if not (String.contains v '\'') then
            pp " %s='%s'" a v
-          else not (String.contains v '"') then
+          else if not (String.contains v '"') then


This is curious. I'm not sure how this would have been compiling before!

shonfeder · 2022-06-23T03:02:56Z

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

@tmattio

CHANGES: 1.3.2 ------ - port from oasis to dune (ocaml/omd#273, @tmattio) 1.3.x ----- - might stop checking validity of HTML tag *names* and accept any XML-parsable tag name. 1.2.5 ----- - only fixes a single bug (an ordered list could be transformed into an unordered list) 1.2.4 ----- - only fixes a single bug (some spaces were wrongly handled in the HTML parsing) 1.2.2/3 ------- - fix a few issues with HTML parsing. 1.2.1 ----- - mainly fixes issues with HTML parsing. 1.2.0 ----- - introduces options `-w` and `-W`. Fixes mostly concern subtle uses of `\n`s in HTML and Markdown outputs. 1.1.2 ----- - fix: some URL-related parsing issues. 1.1.0/1.1.1 ----------- - fix: some HTML-related issues. 1.0.1 ----- - fixes some parsing issues, improves output. (2014-10-02) 1.0.0 ----- - warning: this release is only partially compatible with previous versions. - accept HTML blocks which directly follow each other - fix: accept all XML-compatible attribute names for HTML attributes - fix backslash-escaping for hash-ending ATX-titles + fix Markdown output for Html_block - fix (HTML parsing) bugs introduced in 1.0.0.b and 1.0.0.c - rewrite parser of block HTML to use the updated Omd.t - rewrite parser of inline HTML to use the updated Omd.t - upgrade Omd.t for HTML representation There will not be any newer 0.9.x release although new bugs have been discovered. Thus it's recommended to upgrade to the latest 1.x.y. 0.9.7 ----- - introduction of media:end + bug fixes. If you need to have a version that still has `Tag of extension` instead of `Tag of name * extension` and don't want to upgrade, you may use 0.9.3 0.9.6 ----- - fix a bug (concerning extensions) introduced by 0.9.4. 0.9.5 ----- - bug fix + `Tag of extension` changed to `Tag of name * extension` 0.9.4 ----- - fixes a bug for the new feature 0.9.3 ----- - new feature `media:type="text/omd"`. This version is recommended if you do not use that new feature and want to use 0.9.x 0.9.2 ----- - not released... older versions -------------- - cf. [commit log](https://github.com/ocaml/omd/commits/master)

Port to dune

aefe7a3

sonologico approved these changes Jun 22, 2022

View reviewed changes

shonfeder changed the title ~~Port to dune~~ Port 1.3 to dune Jun 23, 2022

shonfeder approved these changes Jun 23, 2022

View reviewed changes

shonfeder self-requested a review June 23, 2022 01:23

shonfeder added 3 commits June 22, 2022 21:32

Add CI

0bbbaba

Add CI actually

dc29df6

Update test

beeff90

This test has been failing since at least tag 1.3.1, I think there just wasn't any CI to check. So I'm just updating the expected result to match.

shonfeder approved these changes Jun 23, 2022

View reviewed changes

Add base-bigarray dependency

6bc8735

I think this should fix the CI failures on 4.04.2

shonfeder approved these changes Jun 23, 2022

View reviewed changes

Use cache-prefix to try avoiding ocaml compiler conflict

2671373

shonfeder added 4 commits July 1, 2022 12:19

Try pinning the setup ocaml version

542fdb6

Restore dependency on base-bytes

e07a087

Add bigarray and bytes as lib depends

dd0157f

Use the latest setup ocaml

8ac36ff

shonfeder merged commit 244bd36 into ocaml:1.3 Jul 1, 2022

shonfeder mentioned this pull request Jul 1, 2022

[new release] omd (1.3.2) ocaml/opam-repository#21703

Merged

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port 1.3 to dune #273

Port 1.3 to dune #273

tmattio commented Jun 20, 2022

nojb commented Jun 20, 2022

tmattio commented Jun 20, 2022

shonfeder commented Jun 20, 2022

sonologico commented Jun 20, 2022

shonfeder commented Jun 23, 2022

shonfeder left a comment

shonfeder Jun 23, 2022

shonfeder Jun 23, 2022

shonfeder commented Jun 23, 2022

		@@ -1,24 +1,32 @@
		# This file is generated by dune, edit dune-project instead

Port 1.3 to dune #273

Port 1.3 to dune #273

Conversation

tmattio commented Jun 20, 2022

nojb commented Jun 20, 2022

tmattio commented Jun 20, 2022

shonfeder commented Jun 20, 2022

sonologico commented Jun 20, 2022

shonfeder commented Jun 23, 2022

shonfeder left a comment

Choose a reason for hiding this comment

shonfeder Jun 23, 2022

Choose a reason for hiding this comment

shonfeder Jun 23, 2022

Choose a reason for hiding this comment

shonfeder commented Jun 23, 2022