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

Update cppbuild to ensure container has cmake and rapidjson dependencies #1412

Closed
wants to merge 1 commit into from

Conversation

jbkyang-nvi
Copy link
Contributor

No description provided.

@saudet
Copy link
Member

saudet commented Sep 12, 2023

Since this modifies the system, please add that to the workflow instead

@jbkyang-nvi
Copy link
Contributor Author

jbkyang-nvi commented Sep 12, 2023

@saudet This is only for when people want to use javacpp directly within the Triton container. Currently they need to apt get gpg wget rapidjson-dev cmake before they do:

git clone https://github.com/bytedeco/javacpp-presets.git
cd javacpp-presets
mvn clean install --projects .,tritonserver
mvn clean install -f platform --projects ../tritonserver/platform -Djavacpp.platform.host

With the current changes the system installations are done with cppbuild.sh.

If you feel strongly against this change, we can add additional install instructions for TritonCPP + Javacpp on the README on the Triton side.

@saudet
Copy link
Member

saudet commented Sep 12, 2023

Then that's problem with the Docker image of Triton. That should probably be added to this file maybe?
https://github.com/triton-inference-server/server/blob/main/Dockerfile.sdk

@jbkyang-nvi
Copy link
Contributor Author

The developer_tools related bindings are not installed in the Triton container or the Triton SDK container by choice. I don't think it makes sense to install more dependencies in anticipation the the user will use CAPI bindings or Java CAPI bindings

@saudet
Copy link
Member

saudet commented Sep 14, 2023

I suppose, but you could offer other container images that do have the required build dependencies. Anyway, most Java developers don't build from source, so this isn't something we need. Just offer prebuilt binaries and they will use those.

@jbkyang-nvi
Copy link
Contributor Author

Yep we provide the jars anyway and they can use the bytedeco pre-built binaries. Just wanted a complete set of build instructions so closing this PR and instead submitting #1413 for review 🙏

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.

None yet

2 participants