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

Replace Debian Java requirement with an available version [DI-337] #240

Merged
merged 2 commits into from
Nov 28, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/deb/hazelcast/DEBIAN/control
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Section: imdg
Priority: optional
Architecture: all
Conflicts: ${CONFLICTS}
Depends: java21-sdk-headless
Depends: java21-sdk-headless | default-jdk-headless
Copy link
Contributor

Choose a reason for hiding this comment

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

What if default-jdk-headless ends up being beyond JDK21 when they finally update?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If there is no java21-sdk-headless, there are various outcomes:

  • we don't specify a fallback default-jdk-headless (current behaviour), you can't use Hazelcast (👎)
  • we specify a fallback default-jdk-headless
    • it's older
      • it's too old, Hazelcast doesn't work (👎)
      • it's old, but not too old (e.g. 17) - Hazelcast works (👍)
    • it's 21 - Hazelcast works (👍)
    • it's newer
      • (say) 22 is close enough - Hazelcast works (👍)
      • it's too new (say 99), Hazelcast doesn't work (👎)

At least this way there's a chance it works.

Copy link
Contributor

Choose a reason for hiding this comment

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

At least this way there's a chance it works.

Yes totally agree but if its >21 than this will break our Java policy and if it doesn't work then we are back to current situation

Wondering why this was not flagged with PR builder or something? Not sure I fully grasp #237. Is this compile or runtime issue?

I think if we can't guarantee default-jdk-headless will confirm to our Java policy than may be just use JDK17?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At least this way there's a chance it works.

Yes totally agree but if its >21 than this will break our Java policy and if it doesn't work then we are back to current situation

The other alternatives are that:

  • We don't list a JDK as a prerequisite and let the users install it themselves
  • We leave as-is and Debian is broken until they include JDK21 (which might be a while)

Wondering why this was not flagged with PR builder or something?
Because we test our Debian packages on Ubuntu, where there is a java21-sdk-headless.
I'm investigating this seperately.

Not sure I fully grasp #237. Is this compile or runtime issue?

It's an installation issue (which I guess for this project would be defined as runtime).
When you install Hazelcast, it should install the listed prerequisites as well. But they aren't available, so the installation fails.

I think if we can't guarantee default-jdk-headless will confirm to our Java policy than may be just use JDK17?

But then our JDKs are inconsistent between platforms... (no right answers here).

Copy link
Contributor

Choose a reason for hiding this comment

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

But then our JDKs are inconsistent between platforms... (no right answers here).

Yes very true.

May be Depends: java21-sdk-headless | java17-sdk-headless so its pinned either way?
In Docker we don't set default and I assume we will update all JDK usages when we update our Java policy

Just wondering if there a way to install JDk21 from a different location?

I am just giving my novice 2 cents so lets see what @ldziedziul has to say.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just wondering if there a way to install JDk21 from a different location?

There is, but as it's not via apt I don't think you can tag it as a pre-requisite the same way so not useful for our purposes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Needs backporting?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Needs backporting?

Yes - already in the description.

Maintainer: Hazelcast Platform Team <[email protected]>
Description: A tool that allows users to install & run Hazelcast
Homepage: https://www.hazelcast.com/
Expand Down