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 the testing for JDK 8 /11 by using --build-arg #274

Merged
merged 1 commit into from
May 9, 2022

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented May 4, 2022

Noticed while working on building an image on JDK 17 to start testing ome/bioformats#3796 and ome/bioformats#3815 in our CI

The previous logic was based on environment variables and effectively using the same base image for both matrix builds.
486cddd uses the --build-arg option and also uses the <java_version>-slim-bullseye Docker tag to ensure the image is Debian-based.

To test this build look at the raw logs of the JDK8 and the JDK11 builds. Without this PR, the logs should indicate /usr/local/openjdk-8 in both cases. With this PR the JDK version should match the matrix parameter

The previous logic using environment variable was effectively using
the same base image
Pass the <version>-slim-bullseye Docker tag to ensure the image is
based on Debian.
@sbesson sbesson requested review from a team and melissalinkert and removed request for a team May 9, 2022 08:00
@joshmoore
Copy link
Member

LGTM. Is your plan to add in 17 to this repo or only consume this downstream?

@sbesson
Copy link
Member Author

sbesson commented May 9, 2022

LGTM. Is your plan to add in 17 to this repo or only consume this downstream?

😄 Ironically, this branch was original called something like jdk17 and was adding 17 to the matrix until I realised that the build argument logic was broken and also that several low-level components will need some JDK 17 adjustements.

I went for a PR to fix the first issue and I'll discuss the second one this afternoon at the weekly Formats meetings.

@sbesson
Copy link
Member Author

sbesson commented May 9, 2022

Merging as this only affect the GitHub Actions builds. Next steps will be to:

  • update the OME Jenkins infrastructure to start alternating between JDK versions (8 and 11 initially and planning to add 17 eventually)
  • start reviewing the low-level Java components, adding 17 to the matrix GitHub builds and fixing build issues as they arise

@sbesson sbesson merged commit 7761ca3 into ome:master May 9, 2022
@sbesson sbesson deleted the build_arg branch May 9, 2022 16:00
@sbesson
Copy link
Member Author

sbesson commented May 10, 2022

See ome/devspace#194 for the update of the Jenkins infrastructure to rotate between JDK versions.

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