Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
zakkak opened this issue Oct 17, 2023 · 6 comments · Fixed by #67
Closed

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

zakkak opened this issue Oct 17, 2023 · 6 comments · Fixed by #67

Comments

@zakkak
Copy link
Collaborator

zakkak commented Oct 17, 2023

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
Copy link
Member

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
Copy link
Collaborator Author

zakkak commented Oct 17, 2023

I was thinking about doing something similar to:

export async function findLatestGraalVMJDKCEJavaVersion(
majorJavaVersion: string
): Promise<string> {
const matchingRefs = await getMatchingTags(
`${GRAALVM_JDK_TAG_PREFIX}${majorJavaVersion}`
)
const lowestNonExistingVersion = '0.0.1'
let highestVersion = lowestNonExistingVersion
const versionNumberStartIndex = `refs/tags/${GRAALVM_JDK_TAG_PREFIX}`.length
for (const matchingRef of matchingRefs) {
const currentVersion = matchingRef.ref.substring(versionNumberStartIndex)
if (
semverValid(currentVersion) &&
semverGt(currentVersion, highestVersion)
) {
highestVersion = currentVersion
}
}
if (highestVersion === lowestNonExistingVersion) {
throw new Error(
`Unable to find the latest Java version for '${majorJavaVersion}'. Please make sure the java-version is set correctly. ${c.ERROR_HINT}`
)
}
return highestVersion
}

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?

nscuro added a commit to DependencyTrack/hyades that referenced this issue Oct 17, 2023
Caused by graalvm/setup-graalvm#64

Signed-off-by: Niklas <nscuro@protonmail.com>
@fniephaus
Copy link
Member

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
Copy link
Collaborator Author

zakkak commented Oct 17, 2023

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
Copy link
Collaborator Author

zakkak commented Oct 24, 2023

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

@fniephaus
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants