-
Notifications
You must be signed in to change notification settings - Fork 134
fix(python-sdk): chmod CLI binary at build time instead of runtime #3179
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| """Build script that ensures the CLI binary is executable before wheel packaging.""" | ||
|
|
||
| import os | ||
| import stat | ||
| from pathlib import Path | ||
|
|
||
| from setuptools import setup | ||
| from setuptools.command.build_py import build_py | ||
|
|
||
|
|
||
| class BuildPyWithExecutableBinary(build_py): | ||
| """Custom build_py that ensures the CLI binary has execute permissions.""" | ||
|
|
||
| def run(self): | ||
| super().run() | ||
| # After files are copied to build dir, chmod the binary | ||
| if self.build_lib: | ||
| binary_path = Path(self.build_lib) / 'superdoc_sdk_cli_darwin_arm64' / 'bin' / 'superdoc' | ||
| if binary_path.exists(): | ||
| current_mode = binary_path.stat().st_mode | ||
| binary_path.chmod(current_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) | ||
|
|
||
|
|
||
| setup(cmdclass={'build_py': BuildPyWithExecutableBinary}) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,15 +10,15 @@ | |
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The four Unix setup.py files are byte-identical except for one string (the module name); the Windows one is a stub. Five files, one logical thing. Duplication like this is fine in a code-only PR — a small surface, easy to revisit. This PR is infrastructure: wheel builds ship binaries to every consumer, and we're literally fixing a wheel-build bug right now. The next permission edge case will be fixed in one file and forgotten in the other three, and we'll ship asymmetric wheels. Generate from python-embedded-cli-targets.mjs (already the source of truth), or extract a shared _build_helpers.py so each setup.py is a one-line shim. Worth doing in this PR — the duplication exists because of this change, and "we'll unify it later" is how five copies become eight. |
||
|
|
||
| def get_binary_path() -> str: | ||
| """Return the absolute path to the bundled CLI binary, ensuring it is executable.""" | ||
| """Return the absolute path to the bundled CLI binary. | ||
|
|
||
| The binary is made executable at build time via setup.py's build_py hook, | ||
| so no runtime chmod is needed. | ||
| """ | ||
| binary = resources.files(__package__).joinpath('bin', _BINARY_NAME) | ||
| path = str(binary) | ||
|
|
||
| if not os.path.isfile(path): | ||
| raise FileNotFoundError(f'CLI binary not found: {path}') | ||
|
|
||
| if os.name != 'nt': | ||
| mode = os.stat(path).st_mode | ||
| os.chmod(path, mode | 0o111) | ||
|
|
||
| return path | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| """Build script that ensures the CLI binary is executable before wheel packaging.""" | ||
|
|
||
| import os | ||
| import stat | ||
| from pathlib import Path | ||
|
|
||
| from setuptools import setup | ||
| from setuptools.command.build_py import build_py | ||
|
|
||
|
|
||
| class BuildPyWithExecutableBinary(build_py): | ||
| """Custom build_py that ensures the CLI binary has execute permissions.""" | ||
|
|
||
| def run(self): | ||
| super().run() | ||
| # After files are copied to build dir, chmod the binary | ||
| if self.build_lib: | ||
| binary_path = Path(self.build_lib) / 'superdoc_sdk_cli_darwin_x64' / 'bin' / 'superdoc' | ||
| if binary_path.exists(): | ||
| current_mode = binary_path.stat().st_mode | ||
| binary_path.chmod(current_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) | ||
|
|
||
|
|
||
| setup(cmdclass={'build_py': BuildPyWithExecutableBinary}) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| """Build script that ensures the CLI binary is executable before wheel packaging.""" | ||
|
|
||
| import os | ||
| import stat | ||
| from pathlib import Path | ||
|
|
||
| from setuptools import setup | ||
| from setuptools.command.build_py import build_py | ||
|
|
||
|
|
||
| class BuildPyWithExecutableBinary(build_py): | ||
| """Custom build_py that ensures the CLI binary has execute permissions.""" | ||
|
|
||
| def run(self): | ||
| super().run() | ||
| # After files are copied to build dir, chmod the binary | ||
| if self.build_lib: | ||
| binary_path = Path(self.build_lib) / 'superdoc_sdk_cli_linux_arm64' / 'bin' / 'superdoc' | ||
| if binary_path.exists(): | ||
| current_mode = binary_path.stat().st_mode | ||
| binary_path.chmod(current_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) | ||
|
|
||
|
|
||
| setup(cmdclass={'build_py': BuildPyWithExecutableBinary}) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| """Build script that ensures the CLI binary is executable before wheel packaging.""" | ||
|
|
||
| import os | ||
| import stat | ||
| from pathlib import Path | ||
|
|
||
| from setuptools import setup | ||
| from setuptools.command.build_py import build_py | ||
|
|
||
|
|
||
| class BuildPyWithExecutableBinary(build_py): | ||
| """Custom build_py that ensures the CLI binary has execute permissions.""" | ||
|
|
||
| def run(self): | ||
| super().run() | ||
| # After files are copied to build dir, chmod the binary | ||
| if self.build_lib: | ||
| binary_path = Path(self.build_lib) / 'superdoc_sdk_cli_linux_x64' / 'bin' / 'superdoc' | ||
| if binary_path.exists(): | ||
| current_mode = binary_path.stat().st_mode | ||
| binary_path.chmod(current_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) | ||
|
|
||
|
|
||
| setup(cmdclass={'build_py': BuildPyWithExecutableBinary}) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| """Build script for Windows platform package. | ||
|
|
||
| Windows doesn't use Unix file permissions, so no chmod is needed. | ||
| This setup.py exists for consistency across all platform packages. | ||
| """ | ||
|
|
||
| from setuptools import setup | ||
|
|
||
| setup() |
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.
if the path doesn't exist it will just fail silently.