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

Tweaks for Mac compilation. #409

Merged
merged 4 commits into from
Jan 19, 2020
Merged

Tweaks for Mac compilation. #409

merged 4 commits into from
Jan 19, 2020

Conversation

Gamnn
Copy link
Contributor

@Gamnn Gamnn commented Jan 18, 2020

Allows building with homebrew-installed qwt, and works around QT bug on macOS 10.12 (and possibly others) where ld-analyse wouldn't refresh after pressing frame buttons.

@Gamnn Gamnn requested a review from simoninns January 18, 2020 02:33
Copy link
Collaborator

@simoninns simoninns left a comment

Choose a reason for hiding this comment

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

Seems to me that most of these changes are because QWT and opencv2 aren't installed in the correct place within the build environment. Would it not be a lot neater to correct the build environment rather than the project - especially since there is no way to test building in proprietary OSs, so the chance of regression is very high?

The Qt bug work-arounds are a different thing; that just is as it is.

@atsampson
Copy link
Collaborator

Adding -I/usr/include/qwt then doing #include <qwt_whatever.h> is probably the neatest fix - that's what GNURadio does, for example. (While installing Qwt's headers into a subdirectory isn't the default in Qwt's build system, it's the usual practice in GNU/Linux distros to reduce /usr/include pollution - e.g. anything Debian- or CentOS-derived has that layout - so I'd be tempted to suggest that Homebrew should do the same.)

For isnanf, add #include <cmath> and change the code to use isnan instead? It has overloads for float and double in C++ (and in C these days).

It would be a good idea to use git rebase -i and git push -f to squash your updates into your original commit so you don't break bisection.

@Gamnn
Copy link
Contributor Author

Gamnn commented Jan 18, 2020

Fixed as @atsampson suggested. I think the homebrew-specific stuff should be okay for now; the paths are no longer version-specific at least. So it shouldn't break. I'd like to keep the mac building instructions simple by just needing to run brew install, and not have to do manual linking.

As an aside, I think there'll still be building issues on some other non-Ubuntu distros, due to Ubuntu's qwt-qt5 naming on it instead of qwt, but I'm uncertain of an easy work-around for that. I've left a comment in the code as a hint though.

@simoninns simoninns merged commit 091653e into happycube:master Jan 19, 2020
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.

3 participants