feat: add VS 2022 & boost 1.78 & python 3.10 support

[sssooonnnggg]

feat: add VS 2022 & boost 1.78 & python 3.10 support

CMake version >= 3.24 required

[e4lam]

Which VS2022 version did you build it in? As a heads up, I've also been building USD with VS2022 v17.3.1 and found a serious bug in the compiler that made USD unusable. The first symptom of it being that the testSdfLayer unit test fails. I still have other test failures but I haven't gone far enough down to road to confirm which ones are legitimate failures or not. (Some of the ones I get are because the unit test can't find PySide for some reason.)

For reference, the bug I reported is here:
https://developercommunity.visualstudio.com/t/cl-19321933-ternary-operator-bug-call/10126287

In the meantime, I got things going by applying this patch in my local build to workaround it:

diff -urN --strip-trailing-cr USD-py3.9/pxr/usd/sdf/layer.cpp USD-py3.9.new/pxr/usd/sdf/layer.cpp
--- USD-py3.9/pxr/usd/sdf/layer.cpp     2022-08-16 17:00:31.924779700 -0400
+++ USD-py3.9.new/pxr/usd/sdf/layer.cpp 2022-08-19 23:24:06.588275300 -0400
@@ -727,8 +727,12 @@
     // If we're trying to open an anonymous layer, do not try to compute the
     // real path for it.
     ArAssetInfo assetInfo;
-    string resolvedLayerPath = isAnonymous ? layerPath :
-        Sdf_ResolvePath(layerPath, computeAssetInfo ? &assetInfo : nullptr);
+    string resolvedLayerPath;
+    if (isAnonymous)
+        resolvedLayerPath = layerPath;
+    else
+        resolvedLayerPath = Sdf_ResolvePath(layerPath,
+                                    computeAssetInfo ? &assetInfo : nullptr);

     // Merge explicitly-specified arguments over any arguments
     // embedded in the given identifier.

[sssooonnnggg]

Which VS2022 version did you build it in? As a heads up, I've also been building USD with VS2022 v17.3.1 and found a serious bug in the compiler that made USD unusable. The first symptom of it being that the testSdfLayer unit test fails. I still have other test failures but I haven't gone far enough down to road to confirm which ones are legitimate failures or not. (Some of the ones I get are because the unit test can't find PySide for some reason.)

For reference, the bug I reported is here: https://developercommunity.visualstudio.com/t/cl-19321933-ternary-operator-bug-call/10126287

In the meantime, I got things going by applying this patch in my local build to workaround it:

diff -urN --strip-trailing-cr USD-py3.9/pxr/usd/sdf/layer.cpp USD-py3.9.new/pxr/usd/sdf/layer.cpp
--- USD-py3.9/pxr/usd/sdf/layer.cpp     2022-08-16 17:00:31.924779700 -0400
+++ USD-py3.9.new/pxr/usd/sdf/layer.cpp 2022-08-19 23:24:06.588275300 -0400
@@ -727,8 +727,12 @@
     // If we're trying to open an anonymous layer, do not try to compute the
     // real path for it.
     ArAssetInfo assetInfo;
-    string resolvedLayerPath = isAnonymous ? layerPath :
-        Sdf_ResolvePath(layerPath, computeAssetInfo ? &assetInfo : nullptr);
+    string resolvedLayerPath;
+    if (isAnonymous)
+        resolvedLayerPath = layerPath;
+    else
+        resolvedLayerPath = Sdf_ResolvePath(layerPath,
+                                    computeAssetInfo ? &assetInfo : nullptr);

     // Merge explicitly-specified arguments over any arguments
     // embedded in the given identifier.

I use VS2022 17.1.1，it seems that this version has right output for your rvalue test

[sunyab]

Filed as internal issue #USD-7582

[seando-adsk]

I would suggest changing line 395 of build_usd.py from:
if IsVisualStudio2019OrGreater():
to:
if generator and 'Visual Studio' in generator and IsVisualStudio2019OrGreater():
The code now is assuming that on Windows the generator being used is Visual Studio, but there are other generators (such as Ninja) that work on Windows.

Otherwise all the changes look fine.

[sunyab]

Thank you for looking into this! I left one note below but otherwise this seems good to me.

Also, I think @seando-adsk 's comment about the code at line 395 is valid but it's a pre-existing issue and I think doesn't need to be addressed in this change. I'm actually not sure if specifying the platform name causes any problems for generators that don't support them -- the CMake docs imply that the name is just made available for generators to use only if they want to.

Thanks!

[sssooonnnggg]

Thanks for reviewing, boost version is depending on python version on Linux now.

[Tisten]

You ought to bump the required cmake version to 3.24 as well: https://github.com/PixarAnimationStudios/USD/blob/203806dfa0c9bf66c34d5cc58697101849de6fe3/build_scripts/build_usd.py#L2181

[meshula]

@Tisten Good note on the cmake version bump, that is required for sure or people will run into problems. The support for VS2022 was added in 3.21. (https://cmake.org/cmake/help/latest/generator/Visual%20Studio%2017%202022.html) Are you aware of fixes after 3.21 for VS2022 that might be critical? I'm not arguing for one version or another, just checking what the minimum viable is.

[Tisten]

Well, the initial comment by sssooonnnggg was:

CMake version >= 3.24 required

And that has been my experience as well, 3.22 didn't work but 3.24 did. Can't really say which fix it is that makes or breaks it.

[sssooonnnggg]

You ought to bump the required cmake version to 3.24 as well:

https://github.com/PixarAnimationStudios/USD/blob/203806dfa0c9bf66c34d5cc58697101849de6fe3/build_scripts/build_usd.py#L2181

Done.

[Danielmelody]

Requiring newest cmake while not force using 1.78 seems unreasonable. A user would have to install cmake 3.24 for all windows build, even with VS2019 or VS2017

[sssooonnnggg]

A user would have to install cmake 3.24 for all windows build, even with VS2019 or VS2017

Yes, I agree that it seems unreasonable.

In this PR, boost 1.78 is forced on Windows, CMake lower than 3.24 will not find boost 1.78 correctly, so CMake 3.24 is required.

[Danielmelody]

So, in my opinion, we should not force using 1.78, only if we use python > 3.8, the cmake and boost's version requirement should be present

[SimonBoorer]

Is there a reason why we can't just use -A x64 in USD/build_usd.py and not explicitly specify a generator? The CMAKE_GENERATOR_PLATFORM documentation that the -A option refers to says that this is supported by VS 2005 and above:
https://cmake.org/cmake/help/v3.14/manual/cmake.1.html#options
https://cmake.org/cmake/help/v3.14/variable/CMAKE_GENERATOR_PLATFORM.html#variable:CMAKE_GENERATOR_PLATFORM

Is this not the "better way" that the TODO comment refers to?

[pmolodo]

Pinging this because we're looking to add Python 3.10 and VS2022 support as well. Are there still outstanding issues to be addressed for this MR?

[meshula]

It's good as is! We'll update this issue after the holidays!

[seando-adsk]

FYI, at Autodesk we are building USD with python 3.10 binding support. The one thing I had to do was update boost version to 1.76 on all platforms.

[pmolodo]

Saw the recent commits related to this, which is great! Think the only thing still left from this PR is the branching of boost + cmake versions if using python 3.10.

Also - I noticed a typo, and filed an issue, though you're probably already on it:

#2143

Thanks again!

[meshula]

@pmolodo Thanks for the notes. I see the bit you are referring to to get 3.10 support on Linux.

[pmolodo]

@pmolodo Thanks for the notes. I see the bit you are referring to to get 3.10 support on Linux.

Since we're deviating slightly from this PR anyway... I actually think we need to branch based on python version for all platforms, not just Linux. As @seando-adsk noted above, boost needs to be updated to at least 1.76 for any platform that's compiling against python-3.10. I'm thinking we should just force to boost 1.78 and cmake 3.24 for all platforms, for consistency, if python >= 3.10.

I'm happy to make a PR for this myself if that would be easier.

[meshula]

It makes sense to have consistent versions on all platforms py310's selected. I'm working it through now.