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

Enable building of java libraries #72

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Hoeze
Copy link

@Hoeze Hoeze commented Sep 9, 2022

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@conda-forge-linter
Copy link

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:

  • Recipes should usually depend on matplotlib-base as opposed to matplotlib so that runtime environments do not require large packages like qt.

@Hoeze
Copy link
Author

Hoeze commented Sep 9, 2022

@conda-forge-admin, please rerender

@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2022

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/ray-packages-feedstock/actions/runs/3022490864.

@Hoeze
Copy link
Author

Hoeze commented Sep 14, 2022

Hi @h-vetinari, do you know why unzip cannot be found when building the linux packages?
Neither adding it to yum_requirements nor to the build requirements section was helping.
Any help would be highly appreciated!

@h-vetinari
Copy link
Member

Hi @h-vetinari, do you know why unzip cannot be found when building the linux packages? Neither adding it to yum_requirements nor to the build requirements section was helping. Any help would be highly appreciated!

Hey @Hoeze, sorry for the large delay here, pretty underwater ATM... Unzip should be there as a build-dep (both in the meta.yaml, and at build time in CI), but it seems that bazel is looking for it in a different place than our $PREFIX. Might need checking which paths bazel checks for this, and then adding $PREFIX or adding a symlink in a place that bazel looks in.... Not so much experience with bazel though, so I'm of limited help here, and no capacity to google & chase this down myself ATM.

@scv119
Copy link

scv119 commented Oct 7, 2022

@jovany-wang do you know how much work is needed to get java in conda?

@jovany-wang
Copy link

I don't know how conda package works, what's the differences to the wheel package?

@h-vetinari
Copy link
Member

@jovany-wang do you know how much work is needed to get java in conda?

Java is available in conda. Java bindings for ray are just a question of getting things to build in our infrastructure, which imposes requirements that libraries get installed in $PREFIX, to make the packaged artefacts relocatable.

I don't know how conda package works, what's the differences to the wheel package?

There are a huge amount of differences... 🙃
Mostly that we very rigorously track the ABI impact of various libraries (and rebuild where necessary), that we unvendor things wherever possible, that we strongly prefer shared libraries, and that we build all packages with the same compiler stack (which would otherwise be another source of potential ABI incompatibility).

@Hoeze
Copy link
Author

Hoeze commented Nov 7, 2022

@jovany-wang I think if unzip would be available during bazel build, my patch would work.
The problem is that currently the java libraries just do not get built and therefore are not included in the package.
See also the build log.

@jovany-wang
Copy link

The problem is that currently the java libraries just do not get built and therefore are not included in the package.

Didn't understand which libraries you meant here.

@Hoeze
Copy link
Author

Hoeze commented Nov 8, 2022

@jovany-wang See #64.
The pypi package contains <package_root>/jars/ray_dist.jar which is necessary for raydp to work.
Its building is just not enabled in conda yet.

@jovany-wang
Copy link

jovany-wang commented Nov 8, 2022

The pypi package contains <package_root>/jars/ray_dist.jar which is necessary for raydp to work.
Its building is just not enabled in conda yet.

So, backing the question to: how do you build it in conda(ps: I know nothing about building in conda...)?
not sure whether RAY_INSTALL_JAVA=trueenv var could help that.

@Hoeze
Copy link
Author

Hoeze commented Nov 8, 2022

The pypi package contains <package_root>/jars/ray_dist.jar which is necessary for raydp to work.
Its building is just not enabled in conda yet.

So, backing the question to: how do you build it in conda(ps: I know nothing about building in conda...)? not sure whether RAY_INSTALL_JAVA=trueenv var could help that.

@jovany-wang Yes, it should:
https://github.com/ray-project/ray/blob/2e53f481880e248262690bcf972bb4ee3fee25cc/python/build-wheel-manylinux2014.sh#L51-L54

This is also how your Docker build is configured:
https://github.com/ray-project/ray/blob/ee2a8da87a9bdc360d8e7b99061ed643fb934fc0/ci/docker/base.build.Dockerfile

As I mentioned before, the only issue is the missing unzip command in bazel...

@jovany-wang
Copy link

So anything that you need I do?

@Hoeze
Copy link
Author

Hoeze commented Nov 9, 2022

@jovany-wang Can you try to get the java build working?
I.e. fix the unzip issue in bazel?

@Hoeze
Copy link
Author

Hoeze commented May 13, 2023

Hi, is there any progress on Java support in the package?

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.

5 participants