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

Avoid big-scary cmake warning for optional lib #446

Merged
merged 1 commit into from Dec 1, 2022
Merged

Avoid big-scary cmake warning for optional lib #446

merged 1 commit into from Dec 1, 2022

Conversation

zander
Copy link
Contributor

@zander zander commented Nov 28, 2022

As the lib is optional, lets avoid the big CMake warning when the opencv library is not found. It mostly has info for the maintainers, not for people building this.

Instead add a one line message stating the cause and effect.

@axxel
Copy link
Collaborator

axxel commented Nov 29, 2022

Question: do you have Qt installed or does the complaint from the find_package(Qt5 COMPONENTS Gui Multimedia Quick) line not annoy you as much as the OpenCV one does?

@zander
Copy link
Contributor Author

zander commented Nov 29, 2022

Frankly, I didn't notice Qt not being a non-optional requirement until after I made this PR.

So, to answer your question, I have it installed.

@axxel
Copy link
Collaborator

axxel commented Nov 30, 2022

I see. I checked the cmake output of your PR on the CI system where there is neither Qt nor OpenCV installed: https://github.com/zxing-cpp/zxing-cpp/actions/runs/3564843023/jobs/6010005518

The output now looks like:

CMake Warning at example/CMakeLists.txt:26 (find_package):
  By not providing "FindQt5.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "Qt5", but
  CMake did not find one.

  Could not find a package configuration file provided by "Qt5" with any of
  the following names:

    Qt5Config.cmake
    qt5-config.cmake

  Add the installation prefix of "Qt5" to CMAKE_PREFIX_PATH or set "Qt5_DIR"
  to a directory containing one of the above files.  If "Qt5" provides a
  separate development package or SDK, be sure it has been installed.


INFO: Missing OpenCV dependency, skipping building of example ZXingOpenCV

Would you be willing to ammend your PR to do the same thing for Qt, so we get a consistent behavior?

example/CMakeLists.txt Outdated Show resolved Hide resolved
@axxel
Copy link
Collaborator

axxel commented Dec 1, 2022

Thanks for the update. It builds again. Looking at the output now:

INFO: Qt not found, skipping examples based on them. (need modules Multimedia + Quick)
INFO: Missing OpenCV dependency, skipping building of example ZXingOpenCV

I'd like to kindly ask to make one minor improvement: please use the same phrasing for both cases as they convey the exact same kind of information. Here is my proposal:

INFO: Qt (Gui/Multimedia/Quick) not found, skipping Qt examples
INFO: OpenCV not found, skipping ZXingOpenCV example

As the libs are optional, lets avoid the big CMake warning when the opencv
library or Qt is not found. It mostly has info for the maintainers, not for
people building this.

Instead add a one line message stating the cause and effect.
@axxel axxel merged commit 4c2c3bd into zxing-cpp:master Dec 1, 2022
@axxel
Copy link
Collaborator

axxel commented Dec 1, 2022

Awesome. Thanks for your contribution (and patience ;)).

@zander zander deleted the cmakeWarn branch December 2, 2022 19:03
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

2 participants