Skip to content

Fix: pybind11_stubgen command due to pybind11 version bump #102

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.

Sign up for GitHub

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

Open
willkill07 wants to merge 1 commit into
base: branch-25.06
Choose a base branch
from
Open

Fix: pybind11_stubgen command due to pybind11 version bump #102

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions cmake/morpheus_utils/python/python_module_tools.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# SPDX-FileCopyrightText: Copyright (c) 2022-2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
# SPDX-FileCopyrightText: Copyright (c) 2022-2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
# SPDX-License-Identifier: Apache-2.0
#
# Licensed under the Apache License, Version 2.0 (the "License");
Expand Down Expand Up @@ -89,6 +89,7 @@ function(morpheus_utils_create_python_package PACKAGE_NAME)
"${project_dir}/pyproject.toml"
"${project_dir}/setup.cfg"
"${project_dir}/setup.py"
"${project_dir}/versioneer.py"
)

list(APPEND wheel_python_files ${wheel_python_project_files})
Expand Down Expand Up @@ -367,7 +368,6 @@ function(morpheus_utils_build_python_package PACKAGE_NAME)

# Change which setup we use if we are using inplace
if(_ARGS_IS_INPLACE)
list(APPEND _pip_command "-e")
Copy link
Copy Markdown
Contributor

@mdemoret-nv mdemoret-nv Apr 17, 2025

Choose a reason for hiding this comment

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

Why was this removed? This will cause the files to be copied and leads to some confusion.

Copy link
Copy Markdown
Member Author

@willkill07 willkill07 Apr 17, 2025

Choose a reason for hiding this comment

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

I was told it was unnecessary 🤷🏻‍♂️

list(APPEND _pip_command "${sources_source_dir}")
else()
list(APPEND _pip_command "${sources_binary_dir}")
Expand Down Expand Up @@ -622,10 +622,11 @@ macro(__create_python_library MODULE_NAME)
# Before installing, create the custom command to generate the stubs
add_custom_command(
OUTPUT ${module_binary_stub_file}
COMMAND ${Python3_EXECUTABLE} -m pybind11_stubgen ${TARGET_NAME} --no-setup-py --log-level WARN -o ./ --root-module-suffix \"\"
COMMAND env PYTHONPATH=${module_root_binary_dir}:$PYTHONPATH ${Python3_EXECUTABLE} -m pybind11_stubgen ${TARGET_NAME} --no-setup-py --log-level WARN -o ./ --root-module-suffix ""
Copy link
Copy Markdown
Contributor

@mdemoret-nv mdemoret-nv Apr 17, 2025

Choose a reason for hiding this comment

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

I think using the CMake command -E is preferred here. Something like add_custom_target(newtarget ${CMAKE_COMMAND} -E env NAME=VALUE somecommand)

Copy link
Copy Markdown
Member Author

@willkill07 willkill07 Apr 17, 2025

Choose a reason for hiding this comment

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

add_custom_target has the following behavior:

The target has no output file and is always considered out of date.

I don't think we want this which is why we have add_custom_command generate the stub file since we can easily specify the output file.

DEPENDS ${PYTHON_ACTIVE_PACKAGE_NAME}-modules $<TARGET_OBJECTS:${TARGET_NAME}>
COMMENT "Building stub for python module ${TARGET_NAME}..."
WORKING_DIRECTORY ${module_root_binary_dir}
VERBATIM
)

# Add a custom target to ensure the stub generation runs
Expand Down