conda-forge / openjdk-feedstock

A conda-smithy repository for openjdk.
BSD 3-Clause "New" or "Revised" License
4 stars 35 forks source link

Move libraries to /lib/jvm #100

Closed Hoeze closed 2 years ago

Hoeze commented 2 years ago

This PR fixes issues with colliding packages by moving the openjdk installation to $CONDA_PREFIX/lib/jvm, similar how other linux distributions do. Solves e.g. https://github.com/conda-forge/openjdk-feedstock/issues/86

Checklist

conda-forge-linter commented 2 years ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

Hoeze commented 2 years ago

@conda-forge-admin, please rerender

Hoeze commented 2 years ago

@CJ-Wright, @izahn, @johanneskoester, @mingwandroid, @sodre, @xhochy: This PR is ready and could be merged if you agree :)

Hoeze commented 2 years ago

@xhochy Kind push :) Would you mind merging this PR to solve #86?

izahn commented 2 years ago

@conda-forge-admin please rerender

conda-forge-linter commented 2 years ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

izahn commented 2 years ago

I'm thumbs-up on this, but I wish we had more comprehensive tests to make sure this doesn't break anything.

xhochy commented 2 years ago

I'm not 100% certain this works in all cases. I would love to test the interaction with r-base/ r-rjava first before merging. I know that I had issues in the past with them and JVM paths.

Hoeze commented 2 years ago

IMHO, if r-java would really have problems with reading JAVA_HOME, then this should be fixed in r-java. It's a worse idea to keep the status-quo for a single downstream library as it (possibly) breaks a lot of other packages.

Anyways, how would one test this? Is there some kind of conda-forge-testing channel one could use?

Hoeze commented 2 years ago

@xhochy @izahn Any way forward on this PR?

izahn commented 2 years ago

Yes, let's add a test for rJava and see what happens. Something like https://github.com/izahn/openjdk-feedstock/commit/02bd25d43b96e4cde4b902d6fe0bb8b4bc5a0801 should do it.

Hoeze commented 2 years ago

Alright, I created #104 to test your changes :)

izahn commented 2 years ago

@xhochy it looks like rJava is OK, tested in https://github.com/conda-forge/openjdk-feedstock/pull/104. Any other concerns?

Hoeze commented 2 years ago

Thanks a lot for reviewing and merging this PR @izahn @xhochy!

christophfink commented 2 years ago

Hi @Hoeze : Thanks for merging this.

I noticed that OpenJDK 11 for Windows has not been rebuilt (or at least has not landed in conda-forge).

It’s kinda important for us over at https://github.com/r5py/r5py - could you look into why the version was not rebuilt? Thanks!

Hoeze commented 2 years ago

Hi @christophfink, I created a PR to trigger a rebuild, lets see :) https://github.com/conda-forge/openjdk-feedstock/pull/106

If this does not help, I'd suggest you to open an issue and ask for package maintainers' help.

christophfink commented 2 years ago

Thanks!

mariusvniekerk commented 2 years ago

Has this change been backported to openjdk11 as well?

Hoeze commented 2 years ago

@mariusvniekerk Yes, but I think it was not pushed to the conda-forge repo. See #106. Some maintainer (@xhochy ?) would have to merge this to trigger the push to conda-forge.

xhochy commented 2 years ago

Done.