-
Notifications
You must be signed in to change notification settings - Fork 5.9k
【Hackathon 9th No.109】[CppExtension] Support build Custom OP in setuptools 80+ and support install extension via pip install . --no-build-isolation
#76008
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
Changes from 19 commits
a3bb2d7
3ed2fdb
0662252
e10f18b
872e846
3bfa9c3
055d1b0
0655f0f
99a4611
2e80847
9a94332
05a4114
6305767
fff063e
6103552
545b0fa
1dca083
c3fc22b
ba9349b
c1ebffc
c74e819
8e49d98
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,9 +25,13 @@ | |
| import setuptools | ||
| import sys | ||
| import paddle | ||
| import site | ||
|
|
||
| from setuptools.command.easy_install import easy_install | ||
| from setuptools.command.build_ext import build_ext | ||
| from distutils.command.build import build | ||
| from setuptools.command.install import install | ||
|
|
||
|
|
||
| from .extension_utils import ( | ||
| add_compile_flag, | ||
|
|
@@ -55,9 +59,9 @@ | |
| ) | ||
| from .extension_utils import _reset_so_rpath, clean_object_if_change_cflags | ||
| from .extension_utils import ( | ||
| bootstrap_context, | ||
| get_build_directory, | ||
| add_std_without_repeat, | ||
| custom_write_stub, | ||
| ) | ||
|
|
||
| from .extension_utils import ( | ||
|
|
@@ -235,6 +239,11 @@ def setup(**attr: Any) -> None: | |
| assert 'easy_install' not in cmdclass | ||
| cmdclass['easy_install'] = EasyInstallCommand | ||
|
|
||
| # Compatible with wheel installation via `pip install .` | ||
| # Note: This is rarely used with modern pip, which uses bdist_wheel instead | ||
| assert 'install' not in cmdclass | ||
| cmdclass['install'] = InstallCommand | ||
|
|
||
| # Note(Aurelius84): Add rename build_base directory hook in build command. | ||
| # To avoid using same build directory that will lead to remove the directory | ||
| # by mistake while parallelling execute setup.py, for example on CI. | ||
|
|
@@ -246,9 +255,7 @@ def setup(**attr: Any) -> None: | |
| # See http://peak.telecommunity.com/DevCenter/setuptools#setting-the-zip-safe-flag | ||
| attr['zip_safe'] = False | ||
|
|
||
| # switch `write_stub` to inject paddle api in .egg | ||
| with bootstrap_context(): | ||
| setuptools.setup(**attr) | ||
| setuptools.setup(**attr) | ||
|
|
||
|
|
||
| def CppExtension( | ||
|
|
@@ -849,8 +856,36 @@ def _clean_intermediate_files(self): | |
| os.remove(os.path.join(root, file)) | ||
| print(f"Removed: {os.path.join(root, file)}") | ||
|
|
||
| def _generate_python_api_file(self) -> None: | ||
| """ | ||
| Generate the top-level python api file (package stub) alongside the | ||
| built shared library in build_lib. This replaces the legacy bdist_egg | ||
| write_stub mechanism that is no longer triggered in setuptools >= 80. | ||
| """ | ||
| try: | ||
| outputs = self.get_outputs() | ||
| if not outputs: | ||
| return | ||
| # We only support a single extension per setup() | ||
| so_path = os.path.abspath(outputs[0]) | ||
| so_name = os.path.basename(so_path) | ||
| build_dir = os.path.dirname(so_path) | ||
| # The package name equals distribution name | ||
| pkg_name = self.distribution.get_name() | ||
|
||
| pyfile = os.path.join(build_dir, f"{pkg_name}.py") | ||
| # Write stub; it will reference the _pd_ renamed resource at import time | ||
| custom_write_stub(so_name, pyfile) | ||
| except Exception as e: | ||
| raise RuntimeError( | ||
| f"Failed to generate python api file: {e}" | ||
| ) from e | ||
|
|
||
| def run(self): | ||
| super().run() | ||
|
|
||
| # Compatible with wheel installation via `pip install .` | ||
| self._generate_python_api_file() | ||
|
|
||
| self._clean_intermediate_files() | ||
|
|
||
|
|
||
|
|
@@ -926,6 +961,169 @@ def initialize_options(self) -> None: | |
| self.build_base = self._specified_build_base | ||
|
|
||
|
|
||
| class InstallCommand(install): | ||
| """ | ||
| Extend install Command to: | ||
| 1) choose an install dir that is actually importable (on sys.path) | ||
| 2) ensure a single top-level entry for the package in site/dist-packages so | ||
| legacy tests that expect a sole artifact (egg/package) keep working | ||
| 3) rename the compiled library to *_pd_.so to avoid shadowing the python stub | ||
| """ | ||
|
|
||
| def finalize_options(self) -> None: | ||
| super().finalize_options() | ||
|
|
||
| install_dir = ( | ||
| getattr(self, 'install_lib', None) | ||
| or getattr(self, 'install_purelib', None) | ||
| or getattr(self, 'install_platlib', None) | ||
| ) | ||
| if not install_dir or not os.path.isdir(install_dir): | ||
| return | ||
| pkg = self.distribution.get_name() | ||
| # Check if dist-info exists | ||
| has_dist_info = any( | ||
| name.endswith('.dist-info') and name.startswith(pkg) | ||
| for name in os.listdir(install_dir) | ||
| ) | ||
| # If dist-info exists, we are installing a wheel, so we are done | ||
| if has_dist_info: | ||
| return | ||
|
|
||
| # Build candidate site dirs: global + user + entries already on sys.path | ||
| candidates = [] | ||
| candidates.extend(site.getsitepackages()) | ||
| usp = site.getusersitepackages() | ||
| if usp: | ||
| candidates.append(usp) | ||
| for sp in sys.path: | ||
| if isinstance(sp, str) and sp.endswith( | ||
| ('site-packages', 'dist-packages') | ||
| ): | ||
| candidates.append(sp) | ||
| # De-dup while preserving order | ||
| seen = set() | ||
| ordered = [] | ||
| for c in candidates: | ||
| if c and c not in seen: | ||
| seen.add(c) | ||
| ordered.append(c) | ||
| # Prefer a candidate that is actually on sys.path | ||
| target = None | ||
| for c in ordered: | ||
| if c in sys.path and os.path.isdir(c): | ||
| target = c | ||
| break | ||
| # Fallback: pick the first existing candidate | ||
| if target is None: | ||
| for c in ordered: | ||
| if os.path.isdir(c): | ||
| target = c | ||
| break | ||
| if target: | ||
| option_dict = self.distribution.get_option_dict('install') | ||
|
|
||
| if 'install_lib' not in option_dict: | ||
| self.install_lib = target | ||
|
|
||
| if 'install_purelib' not in option_dict: | ||
| self.install_purelib = target | ||
|
|
||
| if 'install_platlib' not in option_dict: | ||
| self.install_platlib = target | ||
|
|
||
| def run(self, *args: Any, **kwargs: Any) -> None: | ||
| super().run(*args, **kwargs) | ||
|
|
||
| install_dir = ( | ||
| getattr(self, 'install_lib', None) | ||
| or getattr(self, 'install_purelib', None) | ||
| or getattr(self, 'install_platlib', None) | ||
| ) | ||
| if not install_dir or not os.path.isdir(install_dir): | ||
| return | ||
| pkg = self.distribution.get_name() | ||
| # Check if dist-info exists | ||
| has_egg_info = any( | ||
| name.endswith('.egg-info') and name.startswith(pkg) | ||
| for name in os.listdir(install_dir) | ||
| ) | ||
| # If egg-info exists, we are installing a source distribution, we need to | ||
| # reorganize the files | ||
| if has_egg_info: | ||
| # First rename the shared library if present at top-level | ||
| self._rename_shared_library() | ||
| # Then canonicalize layout to a single top-level entry for this package | ||
| self._single_entry_layout() | ||
|
|
||
| def _rename_shared_library(self) -> None: | ||
| install_dir = ( | ||
| getattr(self, 'install_lib', None) | ||
| or getattr(self, 'install_purelib', None) | ||
| or getattr(self, 'install_platlib', None) | ||
| ) | ||
| if not install_dir or not os.path.isdir(install_dir): | ||
| return | ||
| pkg = self.distribution.get_name() | ||
| suffix = ( | ||
| '.pyd' | ||
| if IS_WINDOWS | ||
| else ('.dylib' if OS_NAME.startswith('darwin') else '.so') | ||
| ) | ||
| old = os.path.join(install_dir, f"{pkg}{suffix}") | ||
| new = os.path.join(install_dir, f"{pkg}_pd_{suffix}") | ||
| if os.path.exists(old): | ||
| if os.path.exists(new): | ||
| os.remove(new) | ||
| os.rename(old, new) | ||
|
|
||
| def _single_entry_layout(self) -> None: | ||
| """ | ||
| Ensure only one top-level item in install_dir contains the package name by: | ||
| - moving {pkg}.py -> {pkg}/__init__.py | ||
| - moving {pkg}_pd_.so -> {pkg}/{pkg}_pd_.so | ||
| - removing any {pkg}-*.egg-info left by setuptools install (only if dist-info exists) | ||
| This keeps legacy tests that scan os.listdir(site_dir) happy. | ||
| """ | ||
| install_dir = ( | ||
| getattr(self, 'install_lib', None) | ||
| or getattr(self, 'install_purelib', None) | ||
| or getattr(self, 'install_platlib', None) | ||
| ) | ||
| if not install_dir or not os.path.isdir(install_dir): | ||
| return | ||
| pkg = self.distribution.get_name() | ||
| # Prepare paths | ||
| pkg_dir = os.path.join(install_dir, pkg) | ||
| py_src = os.path.join(install_dir, f"{pkg}.py") | ||
| # Find compiled lib (renamed or not) | ||
| suf_so = ( | ||
| '.pyd' | ||
| if IS_WINDOWS | ||
| else ('.dylib' if OS_NAME.startswith('darwin') else '.so') | ||
| ) | ||
| so_candidates = [ | ||
| os.path.join(install_dir, f"{pkg}_pd_{suf_so}"), | ||
| os.path.join(install_dir, f"{pkg}{suf_so}"), | ||
| ] | ||
| so_src = next((p for p in so_candidates if os.path.exists(p)), None) | ||
| # Create package dir | ||
| if not os.path.isdir(pkg_dir): | ||
| os.makedirs(pkg_dir, exist_ok=True) | ||
| # Move python stub to package/__init__.py if exists | ||
| if os.path.exists(py_src): | ||
| py_dst = os.path.join(pkg_dir, "__init__.py") | ||
| if os.path.exists(py_dst): | ||
| os.remove(py_dst) | ||
| os.replace(py_src, py_dst) | ||
| # Move shared lib into the package dir if exists | ||
| if so_src and os.path.exists(so_src): | ||
| so_dst = os.path.join(pkg_dir, os.path.basename(so_src)) | ||
| if os.path.exists(so_dst): | ||
| os.remove(so_dst) | ||
| os.replace(so_src, so_dst) | ||
|
|
||
|
|
||
| def load( | ||
| name: str, | ||
| sources: Sequence[str], | ||
|
|
||
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.
啊 好快的删,没事我周一问问其他人再确定,反正就是一个 revert
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.
昨天忘记说了,昨天和 @zyfncg 讨论的结论是先不为 79-、80+ 单独添加兼容性逻辑,单独解决一下 FD 里的问题就可以了,解决完 FD 问题后如果其他场景有问题反馈再为 79-、80+ 单独添加兼容性逻辑
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.
有个地方没想明白,fd 为啥要把安装好的包 copy 到 fd 目录里面?方便后续一并卸载?应该也不是啊,它用的是 cp 而不是 mv ... ...
fd 如果要改的话,把现在生成的两个目录一起 copy 过去应该就可以了 ~
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.
按我理解是,这些自定义算子
fastdeploy_ops只是中间产物(仅算子实现),最终会被打包到fastdeploy包(含 Python 实现)里进行发布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.
相当于,将这些算子统一放到 fd 的命名空间中进行管理,而不是作为一个个单独的包?
OK ~ 那咱们这里还需要做啥?
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.
这个?现在是需要咱们这里修改兼容 fd 目前的实现方式?
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.
不一定是框架兼容,可以修改 FD 的 build 脚本
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.
嗯 ~ 我的意思也是,咱们这个 pr 应该不需要修改,最好是修改 fd 那边的脚本 ~ 那我给 fd 那边提个 pr ?
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.
嗯,如果确定不是这个 PR 导致的 bug,可以提 PR 改动下,不过那边的 PR 应该兼容两种方式,因为不能保证所有人都用 develop
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.
--install-lib之前是有问题的,前两天最新的 commit ba9349b 已经改了 ~fd 那边我这两天提个 pr 改一下 ~