-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[MLIR][Python] Fix stubgen/PYTHONPATH collision/bug #161307
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
[MLIR][Python] Fix stubgen/PYTHONPATH collision/bug #161307
Conversation
@llvm/pr-subscribers-mlir Author: Maksim Levental (makslevental) ChangesIf Full diff: https://github.com/llvm/llvm-project/pull/161307.diff 1 Files Affected:
diff --git a/mlir/cmake/modules/AddMLIRPython.cmake b/mlir/cmake/modules/AddMLIRPython.cmake
index 208cbdd1dd535..1127f9410e488 100644
--- a/mlir/cmake/modules/AddMLIRPython.cmake
+++ b/mlir/cmake/modules/AddMLIRPython.cmake
@@ -173,9 +173,29 @@ function(mlir_generate_type_stubs)
if(ARG_VERBOSE)
message(STATUS "Generating type-stubs outputs ${_generated_type_stubs}")
endif()
+
+ # If PYTHONPATH is set and points to the build location of the python package then when stubgen runs, _mlir will get
+ # imported twice and bad things will happen (e.g., Assertion `!instance && “PyGlobals already constructed”’).
+ # This happens because mlir is a namespace package and so importer/loader can't distinguish between
+ # mlir._mlir_libs._mlir and _mlir_libs._mlir imported from CWD.
+ # So try to filter out any entries in PYTHONPATH that end in "MLIR_BINDINGS_PYTHON_INSTALL_PREFIX/.."
+ # (e.g., python_packages/mlir_core/).
+ set(_pythonpath "$ENV{PYTHONPATH}")
+ cmake_path(SET _install_prefix NORMALIZE "${MLIR_BINDINGS_PYTHON_INSTALL_PREFIX}/..")
+ string(REGEX REPLACE "/$" "" _install_prefix "${_install_prefix}")
+ if(WIN32)
+ set(_path_sep ";")
+ else()
+ set(_path_sep ":")
+ # `;` is the CMake list delimiter so Windows paths are automatically lists
+ # but Unix paths can be made into lists by replacing `:` with `;`
+ string(REPLACE "${_path_sep}" ";" _pythonpath "${_pythonpath}")
+ endif()
+ list(FILTER _pythonpath EXCLUDE REGEX "(${_install_prefix}|${_install_prefix}/)$")
+ string(JOIN "${_path_sep}" _pythonpath ${_pythonpath})
add_custom_command(
OUTPUT ${_generated_type_stubs}
- COMMAND ${_nb_stubgen_cmd}
+ COMMAND ${CMAKE_COMMAND} -E env PYTHONPATH=${_pythonpath} ${_nb_stubgen_cmd}
WORKING_DIRECTORY "${CMAKE_CURRENT_FUNCTION_LIST_DIR}"
DEPENDS "${ARG_DEPENDS_TARGETS}"
DEPFILE "${_depfile}"
|
fd0ec66
to
13f2900
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried on my local and it did fix the issue. Thanks!
13f2900
to
55eeaa5
Compare
55eeaa5
to
be8ebc8
Compare
be8ebc8
to
583dec4
Compare
If `PYTHONPATH` is set and points to the build location of the python bindings package then when stubgen runs, `_mlir` will get imported twice and bad things will happen (e.g., `Assertion !instance && “PyGlobals already constructed”’`). This happens because `mlir` is a namespace package and the importer/loader can't distinguish between `mlir._mlir_libs._mlir` and `_mlir_libs._mlir` imported from `CWD`. Or something like that. The fix is to filter out any entries in `PYTHONPATH` that end in `MLIR_BINDINGS_PYTHON_INSTALL_PREFIX/..` (e.g., `python_packages/mlir_core/`).
If
PYTHONPATH
is set and points to the build location of the python bindings package then when stubgen runs,_mlir
will get imported twice and bad things will happen (e.g.,Assertion !instance && “PyGlobals already constructed”’
). This happens becausemlir
is a namespace package and the importer/loader can't distinguish betweenmlir._mlir_libs._mlir
and_mlir_libs._mlir
imported fromCWD
. Or something like that. The fix is to filter out any entries inPYTHONPATH
that end inMLIR_BINDINGS_PYTHON_INSTALL_PREFIX/..
(e.g.,python_packages/mlir_core/
).