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

Workaround for JBR JDK used by Compose Desktop Gradle Plugin #62

Conversation

Goooler
Copy link
Contributor

@Goooler Goooler commented Jun 19, 2024

Comment on lines +16 to +17
# TODO: replace this once https://github.com/actions/setup-java/pull/637 gets merged.
- uses: gmitch215/setup-java@99fc2135f7f7b08068e180cb5f29340f9de70720
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
# TODO: replace this once https://github.com/actions/setup-java/pull/637 gets merged.
- uses: gmitch215/setup-java@99fc2135f7f7b08068e180cb5f29340f9de70720
- uses: actions/setup-java@v4

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not necessary here, we can revert the change, just ensure gmitch215/setup-java can be correctly used in .github/workflows/release.yml.

java-version: 17
- uses: gradle/actions/setup-gradle@v3
- run: ./gradlew packageReleaseDmg
- run: ./gradlew build
Copy link
Owner

Choose a reason for hiding this comment

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

The build task doesn't seem to run the "release" configuration and therefore doesn't run proguard on the jar files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As renameDmg depended on packageDmg instead of packageReleaseDmg, I correct it now.

Check https://github.com/Goooler/kotlin-explorer/actions/runs/9576053362/job/26401925728#step:5:58

@@ -15,12 +15,13 @@ jobs:
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
- uses: actions/setup-java@v4
# TODO: replace this once https://github.com/actions/setup-java/pull/637 gets merged.
- uses: gmitch215/setup-java@99fc2135f7f7b08068e180cb5f29340f9de70720
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not super comfortable with this. I'd rather have my own fork to make sure the action doesn't do anything unexpected in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pinning on this commit or waiting for actions/setup-java#637 to be merged. Might hold this PR and release arm64 manually during these days.

@romainguy
Copy link
Owner

Changed my mind, it's too painful without this. Thanks for the contribution @Goooler, it's very helpful!

@romainguy romainguy merged commit 5f15866 into romainguy:main Aug 6, 2024
1 check passed
@Goooler Goooler deleted the compose-multiplatform-plugin-toolchian-honoring branch August 6, 2024 23:01
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.

Latest update (1.4.4) breaks the app
2 participants