Skip to content

Conversation

@robertmaynard
Copy link
Contributor

In support of building a stable C interface for libcuvs I have moved all C code into a separate directory for both the headers and sources. This will make it easier to install just 'C' language headers, make it easier to apply code rules for C headers, and in the future add more CI checks for our C API.

@robertmaynard
Copy link
Contributor Author

By moving this into a separate header it will reduce the complexity of reviewing #1348

@benfred
Copy link
Member

benfred commented Sep 23, 2025

Rather than put in the c headers incpp/include/cuvs/c/ - how about we create a top level c directory like we have for cpp, go, rust, python, and put the C headers and source in there rather than in the cpp/ directory? (like c/include/cuvs/neighbors/cagra.h instead of cpp/include/cuvs/c/neighbors/cagra.h )

Right now the user has to add an extra /c/ to the path when including the header files - and I'd rather avoid changing what the include path looks like .

@robertmaynard robertmaynard changed the base branch from branch-25.10 to branch-25.12 September 25, 2025 13:32
@robertmaynard
Copy link
Contributor Author

Rather than put in the c headers incpp/include/cuvs/c/ - how about we create a top level c directory like we have for cpp, go, rust, python, and put the C headers and source in there rather than in the cpp/ directory? (like c/include/cuvs/neighbors/cagra.h instead of cpp/include/cuvs/c/neighbors/cagra.h )

Right now the user has to add an extra /c/ to the path when including the header files - and I'd rather avoid changing what the include path looks like .

This opens us to incorrect installs of libcuvs or libcuvs_c since when we install we flatten everything to the same install location of include/cuvs/. That means we would have to either manually validate that we don't introduce any .h files under /cpp/include or use some of the tooling from package managers like conda to warn us.

The current model of the source tree layout is that the cpp, rust, go are all roots for build invcocations. If we elevate c to that level ( happy to do so ) we should also seriously consider making it a seperate build instead of a component of the cpp build.

@robertmaynard
Copy link
Contributor Author

I am happy to try moving everything over to cuvs/c/src and cuvs/c/include while we discuss if we like that layout / side-effects

@robertmaynard robertmaynard force-pushed the fea/move_capi_files_to_separate_directory branch from 6877a1b to 782af85 Compare September 25, 2025 14:07
@robertmaynard robertmaynard force-pushed the fea/move_capi_files_to_separate_directory branch from 782af85 to 03c0a2c Compare October 1, 2025 20:57
@robertmaynard robertmaynard force-pushed the fea/move_capi_files_to_separate_directory branch from 805a825 to 80df255 Compare October 7, 2025 14:29
@robertmaynard robertmaynard requested review from a team as code owners October 7, 2025 19:46
@mythrocks
Copy link
Contributor

/ok to test 8afc1c1

@robertmaynard robertmaynard requested a review from a team as a code owner October 8, 2025 13:06
Copy link
Member

@benfred benfred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for these changes! I think this looks great, and I like how clean the separation is between the c and c++ code with this change -

@robertmaynard robertmaynard requested a review from a team as a code owner October 13, 2025 13:47
@robertmaynard
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 895515c into rapidsai:branch-25.12 Oct 13, 2025
161 of 164 checks passed
@robertmaynard robertmaynard deleted the fea/move_capi_files_to_separate_directory branch October 13, 2025 19:23
enp1s0 pushed a commit to enp1s0/cuvs that referenced this pull request Oct 22, 2025
In support of building a stable C interface for libcuvs I have moved all `C` code into a separate directory for both the headers and sources. This will make it easier to install just 'C' language headers, make it easier to apply code rules for C headers, and in the future add more CI checks for our C API.

Authors:
  - Robert Maynard (https://github.com/robertmaynard)
  - MithunR (https://github.com/mythrocks)
  - Ben Frederickson (https://github.com/benfred)

Approvers:
  - Kyle Edwards (https://github.com/KyleFromNVIDIA)
  - Ben Frederickson (https://github.com/benfred)

URL: rapidsai#1357
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Introduces a breaking change feature request New feature or request

Development

Successfully merging this pull request may close these issues.

4 participants