Skip to content

Conversation

makslevental
Copy link
Contributor

@makslevental makslevental commented Sep 9, 2025

This PR fixes 4 things related to stubgen:

  1. Support for both find_package(nanobind) and FetchContent_Declare(nanobind);
  2. Disambiguate stubgen target names by package prefix correctly (prior using a global MLIR_PYTHON_PACKAGE_PREFIX caused collisions for some users);
  3. Fix generation for declare_mlir_python_extension(StandalonePythonSources.NanobindExtension);
  4. Emits DEPFILEs for the add_custom_command to prevent regenerating each build.

@llvmbot llvmbot added the mlir label Sep 9, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 9, 2025

@llvm/pr-subscribers-mlir

Author: Maksim Levental (makslevental)

Changes

FetchContent reports something like /build/CMakeFiles/pkgRedirects for nanobind_DIR, and the correct thing for nanobind_SOURCE_DIR.


Full diff: https://github.com/llvm/llvm-project/pull/157589.diff

1 Files Affected:

  • (modified) mlir/cmake/modules/AddMLIRPython.cmake (+6)
diff --git a/mlir/cmake/modules/AddMLIRPython.cmake b/mlir/cmake/modules/AddMLIRPython.cmake
index 85c80276c1bcf..cd9c00df4c82c 100644
--- a/mlir/cmake/modules/AddMLIRPython.cmake
+++ b/mlir/cmake/modules/AddMLIRPython.cmake
@@ -116,10 +116,16 @@ function(generate_type_stubs MODULE_NAME DEPENDS_TARGET MLIR_DEPENDS_TARGET OUTP
     ""
     "OUTPUTS"
     ${ARGN})
+  # for people doing pip install nanobind
   if(EXISTS ${nanobind_DIR}/../src/stubgen.py)
     set(NB_STUBGEN "${nanobind_DIR}/../src/stubgen.py")
   elseif(EXISTS ${nanobind_DIR}/../stubgen.py)
     set(NB_STUBGEN "${nanobind_DIR}/../stubgen.py")
+  # for people using FetchContent or ExternalProject
+  elseif(EXISTS ${nanobind_SOURCE_DIR}/src/stubgen.py)
+    set(NB_STUBGEN "${nanobind_SOURCE_DIR}/src/stubgen.py")
+  elseif(EXISTS ${nanobind_SOURCE_DIR}/stubgen.py)
+    set(NB_STUBGEN "${nanobind_SOURCE_DIR}/stubgen.py")
   else()
     message(FATAL_ERROR "generate_type_stubs(): could not locate 'stubgen.py'!")
   endif()

@makslevental makslevental force-pushed the users/makslevental/fix-stubgen-fetchcontent branch 2 times, most recently from 423916e to 346ef34 Compare September 9, 2025 01:25
@llvmbot llvmbot added the mlir:python MLIR Python bindings label Sep 9, 2025
@makslevental makslevental marked this pull request as draft September 9, 2025 19:32
@makslevental makslevental force-pushed the users/makslevental/fix-stubgen-fetchcontent branch from 3dfc927 to 8f5523c Compare September 9, 2025 19:33
@makslevental makslevental changed the title [MLIR][Python] fix stubgen for FetchContent users [MLIR][Python] fix stubgen again Sep 9, 2025
@makslevental makslevental changed the title [MLIR][Python] fix stubgen again [MLIR][Python] fix stubgen for downstream users Sep 9, 2025
@makslevental makslevental force-pushed the users/makslevental/fix-stubgen-fetchcontent branch from 8f5523c to 308c656 Compare September 9, 2025 20:33
@makslevental makslevental marked this pull request as ready for review September 9, 2025 21:06
@makslevental makslevental force-pushed the users/makslevental/fix-stubgen-fetchcontent branch from 308c656 to ae0eaf3 Compare September 9, 2025 21:06
@makslevental
Copy link
Contributor Author

@joker-eph can you try this patch and see if it fixes this error that you mentioned:

# RUN: at line 7
"/home/mamini/bin/cmake-3.26.4-linux-x86_64/bin/cmake" --build . --target check-standalone | tee /home/mamini/projects/llvm-project/build/tools/mlir/test/Examples/standalone/Output/test.toy.tmp
# executed command: /home/mamini/bin/cmake-3.26.4-linux-x86_64/bin/cmake --build . --target check-standalone
# .---command stderr------------
# | ninja: error: '/home/mamini/projects/llvm-project/mlir/python/mlir/_mlir_libs/_mlirRegisterEverything.pyi', needed by 'python_packages/standalone/mlir_standalone/_mlirRegisterEverything.pyi', missing and no known rule to make it
# `-----------------------------
# error: command failed with exit status: 1
# executed command: tee /home/mamini/projects/llvm-project/build/tools/mlir/test/Examples/standalone/Output/test.toy.tmp

Copy link
Member

@rkayaith rkayaith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't fully follow the cmake-fu, but I've tried this out and it does indeed fix the issues I hit downstream.

@joker-eph
Copy link
Collaborator

joker-eph commented Sep 9, 2025

Right now it still fails:

# | ninja: error: 'build/tools/mlir/python/_mlir_libs/_mlir/__init__.pyi', needed by 'python_packages/standalone/mlir_standalone/_mlir/__init__.pyi', missing and no known rule to make it

@makslevental makslevental force-pushed the users/makslevental/fix-stubgen-fetchcontent branch from ae0eaf3 to ab5f242 Compare September 9, 2025 22:27
@makslevental
Copy link
Contributor Author

makslevental commented Sep 9, 2025

Right now it still fails:

# | ninja: error: 'build/tools/mlir/python/_mlir_libs/_mlir/__init__.pyi', needed by 'python_packages/standalone/mlir_standalone/_mlir/__init__.pyi', missing and no known rule to make it

can you paste your CMake configure

@joker-eph
Copy link
Collaborator

I'm using the "getting started" basic command somehow:

cmake -G Ninja ../llvm    -DLLVM_ENABLE_PROJECTS=mlir    -DLLVM_BUILD_EXAMPLES=ON    -DLLVM_TARGETS_TO_BUILD="Native;NVPTX;AMDGPU"    -DCMAKE_BUILD_TYPE=Release    -DLLVM_ENABLE_ASSERTIONS=ON  -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DLLVM_ENABLE_LLD=ON   -DLLVM_CCACHE_BUILD=ON -DMLIR_ENABLE_BINDINGS_PYTHON=ON 

@makslevental makslevental force-pushed the users/makslevental/fix-stubgen-fetchcontent branch from 71aa6b7 to 5f4f855 Compare September 10, 2025 00:46
@makslevental makslevental force-pushed the users/makslevental/fix-stubgen-fetchcontent branch 2 times, most recently from 52a834b to 86169c9 Compare September 10, 2025 01:41
@makslevental makslevental force-pushed the users/makslevental/fix-stubgen-fetchcontent branch 10 times, most recently from b2caa44 to f147cb9 Compare September 10, 2025 04:22
@makslevental makslevental force-pushed the users/makslevental/fix-stubgen-fetchcontent branch 9 times, most recently from bfef1dc to 6b6d435 Compare September 10, 2025 06:54
@makslevental makslevental force-pushed the users/makslevental/fix-stubgen-fetchcontent branch 2 times, most recently from b03c3a7 to 8da6462 Compare September 10, 2025 06:57
Remove GENERATE from add_python_modules
Remove GENERATE from add_python_modules
@makslevental
Copy link
Contributor Author

Closed in favor of #157853

@makslevental makslevental deleted the users/makslevental/fix-stubgen-fetchcontent branch September 11, 2025 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mlir:python MLIR Python bindings mlir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants