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

Fix building with cmake on windows #806

Merged
merged 2 commits into from
Nov 1, 2022

Conversation

oyvindln
Copy link
Contributor

@oyvindln oyvindln commented Oct 17, 2022

Neither qwt nor pkg-config seems to include a .pc file on windows, so this borrows a findQwt script from QGis to make handling it easier.

The other addition needed was to define _USE_MATH_DEFINES as M_PI is a non-standard extension and as such not available from <cmath> when compiling with MSVC otherwise.

Now builds with cmake+msbuild on windows if the needed packages are installed via vcpkg and using the ms dev environment command line and vcpkg cmake toolchain file.

ld-ldf-reader is still disabled for windows as it uses some linux/unix-specific functions, also disable it for macos as @kaliohelix on discord reported it didn't build properly there either.

@atsampson
Copy link
Collaborator

Please only fall back to FindQwt if pkg-config doesn't work (and to be honest, I think you'd do better to report a bug in the Windows packaging of Qwt you're using if they aren't providing the .pc file). As we found when testing this across different Linux distributions, FindQwt's approach of guessing wildly at installation directories does not work reliably.

@oyvindln
Copy link
Contributor Author

oyvindln commented Oct 17, 2022

Ok, it should now try with pkg-config first.

Which distros does it fail on? It's a different find script than what was used last previously.

It looks like qwt explicitly disables pkg-config generation on non-UNIX systems:
https://sourceforge.net/p/qwt/git/ci/develop/tree/qwtconfig.pri
Also Qt 6.0 and 6.1 lacked some pkg-config stuff. It's possible the config to enable it for qwt works on win but I don't know.

@oyvindln
Copy link
Contributor Author

Also changed the conditional for ld-ldf-reader, seems the LINUX variable was added only in the last version.

@oyvindln
Copy link
Contributor Author

bump @atsampson

@happycube happycube merged commit 0983625 into happycube:master Nov 1, 2022
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