Refactor: unify PTO-ISA resolve + auto-clone#555
Merged
ChaoZheng109 merged 1 commit intohw-native-sys:mainfrom Apr 14, 2026
Merged
Refactor: unify PTO-ISA resolve + auto-clone#555ChaoZheng109 merged 1 commit intohw-native-sys:mainfrom
ChaoZheng109 merged 1 commit intohw-native-sys:mainfrom
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 refactors the pto-isa dependency management logic, moving it from code_runner.py to a dedicated pto_isa.py module. The changes centralize repository cloning, commit pinning, and path resolution, while introducing file locking to handle concurrent access and an update_if_exists flag for CI environments. Review feedback suggests removing an unused verbose parameter and unifying git command execution in pto_isa.py, as well as using an existing helper function to replace a redundant directory check.
ChaoWao
force-pushed
the
refactor/unify-pto-isa-auto-clone
branch
3 times, most recently
from
1fdb46c to
ffdbc28
Compare
…helper
PTO-ISA unification
-------------------
Before this change there were two parallel implementations of
"ensure PTO-ISA is available":
simpler_setup/pto_isa.py::ensure_pto_isa_root()
- Only looked up (env var -> default path), raised OSError if missing.
- Called by scene_test.py, so `pytest` and standalone `python test_*.py`
hit OSError on a fresh checkout, with an error message that literally
told the user to go run ci.py or run_example.py first. A real
onboarding papercut.
simpler_setup/code_runner.py::_ensure_pto_isa_root()
- Full clone + commit pin + lock + fetch-latest logic (~280 lines).
- Called by ci.py and run_example.py (via CodeRunner).
Unify both behind a single API in simpler_setup/pto_isa.py. All four user
entry points (pytest, standalone test_*.py, ci.py, run_example.py) now
auto-clone on first use.
Move default clone target
examples/scripts/_deps/pto-isa -> PROJECT_ROOT/build/pto-isa
- `build/` is the repo's canonical artifact directory (already gitignored,
already hosts `build/lib/` and `build/cache/`).
- Per-repo/worktree/venv isolation via PROJECT_ROOT means concurrent
worktrees each get their own clone and don't race on commit pins.
- One-time re-clone for existing dev machines; acceptable.
New signature:
ensure_pto_isa_root(
commit: Optional[str] = None,
clone_protocol: str = "ssh",
update_if_exists: bool = False, # fetch origin/HEAD when True + no commit
verbose: bool = False,
) -> str
- scene_test calls with defaults -> auto-clone on first run, never touch
an existing clone (no network on every pytest run).
- ci.py / CodeRunner pass update_if_exists=True + verbose=True to preserve
"always ensure latest" behaviour; commit pin still wins when provided.
code_runner.py loses ~280 lines of PTO-ISA code and the fcntl import;
ci.py updates its two import sites to reach pto_isa directly
(ensure_pto_isa_root, checkout_pto_isa_commit, get_pto_isa_clone_path).
The "Cloning pto-isa to X ..." heads-up is emitted at WARNING level so it
surfaces even in pytest / standalone paths that have no basicConfig —
otherwise users would see a 30-60s silent hang on first run.
Shared --log-level helper
-------------------------
Add simpler_setup/log_config.py exposing LOG_LEVEL_CHOICES,
DEFAULT_LOG_LEVEL ("info"), and configure_logging(log_level).
All three CLI entry points now share one spelling of the flag, all default
to INFO:
ci.py --log-level error|warn|info|debug
examples/scripts/run_example.py --log-level ... (plus -v / --silent shortcuts)
python test_*.py (run_module) --log-level ...
The helper also propagates PTO_LOG_LEVEL env so ci.py's runtime-isolation
subprocesses inherit the level.
pytest is untouched — it has its own --log-cli-level / pyproject log_cli_level
and doesn't need a layered flag.
Verification
------------
- pytest tests/ut/py: 121 passed, 5 skipped (unchanged from main)
- tools/verify_packaging.sh: all 5 install modes × 4 entry points green
- All 3 CLI entry points expose --log-level choices with default info
ChaoWao
force-pushed
the
refactor/unify-pto-isa-auto-clone
branch
from
ffdbc28 to
4a75503
Compare
ChaoZheng109
approved these changes
Apr 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Single source of truth for locating / cloning / pinning the PTO-ISA repo. Before: two parallel implementations disagreed on whether to auto-clone, so
pytestandpython test_*.pyfailed on fresh checkouts with a hostile error message ("run ci.py first"). After: all four user entry points (pytest, standalone, ci.py, run_example.py) auto-clone on first use.Test plan
One-time migration cost
Existing dev machines and self-hosted runners have `examples/scripts/_deps/pto-isa/` from the old path. First run after this merge will auto-clone to `build/pto-isa/` — ~1 minute, one-time. Old location can be manually deleted (it's gitignored already).