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

Add the JVM Implementation after the name of the Vendor #140

Merged
merged 4 commits into from
Apr 6, 2021

Conversation

delgurth
Copy link
Contributor

In order to fix #46 and #57 I've added the jvm_impl after the vendor.

Unfortunately this will break the names configured previously in .tools-versions if you had selected one of the openj9 JDK's. Not sure how you can do this backwards compatible.

@joschi
Copy link
Contributor

joschi commented Apr 5, 2021

I'm not a big fan of breaking consumer again (sorry for the last time) but I also see the advantage of putting the JVM implementation into the name tag.

This being said, I think GraalVM releases (e. g. graalvm-graalvm-21.0.0.2+java11) would look a bit weird, wouldn't they?

@delgurth
Copy link
Contributor Author

delgurth commented Apr 5, 2021

I'm not a big fan of breaking consumer again (sorry for the last time) but I also see the advantage of putting the JVM implementation into the name tag.

Unfortunately I do not see a proper way of dealing with #46 and #57 without introducing a breaking change. And my hope is that people will understand the added value of this one.

This being said, I think GraalVM releases (e. g. graalvm-graalvm-21.0.0.2+java11) would look a bit weird, wouldn't they?

Good point, missed that one :( Guess graalvm should be in the excemption list, just as hotspot. So only openj9 needs special treatment at this moment I guess?

@delgurth delgurth marked this pull request as draft April 5, 2021 08:52
@delgurth
Copy link
Contributor Author

delgurth commented Apr 5, 2021

@joschi changed it to only do something for openj9. This makes it fragile in case a new jvm_impl is introduced, but I'm no jq hero unfortunately.

@delgurth delgurth marked this pull request as ready for review April 5, 2021 09:33
@halcyon
Copy link
Owner

halcyon commented Apr 6, 2021

@delgurth tremendous work, thank you! I'll merge it once you've had a chance to resolve conflicts.

@delgurth
Copy link
Contributor Author

delgurth commented Apr 6, 2021

@halcyon thanks. My git says I'm up to date with upstream/master and GitHub says I'm not. Will check what's going wrong in a few hours.

@delgurth delgurth marked this pull request as draft April 6, 2021 16:55
@delgurth delgurth force-pushed the add-jvm-impl-after-vendor branch from 87fc85c to ab06cc0 Compare April 6, 2021 17:03
@delgurth delgurth marked this pull request as ready for review April 6, 2021 17:03
@delgurth delgurth marked this pull request as draft April 6, 2021 17:09
@delgurth delgurth force-pushed the add-jvm-impl-after-vendor branch from ab06cc0 to af81c35 Compare April 6, 2021 17:28
@delgurth delgurth marked this pull request as ready for review April 6, 2021 17:34
@delgurth
Copy link
Contributor Author

delgurth commented Apr 6, 2021

I was messing up git history. Fixed the history now for this pull request. 2 more to go.

@halcyon halcyon merged commit 150e793 into halcyon:master Apr 6, 2021
@delgurth delgurth deleted the add-jvm-impl-after-vendor branch April 6, 2021 20:16
delgurth added a commit to delgurth/asdf-java that referenced this pull request Apr 10, 2021
adoptopenjdk-8.0.252+9.1.openj9-0.20.0 no longer exists, it's now adoptopenjdk-openj9-8.0.252+9.1.openj9-0.20.0
halcyon pushed a commit that referenced this pull request Aug 7, 2021
adoptopenjdk-8.0.252+9.1.openj9-0.20.0 no longer exists, it's now adoptopenjdk-openj9-8.0.252+9.1.openj9-0.20.0
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.

Add support for "latest" versions
3 participants