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

MAYA-105621 - MayaUSD: install USD from artifact beside plugin #631

Merged
merged 5 commits into from
Jul 10, 2020

Conversation

seando-adsk
Copy link
Collaborator

MAYA-105621 - MayaUSD: install USD from artifact beside plugin

  • When given a USD relative path, set mod file and path/pythonpath accordingly.
  • Specific mod file for Windows (with paths) and Linux/Mac. Added .mod specific tags.

* When given a USD relative path, set mod file and path/pythonpath
  accordingly.
* Specific mod file for Windows (with paths) and Linux/Mac.
  Added .mod specific tags.
Comment on lines +180 to +184
if (DEFINED MAYAUSD_TO_USD_RELATIVE_PATH)
set(USD_INSTALL_LOCATION "${CMAKE_INSTALL_PREFIX}/${MAYAUSD_TO_USD_RELATIVE_PATH}")
else()
set(USD_INSTALL_LOCATION ${PXR_USD_LOCATION})
endif()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When given a USD relative path (from the ADSK internal build system) we will use that for the USD path in the .mod file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: "USD_INSTALL_LOCATION" is a variable inside the module template files.

CMakeLists.txt Outdated
Comment on lines 185 to 189
if (IS_WINDOWS)
configure_file(mayaUSD_Win.mod.template ${PROJECT_BINARY_DIR}/mayaUSD.mod)
else()
configure_file(mayaUSD.mod.template ${PROJECT_BINARY_DIR}/mayaUSD.mod)
endif()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created Windows specific .mod file (with PATH) and one for Linux/Mac (no PATH)

cmake/usd.cmake Outdated
Comment on lines 18 to 22
if (DEFINED MAYAUSD_TO_USD_RELATIVE_PATH)
set(USD_INSTALL_LOCATION "${CMAKE_INSTALL_PREFIX}/${MAYAUSD_TO_USD_RELATIVE_PATH}")
else()
set(USD_INSTALL_LOCATION ${PXR_USD_LOCATION})
endif()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again when given a USD relative path, use that for the PATH and PYTHONPATH.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realized (from a clean build) that this change breaks the build. We need the PATH and PYTHONPATH to usd during the configure step for generating the schemas. So for that step we need to use USD from it's original location.

But then for running the tests we would want PATH & PYTHONPATH to point to the USD in the install folder. I need to think some more how to fix this.

@@ -1,32 +1,37 @@
+ USD ${USD_VERSION} ${PXR_USD_LOCATION}
+ USD ${USD_VERSION} ${USD_INSTALL_LOCATION}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to new variable which is set from top-level CMakeLists.txt above.

Comment on lines +2 to +6
icons:
plug-ins:
presets:
scripts:
resources:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use Maya module specific tags to not add anything specific for these paths since USD is not a Maya plugin and doesn't need Maya to know about its icons/presets/etc.

Comment on lines 3 to 4
PATH+:=bin
PATH+:=lib
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These PATH vars are no needed on Linux/Mac. There is a Windows specific .mod template now.

Comment on lines +10 to +12
icons: maya/lib/usd/usdMaya/resources
plug-ins: maya/plugin
scripts: maya/lib/usd/usdMaya/resources
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use module tags rather than setting the vars directly.

Comment on lines 8 to 9
PATH+:=bin
PATH+:=lib
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Windows specific mod template which just has these extra PATH lines. Otherwise it's identical.

Copy link
Contributor

@HamedSabri-adsk HamedSabri-adsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, Lgtm! Thank you.

@HamedSabri-adsk HamedSabri-adsk added the build Related to building maya-usd repository label Jul 7, 2020
* Moved the Maya module template files into sub-folder in repo.
@seando-adsk seando-adsk added the do-not-merge-yet Development is not finished, PR not ready for merge label Jul 7, 2020
* Reverted previous change to usd.cmake (adjustment to PATH/PYTHONPATH).
  Instead made that adjustment in test.cmake.
cmake/test.cmake Outdated
Comment on lines 200 to 215
# Adjust PATH and PYTHONPATH to include USD.
# These should come last (esp PYTHONPATH, in case another module is overriding
# with pkgutil)
list(APPEND mayaUsd_varname_PATH $ENV{PATH})
list(APPEND mayaUsd_varname_PYTHONPATH $ENV{PYTHONPATH})
if (DEFINED MAYAUSD_TO_USD_RELATIVE_PATH)
set(USD_INSTALL_LOCATION "${CMAKE_INSTALL_PREFIX}/${MAYAUSD_TO_USD_RELATIVE_PATH}")
else()
set(USD_INSTALL_LOCATION ${PXR_USD_LOCATION})
endif()
list(APPEND mayaUsd_varname_PYTHONPATH
"${USD_INSTALL_LOCATION}/lib/python")
if(IS_WINDOWS)
list(APPEND mayaUsd_varname_PATH
"${USD_INSTALL_LOCATION}/bin")
list(APPEND mayaUsd_varname_PATH
"${USD_INSTALL_LOCATION}/lib")
endif()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reverted my change in usd.cmake where I was using the USD from install folder because it was wrong. In our internal builds USD is only copied to install folder during install stage. During configure stage we'll need to use USD from the given input USD location.

However at test time we now check if we were given a USD relative path. If so we use'll that one and add USD to the PYTHONPATH from there.

Here I've only add USD to PATH when on Windows, just like usd.cmake does.

I believe that even all the PATH items above should only need to be done on Windows, but I didn't change them so as to not pollute this PR.

Copy link
Contributor

@HamedSabri-adsk HamedSabri-adsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm. Thanks Sean.

@seando-adsk seando-adsk removed the do-not-merge-yet Development is not finished, PR not ready for merge label Jul 8, 2020
* Fix for test failure on OSX, but also affects Windows/Linux.
Comment on lines +218 to 225
# NOTE: this should come after any setting of PATH/PYTHONPATH so
# that our entries will come first.
# Inherit any existing PATH/PYTHONPATH, but keep it at the end.
# This is needed (especially for PATH) because we will overwrite
# both with the values from our list and we need to keep any
# system entries.
list(APPEND mayaUsd_varname_PATH $ENV{PATH})
list(APPEND mayaUsd_varname_PYTHONPATH $ENV{PYTHONPATH})
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realized this is important, especially for PATH. We will overwrite PATH with the value(s) set here. So we need to make sure we append the existing PATH otherwise we'll erase all the system entries and things won't work.

Copy link
Contributor

@HamedSabri-adsk HamedSabri-adsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! Thanks Sean.

@seando-adsk seando-adsk requested a review from mattyjams July 10, 2020 18:02
@seando-adsk
Copy link
Collaborator Author

@mattyjams Can you have quick look at these changes to make sure nothing conflicts with the way you are building? Most of the changes are around "MAYAUSD_TO_USD_RELATIVE_PATH" which we use in our internal build system to generate relative rpaths. I split the Maya module template file into two (one Windows specific with PATH entries, and the other for Linux/Mac which use rpath). I also added some extra tags so Maya doesn't add extra paths for icons/scripts/etc for USD which aren't needed or wanted, since it is not a Maya plugin.

Copy link
Contributor

@mattyjams mattyjams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seando-adsk: Yeah, this shouldn't affect us internally at all. Our internal build system is completely separate and ignores anything CMake-related.

Thanks!

@kxl-adsk kxl-adsk merged commit 9e395a6 into dev Jul 10, 2020
@kxl-adsk kxl-adsk deleted the donnels/MAYA-105621/install_usd_to_install_folder branch July 10, 2020 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Related to building maya-usd repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants