Page MenuHome

Cleanup: CMake warnings related to "find package" modules
ClosedPublic

Authored by Sebastian Parborg (zeddb) on Aug 7 2020, 2:53 PM.

Details

Summary

When creating the build directory with cmake it would print warnings like:

CMake Warning (dev) at /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:273 (message):
  The package name passed to `find_package_handle_standard_args` (ALEMBIC)
  does not match the name of the calling package (Alembic).  This can lead to
  problems in calling code that expects `find_package` result variables
  (e.g., `_FOUND`) to follow a certain pattern.

I looked at how we did it for other modules and made the package with these warnings adopt the same style.

I also removed the EMBREE_LIBRARY variable as it seems like it was not used.
For me this showed up as a semi warning as module as set to FOUND but the LIBRARY was set to NOT_FOUND.
This seems to have been an oversight as the embree library consists of may smaller libraries, and we correctly search for those.
(@Brecht Van Lommel (brecht) or is it possible to build embree as a big blob of a library?)

With this I also noticed that some modules state that they set and use the _LIBRARY variable but the do in fact not do this.
I removed these comments from those files.

I've tested these changes and I don't think it will break anything, but I will put this up to review to make sure.

Diff Detail

Event Timeline

Sebastian Parborg (zeddb) requested review of this revision.Aug 7 2020, 2:53 PM
Sebastian Parborg (zeddb) created this revision.

We use none of the findpackages on windows currently (although it be nice to transition to it at one point now we can constrain cmake to our libfolder) so testing on windows doesn't make a whole lot of sense right now.

just skimming over the changes LGTM though.

This revision is now accepted and ready to land.Aug 7 2020, 3:10 PM

Looks good overall, except one thing.

build_files/cmake/Modules/FindOSL.cmake
88

I think this needs FOUND_VAR OSL_FOUND.

CMake will automatically do uppercase, but it can't abbreviate.

Brecht Van Lommel (brecht) requested changes to this revision.Aug 7 2020, 3:12 PM
This revision now requires changes to proceed.Aug 7 2020, 3:12 PM
Sebastian Parborg (zeddb) marked an inline comment as done.

Fixed the OSL naming issue as pointed out by Brecht.

This revision is now accepted and ready to land.Aug 7 2020, 4:42 PM

Ugh, forgot to include this review in the commit message. -_-

rBbe83b8f456634cb627ef12a4904a5916a3d880f3