ocaml / setup-ocaml

GitHub Action for the OCaml programming language
https://ocaml.org/
MIT License
199 stars 39 forks source link

3.0.0-alpha: Windows: bash script fails with "syntax error: unexpected end of file" #815

Closed cknitt closed 5 months ago

cknitt commented 5 months ago

Another 3.0.0-alpha issue, I hope this one makes more sense than my previous one 🙈.

So once correctly configured, setup-ocaml itself works perfectly fine on Windows, and we can use the OCaml 5.2.0 compiler to build our project.

However, after the setup-ocaml step and our successful OCaml build step, we have the following step:

      - name: "Check if syntax subfolder has changed"
        id: syntax-diff
        shell: bash
        run: |
          if git diff --name-only --exit-code HEAD^ HEAD -- jscomp/syntax; then
            echo "syntax_status=unchanged" >> $GITHUB_ENV
          else
            echo "syntax_status=changed" >> $GITHUB_ENV
          fi

This step works fine when using setup-ocaml 2.x.x, but fails with 3.0.0-alpha with the following error message:

Run if git diff --name-only --exit-code HEAD^ HEAD -- jscomp/syntax; then
D:\a\_temp\d426cc8d-884f-4bdc-a18e-a[70](https://github.com/rescript-lang/rescript-compiler/actions/runs/9594206863/job/26456471252?pr=6819#step:18:71)af2c1cc23.sh:
line 6: syntax error: unexpected end of file
Error: Process completed with exit code 2.

So it seems that the setup-ocaml step changes something in the environment, causing this error.

See https://github.com/rescript-lang/rescript-compiler/actions/runs/9601379213/job/26481410755.

smorimoto commented 5 months ago

Remove this step: https://github.com/rescript-lang/rescript-compiler/actions/runs/9601379213/workflow#L183-L187

smorimoto commented 5 months ago

This issue is probably unrelated to the action.

cknitt commented 5 months ago

Remove this step: https://github.com/rescript-lang/rescript-compiler/actions/runs/9601379213/workflow#L183-L187

I should have mentioned that I have already tried this, but it didn't help. Also, this did work fine with the setup-ocaml 2.x.x.

This is definitely a new issue with 3.0.0-beta. Here is a standalone repro:

https://github.com/cknitt/setup-ocaml-repro/blob/main/.github/workflows/ci.yml https://github.com/cknitt/setup-ocaml-repro/actions/runs/9745608001/job/26893980512

So please reopen this issue @smorimoto

smorimoto commented 5 months ago

The Actions runner may be using the wrong bash:

image
smorimoto commented 5 months ago

https://github.com/marketplace/actions/install-cygwin-action#line-endings

smorimoto commented 5 months ago

You can use >- instead:

- name: Check if syntax subfolder has changed
  id: syntax-diff
  shell: bash
  env:
    SHELLOPTS: igncr
  run: |
    if git diff --name-only --exit-code HEAD^ HEAD -- jscomp/syntax; then
      echo "syntax_status=unchanged" >>"$GITHUB_ENV"
    else
      echo "syntax_status=changed" >>"$GITHUB_ENV"
    fi
cknitt commented 5 months ago

@smorimoto Thanks, but the proposed workaround with >- does NOT solve the problem.

See

https://github.com/cknitt/setup-ocaml-repro/blob/main/.github/workflows/ci.yml https://github.com/cknitt/setup-ocaml-repro/actions/runs/9761746852/job/26943486150

Why does the 3.0.0 extension need to modify the path in such a way that a different bash binary becomes the default when that obviously wasn't necessary in 2.x.x?

This particular bash issue may not be the only side effect of this new behavior.

cknitt commented 5 months ago

Ah, overlooked the SHELLOPTS: igncr, sorry, will try that, too.

However, the question remains: Why this change?

cknitt commented 5 months ago

Ok, I can confirm that SHELLOPTS: igncr is the correct workaround.

smorimoto commented 5 months ago

A better way to set it on Windows only is:

- name: Set SHELLOPTS=igncr on Windows
  if: runner.os == 'Windows'
  run: echo "SHELLOPTS=igncr" >>"$GITHUB_ENV"
smorimoto commented 5 months ago

The reason you need to do this from v3 is that, until v2, the opam you call was not the real opam: https://github.com/ocaml/setup-ocaml/blob/970719e1fee4d99a8f464db850f5ef30028f68c8/packages/setup-ocaml/src/opam.ts#L240-L248

cknitt commented 5 months ago

A better way to set it on Windows only is:

Thanks for the hint!

ygrek commented 3 months ago

does it make sense to have igncr as default on windows runners?