search

halcyon / asdf-java

A Java plugin for asdf-vm.
MIT License
455 stars 86 forks source link

Support for integrating with macOS /usr/libexec/java_home #112

Closed fcrespo82 closed 3 years ago

fcrespo82 commented 3 years ago

This pull request is aimed to fix #33

I kept the work to a minimum for a proof of concept. It the maintainer likes the approach I can further polish the code.

All feedback appreciated.

halcyon commented 3 years ago

LGTM

@joschi Any thoughts?

fcrespo82 commented 3 years ago

Investigating about Zulu and Liberica SDKs that have special treatment on the plugin, I found that with Zulu I can get by with a simple change in the path were macOS files are symlinked. But for Liberica, it appears that the files for the integration are only available in "PKG" and "DMG" downloads.

What @halcyon and @joschi think would be a better course of action?

Treat this different cases, or add a note in the readme that not all JDK offer the integration?

Note that for liberica it now should download the "PKG" or "DMG" option (not the tar.gz), changing the file that the metadata is lists in its endpoint and proceed the extraction in a whole new way

The note could be something like:

Note: Not all distributions of Java JDK packages offer this integration (eg. liberica). This option only works for packages that do offer that integration.

joschi commented 3 years ago

But for Liberica, it appears that the files for the integration are only available in "PKG" and "DMG" downloads.

While it seems to be possible to install PKG and DMG files in a script, it looks quite error prone. I'm not sure whether the additional effort to handle edge cases there would be worth it.

Install PKG: https://apple.stackexchange.com/a/72227/130823 Install DMG: https://zeckli.github.io/en/2017/10/06/mac-install-dmg-through-command-line-en.html

fcrespo82 commented 3 years ago

There is support for extracting the pkg directly with pkgutil --expand-full but the option is undocumented, and as so it can go away anytime. I agree that it is starting to be a little convoluted.

In the mean time I thought of another option for this.

If it can't find, I think it is possible to generate a dynamic Info.plist to be able to accomplish the same integration, but this needs more testing.

Where should we go from here? Basic integration with a note and investigate further on the other options?

halcyon commented 3 years ago

There is support for extracting the pkg directly with pkgutil --expand-full but the option is undocumented, and as so it can go away anytime. I agree that it is starting to be a little convoluted.

In the mean time I thought of another option for this.

If it can't find, I think it is possible to generate a dynamic Info.plist to be able to accomplish the same integration, but this needs more testing.

Where should we go from here? Basic integration with a note and investigate further on the other options?

Dynamically generating Info.plist is an interesting solution. Would it be vulnerable to a similar problem as the undocumented --expand-full if Apple decided to make changes to how they store their package metadata?

The work done so far is great, thank you! Tackling these JDK outliers in another issue/PR sounds like a good idea.

fcrespo82 commented 3 years ago

Would it be vulnerable to a similar problem as the undocumented --expand-full if Apple decided to make changes to how they store their package metadata?

It is possible that it would break, but I think this change is very unlikely to happen.

The work done so far is great, thank you! Tackling these JDK outliers in another issue/PR sounds like a good idea.

Nice, I will polish the code and mark it as ready for review.

fcrespo82 commented 3 years ago

Hi folks, this PR should be ready for review and merge.

Let me know if I need to do any changes.

Thanks.