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

Adjust wheel build tags appropriately for new built component #512

Closed
vyasr opened this issue Oct 23, 2024 · 1 comment
Closed

Adjust wheel build tags appropriately for new built component #512

vyasr opened this issue Oct 23, 2024 · 1 comment

Comments

@vyasr
Copy link
Contributor

vyasr commented Oct 23, 2024

With the addition of curl to the build, the libkvikio wheels now have a compiled component. They are not being run through auditwheel, though, and I'm not sure auditwheel would even detect this as a valid wheel to audit because libcurl is built as a static library so there are no DSOs in the wheel. The problem is that the libkvikio wheels are now being shipped with linux rather than manylinux tags, so they won't be as portable as we need them to be. A bigger problem is that the libcurl archive may not be portable anyway if it embeds any noncompliant symbols, but we have no easy way to check that without running auditwheel. I think what we really want is for curl to only be built as part of the C++ interface (i.e. when the Python build or some other consumer like libcudf finds the C++), not as part of the C++ build.

rapids-bot bot pushed a commit that referenced this issue Nov 6, 2024
Moving libcurl dependent functions to `remote_handle.cpp` (let's move the rest in a follow up PR) and setup building `libkvikio.so`.

### Background 

Now that KvikIO has evolved into a standalone IO library that works without cuFile and CUDA, I think it makes sense to move to a shared library. Originally, KvikIO was a very thin wrapper around cuFile, which is why we decided to keep it header-only (not because of templates). 

**Pros**:
  - Enables us to statically compile libcurl into `libkvikio.so`. 
    * Required by our wheels. If we don’t do this, we will have to find another solution to #512 
  - Avoid having to compile libcurl in downstream projects. Currently, cudf must compile libcurl in every build. 
  - Reduce compile time in CI, both for KvikIO and downstream projects. 
  - Ease development by not having to rebuild downstream projects like cudf every time KvikIO is modified. 

**Cons**: 
 - Projects cannot vendoring KvikIO as a header-only project. As far as we know, nobody does this.

Authors:
  - Mads R. B. Kristensen (https://github.com/madsbk)
  - Paul Taylor (https://github.com/trxcllnt)

Approvers:
  - Robert Maynard (https://github.com/robertmaynard)
  - Vyas Ramasubramani (https://github.com/vyasr)
  - Bradley Dice (https://github.com/bdice)

URL: #527
@vyasr
Copy link
Contributor Author

vyasr commented Nov 7, 2024

This was resolved in #527 by compiling and shipping a shared library for libkvikio and statically linking libcurl into that while hiding its symbols so that we would stop shipping libcurl.

@vyasr vyasr closed this as completed Nov 7, 2024
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

No branches or pull requests

1 participant