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.

Sign up for GitHub

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

Jump to bottom

Add Support for JetBrains Runtime #637

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Add Support for JetBrains Runtime #637

wants to merge 10 commits into from

Conversation

gmitch215
Copy link

@gmitch215 gmitch215 commented Jun 5, 2024 •
edited
Loading

Description:
This PR adds support for the JetBrains Runtime, which is required in dependencies like jewel and useful for Compose Multiplatform.

The PR uses the binaries listed in Binaries for Developers , using the JBRSDK with JCEF flavor as shown in the releases page. It uses the GitHub API to find and parse versions, then calculates the URL using cache-redirector.jetbrains.com with the parsed SemVer, OS, Arch, and build number, which all output a .tar.gz file. Left a few comments in there for maintainability.

Related issue:

Check list:

  • Mark if documentation changes are required.
  • Mark if tests were added or updated to cover the changes.

Notes
Tested using jetbrains-installer.test.ts. Most of the tests were copied from other installers, but some I couldn't figure out how to bring over, so additional work may be required. I also copied downloadTool from the Temurin installer.

I also executed npm run build for dist/setup/index.js. I think something else was also thrown because I was on TypeScript 5.3, so I'm not sure how that will affect ncc.

@gmitch215
Add Support for JetBrains Runtime
bee7c85 
- Add Installer, Models
- Includes Tests & Test Manifest Data
- Add to `e2e-versions.yml`
- Run `npm run build`
- Update README.md
@gmitch215 gmitch215 requested a review from a team as a code owner June 5, 2024 20:55
@Goooler
Copy link

Goooler commented Jun 19, 2024

I'm using your branch as a workaround

  # TODO: replace this once https://github.com/actions/setup-java/pull/637 gets merged.
  - uses: gmitch215/setup-java@07ea605e4b1f0f8b0621155fb1722d6094c3e183
    with:
      distribution: 'jetbrains'
      java-version: 17

with an error:

Installed distributions
  Error: No supported distribution was found for input jetbrains

is there anything I can do?

See:

@gmitch215
Copy link
Author

Hmm, I'll debug and get back here later.

@gmitch215
Copy link
Author

Seems I just forgot to add it to the map of recognized runtimes to their IDs. Update your commit hash and it should succeed.

My project passed with the following configuration:

      - name: Setup JBR 17
        uses: gmitch215/setup-java@7f3ab2648f3d20faa20c4fe6e52a455757e0f38a
        with:
          distribution: 'jetbrains'
          java-version: 17
          cache: 'gradle'

Output:
image

@Goooler
Copy link

Goooler commented Jun 19, 2024

It could be picked correctly now!

But seems some libs are missing in this JDK installed.

Execution failed for task ':checkRuntime'.
You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.
> Failed to check JDK distribution: 'jlink', 'jpackage' are missing

  JDK distribution path: /Users/runner/hostedtoolcache/Java_JetBrains_jdk/17.0.11-1312.2/arm64/Contents/Home

See https://github.com/Goooler/kotlin-explorer/actions/runs/9575738587/job/26401058613.

@Goooler
Copy link

Goooler commented Jun 19, 2024

The JDK installed by Gradle toolchain worked fine.

Invalid Java installation found at '/Users/runner/.gradle/jdks/jetbrains_s_r_o_-17-aarch64-os_x/jbrsdk_jcef-17.0.11-osx-aarch64-b1207.24/Contents/Home' (Auto-provisioned by Gradle). It will be re-checked in the next build. This might have performance impact if it keeps failing. Run the 'javaToolchains' task for more details.

Seems we need Binaries for developers instead of Binaries for launching IntelliJ IDEA, which is installed by your action. Check https://github.com/JetBrains/JetBrainsRuntime/releases/tag/jbr-release-17.0.11b1312.2.

@gmitch215
Copy link
Author

gmitch215 commented Jun 19, 2024 •
edited
Loading

I switched the URL to the ones listed in the Binaries for Developers section, still getting the same errors. Going to do some more research.

@gmitch215
Copy link
Author

gmitch215 commented Jun 19, 2024 •
edited
Loading

I guessed correctly! :checkRuntime seems to pass.

      - name: Setup JBR 17
        uses: gmitch215/setup-java@99fc2135f7f7b08068e180cb5f29340f9de70720
        with:
          distribution: 'jetbrains'
          java-version: 17
          cache: 'gradle'

(This action failed due to below)

image

Note: Running on windows-latest seems to fail, so I'll fix that too.

@Goooler Goooler mentioned this pull request Jun 19, 2024
Open
@Goooler
Copy link

Goooler commented Jun 19, 2024

99fc213 works for me now, someone who needs this can ref romainguy/kotlin-explorer#62.

Thanks for your quick response!

@gmitch215
Copy link
Author

Got windows-latest to build now. Everything else works fine too.

      - name: Setup JBR 17
        uses: gmitch215/setup-java@e1f4c15facc0cf68c0e764340e374be034e4a859
        with:
          distribution: 'jetbrains'
          java-version: 17
          cache: 'gradle'

@gmitch215 gmitch215 mentioned this pull request Jun 28, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

JDK support distribution JBR (jetbrains runtime version) Support JetBrainsRuntime distribution
2 participants
@gmitch215 @Goooler