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

Use JabRef's JDK21 build #10004

Merged
merged 13 commits into from
Jun 28, 2023
Merged

Use JabRef's JDK21 build #10004

merged 13 commits into from
Jun 28, 2023

Conversation

koppor
Copy link
Member

@koppor koppor commented Jun 11, 2023

Background:

Idea:

  • Use stable JDK whereever possible (tests, gradle execution, ...)
  • Use patched JDK21 when building end user binaries.

Binaries:

Ancient stories:

Before jpackage, we used install4j - see #5312 for details.

Mandatory checks

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

@JabRef JabRef deleted a comment from github-actions bot Jun 11, 2023
@@ -47,6 +47,10 @@ java {
// Workaround needed for Eclipse, probably because of https://github.com/gradle/gradle/issues/16922
// Should be removed as soon as Gradle 7.0.1 is released ( https://github.com/gradle/gradle/issues/16922#issuecomment-828217060 )
modularity.inferModulePath.set(false)

toolchain {
languageVersion = JavaLanguageVersion.of(20)
Copy link
Member

Choose a reason for hiding this comment

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

That needs to be 21

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought,, 20 is the latest release version (21 is in ramp down, isn't it? - https://openjdk.org/projects/jdk/21/). That version is used generally (tests, ...). Exception; When using jlink, 21 is used. I thought, it is a good idea.to modify one workflow only instead of changing all (JDK custom build download etc)

Copy link
Member Author

Choose a reason for hiding this comment

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

I change this with sed in deployment.yml

@Siedlerchr
Copy link
Member

Siedlerchr commented Jun 12, 2023

Can't we just use the GitHub artifacts that are build and upload them to our server?

Think this pr is a bit too early

@Siedlerchr Siedlerchr closed this Jun 12, 2023
@koppor
Copy link
Member Author

koppor commented Jun 12, 2023

Can't we just use the GitHub artifacts that are build and upload them to our server?

Artifacts are cleared after 30days. Should we build the JDK each month?

Think this pr is a bit too early

I assume the patch will take two or three JDK releases until it is included. Don't want to wait such a long time.

I also see this PR as unblocker for a release using java-keyring to store passwords "properly".

@Siedlerchr
Copy link
Member

Siedlerchr commented Jun 12, 2023

I mean building the jdk once using CI (like they do), then uploading the artifact to our server
https://github.com/openjdk/jdk/blob/master/.github/workflows/build-linux.yml

Seems to be easier to copy the existing workflow

@koppor
Copy link
Member Author

koppor commented Jun 12, 2023

The build is even created automatically. main ("OpenJDK GHA Sanity Checks'") calls all build workflows.

Example: https://github.com/JabRef/jdk/actions/runs/5239968107/jobs/9460305999

grafik

Windows fails at GetJTreg (as ist also does at the main repository)

@koppor
Copy link
Member Author

koppor commented Jun 12, 2023

grafik

@koppor
Copy link
Member Author

koppor commented Jun 12, 2023

The artifacts will be available after the complete run. See actions/upload-artifact#181 for more information.

@Siedlerchr
Copy link
Member

Yes, we just need to get this jtreg download fixed

@koppor
Copy link
Member Author

koppor commented Jun 12, 2023

Yes, we just need to get this jtreg download fixed

They also happen at the main repository (https://github.com/JabRef/jdk/actions/runs/5239968107/jobs/9460306443). Thus, I hope, someone of the OpenJDK team picks up.

@koppor koppor reopened this Jun 12, 2023
@koppor
Copy link
Member Author

koppor commented Jun 12, 2023

at 14c2fef (#10004), I updated to the build generated at JabRef/jdk21#1.

@koppor
Copy link
Member Author

koppor commented Jun 12, 2023

Build available at https://builds.jabref.org/pull/10004/merge/

The about dialog is of strange size here (Windows). Does anyone else also experience issues here?

image

@koppor koppor marked this pull request as ready for review June 28, 2023 19:56
@koppor
Copy link
Member Author

koppor commented Jun 28, 2023

We need to move forward because of JabRef release blocking issues. In case something breaks, we will need to investigate then.

@koppor koppor merged commit 456cd0e into main Jun 28, 2023
@koppor koppor deleted the use-jdk21 branch June 28, 2023 19:57
@Siedlerchr
Copy link
Member

Siedlerchr commented Jun 29, 2023 via email

@koppor
Copy link
Member Author

koppor commented Jun 29, 2023

We will also probably add notarizations and singing for the mac jdk version as well

I put #10041 to the v5.10 milestone.

@Siedlerchr
Copy link
Member

jdk 21 mac path is probably:
/Users/christophs/Downloads/jdk-21.jdk/Contents/Home/bin

signing overall of the jdk https://github.com/openjdk/jdk/blob/master/doc/building.md#macos-1

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.

3 participants