Package ptoas CLI in wheel#909
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a ptoas CLI entry point to the ptoas wheel, packaging the unified runtime binaries and libraries, and implementing a Python launcher (_launcher.py) to execute the packaged binary with the correct environment variables. Feedback on the changes includes: importing NoReturn from typing to avoid a NameError, adding a fallback path in the launcher to support editable/development installations where the _runtime directory is not packaged, and simplifying a redundant conditional check in the _has_cli_option helper function.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| from __future__ import annotations | ||
|
|
||
| import os | ||
| import sys | ||
| from pathlib import Path |
There was a problem hiding this comment.
The type hint NoReturn is used for the main function, but it is not imported from the typing module. This will cause static type checkers (like mypy or pyright) to fail with an undefined name error.
| from __future__ import annotations | |
| import os | |
| import sys | |
| from pathlib import Path | |
| from __future__ import annotations | |
| import os | |
| import sys | |
| from pathlib import Path | |
| from typing import NoReturn |
| def main() -> "NoReturn": | ||
| package_root = Path(__file__).resolve().parent | ||
| runtime_root = package_root / "_runtime" | ||
| binary = runtime_root / "bin" / "ptoas" | ||
| if not binary.is_file(): | ||
| raise SystemExit( | ||
| f"wheel runtime is missing the packaged ptoas binary: {binary}" | ||
| ) |
There was a problem hiding this comment.
In an editable/development installation (e.g., pip install -e .), the _runtime directory is not packaged or copied into the source tree (ptodsl/ptoas/_runtime). As a result, when running the ptoas CLI in editable mode, binary.is_file() will be False and the launcher will fail with a SystemExit error.
To support editable/development installations, we should add a fallback to locate the binary in the build/install directory (e.g., using the PTO_INSTALL_DIR environment variable or defaulting to the in-repo install directory relative to the package root).
| def main() -> "NoReturn": | |
| package_root = Path(__file__).resolve().parent | |
| runtime_root = package_root / "_runtime" | |
| binary = runtime_root / "bin" / "ptoas" | |
| if not binary.is_file(): | |
| raise SystemExit( | |
| f"wheel runtime is missing the packaged ptoas binary: {binary}" | |
| ) | |
| def main() -> "NoReturn": | |
| package_root = Path(__file__).resolve().parent | |
| runtime_root = package_root / "_runtime" | |
| # Fallback for editable/development installs where _runtime is not packaged in the source tree | |
| if not runtime_root.exists(): | |
| env_install_dir = os.environ.get("PTO_INSTALL_DIR") | |
| if env_install_dir: | |
| runtime_root = Path(env_install_dir) | |
| else: | |
| # Default to in-repo install directory: repo_root/install | |
| runtime_root = package_root.parent.parent / "install" | |
| binary = runtime_root / "bin" / "ptoas" | |
| if not binary.is_file(): | |
| raise SystemExit( | |
| f"wheel runtime is missing the packaged ptoas binary: {binary}" | |
| ) |
| def _has_cli_option(argv: list[str], option: str) -> bool: | ||
| option_with_value = f"{option}=" | ||
| for index, arg in enumerate(argv): | ||
| if arg == option or arg.startswith(option_with_value): | ||
| return True | ||
| if arg == option and index + 1 < len(argv): | ||
| return True | ||
| return False |
There was a problem hiding this comment.
The second if condition in _has_cli_option is redundant and unreachable. If arg == option is true, the first if condition (arg == option or arg.startswith(option_with_value)) will already evaluate to True and return True.
We can simplify the function to remove the redundant check and the unused index variable.
def _has_cli_option(argv: list[str], option: str) -> bool:
option_with_value = f"{option}="
for arg in argv:
if arg == option or arg.startswith(option_with_value):
return True
return False
Codex Review该评论由 review 机器人自动更新。
SummaryReview failed at stage Findings未生成结构化 findings，因为 review 过程提前失败。 Log Tail |
Zhendong404
force-pushed
the
pack-ptoas-in-wheel
branch
3 times, most recently
from
b233ce6 to
1bc5ad5
Compare
Zhendong404
force-pushed
the
pack-ptoas-in-wheel
branch
from
29dcfa5 to
51aaf66
Compare
2 participants
No description provided.