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-109891: Remove dependency on boost filesystem/system. #1182

Merged
merged 3 commits into from
Feb 22, 2021

Conversation

HamedSabri-adsk
Copy link
Contributor

@HamedSabri-adsk HamedSabri-adsk commented Feb 16, 2021

This PR removes the dependency on boost filesystem/system entirely from the project. Until the transition to C++17 std::filesystem, ghc::filesystem must be used as an alternative across the project.

When building USD, --with-system and --with-filesystem will no longer need to be passed.

@HamedSabri-adsk HamedSabri-adsk added the core Related to core library label Feb 16, 2021
@hamedsabri
Copy link

So all the PFs passed but "2020_python2_osx" when building AL_USDMaya. Very interesting.

Looking at the logs to see what is happening.

Hmmm, this is the PERFECT example of the pains dealing with Boost.

@pmolodo
Copy link
Contributor

pmolodo commented Feb 16, 2021

Looks good!

Copy link
Contributor

@pmolodo pmolodo left a comment

Choose a reason for hiding this comment

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

Looks good!

@HamedSabri-adsk
Copy link
Contributor Author

@pmolodo

Following up on yesterday's PF failure on "2020_python2_osx", I noticed this linker error in the log which I was not expecting:

[ 85%] Linking CXX shared library libAL_USDMaya.dylib
Undefined symbols for architecture x86_64:
 "boost::system::system_category()", referenced from:
   boost::mutex::mutex() in LayerManager.cpp.o
   boost::condition_variable::condition_variable() in LayerManager.cpp.o
   boost::unique_lock<boost::shared_mutex>::lock() in LayerManager.cpp.o
   boost::condition_variable::wait(boost::unique_lock<boost::mutex>&) in LayerManager.cpp.o
   boost::unique_lock<boost::mutex>::lock() in LayerManager.cpp.o
   boost::mutex::lock() in LayerManager.cpp.o
   boost::unique_lock<boost::mutex>::unlock() in LayerManager.cpp.o

What this means that there is still a dependency on "boost::system" coming from "boost::thread".

To work around it, one can add "BOOST_SYSTEM_NO_DEPRECATED" compiler definition.

$<$<BOOL:${IS_MACOSX}>:BOOST_SYSTEM_NO_DEPRECATED>

And this indeed works when I encounter the same problem on "2022 OSX python_3". But some how it didn't work for '2020_python2_osx'?

The answer lies in the insane combination of boost versions and additional components that comes with the different version of USD:

master Windows python3   ---------->  USD v[0.21.02] + Boost v[1.70.0] + [thread/chrono/date_time/python]
master Linux python3     ---------->  USD v[0.21.02] + Boost v[1.70.0] + [thread/python]
master OSX python3       ---------->  USD v[0.21.02] + Boost v[1.70.0] + [thread/python]
2020 Windows python2     ---------->  USD v[0.20.11] + Boost v[1.70.0] + [thread/chrono/date_time/python]
2020 Linux python2       ---------->  USD v[0.20.11] + Boost v[1.61.0] + [filesystem/system]
2020 OSX python2         ---------->  USD v[0.20.11] + Boost v[1.61.0] + [filesystem/system]
2022 Windows python2     ---------->  USD v[0.20.11] + Boost v[1.70.0] + [thread/chrono/date_time/python]
2022 Linux python2       ---------->  USD v[0.20.11] + Boost v[1.61.0] + [thread/python/system/chrono/date_time/atomic]
2022 Windows python3     ---------->  USD v[0.20.11] + Boost v[1.70.0] + [thread/chrono/date_time/python]
2022 Linux python3       ---------->  USD v[0.20.11] + Boost v[1.70.0] + [thread/python]
2022 OSX python3         ---------->  USD v[0.20.11] + Boost v[1.70.0] + [thread/python]

On MacOS/Linux with Boost v1.61.0, linking against the boost::thread (a.k.a "--with-thread"), will automatically bring in boost::system (a.k.a --with-system) and boost::filesystem (a.k.a --with-filesystem).

And somehow on Boost v1.70.0, BOOST_SYSTEM_NO_DEPRECATED works with the present of system/filesystem shared libraries and not with 1.61.0.

At this point there are two solutions:

1- Change our internal infrastructure to make "2020 OSX python2" link against boost "v1.70.0". Again, USD v20.11 brings in Boost v1.61.0. Not really sure about the reasons why a single boost version could not be used.

https://github.com/PixarAnimationStudios/USD/blob/d8a405a1344480f859f025c4f97085143efacb53/build_scripts/build_usd.py#L626

2- Stop relying on boost::thread all together for good. Animal Logic plugin is the only place in the entire project that has a dependency on "boost::thread" and they are using only 3 thread API calls in only one file (LayerManager[.h][.cpp]). There is already an equivalent for these calls in C++14 standard library .

I chose option number 2 since it is more logical and avoids us from making any additional internal changes to our build pipeline.

Also I have created two additional PRs which are orthogonal to this PR.

#1183
#1184

Once all three PRs are merged, we will have one dependency on boost and that would be Boost::Python.

Copy link
Contributor

@fabal fabal left a comment

Choose a reason for hiding this comment

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

Thanks @HamedSabri-adsk for this 👍

@kxl-adsk kxl-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Feb 22, 2021
@kxl-adsk kxl-adsk merged commit aed181f into dev Feb 22, 2021
@kxl-adsk kxl-adsk deleted the sabrih/MAYA-109891/ghc_file_systemf branch February 22, 2021 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Related to core library ready-for-merge Development process is finished, PR is ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants