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.

Sign up for GitHub

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

Jump to bottom

AL unit tests fixes for all three platforms #198

Merged
merged 19 commits into from
Jan 28, 2020
Merged

AL unit tests fixes for all three platforms #198

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
e5b96a3
Fixes for running AL unit tests on all three platforms both in Debug …
Jan 13, 2020
8a79b15
New changes based on Sean's feedbacks.
Jan 14, 2020
32682ed
1. Python tempfile.NamedTemporaryFile doesn't work well on Windows wh…
Jan 16, 2020
26e3834
Address the issues with GTest library based on the review feedbacks!
Jan 20, 2020
e047281
make sure these variables are wrapped around the quotes.
Jan 20, 2020
7305809
Code improvements based on the review feedbacks:
Jan 20, 2020
b11cb1d
Use ${MAYA_PY_EXECUTABLE} instead.
Jan 20, 2020
4ef2f16
Fix indentation!
Jan 20, 2020
33850d1
Simplify things a bit by removing the relative path!
Jan 20, 2020
65f6839
Fix issues related to TestUSDMayaPython!
Jan 21, 2020
d11f15f
Fix a regression!
Jan 21, 2020
b34dc94
Make sure the exit code is set based on the Testprogram result!
Jan 21, 2020
fc366ec
Clean up some old logics that was super confusing!
Jan 22, 2020
96156e7
Add final touches!
Jan 22, 2020
77fc0a7
Fix TestUSDMayaPython test cases on Windows!
Jan 23, 2020
478d741
Merge branch 'dev' into autodesk/MAYA-100225/fixes_al_unit_tests
HamedSabri-adsk Jan 24, 2020
1f0d7c6
[al] fix quoting inside of install(CODE ...)
Jan 24, 2020
0f84986
Fix Linux build!
Jan 24, 2020
dd8f380
Fix a regression that was recently introduced in commitID: 85010f26 w…
Jan 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,11 @@ if(NOT WIN32)
endif()
string(REPLACE ";" " " CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")

# Use RUNPATH instead of RPATH for all shared libs and executables on Linux
if(IS_LINUX)
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl,--enable-new-dtags")
set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl,--enable-new-dtags")
endif()
seando-adsk marked this conversation as resolved.
Show resolved Hide resolved

#==============================================================================
# Tests
Expand All @@ -118,6 +123,8 @@ if (BUILD_TESTS)
if (UFE_FOUND)
add_subdirectory(test/lib/ufe)
endif()

set(MAYAUSD_GTEST_PATH "PATH+:=lib/gtest")
endif()

#==============================================================================
Expand Down
8 changes: 6 additions & 2 deletions cmake/CMakeLists_googletest_download.txt.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,22 @@ project(googletest-download NONE)
# https://github.com/abseil/googletest/blob/master/googletest/README.md
# need to force the use of the shared C run-time.


include(ExternalProject)

set(FORCE_SHARED_CRT "")
if(MSVC)
set(FORCE_SHARED_CRT -DFORCE_SHARED_CRT=OFF)
endif()

set(OSX_RPATH "")
if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
set(OSX_RPATH -DCMAKE_MACOSX_RPATH=1)
endif()

ExternalProject_Add(googletest
GIT_REPOSITORY https://github.com/google/googletest.git
GIT_TAG master
SOURCE_DIR "${GOOGLETEST_BUILD_ROOT}/googletest-src"
BINARY_DIR "${GOOGLETEST_BUILD_ROOT}/googletest-build"
CMAKE_ARGS "-DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}" "-DCMAKE_INSTALL_PREFIX=${GOOGLETEST_BUILD_ROOT}/googletest-install" "-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}" "-Dgtest_force_shared_crt=ON" "-DCMAKE_POSITION_INDEPENDENT_CODE=ON" -DFORCE_SHARED_CRT=OFF ${FORCE_SHARED_CRT}
CMAKE_ARGS "-DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}" "-DCMAKE_INSTALL_PREFIX=${GOOGLETEST_BUILD_ROOT}/googletest-install" "-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}" "-Dgtest_force_shared_crt=ON" "-DBUILD_SHARED_LIBS=1" "-DCMAKE_MACOSX_RPATH=1" "${OSX_RPATH}" "-DCMAKE_POSITION_INDEPENDENT_CODE=ON" ${FORCE_SHARED_CRT}
pmolodo marked this conversation as resolved.
Show resolved Hide resolved
)
7 changes: 6 additions & 1 deletion cmake/CMakeLists_googletest_src.txt.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,15 @@ if(MSVC)
set(FORCE_SHARED_CRT -DFORCE_SHARED_CRT=OFF)
endif()

set(OSX_RPATH "")
if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
set(OSX_RPATH -DCMAKE_MACOSX_RPATH=1)
endif()

ExternalProject_Add(googletest
DOWNLOAD_COMMAND ""
UPDATE_COMMAND ""
SOURCE_DIR "${GOOGLETEST_SRC_DIR}"
BINARY_DIR "${GOOGLETEST_BUILD_ROOT}/googletest-build"
CMAKE_ARGS "-DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}" "-DCMAKE_INSTALL_PREFIX=${GOOGLETEST_BUILD_ROOT}/googletest-install" "-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}" "-Dgtest_force_shared_crt=ON" "-DCMAKE_POSITION_INDEPENDENT_CODE=ON" ${FORCE_SHARED_CRT}
CMAKE_ARGS "-DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}" "-DCMAKE_INSTALL_PREFIX=${GOOGLETEST_BUILD_ROOT}/googletest-install" "-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}" "-Dgtest_force_shared_crt=ON" "-DBUILD_SHARED_LIBS=1" "${OSX_RPATH}" -DCMAKE_POSITION_INDEPENDENT_CODE=ON" ${FORCE_SHARED_CRT}
)
30 changes: 21 additions & 9 deletions cmake/Googletest.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,20 +53,26 @@ macro(fetch_googletest)
set(GTEST_ROOT ${GOOGLETEST_BUILD_ROOT}/googletest-install CACHE path "GoogleTest installation root")
endif()

# FindGTest should get call after GTEST_ROOT is set
find_package(GTest QUIET)

seando-adsk marked this conversation as resolved.
Show resolved Hide resolved
# https://gitlab.kitware.com/cmake/cmake/issues/17799
# FindGtest is buggy when dealing with Debug build.
# FindGtest is buggy when dealing with Debug build.
if (CMAKE_BUILD_TYPE MATCHES Debug AND GTEST_FOUND MATCHES FALSE)
message("Setting GTest libraries with debug...")

if (GTEST_LIBRARY_DEBUG MATCHES GTEST_LIBRARY_DEBUG-NOTFOUND)
set(gtest_library "")
set(gtest_main_library "")
if(WIN32)
set(gtest_library lib/gtestd.lib)
set(gtest_main_library lib/gtest_maind.lib)
else()
set(gtest_library lib64/libgtestd.a)
set(gtest_main_library lib64/libgtest_maind.a)
set(gtest_library "")
set(gtest_main_library "")
if(${CMAKE_SYSTEM_NAME} MATCHES "Windows")
set(gtest_library bin/gtestd.dll)
set(gtest_main_library bin/gtest_maind.dll)
elseif(${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
set(gtest_library lib/libgtestd.dylib)
set(gtest_main_library lib/libgtest_maind.dylib)
elseif(${CMAKE_SYSTEM_NAME} MATCHES "Linux")
set(gtest_library lib64/libgtestd.so)
set(gtest_main_library lib64/libgtest_maind.so)
endif()
set(GTEST_INCLUDE_DIRS ${GOOGLETEST_BUILD_ROOT}/googletest-install/include)
pmolodo marked this conversation as resolved.
Show resolved Hide resolved
set(GTEST_LIBRARY_DEBUG ${GOOGLETEST_BUILD_ROOT}/googletest-install/${gtest_library})
Expand All @@ -79,4 +85,10 @@ macro(fetch_googletest)
set(GTEST_MAIN_LIBRARIES ${GTEST_MAIN_LIBRARY})
endif()

set(GTEST_LIBS ${GTEST_LIBRARIES})
if(${CMAKE_SYSTEM_NAME} MATCHES "Windows" AND NOT CMAKE_BUILD_TYPE MATCHES Debug)
set(GTEST_LIBS "${GTEST_ROOT}/bin/gtest.dll")
pmolodo marked this conversation as resolved.
Show resolved Hide resolved
endif()

install(FILES ${GTEST_LIBS} DESTINATION ${CMAKE_INSTALL_PREFIX}/lib/gtest)
seando-adsk marked this conversation as resolved.
Show resolved Hide resolved
endmacro()
12 changes: 3 additions & 9 deletions cmake/modules/FindMaya.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
# MAYA_INCLUDE_DIRS Path to the devkit's include directories
# MAYA_API_VERSION Maya version (6-8 digits)
#
# IMPORTANT: Currently, there's only support for OSX platform and Maya version 2017 because of ABI issues with libc++.

#=============================================================================
# Copyright 2011-2012 Francisco Requena <frarees@gmail.com>
Expand Down Expand Up @@ -69,21 +68,14 @@ else(IS_LINUX)
endif()

if(IS_MACOSX)
# Note: according to official Autodesk sources (and how it sets up
# MAYA_LOCATION itself), MAYA_LOCATION should include Maya.app/Contents
# on MacOS - ie:
# /Applications/Autodesk/maya2019/Maya.app/Contents
# However, for legacy reasons, and for maximum compatibility, setting
# it to the installation root is also supported, ie:
# /Applications/Autodesk/maya2019

pmolodo marked this conversation as resolved.
Show resolved Hide resolved
find_path(MAYA_BASE_DIR
include/maya/MFn.h
HINTS
"${MAYA_LOCATION}/../.."
"$ENV{MAYA_LOCATION}/../.."
"${MAYA_LOCATION}"
"$ENV{MAYA_LOCATION}"
"/Applications/Autodesk/maya2020"
"/Applications/Autodesk/maya2019"
"/Applications/Autodesk/maya2018"
"/Applications/Autodesk/maya2017"
Expand All @@ -110,6 +102,7 @@ elseif(IS_LINUX)
HINTS
"${MAYA_LOCATION}"
"$ENV{MAYA_LOCATION}"
"/usr/autodesk/maya2020-x64"
"/usr/autodesk/maya2019-x64"
"/usr/autodesk/maya2018-x64"
"/usr/autodesk/maya2017-x64"
Expand All @@ -135,6 +128,7 @@ elseif(IS_WINDOWS)
HINTS
"${MAYA_LOCATION}"
"$ENV{MAYA_LOCATION}"
"C:/Program Files/Autodesk/Maya2020"
"C:/Program Files/Autodesk/Maya2019"
"C:/Program Files/Autodesk/Maya2018"
"C:/Program Files/Autodesk/Maya2017"
Expand Down
2 changes: 1 addition & 1 deletion cmake/utils.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,4 +273,4 @@ function(mayaUsd_copyDirectory target)
)

endforeach()
endfunction()
endfunction()
1 change: 1 addition & 0 deletions lib/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ if(UFE_FOUND)
target_link_libraries(${LIBRARY_NAME} ${UFE_LIBRARY})
endif()

# handle run-time search paths
if(IS_MACOSX OR IS_LINUX)
mayaUsd_init_rpath(rpath "plugin")
if(WANT_USD_RELATIVE_PATH)
Expand Down
1 change: 1 addition & 0 deletions mayaUSD.mod.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ PATH+:=lib
PYTHONPATH+:=lib/python
PXR_PLUGINPATH_NAME+:=lib/usd
VP2_RENDER_DELEGATE_PROXY=1
${MAYAUSD_GTEST_PATH}

+ MayaUSD ${MAYAUSD_VERSION} ${CMAKE_INSTALL_PREFIX}/plugin/adsk
MAYA_PLUG_IN_PATH+:=plugin
Expand Down
2 changes: 1 addition & 1 deletion plugin/adsk/plugin/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ target_link_libraries(${PLUGIN_PACKAGE}

MAYA_SET_PLUGIN_PROPERTIES(${PLUGIN_PACKAGE})

# rpath setup
# handle run-time search paths
if(IS_MACOSX OR IS_LINUX)
mayaUsd_init_rpath(rpath "plugin")
if(WANT_USD_RELATIVE_PATH)
Expand Down
2 changes: 1 addition & 1 deletion plugin/al/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,4 +98,4 @@ get_property(PYTHON_LIBRARY_LOCATION GLOBAL PROPERTY GLOBAL_PYTHON_LIBRARY_LOCAT
configure_file(ALUsdMayaConfig.cmake.in ${PROJECT_BINARY_DIR}/ALUsdMayaConfig.cmake @ONLY)

install(CODE "message(STATUS \"POST INSTALL: Compiling python/pyc for ${AL_INSTALL_PREFIX} ... \")")
install(CODE "execute_process(COMMAND ${Python_EXECUTABLE} -m compileall ${AL_INSTALL_PREFIX} )")
install(CODE "execute_process(COMMAND \"${Python_EXECUTABLE}\" -m compileall ${AL_INSTALL_PREFIX} )")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

wrap all the full path variables around quotes in case there are located in weird places with whitespaces and special characters on Windows. Just to be safe!

16 changes: 5 additions & 11 deletions plugin/al/lib/AL_USDMaya/AL/usdmaya/cmds/DebugCommands.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,6 @@ namespace cmds {

AL_MAYA_DEFINE_COMMAND(UsdDebugCommand, AL_usdmaya);

//----------------------------------------------------------------------------------------------------------------------
MArgDatabase UsdDebugCommand::makeDatabase(const MArgList& args)
{
MStatus status;
MArgDatabase database(syntax(), args, &status);
if(!status)
throw status;
return database;
}

//----------------------------------------------------------------------------------------------------------------------
MSyntax UsdDebugCommand::createSyntax()
{
Expand All @@ -66,7 +56,11 @@ MStatus UsdDebugCommand::doIt(const MArgList& argList)
TF_DEBUG(ALUSDMAYA_COMMANDS).Msg("AL_usdmaya_UsdDebugCommand::doIt\n");
try
{
MArgDatabase args = makeDatabase(argList);
MStatus status;
MArgDatabase args(syntax(), argList, &status);
if(!status)
return status;

Comment on lines -69 to +63
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd defer to the Animal Logic folks for the final word here, but it does seem a bit unfortunate to turn all of these previously single-line function calls into multi-line incantations. It results in a fair amount of duplicated code. I get wanting to remove the throw though, and turning it into a function that returns an MStatus through a pointer param would probably mean the call sites would be the same length as they are in this change.

If we do keep it as a function, maybe it could be defined in one common place rather than repeated in each command?

If we ditch the function, can we get rid of the try{}, since makeDatabase() was likely the only thing expected to throw?

AL_MAYA_COMMAND_HELP(args, g_helpText);

const bool listSymbols = args.isFlagSet("-ls");
Expand Down
1 change: 0 additions & 1 deletion plugin/al/lib/AL_USDMaya/AL/usdmaya/cmds/DebugCommands.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ namespace cmds {
class UsdDebugCommand
: public MPxCommand
{
MArgDatabase makeDatabase(const MArgList& args);
public:
AL_MAYA_DECLARE_COMMAND();
private:
Expand Down
2 changes: 1 addition & 1 deletion plugin/al/lib/AL_USDMaya/AL/usdmaya/cmds/EventCommand.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ MSyntax Callback::createSyntax()
syn.makeFlagMultiUse("-pne");
syn.makeFlagMultiUse("-me");
syn.makeFlagMultiUse("-mne");
syn.makeFlagMultiUse("-d");
syn.makeFlagMultiUse("-de");
return syn;
pmolodo marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down
56 changes: 34 additions & 22 deletions plugin/al/lib/AL_USDMaya/AL/usdmaya/cmds/LayerCommands.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,16 +116,6 @@ MSyntax LayerCommandBase::setUpCommonSyntax()
return syntax;
}

//----------------------------------------------------------------------------------------------------------------------
MArgDatabase LayerCommandBase::makeDatabase(const MArgList& args)
{
MStatus status;
MArgDatabase database(syntax(), args, &status);
if(!status)
throw status;
return database;
}

//----------------------------------------------------------------------------------------------------------------------
pmolodo marked this conversation as resolved.
Show resolved Hide resolved
nodes::ProxyShape* LayerCommandBase::getShapeNode(const MArgDatabase& args)
{
Expand Down Expand Up @@ -236,7 +226,10 @@ MStatus LayerGetLayers::doIt(const MArgList& argList)
TF_DEBUG(ALUSDMAYA_COMMANDS).Msg("LayerGetLayers::doIt\n");
try
{
MArgDatabase args = makeDatabase(argList);
MStatus status;
MArgDatabase args(syntax(), argList, &status);
if(!status)
return status;
AL_MAYA_COMMAND_HELP(args, g_helpText);

nodes::ProxyShape* proxyShape = getShapeNode(args);
Expand Down Expand Up @@ -374,7 +367,10 @@ MStatus LayerCreateLayer::doIt(const MArgList& argList)
TF_DEBUG(ALUSDMAYA_COMMANDS).Msg("LayerCreateLayer::doIt\n");
try
{
MArgDatabase args = makeDatabase(argList);
MStatus status;
MArgDatabase args(syntax(), argList, &status);
if(!status)
return status;

AL_MAYA_COMMAND_HELP(args, g_helpText);

Expand Down Expand Up @@ -585,7 +581,11 @@ MStatus LayerCurrentEditTarget::doIt(const MArgList& argList)
{
try
{
MArgDatabase args = makeDatabase(argList);
MStatus status;
MArgDatabase args(syntax(), argList, &status);
if(!status)
return status;

AL_MAYA_COMMAND_HELP(args, g_helpText);
if(args.isQuery())
{
Expand Down Expand Up @@ -774,7 +774,11 @@ MStatus LayerSave::doIt(const MArgList& argList)
{
try
{
MArgDatabase args = makeDatabase(argList);
MStatus status;
MArgDatabase args(syntax(), argList, &status);
if(!status)
return status;

AL_MAYA_COMMAND_HELP(args, g_helpText);

MString layerName = args.commandArgumentString(0);
Expand Down Expand Up @@ -899,7 +903,11 @@ MStatus LayerSetMuted::doIt(const MArgList& argList)
{
try
{
MArgDatabase args = makeDatabase(argList);
MStatus status;
MArgDatabase args(syntax(), argList, &status);
if(!status)
return status;

AL_MAYA_COMMAND_HELP(args, g_helpText);

MString layerName = args.commandArgumentString(0);
Expand Down Expand Up @@ -985,7 +993,11 @@ MStatus LayerManager::doIt(const MArgList& argList)
{
try
{
MArgDatabase args = makeDatabase(argList);
MStatus status;
MArgDatabase args(syntax(), argList, &status);
if(!status)
return status;

AL_MAYA_COMMAND_HELP(args, g_helpText);

nodes::LayerManager* layerManager = nodes::LayerManager::findManager();
Expand Down Expand Up @@ -1125,12 +1137,12 @@ void constructLayerCommandGuis()
const char* const LayerCreateLayer::g_helpText = R"(
LayerCreateLayer Overview:

This command provides a way to create new layers in Maya. The Layer identifier passed into the -o will attempt to find the layer,
and if it doesn't exist then it is created. If a layer is created, it will create a AL::usdmaya::nodes::Layer which will contain a SdfLayerRefPtr
This command provides a way to create new layers in Maya. The Layer identifier passed into the -o will attempt to find the layer,
and if it doesn't exist then it is created. If a layer is created, it will create a AL::usdmaya::nodes::Layer which will contain a SdfLayerRefPtr
to the layer opened with -o.

This command is currently used in our pipeline to create layers on the fly. These layers may then be targeted by an EditTarget for edits
and these edits are saved into the maya scene file.
and these edits are saved into the maya scene file.

If no identifier is passed, the stage's root layer is used as the parent.

Expand Down Expand Up @@ -1227,8 +1239,8 @@ LayerCurrentEditTarget Overview:
LayerCurrentEditTarget -l "anon:0x136d9050" -fid -proxy "ProxyShape1"


There are some caveats here though. If no TargetPath and SourcePath prim paths are specified,
USD will only allow you to set an edit target into what is known as the current layer stack.
There are some caveats here though. If no TargetPath and SourcePath prim paths are specified,
USD will only allow you to set an edit target into what is known as the current layer stack.
These layers can be determined using the following command:

LayerGetLayers -stack "ProxyShape1";
Expand Down
5 changes: 0 additions & 5 deletions plugin/al/lib/AL_USDMaya/AL/usdmaya/cmds/LayerCommands.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,6 @@ class LayerCommandBase
/// \return the stage from a proxy shape node (if found in the args)
UsdStageRefPtr getShapeNodeStage(const MArgDatabase& args);

/// \brief construct the arg data base from an args list. If any errors are found during parsing, an error will be thrown
/// \param args the args list passed to the MPxCommand from Maya
/// \return the new data base
MArgDatabase makeDatabase(const MArgList& args);

// /// \brief hunt for the first node of the specified type found in the selection list object args
// /// \param args the pre-parsed argument data base
// /// \param typeId the typeId of the object type that may appear as one of the selected objects
Expand Down
Loading