fix: 이식성 잔여 — python3, 개행 경계 순회, 캐시 resolver 정렬 2중 결함 - #186
Conversation
…lity remainder
ml-toolkit: python -> python3 in the three places it is resolved from PATH.
environment-setup.md:318 is the one that mattered -- `python -m venv .venv` runs
BEFORE the venv is activated, so it resolves against the system PATH, and macOS
12.3 removed /usr/bin/python; the block labels itself "# Linux/Mac", so a Mac
reader following it could not create the environment at all. Lines 329 and 348
run after activation and are deliberately left alone. The gpu-parallel-pipeline
call site and check_gpu_memory.py's shebang + usage docstring follow for
consistency; those two only change the error message, since the script exits on
torch.cuda.is_available() anyway.
ppt-yeong-style: render-qa.sh iterated grep -REl output with `for f in $b_hits`,
which word-splits on IFS and glob-expands. Reproduced with a spaced DECK_ROOT:
"/tmp/qa test/svg_output/P03.svg" arrived as "/tmp/qa (0)" and
"test/svg_output/P03.svg (0)" -- two half-paths whose grep failed into a
discarded stderr. Now newline-bounded, giving the whole path and a real count.
The gate verdict was never affected (fail=1 is set before this loop); this is the
report the user reads to locate the leak. Not macOS-specific.
project-init: backported cr-fix's cache resolver, which fixes two independent
defects. The fallback was a plain lexicographic sort, so 0.6.1 outranked 0.10.0
and the resolver would pick an older cached version once a component reached two
digits. Sorting the full path is worse and is not what the audit found: measured
with zeta/0.4.0 against alpha/0.10.0, both plain sort AND sort -V pick
zeta/0.4.0, because the marketplace directory name is compared before the
version ever is. Prefixing the basename as the sort key closes both. The comment
claiming macOS lacks sort -V is also wrong -- Apple's FreeBSD sort has had it
since 10.13 -- and it was being used to justify the lexicographic fallback, so
the reasoning is corrected while the capability probe stays for older userlands.
Also in project-init: gh-repo-create-flow.md used ${VISIBILITY,,} twice, a Bash
4 expansion that is a bad substitution on macOS /bin/bash 3.2. Replaced with the
POSIX to_lower + case token match the executable procedure already uses, so the
raw answer is never passed straight into a CLI flag. And HAS_CODE was built with
`| head -1 | wc -l`, which BSD wc right-pads; it now returns yes/no like the
HAS_GIT and HAS_CLAUDE lines beside it, and stops at the first match.
Closes #177
Closes #178
Closes #179
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe changes synchronize patch versions for three plugins, update Python interpreter handling, preserve QA paths containing whitespace, and revise project initialization visibility, cache-version, and source-detection logic. ChangesPlugin maintenance
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 9✅ Passed checks (9 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe21c7edbb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ```bash | ||
| # Create virtual environment | ||
| python -m venv .venv | ||
| python3 -m venv .venv |
There was a problem hiding this comment.
이 블록은 바로 아래에 Windows용 활성화 경로도 제공하지만, 일반적인 Windows Python 설치는 python3.exe 대신 py -3 또는 python을 노출하므로 변경된 첫 명령에서 환경 생성이 중단됩니다. macOS의 python 부재를 해결하면서 Windows 흐름을 회귀시키지 않도록 OS별 명령을 병기하거나 인터프리터를 탐지해 선택하세요.
AGENTS.md reference: AGENTS.md:L287-L287
Useful? React with 👍 / 👎.
The venv-creation line is shared by a block that carries a Windows branch two lines down (`.venv\Scripts\activate # Windows`), so it has to serve all three platforms. Switching it to `python3 -m venv` fixed macOS 12.3+, which has no `python`, and broke a python.org Windows install, which ships `python.exe` and the `py` launcher but no `python3.exe`. Both spellings are now given with per-OS comments, matching the idiom the activation line two rows below already uses. The comment says why the line is sensitive at all: it runs before activation, so it resolves against the system PATH rather than the venv. Swept the rest of the file: the other Windows-branch block uses `uv venv` and names no interpreter, and the two `python -c` calls run after activation, where a venv provides `python` on every platform. No sibling occurrences. Addresses the Codex P1 on PR #186.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 580120c100
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fi | ||
| [ -d "$SKILL_DIR" ] || { echo "gpu-parallel-pipeline: skill dir not resolved" >&2; exit 1; } | ||
| python "$SKILL_DIR/scripts/check_gpu_memory.py" | ||
| python3 "$SKILL_DIR/scripts/check_gpu_memory.py" |
There was a problem hiding this comment.
Windows CUDA 환경에서 python.org 설치본처럼 python/py만 제공하고 python3.exe는 없는 경우, 메모리 계획 단계가 스크립트 실행 전에 command not found로 중단됩니다. 기존 venv 문서 위치와 별개로 이 실행 경로도 이번 커밋에서 python에서 python3로 바뀌었으므로, python3·python·py -3를 순서대로 탐지하거나 현재 런타임의 인터프리터를 사용하도록 분기하세요.
AGENTS.md reference: AGENTS.md:L285-L287
Useful? React with 👍 / 👎.
The audit rated this call site cosmetic, reasoning that macOS has no CUDA so the script exits either way and only the error message differs. That holds for macOS and not for Windows, where CUDA is available and the memory-planning step is a real feature -- so switching `python` to `python3` traded a macOS-only message change for a Windows functional break. No single spelling covers all three platforms: macOS 12.3+ dropped `python`, and a python.org Windows install ships `python` and the `py` launcher but no `python3`. Probes python3 -> python -> py -3 and fails loudly when none is present. Held in an indexed array (Bash 2+, so 3.2-safe; only associative arrays need Bash 4) so `py -3` survives as two words under quoting -- `"$PY" script.py` would look for an executable literally named "py -3", and dropping the quotes would break on a SKILL_DIR containing a space. Verified across four PATH scenarios: python3-only picks python3, python-only picks python, py-only picks `py -3`, and an empty PATH exits 1 with a message. Addresses the second Codex P1 on PR #186.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d56e71a86c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if command -v python3 >/dev/null 2>&1; then PY=(python3) | ||
| elif command -v python >/dev/null 2>&1; then PY=(python) | ||
| elif command -v py >/dev/null 2>&1; then PY=(py -3) |
There was a problem hiding this comment.
python3가 3.8이고 python이 PyTorch를 설치한 3.10+ 환경을 가리키는 시스템에서는 첫 분기가 무조건 선택되어, check_gpu_memory.py의 float | None 문법을 파싱하지 못하고 메모리 계획이 중단됩니다. help command가 설명하듯 command -v는 명령의 설명/위치만 확인하므로 버전이나 실행 가능성은 보장하지 않습니다. 기존 코멘트 이후 추가된 우선순위 선택기가 이 검증 없이 후보를 확정한다는 것이 새 근거이므로, 각 후보에 3.10+ 및 import torch probe를 실행해 통과한 인터프리터를 선택하거나 스크립트를 지원 최저 버전 문법으로 낮추세요.
AGENTS.md reference: AGENTS.md:L287-L287
Useful? React with 👍 / 👎.
cr-fix iter 3 — Codex P1 (SKILL.md:137) 를 skip 합니다지적: 전제가 성립하지 않습니다. 해당 스크립트는 11행에 검증한 것:
스크립트는 Python 3.7+ 에서 동작하므로 선택기가 보상해야 할 버전 제약이 없습니다. 그 상태에서 후보마다 일반론( |
1 participant
macOS/BSD 이식성 감사의 남은 코드 수정 묶음. 플러그인 3개(ml-toolkit, ppt-yeong-style, project-init)만 건드리므로 PR A/B 와
marketplace.json충돌이 없다.Closes #177
Closes #178
Closes #179
Refs #180 (project-init 부분 —
gh-repo-create-flow.md의${VISIBILITY,,}2곳,new-procedure.md:69의wc -l)1.
python->python33곳 (#177)macOS 12.3 (Monterey) 부터
/usr/bin/python이 제거됐다.실질적으로 문제가 되는 건 한 곳이다.
cv-notebook/references/environment-setup.md:318의python -m venv .venv는 venv 활성화 이전(활성화는 321행)에 실행되므로 시스템 PATH 에서 해석된다. 그 블록이# Linux/Mac으로 자기 대상을 명시하고 있어, 문서대로 따라 한 Mac 사용자는 환경 자체를 못 만든다.같은 파일 329·348 행의
python은 활성화 이후라 venv 인터프리터를 쓴다 — 의도적으로 손대지 않았다.나머지 둘(
gpu-parallel-pipeline/SKILL.md:130,check_gpu_memory.pyshebang + docstring)은 대상 스크립트가torch.cuda.is_available()실패 시 즉시 exit 하므로 macOS 에서는 인터프리터가 있어도 유의미한 결과가 안 나온다. 차이는 오류 메시지뿐이라 cosmetic 이고, 일관성 차원에서 같이 고쳤다.check_gpu_memory.py:1은 감사에서 회의적 검증 판정이 기록되지 않은 유일한 항목(verdict: null)이었다. 고치되 "검증됨" 으로 기록하지 않는다.2.
render-qa.sh개행 경계 순회 (#178)grep -REl의 결과를for f in $b_hits로 순회하면 IFS 단어 분리 + glob 확장이 일어난다. 공백이 든DECK_ROOT로 재현했다.조각난 경로에서 grep 이 실패하고 그 stderr 는
2>/dev/null로 삼켜져, FAIL 상세가 반쪽 경로 +(0)으로 오염된다.게이트 판정 자체는 원래 무사했다 —
fail=1이 이 루프보다 먼저 서기 때문이다. 손상되는 건 사용자가 leak 위치를 찾을 때 읽는 리포트다. macOS 전용이 아니고 Linux 에서 동일하게 재현된다.3. project-init 캐시 resolver 백포트 (#179)
감사는 폴백이 사전순이라
0.6.1이0.10.0을 이긴다는 것을 잡았다. 고치다 보니 독립된 결함이 하나 더 있었다.zeta/0.4.0대alpha/0.10.0으로 측정했다 (정답은alpha/0.10.0):sortzeta/project-init/0.4.0sort -Vzeta/project-init/0.4.0sort -Valpha/project-init/0.10.0alpha/project-init/0.10.0전체 경로를 정렬하면
sort -V조차 틀린다 — marketplace 디렉터리 이름이 버전 성분보다 먼저 비교되어 버전에 도달하지도 못한다. basename 을 정렬 키로 앞세우는 것이 두 결함을 동시에 닫는다.plugins/github-dev/skills/cr-fix/SKILL.md가 이미 쓰는 형태이고tests/run-tests.sh의 "fallback sort: 2.10.0 outranks 2.9.0" 케이스가 그걸 가드하고 있다 — 백포트가 누락돼 있었을 뿐이다.주석도 고쳤다. "macOS / BSD sort lacks -V" 는 사실이 아니다 (Apple 의 FreeBSD sort 는 10.13 부터
-V지원, synopsis[-bcCdfghiRMmnrsuVz]). 문제는 그 틀린 전제로 "X.Y.Z under 10 이면 충분" 이라며 사전순 폴백을 정당화하고 있었다는 점이다. capability probe 자체는 구형 유저랜드 대비로 유지한다.4.
${VISIBILITY,,}2곳 +HAS_CODE(#180 부분)gh-repo-create-flow.md:70,92— Bash 4 전용 확장이라 macOS/bin/bash3.2 에서bad substitution. 실행되는 절차(new-procedure.mdPhase 4/6)는 이미 POSIXto_lower+case토큰 매칭으로 해결돼 있었으므로 그 형태를 그대로 미러했다 — 새로 설계하지 않았다. 덤으로 사용자 답변을 CLI 플래그에 날것으로 넘기지 않게 된다.new-procedure.md:69—HAS_CODE가| head -1 | wc -l이라 BSDwc우측 정렬 패딩으로" 1"이 된다. 이웃HAS_GIT/HAS_CLAUDE는 깨끗한 yes/no 였다. 같은 형태로 맞추고-print -quit으로 첫 매치에서 멈추게 했다. (감사에서 "읽는 곳이 없는 dead variable" 로 판정이 갈렸던 항목 — 저장소 전역git grep HAS_CODE는 자기 정의 한 줄뿐인 게 맞다. 삭제 대신 이웃과 일관되게 살렸다.)검증
sync-codex-manifests.mjs --checksync-hermes-manifests.mjs --checkcheck-doc-consistency.mjscheck-skill-tool-portability.mjs --checkmock-load-hermes.pycr-fix/tests/run-tests.shbash -n render-qa.shpy_compile check_gpu_memory.py버전 범프:
ml-toolkit 1.4.3 -> 1.4.4,ppt-yeong-style 0.9.3 -> 0.9.4,project-init 0.6.1 -> 0.6.2,metadata.version 2.7.2 -> 2.7.3. Codex 매니페스트 + Hermes 어댑터 재생성 (ml-toolkit·ppt-yeong-style 은 HERMES_ELIGIBLE).실제 macOS 하드웨어 검증은 없다(unverified). 위 재현은 전부 Linux 에서의 등가 시뮬레이션이다.
Summary by CodeRabbit
python3(including venv creation guidance).gh repo createvisibility examples for macOS-compatible behavior.