From 2471c60beb61f753a93596d9ae7e5fcc3e6a62c7 Mon Sep 17 00:00:00 2001 From: Louis Gesbert Date: Fri, 17 May 2024 15:32:51 +0200 Subject: [PATCH 1/8] Update some dependencies - more recent sedlex fixes a bug that needed a workaround in our code - we need recent dates_calc to avoid extra runtime dependency on `Str` that our build system won't handle --- catala.opam | 4 ++-- compiler/surface/lexer.cppo.ml | 6 ------ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/catala.opam b/catala.opam index 4650932f..ac9ac008 100644 --- a/catala.opam +++ b/catala.opam @@ -22,7 +22,7 @@ depends: [ "bindlib" {>= "6.0"} "cmdliner" {>= "1.1.0"} "cppo" {>= "1"} - "dates_calc" {>= "0.0.4"} + "dates_calc" {>= "0.0.6"} "dune" {>= "3.11"} "js_of_ocaml-ppx" {= "4.1.0"} "menhir" {>= "20200211"} @@ -31,7 +31,7 @@ depends: [ "ocamlfind" {!= "1.9.5"} "ocamlgraph" {>= "1.8.8"} "re" {>= "1.10"} - "sedlex" {>= "2.4"} + "sedlex" {>= "3.1"} "uutf" {>= "1.0.3"} "ubase" {>= "0.05"} "unionFind" {>= "20220109"} diff --git a/compiler/surface/lexer.cppo.ml b/compiler/surface/lexer.cppo.ml index 9e0ac90a..d325c6fb 100644 --- a/compiler/surface/lexer.cppo.ml +++ b/compiler/surface/lexer.cppo.ml @@ -778,9 +778,6 @@ let lex_raw (lexbuf : lexbuf) : token = | _ -> ( (* Nested match for lower priority; `_` matches length 0 so we effectively retry the sub-match at the same point *) - let lexbuf = lexbuf in - (* workaround sedlex bug, see https://github.com/ocaml-community/sedlex/issues/12 - (fixed in 3.1) *) match%sedlex lexbuf with | Star (Compl '\n'), ('\n' | eof) -> LAW_TEXT (Utf8.lexeme lexbuf) | _ -> L.raise_lexer_error (Pos.from_lpos prev_pos) prev_lexeme) @@ -817,9 +814,6 @@ let lex_law (lexbuf : lexbuf) : token = | _ -> ( (* Nested match for lower priority; `_` matches length 0 so we effectively retry the sub-match at the same point *) - let lexbuf = lexbuf in - (* workaround sedlex bug, see https://github.com/ocaml-community/sedlex/issues/12 - (fixed in 3.1) *) match%sedlex lexbuf with | Star (Compl '\n'), ('\n' | eof) -> LAW_TEXT (Utf8.lexeme lexbuf) | _ -> L.raise_lexer_error (Pos.from_lpos prev_pos) prev_lexeme) From 446481182f338c26c3e74ca27cb648f4f0fc98c7 Mon Sep 17 00:00:00 2001 From: Louis Gesbert Date: Mon, 27 May 2024 13:48:05 +0200 Subject: [PATCH 2/8] CI: update base build image We need the new dates_calc and sedlex Debug notes: We need the latest ocamlpro/ocaml image (2024-05-26) to get the release of dates_calc. Unfortunately, it breaks: `pip install gmpy2` could not find pre-built binaries, so it would "transparently" try to recompile and then complain about obscure system packages missing (mp libraries). Indeed the newest image picked up the newer Alpine release (3.20), which is based on a newer musl release (and apparently that's a problem !?). Hopefully the proper python dependencies will become available at some point ? --- Dockerfile | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Dockerfile b/Dockerfile index ba8f9fb1..3f3e4db0 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,7 +2,7 @@ # STAGE 1: setup an opam switch with all dependencies installed # # (only depends on the opam files) -FROM ocamlpro/ocaml:4.14-2024-01-14 AS dev-build-context +FROM ocamlpro/ocaml:4.14-2024-05-26 AS dev-build-context # Image from https://hub.docker.com/r/ocamlpro/ocaml RUN mkdir catala @@ -25,10 +25,6 @@ RUN opam --cli=2.1 switch create catala ocaml-system && \ # Note: just `opam switch create . --deps-only --with-test --with-doc && opam clean` # should be enough once opam 2.2 is released (see opam#5185) -# This is temporary, to avoid pulling in a dependency to Str, until it's merged -# and release into dates_calc -RUN opam --cli=2.1 pin dates_calc.0.0.5 git+https://github.com/AltGr/dates-calc#nostr - # # STAGE 2: get the whole repo and build # From 291a8e05e435ad8bb4b2dc6efff8db712e673572 Mon Sep 17 00:00:00 2001 From: Louis Gesbert Date: Wed, 26 Jun 2024 12:40:29 +0200 Subject: [PATCH 3/8] Specify latest gmpy2 dependency --- runtimes/python/pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtimes/python/pyproject.toml b/runtimes/python/pyproject.toml index 4d194740..e6388497 100644 --- a/runtimes/python/pyproject.toml +++ b/runtimes/python/pyproject.toml @@ -3,7 +3,7 @@ name = "catala-runtime" description = "Runtime libraries needed to execute Catala programs compiled to Python" version = "0.10.0" dependencies = [ - "gmpy2", + "gmpy2 ~= 2.2.0rc1", "typing", "mypy", "python-dateutil", From 52f1f14a0848b3f7d4e40d28e6b35e2542cb34bc Mon Sep 17 00:00:00 2001 From: Louis Gesbert Date: Wed, 26 Jun 2024 17:50:04 +0200 Subject: [PATCH 4/8] Fix for opam 2.1.6 The fix for the annoying bug with --assume-built has not been backported from the 2.2 branch. --- Makefile | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index b6dbd75e..ea06ac9b 100644 --- a/Makefile +++ b/Makefile @@ -99,14 +99,13 @@ prepare-install: dune build @install --promote-install-files install: prepare-install - if [ x$$($(OPAM) --version) = "x2.1.5" ]; then \ - $(OPAM) install . --working-dir; \ - else \ - $(OPAM) install . --working-dir --assume-built; \ - fi + case x$$($(OPAM) --version) in \ + x2.1.5|x2.1.6) $(OPAM) install . --working-dir;; \ + *) $(OPAM) install . --working-dir --assume-built;; \ + esac # `dune install` would work, but does a dirty install to the opam prefix without # registering with opam. -# --assume-built is broken in 2.1.5 +# --assume-built is broken in 2.1.5 and 2.1.6 inst: prepare-install @opam custom-install \ From 153a029b34e73ca7d28eea87d321d1e6f89532a1 Mon Sep 17 00:00:00 2001 From: Louis Gesbert Date: Thu, 27 Jun 2024 10:08:18 +0200 Subject: [PATCH 5/8] Adjust CI for newer alpine --- .github/workflows/harness.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/harness.yml b/.github/workflows/harness.yml index df14b47c..b26b3cc6 100644 --- a/.github/workflows/harness.yml +++ b/.github/workflows/harness.yml @@ -112,7 +112,7 @@ jobs: - name: Install LaTeX deps # This is done late because caching would not benefit compared to # installation through apk (1,5G upload is slow) - run: sudo apk add texlive-xetex texmf-dist-latexextra texmf-dist-pictures font-dejavu groff + run: sudo apk add texlive-xetex texmf-dist-latexextra texmf-dist-binextra texmf-dist-pictures font-dejavu groff - name: Build Catala extra docs run: | cd ~/catala From 2d756698fb2bf78361951e809091fa6226f6c1e5 Mon Sep 17 00:00:00 2001 From: Louis Gesbert Date: Thu, 27 Jun 2024 11:58:53 +0200 Subject: [PATCH 6/8] clerk report: allow to choose diff command Also, detect non-GNU diff or absence of colors, and fallback to a basic view; this is actually more readable in logs or diffs. --- build_system/clerk_driver.ml | 27 +++++++++++++---------- build_system/clerk_report.ml | 41 +++++++++++++++++++++-------------- build_system/clerk_report.mli | 2 +- catala.opam | 1 - 4 files changed, 41 insertions(+), 30 deletions(-) diff --git a/build_system/clerk_driver.ml b/build_system/clerk_driver.ml index 9e7ab20f..c358b783 100644 --- a/build_system/clerk_driver.ml +++ b/build_system/clerk_driver.ml @@ -226,15 +226,18 @@ module Cli = struct ~doc:"Display the full list of tests that have been run" ); ]) - let use_patdiff = + let diff_command = Arg.( value - & flag - & info ["patdiff"] - ~env:(Cmd.Env.info "CATALA_USE_PATDIFF") + & opt ~vopt:(Some None) (some (some string)) None + & info ["diff"] + ~env:(Cmd.Env.info "CATALA_DIFF_COMMAND") ~doc: - "Enable use of the 'patdiff' command for showing test failure \ - details (no effect if the command is not available)") + "Use a standard $(i,diff) command instead of the default \ + side-by-side view. If no argument is supplied, the command will \ + be $(b,patdiff) if available or $(b,diff) otherwise. A supplied \ + argument will be used as diff command with arguments pointing to \ + the reference file and the output file") let ninja_flags = let env = @@ -915,10 +918,10 @@ let test_cmd = (reset_test_outputs : bool) (test_flags : string list) verbosity - (use_patdiff : bool) + (diff_command : string option option) (ninja_flags : string list) = set_report_verbosity verbosity; - Clerk_report.set_display_flags ~use_patdiff (); + Clerk_report.set_display_flags ~diff_command (); ninja_init ~extra:Seq.empty ~test_flags @@ fun build_dir nin_file -> let targets = @@ -988,7 +991,7 @@ let test_cmd = $ Cli.reset_test_outputs $ Cli.test_flags $ Cli.report_verbosity - $ Cli.use_patdiff + $ Cli.diff_command $ Cli.ninja_flags) let run_cmd = @@ -1051,11 +1054,11 @@ let runtest_cmd = $ Cli.single_file) let report_cmd = - let run color debug verbosity use_patdiff build_dir file = + let run color debug verbosity diff_command build_dir file = let _options = Catala_utils.Global.enforce_options ~debug ~color () in let build_dir = Option.value ~default:"_build" build_dir in set_report_verbosity verbosity; - Clerk_report.set_display_flags ~use_patdiff (); + Clerk_report.set_display_flags ~diff_command (); let open Clerk_report in let tests = read_many file in let success = summary ~build_dir tests in @@ -1071,7 +1074,7 @@ let report_cmd = $ Cli.Global.color $ Cli.Global.debug $ Cli.report_verbosity - $ Cli.use_patdiff + $ Cli.diff_command $ Cli.build_dir $ Cli.single_file) diff --git a/build_system/clerk_report.ml b/build_system/clerk_report.ml index f2753d95..d5369164 100644 --- a/build_system/clerk_report.ml +++ b/build_system/clerk_report.ml @@ -33,22 +33,22 @@ type disp_flags = { mutable files : [ `All | `Failed | `None ]; mutable tests : [ `All | `FailedFile | `Failed | `None ]; mutable diffs : bool; - mutable use_patdiff : bool; + mutable diff_command : string option option; } let disp_flags = - { files = `Failed; tests = `FailedFile; diffs = true; use_patdiff = false } + { files = `Failed; tests = `FailedFile; diffs = true; diff_command = None } let set_display_flags ?(files = disp_flags.files) ?(tests = disp_flags.tests) ?(diffs = disp_flags.diffs) - ?(use_patdiff = disp_flags.use_patdiff) + ?(diff_command = disp_flags.diff_command) () = disp_flags.files <- files; disp_flags.tests <- tests; disp_flags.diffs <- diffs; - disp_flags.use_patdiff <- use_patdiff + disp_flags.diff_command <- diff_command let write_to f file = File.with_out_channel f (fun oc -> Marshal.to_channel oc (file : file) []) @@ -81,20 +81,14 @@ let longuest_common_prefix_length s1 s2 = aux 0 let diff_command = + let has_gnu_diff () = + File.process_out ~check_exit:ignore "diff" ["--version"] + |> Re.(execp (compile (str "GNU"))) + in lazy begin - if - disp_flags.use_patdiff - && has_command "patdiff" - && Message.has_color stdout - then - ( ["patdiff"; "-alt-old"; "expected"; "-alt-new"; "result"], - fun ppf s -> - s - |> String.split_on_char '\n' - |> List.filter (( <> ) "") - |> Format.pp_print_list Format.pp_print_string ppf ) - else + match disp_flags.diff_command with + | None when Message.has_color stdout && has_gnu_diff () -> let width = Message.terminal_columns () - 5 in ( [ "diff"; @@ -147,6 +141,21 @@ let diff_command = (String.sub r w (String.length r - w)) | _ -> Format.pp_print_string ppf li)) ppf ) + | Some cmd_opt | (None as cmd_opt) -> + let command = + match cmd_opt with + | Some str -> String.split_on_char ' ' str + | None -> + if Message.has_color stdout && has_command "patdiff" then + ["patdiff"; "-alt-old"; "Reference"; "-alt-new"; "Result"] + else ["diff"; "-u"; "-L"; "Reference"; "-L"; "Result"] + in + ( command, + fun ppf s -> + s + |> String.trim_end + |> String.split_on_char '\n' + |> Format.pp_print_list Format.pp_print_string ppf ) end let print_diff ppf p1 p2 = diff --git a/build_system/clerk_report.mli b/build_system/clerk_report.mli index 40169d96..42355aa6 100644 --- a/build_system/clerk_report.mli +++ b/build_system/clerk_report.mli @@ -43,6 +43,6 @@ val set_display_flags : ?files:[ `All | `Failed | `None ] -> ?tests:[ `All | `FailedFile | `Failed | `None ] -> ?diffs:bool -> - ?use_patdiff:bool -> + ?diff_command:string option option -> unit -> unit diff --git a/catala.opam b/catala.opam index ac9ac008..034f68fe 100644 --- a/catala.opam +++ b/catala.opam @@ -47,7 +47,6 @@ depends: [ "conf-npm" {cataladevmode} "conf-python-3-dev" {cataladevmode} "cpdf" {cataladevmode} - "conf-diffutils" {cataladevmode} "conf-pandoc" {cataladevmode} "z3" {catalaz3mode} "conf-ninja" From 81e2d1810008ea62cff5d314df93bb23305c7045 Mon Sep 17 00:00:00 2001 From: Louis Gesbert Date: Thu, 27 Jun 2024 13:59:25 +0200 Subject: [PATCH 7/8] Don't try to read screen columns without a tty --- compiler/catala_utils/file.ml | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/compiler/catala_utils/file.ml b/compiler/catala_utils/file.ml index 02fbe6f1..f043b3df 100644 --- a/compiler/catala_utils/file.ml +++ b/compiler/catala_utils/file.ml @@ -185,21 +185,24 @@ let process_out ?check_exit cmd args = let () = let default = 80 in let get_terminal_cols () = + let from_env () = + try int_of_string (Sys.getenv "COLUMNS") with Not_found | Failure _ -> 0 + in let count = - try - (* terminfo *) - process_out "tput" ["cols"] |> String.trim |> int_of_string - with Failure _ -> ( + if not Unix.(isatty stdin) then from_env () + else try - (* stty *) - process_out "stty" ["size"] - |> String.trim - |> fun s -> - let i = String.rindex s ' ' + 1 in - String.sub s i (String.length s - i) |> int_of_string - with Failure _ | Not_found | Invalid_argument _ -> ( - try int_of_string (Sys.getenv "COLUMNS") - with Not_found | Failure _ -> 0)) + (* terminfo *) + process_out "tput" ["cols"] |> String.trim |> int_of_string + with Failure _ -> ( + try + (* stty *) + process_out "stty" ["size"] + |> String.trim + |> fun s -> + let i = String.rindex s ' ' + 1 in + String.sub s i (String.length s - i) |> int_of_string + with Failure _ | Not_found | Invalid_argument _ -> from_env ()) in if count > 0 then count else default in From 33dbbadca8c1337b29f62bb3123199ce3862dc5d Mon Sep 17 00:00:00 2001 From: Louis Gesbert Date: Thu, 27 Jun 2024 14:00:11 +0200 Subject: [PATCH 8/8] Workaround pandoc version changing the result of some tests --- tests/literate/good/test_grave_char_en.catala_en | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/literate/good/test_grave_char_en.catala_en b/tests/literate/good/test_grave_char_en.catala_en index 4da62f82..ac32c8df 100644 --- a/tests/literate/good/test_grave_char_en.catala_en +++ b/tests/literate/good/test_grave_char_en.catala_en @@ -1,6 +1,6 @@ > Module Test_grave_char_en -## Law text should be able to contain grave accent '`'. +## Law text should be able to contain backquote chars `` ` ``. This is a block of law text containing `. This allows to: @@ -60,7 +60,8 @@ $ catala latex \textbf{This defines the catala module \texttt{Test\_grave\_char\_en}} -\subsection{Law text should be able to contain grave accent ``'.} +\subsection{Law text should be able to contain backquote chars +\texttt{\textasciigrave{}}.} This is a block of law text containing `. This allows to: