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

Multiple CMake fixes and version updates to resolve compiler warnings #738

Merged
merged 6 commits into from
Dec 20, 2022
Merged

Multiple CMake fixes and version updates to resolve compiler warnings #738

merged 6 commits into from
Dec 20, 2022

Conversation

galenguyer
Copy link
Contributor

@galenguyer galenguyer commented Nov 29, 2022

The output of cmake and make was pretty noisy so I updated some stuff to clean that up. Change list:

  • Boost complains about the use of global placeholders. Defining BOOST_BIND_GLOBAL_PLACEHOLDERS retains the current behavior
  • The declaration of SPDLOG_FMT_EXTERNAL=ON conflicts with the declaration of SPDLOG_FMT_EXTERNAL in the spdlog CMake instructions. Making the declaration identical resolves the conflict (removing the declaration means it doesn't get built right).
  • Compiling on Arch with Boost 1.80 produces deprecation warnings for C++ standards below 14 that will take effect in July 2023. C++17 is the newest supported C++ standard and is supported by Debian 10 and newer as well as Ubuntu 20.04 and newer.
  • Updating C++ to C++14 or newer leads the version of Eigen vendored to throw deprecation warnings for std::unary_negate and std::binary_negate, which are deprecated in C++17 and removed in C++20. The vendored version of Eigen is at least 5 years old, and the deprecation warning is fixed in Eigen 3.3. Eigen 3.4 has also been released and appears to work without issue, so I've updated the vendored library to that version.
  • The OsmoSDR include directory is never found because the wrong pkg-config name is used and the wrong folders are searched in. This is resolved.

@robotastic
Copy link
Owner

I am liking where this is going! Thanks!

@galenguyer galenguyer marked this pull request as ready for review December 4, 2022 19:45
@galenguyer
Copy link
Contributor Author

I've tested that it builds without errors or warnings on Ubuntu 20.04, 22.04, Debian 10 and 11, and Arch Linux. I'll probably enable some good debugging/optimization flags but that's a PR for another time.

galenguyer and others added 6 commits December 5, 2022 13:31
The practice of declaring the Bind placeholders in the
global namespace is deprecated. Setting BOOST_BIND_GLOBAL_PLACEHOLDERS
will retain the old behavior.
The spdlog package sets SPDLOG_FMT_EXTERNAL internally,
and then we re-declare it as SPDLOG_FMT_EXTERNAL=ON. This
causes a warning. By simply setting SPDLOG_FMT_EXTERNAL,
the declaration is the same and the warning stops
Boost.Math will require C++14 or greater in release 1.82,
which is scheduled for July 2023. C++17 is the latest
widely supported version so there seems to be no reason
to not go to it now.
The current version of Eigen that is vendored in lib/lfsr
is 5 to 6 years old (between commits 6bceebfab to 68e8f2b83).
Additionally, increasing the C++ version results in warnings about
std::unary_negate and std::binary_negate, which are deprecated in
C++17 and removed in C++20. Eigen 3.3 contains a fix for this.
The CMake file to find the include dirs for
gnuradio-osmosdr checks the wrong pkg module name,
and has trailing paths appended.
@robotastic
Copy link
Owner

Amazing - let's do this. I made a few small tweaks to also cleanup some of the code related warnings.

@robotastic robotastic merged commit 7c57eb5 into robotastic:master Dec 20, 2022
@gisforgirard
Copy link
Contributor

i have nothing to add i just wanted to say wow what an impressive pull request looks like quite a bit of work went into this thanks Galen!

jessm33 added a commit to jessm33/trunk-recorder-mqtt-status that referenced this pull request Jan 14, 2023
taclane pushed a commit to taclane/trunk-recorder-mqtt-status that referenced this pull request May 22, 2023
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