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

Fix Oracle JDK index generation #243

Merged
merged 1 commit into from
Mar 19, 2024
Merged

Conversation

carlosedp
Copy link
Contributor

Found the issue that led to coursier/coursier#2952.

The Oracle index currently generates:

{
"graalvm-jdk@oracle": {
  "17": "https://download.oracle.com/graalvm/17/latest/graalvm-jdk-17_macos-x64_bin.tar.gz",
  "21": "https://download.oracle.com/graalvm/21/latest/graalvm-jdk-21_macos-x64_bin.tar.gz"

While the correct way is as now:

{
"jdk@graalvm-oracle": {
        "17": "tgz+https://download.oracle.com/graalvm/17/latest/graalvm-jdk-17_macos-x64_bin.tar.gz",
        "21": "tgz+https://download.oracle.com/graalvm/21/latest/graalvm-jdk-21_macos-x64_bin.tar.gz"
      },
"jdk@java-oracle": {
        "17": "tgz+https://download.oracle.com/java/17/latest/jdk-17_macos-x64_bin.tar.gz",
        "21": "tgz+https://download.oracle.com/java/21/latest/jdk-21_macos-x64_bin.tar.gz"
      }
...

Sorry about the hassle.

@carlosedp
Copy link
Contributor Author

Please @ckipp01 / @alexarchambault ... if you can take a look as this messes up with the cs command on some cases (like listing installed JDKs).
Thanks!

Copy link
Collaborator

@ckipp01 ckipp01 left a comment

Choose a reason for hiding this comment

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

Ahh, good catch. I feel like we should have some sort of a test to catch the format of these.

@ckipp01 ckipp01 merged commit 04f1a71 into coursier:master Mar 19, 2024
2 checks passed
@carlosedp
Copy link
Contributor Author

Thanks! Yeah... since there is no docs, it would be nice to have some validation.

@carlosedp carlosedp deleted the fixoracle branch March 19, 2024 17:28
ckipp01 added a commit to ckipp01/jvm-index that referenced this pull request Mar 21, 2024
This is sort of a proposal. Following the comment in
coursier#243 (comment)
by @carlosedp I was going to try and write some docs on the actual
structure of the index, but right away hit on the fact that it's just
this:

```
final case class Index(map: Map[String, Map[String, Map[String, Map[String, String]]]]) {
```

It's pretty impossible to know what these `String`s all are so I figured
maybe it'd be good to start just typing this out a bit more. I used an
opaque type for `Os` here in hopes that it'd make things clearer a bit
throughout the code, and figured we could do that for all the `String`s
in the `Index`, but wanted to see what it would be like to at least do
one of them first. Thoughts on this?
alexarchambault pushed a commit that referenced this pull request Jul 24, 2024
This is sort of a proposal. Following the comment in
#243 (comment)
by @carlosedp I was going to try and write some docs on the actual
structure of the index, but right away hit on the fact that it's just
this:

```
final case class Index(map: Map[String, Map[String, Map[String, Map[String, String]]]]) {
```

It's pretty impossible to know what these `String`s all are so I figured
maybe it'd be good to start just typing this out a bit more. I used an
opaque type for `Os` here in hopes that it'd make things clearer a bit
throughout the code, and figured we could do that for all the `String`s
in the `Index`, but wanted to see what it would be like to at least do
one of them first. Thoughts on this?
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 this pull request may close these issues.

2 participants