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

[C] Add support for CMake package #90

Merged
merged 1 commit into from
Aug 29, 2022

Conversation

kou
Copy link
Member

@kou kou commented Aug 29, 2022

fix #89

Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

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

Thanks!

As you can see, the CMake config is mostly copy-pasted from Arrow - I wonder if you have general thoughts about continuing to do this, or trying to build up the CMake config from first principles (like nanoarrow did)

@lidavidm lidavidm merged commit 06821ec into apache:main Aug 29, 2022
@kou kou deleted the c-manager-cmake-package branch August 29, 2022 13:38
@kou
Copy link
Member Author

kou commented Aug 29, 2022

I think that "trying to build up the CMake config from first principles" is better for maintainability. I think that ADBC can be built with simple build config. (Arrow needs complex build config because it has many components, depends on many libraries and so on...)
Arrow's common CMake modules have some helpful features but it's complex. We'll refactor Arrow's CMake modules in the future Arrow development. Backporting changes from Arrow to ADBC may be difficult.

FYI: The changes for BuildUtils.cmake in this pull request is backported from apache/arrow#13892 .

@lidavidm lidavidm mentioned this pull request Aug 29, 2022
1 task
@lidavidm
Copy link
Member

Makes sense - I filed #92.

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.

[C] Add support for CMake package
2 participants