PixarAnimationStudios / OpenUSD

Universal Scene Description
http://www.openusd.org
Other
5.45k stars 1.13k forks source link

`cmake/macros/shebang.py` runs the wrong code path if `PXR_PYTHON_SHEBANG` fails to be set to the python executable #3047

Open nvmkuruc opened 3 weeks ago

nvmkuruc commented 3 weeks ago

Description of Issue

If PXR_PYTHON_SHEBANG gets set to the empty string in when running cmake, the invocation of cmake/macros/shebang.py will be missing an argument.

shebang.py uses the number of arguments to dispatch two branching code paths. The len(sys.argv) == 4 (which I'll call "substitute") code path replaces \pxrpythonsubst. The len(sys.argv) == 3 (which I'll call "wrapper") code path is a Windows-only code path that sets up a wrapper command.

If PXR_PYTHON_SHEBANG is empty, the arguments for the "substitute (length 4)" code path get routed to the "wrapper (length 3)" code path. The build will succeed but python commands like usdchecker will have the Windows-only wrappers (on non-Windows builds) with the "substitute" arguments instead of the "wrapper" arguments, and the "substitute" code path will never get executed.

It's not clear why PXR_PYTHON_SHEBANG isn't resolving in my environment, but it seems like the build should be hardened against succeeding when it does fail to resolve. (My workaround is to replace PXR_PYTHON_SHEBANG explicitly with PYTHON_EXECUTABLE in the Public.cmake file.)

I'd suggest disaggregating the windows code path out of shebang.py into something like (win_py_wrapper.py). They don't share any logic and would avoid this confusion. It would be clear when running shebang.py that the wrong number of arguments were passed and the build command would (helpfully) fail.

I would be happy to contribute a fix like that if there's agreement that's a useful improvement.

Steps to Reproduce

System Information (OS, Hardware)

WSL2

Package Versions

CMake 3.22

Build Flags

jesschimein commented 3 weeks ago

Filed as internal issue #USD-9564