Skip to content

Conversation

@anmonteiro
Copy link
Contributor

Reason: Syntax & Toolchain for OCaml

CHANGES:

CHANGES:

- fix: make `End_of_line.Convert.lf_to_crlf` compatible with OCaml 4.08
  (@anmonteiro, [reasonml/reason#2898](reasonml/reason#2898))
Copy link
Member

@jmid jmid left a comment

Choose a reason for hiding this comment

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

Thanks!

I see a few runtest red lights.

On 4.08 both packages fail runtest due to small white space differences, e.g.,
https://opam.ci.ocaml.org/github/ocaml/opam-repository/commit/e295755dbcc7e97d813b019ec01f0943e0e54dcf/variant/compilers,4.08,reason.3.17.2,tests

#=== ERROR while compiling reason.3.17.2 ======================================#
# context              2.5.0~rc1 | linux/x86_64 | ocaml-base-compiler.4.08.1 | pinned(https://github.com/reasonml/reason/releases/download/3.17.2/reason-3.17.2.tbz)
# path                 ~/.opam/4.08/.opam-switch/build/reason.3.17.2
# command              ~/.opam/opam-init/hooks/sandbox.sh build dune build -p reason -j 71 @install @runtest
# exit-code            1
# env-file             ~/.opam/log/reason-7-393a8f.env
# output-file          ~/.opam/log/reason-7-393a8f.out
### output ###
# File "test/4.08/type-jsx.t/run.t", line 1, characters 0-0:
# /usr/bin/git --no-pager diff --no-index --color=always -u _build/default/test/4.08/type-jsx.t/run.t _build/default/test/4.08/type-jsx.t/run.t.corrected
# diff --git a/_build/default/test/4.08/type-jsx.t/run.t b/_build/default/test/4.08/type-jsx.t/run.t.corrected
# index 51295c7..a01f956 100644
# --- a/_build/default/test/4.08/type-jsx.t/run.t
# +++ b/_build/default/test/4.08/type-jsx.t/run.t.corrected
# @@ -109,8 +109,8 @@ Print the formatted file
#    module Optional1 = {
#      let createElement = (~required, ~children, ()) => {
#        switch (required) {
# -      | Some(a) => {displayName: a}
# -      | None => {displayName: "nope"}
# +      | Some(a) => { displayName: a }
# +      | None => { displayName: "nope" }
#        };
#      };
#    };

[...]

A quick fix to disable runtest on 4.08 would be to add an additional constraint like the following to both packages:

  "ocaml" {with-test & >= "4.09"}

reason-.3.17.2 is also failing runtest on Alpine for both 4.14 and 5.4 with
https://opam.ci.ocaml.org/github/ocaml/opam-repository/commit/e295755dbcc7e97d813b019ec01f0943e0e54dcf/variant/distributions,alpine-3.22-ocaml-5.4,reason.3.17.2,tests

#=== ERROR while compiling reason.3.17.2 ======================================#
# context              2.5.0~rc1 | linux/x86_64 | ocaml-base-compiler.5.4.0 | pinned(https://github.com/reasonml/reason/releases/download/3.17.2/reason-3.17.2.tbz)
# path                 ~/.opam/5.4/.opam-switch/build/reason.3.17.2
# command              ~/.opam/opam-init/hooks/sandbox.sh build dune build -p reason -j 71 @install @runtest
# exit-code            1
# env-file             ~/.opam/log/reason-7-3faa06.env
# output-file          ~/.opam/log/reason-7-3faa06.out
### output ###
# (cd _build/default/src/refmt && /bin/bash -e -u -o pipefail -c 'echo let version = \"$(git rev-parse --verify HEAD)\"') > _build/default/src/refmt/git_commit.ml
# fatal: not a git repository (or any parent up to mount point /)
# Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
# (cd _build/default/src/refmt && /bin/bash -e -u -o pipefail -c 'echo let short_version = \"$(git rev-parse --short HEAD)\"') > _build/default/src/refmt/git_commit.ml
# fatal: not a git repository (or any parent up to mount point /)
# Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
# File "test/comments-ml.t/run.t", line 1, characters 0-0:
# /usr/bin/git --no-pager diff --no-index --color=always -u _build/default/test/comments-ml.t/run.t _build/default/test/comments-ml.t/run.t.corrected
# diff --git a/_build/default/test/comments-ml.t/run.t b/_build/default/test/comments-ml.t/run.t.corrected
# index f2978d2..3cf4924 100644
# --- a/_build/default/test/comments-ml.t/run.t
# +++ b/_build/default/test/comments-ml.t/run.t.corrected
# @@ -13,6 +13,8 @@ Format the formatted file back
#  
#  Ensure idempotency: first format and second format are the same
#    $ diff formatted.re formatted_back.re
# -  0a1
# -  > 
# +  --- formatted.re
# +  +++ formatted_back.re
# +  @@ -0,0 +1 @@
# +  +
#    [1]

What do you make of this?

A similar quick fix would be to disable runtest on Alpine:

    "@runtest" {with-test & os-distribution != "alpine"}

@anmonteiro
Copy link
Contributor Author

sorry I forgot to remove the @runtest aliases, should be good now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants