Skip to content

Commit 1a8f49f

Browse files
authored
[mlir][python][cmake] Allow skipping nanobind compile options changes. (#123997)
Context: #107103 (comment) This code is brittle, especially when called from a superproject that adds the `nanobind-*` target in a different source directory: ```cmake get_property(all_targets DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} PROPERTY BUILDSYSTEM_TARGETS) ``` The changes here do help with my downstream build, but I'm not sure if using the `MLIR_DISABLE_CONFIGURE_PYTHON_DEV_PACKAGES` option introduced in #117934 is the right fix given that the option is currently scoped directly to one location with a matching name: https://github.com/llvm/llvm-project/blob/7ad8a3da4771ce8abbd146611124104d42a4e63e/mlir/cmake/modules/MLIRDetectPythonEnv.cmake#L4-L5 Some other solutions to consider: 1. Search through an explicit list of target names using `if (TARGET)` 2. Iterate over _all_ targets in the project, not just the targets in the current directory, using code like https://stackoverflow.com/a/62311397 3. Iterate over targets in the directory known to MLIR (`llvm-project/mlir/python`) 4. Move this `target_compile_options` setup into `mlir_configure_python_dev_packages` (I started on this, but that runs into similar issues where the target is defined in a different directory)
1 parent 082b148 commit 1a8f49f

File tree

1 file changed

+5
-2
lines changed

1 file changed

+5
-2
lines changed

mlir/cmake/modules/AddMLIRPython.cmake

+5-2
Original file line numberDiff line numberDiff line change
@@ -671,8 +671,11 @@ function(add_mlir_python_extension libname extname)
671671
${ARG_SOURCES}
672672
)
673673

674-
if (LLVM_COMPILER_IS_GCC_COMPATIBLE OR CLANG_CL)
675-
# Avoids warnings from upstream nanobind.
674+
if (NOT MLIR_DISABLE_CONFIGURE_PYTHON_DEV_PACKAGES
675+
AND (LLVM_COMPILER_IS_GCC_COMPATIBLE OR CLANG_CL))
676+
# Avoid some warnings from upstream nanobind.
677+
# If a superproject set MLIR_DISABLE_CONFIGURE_PYTHON_DEV_PACKAGES, let
678+
# the super project handle compile options as it wishes.
676679
set(nanobind_target "nanobind-static")
677680
if (NOT TARGET ${nanobind_target})
678681
# Get correct nanobind target name: nanobind-static-ft or something else

0 commit comments

Comments
 (0)