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

Improve pxr CMake Config support by using CMakeFindDependencyMacro CMake module instead of raw find_package #3205

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

theblackunknown
Copy link

@theblackunknown theblackunknown commented Aug 4, 2024

Description of Change(s)

This PR improves the pxr CMake Config supports by using find_dependency from CMakeFindDependencyMacro CMake module instead of raw find_package.

Doing so handling of REQUIRED is not needing anymore as it will be transively forwarded from the find_pckage(pxr ...) calls:

  • find_package(pxr REQUIRED ...) results in dependencies being required too
  • find_package(pxr ...) results in dependencies being optional too

More information on CMakeFindDependencyMacro CMake module documentation page.

  • I have verified that all unit tests pass with the proposed changes
  • I have submitted a signed Contributor License Agreement

…CMake module instead of raw `find_package`
@theblackunknown theblackunknown changed the base branch from release to dev August 4, 2024 14:37
@theblackunknown theblackunknown marked this pull request as ready for review August 5, 2024 08:16
@jesschimein
Copy link
Collaborator

Filed as internal issue #USD-9932

@jesschimein
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@theblackunknown
Copy link
Author

It looks like the Linux build went through but the CI was cancelled, any idea what went wrong ?

@sunyab
Copy link
Contributor

sunyab commented Aug 5, 2024

@theblackunknown It looks like the build succeeded but there was some kind of hiccup on the Azure side of things. I requeued the job to double-check.

@theblackunknown
Copy link
Author

Hi @sunyab what are my chances to integrate my change for the next release ?

@asluk asluk added the build Build-related issue/PR label Oct 25, 2024
@parkerlreed
Copy link

parkerlreed commented Dec 28, 2024

Maybe #3310 can be included in this?

Currently unable to build f3d as it depends on usd which then depends on materialx. Fails at OpenGL since nothing includes it in the cmake.

Manually adding find_package(OpenGL REQUIRED) below the pxr worked to get a build going in the mean time.

@theblackunknown
Copy link
Author

Maybe #3310 can be included in this?

Currently unable to build f3d as it depends on usd which then depends on materialx. Fails at OpenGL since nothing includes it in the cmake.

Manually adding find_package(OpenGL REQUIRED) below the pxr worked to get a build going in the mean time.

Looking at your issue @parkerlreed, I think I know what is going on and this requires further improvements which are currently not in this PR.
I would be happy to follow up on those but I would like to get this PR merged first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Build-related issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants