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

Use CURL imported target instead of variables set by find_package #412

Merged

Conversation

shailpatels
Copy link
Contributor

Patch I made locally to fixes #411

Instead of relying on the variables set by find_package this uses an imported target from the curl cmake files to ensure libcurl is available. I'm not fully sure why find_package wasn't getting the full set of curl libraries required, maybe it was due to multiple installations of libcurl or if it was built locally? But it seems like using an imported target is generally preferred in newer cmake

This did require bumping the min version of cmake supported since the imported target was only added in 3.12. I'm not sure if thats an issue or not, an alternative solution would be to use find_package and explicitly request libcurl as a required component

Instead of relying on the variables set by find_package
this uses an imported target from the curl cmake files
to ensure libcurl is available
@SimonKagstrom
Copy link
Owner

I don't think that should be an issue, in general. However, my (old) ARM/PPC etc github action use an older Ubuntu which doesn't have cmake 3.12, so those have to be updated. I can take a look at that, unless you feel like doing it :-)

Also, the Mac OS big sur build can be disabled now.

@shailpatels
Copy link
Contributor Author

I don't think that should be an issue, in general. However, my (old) ARM/PPC etc github action use an older Ubuntu which doesn't have cmake 3.12, so those have to be updated. I can take a look at that, unless you feel like doing it :-)

Also, the Mac OS big sur build can be disabled now.

Sure thing! I bumped Ubuntu to ubuntu22.04 and updated the run-on-arch GH action while I was there to the latest version (assuming thats whats preferred) and the macos big sur job should be disabled now

@SimonKagstrom SimonKagstrom merged commit e969e19 into SimonKagstrom:master Jan 7, 2024
1 of 8 checks passed
@SimonKagstrom
Copy link
Owner

Thanks, merging!

There are still some CI issues, but I'll resolve those. The CI was unstable also before your changes, so not caused by you anyway!

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.

Undefined reference to curl libraries when trying to build kcov
2 participants