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

Allow using system provided json library #1970

Merged
merged 1 commit into from
Dec 26, 2023

Conversation

mattiaverga
Copy link
Contributor

Some Linux distributions have their own Niels Lohmann's json library packaged, this will add an option to use the system provided one (default to use bundled library).

@mattiaverga
Copy link
Contributor Author

I think I should also make indi provide a HAVE_BUNDLED_JSON flag, so indi-3rdparty targets requiring json.h know that they should require system's json library... but I'm not expert in cmake and didn't found a way to do that, yet.

@knro
Copy link
Contributor

knro commented Dec 24, 2023

Can you please resolve the conflicts above?

@mattiaverga
Copy link
Contributor Author

Rebase done.

About indi-3rdparty, currently both project use the INDI_SYSTEM_JSONLIB option manually.
I think I could have 3rdparty to check for the indi provided json library in FindINDI.cmake and require the system library if indilib was built with system json, but I think I would need to change the indi provided json header file name to distinguish from the upstream one. AFAIK, using find_path() will always check in system's paths, so using the same name for indi provided and upstream header file will not distinguish the two...

@knro
Copy link
Contributor

knro commented Dec 25, 2023

I think using a different header name is OK if this makes it work across indi-core and indi-3rdparty reliably in both scenarios.

@mattiaverga
Copy link
Contributor Author

I will update indilib/indi-3rdparty#861 ASAP

@mattiaverga
Copy link
Contributor Author

Do we also want to add a workflow to test build using system libraries (json and http)?

@knro knro merged commit 5718ec7 into indilib:master Dec 26, 2023
11 checks passed
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.

2 participants