graalvm / setup-graalvm

GitHub Action for setting up GraalVM distributions.
https://www.graalvm.org
Universal Permissive License v1.0
192 stars 27 forks source link

Using `version: mandrel-latest` with `java-version: 17` results in 404 error #64

Closed zakkak closed 10 months ago

zakkak commented 11 months ago

When using setup-graalvm with:

      - name: Setup GraalVM
        id: setup-graalvm
        uses: graalvm/setup-graalvm@v1
        with:
          version: 'mandrel-latest'
          java-version: '17'
          ...

the action results in:

Error: Unexpected HTTP response: 404

This happens because setup-graalvm gets the latest release from github without acounting for the java-version. So with the release of Mandrel 23.1 (which no longer includes java17 builds) setup-graalvm tries to access https://github.com/graalvm/mandrel/releases/download/mandrel-23.1.0.0-Final/mandrel-java17-linux-amd64-23.1.0.0-Final.tar.gz which doesn't exist.

In this scenario setup-graalvm needs a way to fetch the latest Mandrel release that includes java 17 builds, e.g. mandrel-23.0.1.2-Final.

I can work on implementing this. We will probably need to add additional tags to our releases, e.g. have mandrel-java21-23.1.0.0-Final along with mandrel-23.1.0.0-Final, mandrel-java17-23.0.1.2-Final and mandrel-java20-23.0.1.2-Final along with mandrel-23.0.1.2-Final etc.

This way we will be able to filter tags based on the java version and then get the latest of them.

cc @jerboaa @karm

fniephaus commented 11 months ago

How would this work? Would you fetch all releases, order them by date, and then find the first release that contains a JDK 17 build? I wouldn't want to hard code latest version because that adds maintenance work, but I also think we should keep the number of requests to a minimum.

Maybe integrating the Foojay API or something similar is an alternative?

zakkak commented 11 months ago

I was thinking about doing something similar to: https://github.com/graalvm/setup-graalvm/blob/6c7d417a1ef253f4d667a69e6a5716927746e251/src/graalvm.ts#L62-L86

That's why I am suggesting introducing new tags that will include the java version in our releases.

Maybe integrating the Foojay API or something similar is an alternative?

Sure, that could work as well. But it would only make sense if we were to use it for GraalVM releases as well. I don't think it's worth having such an alternative just for Mandrel. WDYT?

fniephaus commented 11 months ago

That's why I am suggesting introducing new tags that will include the java version in our releases.

I didn't know about that, makes sense!

Sure, that could work as well. But it would only make sense if we were to use it for GraalVM releases as well. I don't think it's worth having such an alternative just for Mandrel. WDYT?

Agreed, we could consider using Disco API instead, which will avoid users hitting rate limits. On the other hand, there's a new external dependency that is not directly controlled and updated by any of us. So if you plan to add new tags, maybe better if we do something similar to findLatestGraalVMJDKCEJavaVersion().

zakkak commented 11 months ago

Unfortunately using the tags is not going to work since we often tag the source code a coupe of weeks before we actually build and publish the release. E.g. at the moment there is a tag [mandrel-22.3.4.0-Final](https://github.com/graalvm/mandrel/releases/tag/mandrel-22.3.4.0-Final) which doesn't have any assets attached to it resulting in Error: Unexpected HTTP response: 404 if setup-graalvm tries to fetch the release archive from it.

To make matters worse, some times we might even bump the version before we actually release the previous tag (e.g. because of a last minute bugfix, we did this with mandrel-23.0.1.1-Final).

So even if we get the list of tags we the need to find the latest one that has some assets attached to it, so we still need to perform a few extra requests, hopefully no more than a couple.

In the light of this I had a quick look at the Foojay Disco API as well and it looks easier to use (at the cost of an additional dependency as you say).

E.g.

Getting the latest mandrel release working with JDK 17

❯ curl -X 'GET' \
  'https://api.foojay.io/disco/v3.0/packages/jdks?jdk_version=17&distribution=mandrel&architecture=amd64&operating_system=linux&latest=per_distro' \
  -H 'accept: application/json' -s | jq
{
  "result": [
    {
      "id": "4f4c7f41b8134eb229663342b7a1bf91",
      "archive_type": "tar.gz",
      "distribution": "mandrel",
      "major_version": 23,
      "java_version": "23.0.1.2",
      "distribution_version": "23.0.1.2",
      "jdk_version": 17,
      "latest_build_available": true,
      "release_status": "ga",
      "term_of_support": "lts",
      "operating_system": "linux",
      "lib_c_type": "glibc",
      "architecture": "amd64",
      "fpu": "unknown",
      "package_type": "jdk",
      "javafx_bundled": false,
      "directly_downloadable": true,
      "filename": "mandrel-java17-linux-amd64-23.0.1.2-Final.tar.gz",
      "links": {
        "pkg_info_uri": "https://api.foojay.io/disco/v3.0/ids/4f4c7f41b8134eb229663342b7a1bf91",
        "pkg_download_redirect": "https://api.foojay.io/disco/v3.0/ids/4f4c7f41b8134eb229663342b7a1bf91/redirect"
      },
      "free_use_in_production": true,
      "tck_tested": "unknown",
      "tck_cert_uri": "",
      "aqavit_certified": "unknown",
      "aqavit_cert_uri": "",
      "size": 244291230,
      "feature": []
    }
  ],
  "message": "1 package(s) found"
}

Getting the latest mandrel release working with JDK 21

❯ curl -X 'GET' \
  'https://api.foojay.io/disco/v3.0/packages/jdks?jdk_version=21&distribution=mandrel&architecture=amd64&operating_system=linux&latest=per_distro' \
  -H 'accept: application/json' -s | jq
{
  "result": [
    {
      "id": "6b8d545a0484acd21339fc6bc090b61c",
      "archive_type": "tar.gz",
      "distribution": "mandrel",
      "major_version": 23,
      "java_version": "23.1",
      "distribution_version": "23.1",
      "jdk_version": 21,
      "latest_build_available": true,
      "release_status": "ga",
      "term_of_support": "lts",
      "operating_system": "linux",
      "lib_c_type": "glibc",
      "architecture": "amd64",
      "fpu": "unknown",
      "package_type": "jdk",
      "javafx_bundled": false,
      "directly_downloadable": true,
      "filename": "mandrel-java21-linux-amd64-23.1.0.0-Final.tar.gz",
      "links": {
        "pkg_info_uri": "https://api.foojay.io/disco/v3.0/ids/6b8d545a0484acd21339fc6bc090b61c",
        "pkg_download_redirect": "https://api.foojay.io/disco/v3.0/ids/6b8d545a0484acd21339fc6bc090b61c/redirect"
      },
      "free_use_in_production": true,
      "tck_tested": "unknown",
      "tck_cert_uri": "",
      "aqavit_certified": "unknown",
      "aqavit_cert_uri": "",
      "size": 264296774,
      "feature": []
    }
  ],
  "message": "1 package(s) found"
}
zakkak commented 11 months ago

@fniephaus are you OK with me using the Foojay Disco API for this?

fniephaus commented 11 months ago

Sure, I'm ok with that. Keep in mind that the additional dependency is also external, which means your users might not get latest the moment it is out, or even if Disco API is broken or ever decides to remove Mandrel for some reason.