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

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 this issue Apr 19, 2024 · 1 comment

Comments

@nvmkuruc
Copy link
Contributor

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
Copy link

Filed as internal issue #USD-9564

@nvmkuruc nvmkuruc changed the title cmake/macros/shebang.py runs the wrong code path if PXR_PYTHON_SHEBANG fails to find the python executable cmake/macros/shebang.py runs the wrong code path if PXR_PYTHON_SHEBANG fails to be set to the python executable Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants