setup-ocaml icon indicating copy to clipboard operation
setup-ocaml copied to clipboard

Beta versions of the compiler support could be improved

Open kit-ty-kate opened this issue 2 years ago • 9 comments

With the following workflow file: https://github.com/ocaml/merlin/blob/56e0601ad40995421ae9e3537158af30559fb65f/.github/workflows/main.yml

 ocaml-compiler:
          - 4.13.0~beta1

Is not understood as a valid compiler version and will instead expect it to be a package name, which makes opam fail with:

> Install OCaml
  /opt/hostedtoolcache/opam/2.0.9/x86_64/opam switch create . --no-install --packages 4.13.0~beta1
  opam: option `--packages': invalid element in list (`4.13.0~beta1'): Invalid
        character in package name "4.13.0~beta1"
  am switch [OPTION]... [COMMAND] [ARG]...
  Try `opam switch --help' or `opam --help' for more information.
  Error: The process '/opt/hostedtoolcache/opam/2.0.9/x86_64/opam' failed with exit code 2

This is a valid version string for opam and expliciting ocaml-base-compiler.4.13.0~beta1 shouldn't be required.

kit-ty-kate avatar Sep 01 '21 15:09 kit-ty-kate

And using ocaml-base-compiler.4.13.0~beta1 the result is:

/opt/hostedtoolcache/opam/2.0.9/x86_64/opam switch create . --no-install --packages ocaml-base-compiler.4.13.0~beta1
[ERROR] Could not resolve set of base packages:
       The following dependencies couldn't be met:
         - ocaml-base-compiler → ocaml-beta
             unmet availability conditions: enable-ocaml-beta-repository
                ```

voodoos avatar Sep 01 '21 15:09 voodoos

@voodoos - you need to add the ocaml-beta-repository explicitly, then that should work

dra27 avatar Sep 01 '21 16:09 dra27

I don’t think it should be extended to recognise that version string - the input is it intended to be an opam package version, but access to betas could definitely be improved, perhaps more along the lines of:

ocaml-compiler: 4.13.x
enable-beta-compilers: true

or some such. The main thing would be not specify the alpha, beta or rc number

dra27 avatar Sep 01 '21 16:09 dra27

I totally agree. Well in the first place I was expecting setup-ocaml to use opam 2.1 so having a enable-beta-compilers field shouldn't be needed, but since it doesn't i guess this is one way to do it, though I think ocaml-beta-repository should always be set in CI with opam 2.0 in my opinion, that would take care of everything automatically be it on opam 2.0 or 2.1

kit-ty-kate avatar Sep 01 '21 16:09 kit-ty-kate

For the record here is the configuration that works for us:

  build:
    strategy:
      matrix:
        os:
          - macos-latest
          - ubuntu-latest
          - windows-latest
        ocaml-compiler:
          - ocaml-base-compiler.4.13.0~beta1
        
    runs-on: ${{ matrix.os }}

    steps:
      - ...

      - name: Set up OCaml ${{ matrix.ocaml-compiler }}
        uses: ocaml/setup-ocaml@v2
        with:
          # Version of the OCaml compiler to initialise
          ocaml-compiler: ${{ matrix.ocaml-compiler }}
          opam-repositories: |
            default: https://github.com/ocaml/opam-repository.git
            beta: https://github.com/ocaml/ocaml-beta-repository.git

voodoos avatar Sep 02 '21 11:09 voodoos

@kit-ty-kate - you're right, it shouldn't be necessary to have to specify either the beta repository or the fact betas are wanted. Saying 4.13.x gives everything that's needed: the fact it's been added to a workflow now tells you that betas are needed and then that automatically becomes 4.13.0 when the release cycle finishes. The only thing we might want to do is to have a special so that 4.14.x is an error (now) because - at least for now - no one wants to be running CI against OCaml's trunk!

dra27 avatar Sep 03 '21 10:09 dra27

@voodoos' suggestion is good. Also, in the jsoo repository, I wrote something quite similar: https://github.com/ocsigen/js_of_ocaml/blob/c5b53bf1eb034a229a47e1f226097d81b5b279ba/.github/workflows/build.yml#L59-L84

smorimoto avatar Sep 23 '21 18:09 smorimoto

Any chance this could be fixed? Making PRs for adding 4.14 support is really painful

kit-ty-kate avatar Jan 22 '22 15:01 kit-ty-kate

I was just looking at this. There are already enough features to do this, so if we are going to improve this, we have to add a higher level of functionality, and I'm still not sure how meaningful that really is.

smorimoto avatar Jan 22 '22 17:01 smorimoto