allow CMAKE_PREFIX_PATH discovery mechanism to work

[chadrik]

We use a package management that sets CMAKE_PREFIX_PATH, CMAKE_LIBRARY_PATH, and CMAKE_INCLUDE_PATH cmake environment variables for each dependent package pulled into a build. This allows them to be discovered by other projects that use find_path and find_library commands in cmake. Everything works as expected in USD -- the includes and libraries are found by cmake -- but USD's Find*.cmake modules add the root/base dir variables as REQUIRED_VARS even though they are not used elsewhere in the build process or absolutely necessary to find the libraries. These should be optional.

Regarding these changes to FindGLEW.cmake:

-            NO_DEFAULT_PATH
+            NO_SYSTEM_ENVIRONMENT_PATH
+            NO_CMAKE_SYSTEM_PATH

I don't know why NO_DEFAULT_PATH was added in the first place, but this is very restrictive. I opened it up just a bit. From the cmake docs for find_library:

Additional search locations can be specified after the PATHS argument. If ENV var is found in the HINTS or PATHS section the environment variable var will be read and converted from a system environment variable to a cmake style list of paths. For example ENV PATH would be a way to list the system path variable. The argument after DOC will be used for the documentation string in the cache. PATH_SUFFIXES specifies additional subdirectories to check below each search path.

If NO_DEFAULT_PATH is specified, then no additional paths are added to the search. If NO_DEFAULT_PATH is not specified, the search process is as follows:

1. Search paths specified in cmake-specific cache variables. These are intended to be used on the command line with a -DVAR=value. This can be skipped if NO_CMAKE_PATH is passed.
   /lib/ if CMAKE_LIBRARY_ARCHITECTURE is set, and /lib for each in > - CMAKE_PREFIX_PATH

• CMAKE_LIBRARY_PATH
• CMAKE_FRAMEWORK_PATH

2. Search paths specified in cmake-specific environment variables. These are intended to be set in the user’s shell configuration. This can be skipped if NO_CMAKE_ENVIRONMENT_PATH is passed.
   /lib/ if CMAKE_LIBRARY_ARCHITECTURE is set, and /lib for each in > - CMAKE_PREFIX_PATH

• CMAKE_LIBRARY_PATH
• CMAKE_FRAMEWORK_PATH

3. Search the paths specified by the HINTS option. These should be paths computed by system introspection, such as a hint provided by the location of another item already found. Hard-coded guesses should be specified with the PATHS option.

4. Search the standard system environment variables. This can be skipped if NO_SYSTEM_ENVIRONMENT_PATH is an argument.

• PATH and LIB

5. Search cmake variables defined in the Platform files for the current system. This can be skipped if NO_CMAKE_SYSTEM_PATH is passed.

• /lib/ if CMAKE_LIBRARY_ARCHITECTURE is set, and /lib for each in > - CMAKE_SYSTEM_PREFIX_PATH
• CMAKE_SYSTEM_LIBRARY_PATH
• CMAKE_SYSTEM_FRAMEWORK_PATH

6. Search the paths specified by the PATHS option or in the short-hand version of the command. These are typically hard-coded guesses.

In summary, removing NO_DEFAULT_PATH enables 4 additional search routines (3 & 6 are arguments to the command itself and I don't think that NO_DEFAULT_PATH disables these).
I need 1 & 2 enabled, but to be safe I disabled 4 & 5 using NO_SYSTEM_ENVIRONMENT_PATH and NO_CMAKE_SYSTEM_PATH.

Btw, I've signed the Pixar USD CLA.

[sunyab]

Hi @chadrik, thanks for the details! Assigning to myself to handle the merge.

[chadrik]

Sounds good. Any updates?

[sunyab]

Sorry @chadrik, we haven't had a chance to review this internally yet. We'll try to get to this as soon as we can.

[chadrik]

ok. no worries.

[sunyab]

@chadrik, just to confirm -- the changes to the other modules besides FindGLEW.cmake aren't strictly necessary for your build, and was more of a cleanup change, right?

If so, I think we'd prefer to leave those files as-is. While those base dir variables aren't required, they should always be present anyway if the rest of the required variables are found. Also, having them there gives us nicer status messages from CMake when it finds packages, like:

-- Found OpenImageIO: /usr/local/openimageio-1.6.14 (found version "1.6.14")

[chadrik]

just to confirm -- the changes to the other modules besides FindGLEW.cmake aren't strictly necessary for your build, and was more of a cleanup change, right?

@sunyab, all of the changes are required for my build to work. When building I'm not providing OPENEXR_ROOT_DIR, OPENEXR_ROOT_DIR, or OIIO_BASE_DIR, and I'm instead relying on cmake's discovery mechanism to find these packages (for more info, see the link for cmake env variables posted above). Making them required forces me to use them instead of cmake's discovery mechanism, which I don't want to do.

Also, having them there gives us nicer status messages from CMake when it finds packages, like:

It appears that FIND_PACKAGE_HANDLE_STANDARD_ARGS prints the first variable passed to it. As a compromise, we could place the include directory variables first, which would print something close to what you're getting now.

[chadrik]

Hi, any updates? There are a number of people on the mailing lists looking for help using rez to build USD and I think our method of using env variables to add to cmake's search path can really simplify the process, but they can't use it unless this change gets merged. Thanks!

[drwave]

Sunya has been on vacation for the past several weeks. I’m sure he’ll be able to respond some time early next week.

On Oct 15, 2016, at 8:02 AM, Chad Dombrova [email protected] wrote:

Hi, any updates? There are a number of people on the mailing lists looking for help using rez to build USD and I think our method of using env variables to add to cmake's search path can really simplify the process, but they can't use it unless this change gets merged. Thanks!

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub #36 (comment), or mute the thread https:/notifications/unsubscribe-auth/ABmgOk8_Lt_exQHSEzFTJxl7-oizKooEks5q0OrpgaJpZM4JyK2m.

[sunyab]

Hey @chadrik, sorry again for the delay. I discussed your changes with some folks here and we think that idea sounds good. Would you mind moving the OIIO_INCLUDE_DIRS parameter in FindOpenImageIO.cmake to the top of the REQUIRED_VARS section? Once you do that, I'll merge the pull request in.