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

Add cmake support #60

Merged
merged 8 commits into from Nov 2, 2021
Merged

Add cmake support #60

merged 8 commits into from Nov 2, 2021

Conversation

fritzm
Copy link
Contributor

@fritzm fritzm commented Oct 28, 2021

Adds ability to build package with CMake in addition to eupspkg (allows to be included as submodule in other CMake projects, e.g. Qserv.)

@fritzm fritzm requested review from mwittgen and ktlim October 28, 2021 06:38
@fritzm fritzm marked this pull request as ready for review October 28, 2021 06:38
@timj
Copy link
Member

timj commented Oct 28, 2021

Does log really need base? If it doesn't, can you create a github action that builds this package and runs the tests with cmake? You can install utils using pip as daf_butler already does (it looks like you only use one python functions from utils). This would be great since it would allow us to run tests for every PR without relying on jenkins and make sure that no-one breaks cmake.

@fritzm
Copy link
Contributor Author

fritzm commented Oct 28, 2021

Does log really need base?

Good question -- I didn't really re-examine the eups table files etc. in this work. I can try some experiments and see, though...

@timj
Copy link
Member

timj commented Oct 28, 2021

Just try a GitHub build action using cmake and pip install of utils and see what happens. You can look at daf_butler or utils github actions for help and you will need to install some python packages (utils is a better template because it doesn't use conda and just pip installs -- daf_butler show you how to pip install utils into the action environment though).

src/CMakeLists.txt Outdated Show resolved Hide resolved
@fritzm
Copy link
Contributor Author

fritzm commented Nov 2, 2021

@timj Okay, took about a dozen commits to get that straight :-) Squashed and re-pushed. Give a look and see what you think?

@fritzm
Copy link
Contributor Author

fritzm commented Nov 2, 2021

@timj Okay, GHA is running python tests against the cmake-built artifacts now. I will to similar for the sphgeom PR.

Copy link
Member

@timj timj 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 adding the python tests.

@fritzm fritzm merged commit e5750ce into master Nov 2, 2021
@fritzm fritzm deleted the tickets/DM-23308 branch November 2, 2021 21:53
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

3 participants