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

[DOC] Update contributing guide re: info for building RAPIDS JNI #1084

Closed
razajafri opened this issue Apr 18, 2023 · 3 comments · Fixed by #1124
Closed

[DOC] Update contributing guide re: info for building RAPIDS JNI #1084

razajafri opened this issue Apr 18, 2023 · 3 comments · Fixed by #1124
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@razajafri
Copy link
Collaborator

Is your feature request related to a problem? Please describe.
Today, when we build spark-rapids-jni, it builds the libcudfjni but it bypasses the pom inside cudf/java. Which could result in a stale pom.xml inside cudf/java.

Describe the solution you'd like
I assume, it might be possible to build libcudfjni by using it's own pom.xml

@jlowe
Copy link
Member

jlowe commented Apr 18, 2023

I'm not sure this makes sense. spark-rapids-jni wants to build the GPU code as a single shared library with the CUDA runtime only statically linked once. If cudf/java builds libcudf.so, as it normally does, then we cannot build another libcudf.so based on that artifact. We need cudf/java to build static libs, not shared libs. If it builds static libs, cudf/java cannot run its Java tests since there's no .so to load. That's why spark-rapids-jni pulls in the Java sources from cudf/java explicitly, since that's the first point where we can run tests on the "official" artifact for spark-rapids-jni. It's important to ensure the cudf tests on the final artifact, not just some intermediate form). Also if cudf/java builds in some sort of different way for use by spark-rapids-jni, it needs to use a classifier or some other mechanism to distinguish that artifact from the typical artifact for cudf.

I'm not convinced the difficulties of reorganizing the cudf/java pom and artifact naming is worth the benefit, especially since we want to run the cudf/Java tests on the final spark-rapids-jni artifact to ensure everything has been packaged properly.

@gerashegalov
Copy link
Collaborator

We could add clarify this nuance in the CONTRIBUTING.md

@razajafri
Copy link
Collaborator Author

Thank you for the clarification @jlowe

We could add clarify this nuance in the CONTRIBUTING.md

Yeah, that might be helpful.

@mattahrens mattahrens added documentation Improvements or additions to documentation and removed ? - Needs Triage feature request labels Apr 18, 2023
@mattahrens mattahrens changed the title [FEA] Build RAPIDS JNI using it's dedicated pom.xml [DOC] Update contributing guide re: info for building RAPIDS JNI Apr 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants