Skip to content

fix(win): Python detection and proxy support#9

Open
yang12535 wants to merge 2 commits into
mainfrom
fix/win-python-proxy
Open

fix(win): Python detection and proxy support#9
yang12535 wants to merge 2 commits into
mainfrom
fix/win-python-proxy

Conversation

@yang12535

Copy link
Copy Markdown
Owner

Changes

  • _common.ps1: prefer py over python to avoid Store shim; accept 3.7+
  • gh-api.py: add proxy handler reading HTTP_PROXY/HTTPS_PROXY

Tested

Windows 10 + PS5.1 + Python 3.7.2 via py launcher, proxy 127.0.0.1:10808

- _common.ps1: prefer 'py' over 'python' to avoid Microsoft Store shim;
  accept Python 3.7+ (tested working) instead of hard 3.8+ requirement
- gh-api.py: add _install_proxy_handler() to read HTTP_PROXY/HTTPS_PROXY
  and configure urllib ProxyHandler explicitly

Tested on Windows 10 + PS5.1 + Python 3.7.2 via py launcher.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 44388d70e8

ℹ️ 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".

Comment thread scripts/windows/_common.ps1 Outdated
foreach ($c in $candidates) {
$cmd = Get-Command $c -ErrorAction SilentlyContinue
if (-not $cmd) { continue }
$verOutput = & $cmd --version 2>&1

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Probe the py launcher with a Python 3 qualifier

When $c is py, this calls the Windows launcher without a version qualifier; the Python launcher docs describe bare py as using its configured default, while py -3/python3 selects the latest Python 3.x. In Windows environments where PY_PYTHON/py.ini or an older launcher default points to Python 2 and no python3.exe is on PATH, this wrapper still rejects py even though a supported Python 3 is installed, so all Windows commands fail. Please probe and later invoke the launcher with an explicit -3 qualifier for the py candidate.

Useful? React with 👍 / 👎.

Comment thread scripts/gh-api.py Outdated
Comment on lines +18 to +19
os.environ.get("HTTPS_PROXY")
or os.environ.get("https_proxy")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor lowercase proxy overrides

When both uppercase and lowercase proxy variables are present, this manual lookup always chooses HTTPS_PROXY over https_proxy, whereas Python's normal urllib proxy discovery intentionally lets lowercase values override uppercase ones. In environments where an infrastructure-level uppercase proxy is stale or blocked and the user sets lowercase https_proxy for this command, the new handler sends GitHub API requests through the wrong proxy and the wrapper fails; preserving urllib's precedence or delegating to getproxies() avoids that regression.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

已推送 41c1829,完成 Windows 适配修正。

处理内容:

  • Windows Python 探测/调用改为 py -3 优先、python 兜底,不依赖 py3/python3
  • Python API 后端代理处理改为 urllib.request.getproxies(),保留小写 http_proxy/https_proxy 覆盖大写变量的标准优先级。
  • 同步更新 scripts/linux/gh-api.py,保持复制版 Python 后端一致。
  • 更新 README.mdSKILL.mdCHANGELOG.md 的 Windows Python/测试说明。
  • 补了 Windows PowerShell 5.1 兼容的 UTF-8 no BOM 写文件 helper,避免 Set-Content -Encoding utf8NoBOM 在 5.1 下不可用。

实际测试:

  • py -3 -m py_compile 覆盖 scripts/**/*.py
  • bash -n scripts/linux/*.sh scripts/*.sh
  • PowerShell parser 检查分别在 PowerShell 7.6 和 Windows PowerShell 5.1 下通过。
  • py -3 scripts\gh-api.py user -f login 返回 yang12535
  • pwsh.exe -File scripts\windows\gh-user.ps1powershell.exe -File scripts\windows\gh-user.ps1 均返回 yang12535
  • pwsh.exepowershell.exescripts\windows\gh-repo.ps1 yang12535/github-ops url 均返回仓库 URL。
  • 模拟 bare py --version 为 Python 2.7、py -3 --version 为 Python 3.x,Windows wrapper 仍能成功调用 API。
  • 模拟 PATH 中没有 py、只有 python,Windows wrapper 可通过 python fallback 成功调用 API。
  • 代理优先级测试确认 lowercase https_proxy 覆盖 uppercase HTTPS_PROXY,并同时覆盖 scripts/gh-api.pyscripts/linux/gh-api.py

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves Windows compatibility for the PowerShell wrappers by making Python discovery more reliable (avoiding the Microsoft Store shim / unintended Python 2 via py defaults) and adds proxy support to the Python urllib backend so requests can flow through standard proxy configuration.

Changes:

  • Update Windows wrappers to prefer py -3 (fallback python) and relax the minimum supported Python version to 3.7+.
  • Add proxy discovery/handling in gh-api.py via urllib.request.getproxies() and a ProxyHandler opener.
  • Improve Windows PowerShell 5.1 compatibility by writing temp JSON payload files as UTF-8 without BOM via a .NET helper.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
SKILL.md Documents Windows Python invocation guidance (py -3 then python).
scripts/windows/gh-pr.ps1 Uses UTF-8 no-BOM writer for temp JSON payload files.
scripts/windows/gh-issue.ps1 Uses UTF-8 no-BOM writer for temp JSON payload files.
scripts/windows/_common.ps1 Implements Python discovery preferring py -3, and adds Write-Utf8NoBomFile.
scripts/linux/gh-api.py Installs urllib proxy handler based on discovered proxies.
scripts/gh-api.py Installs urllib proxy handler based on discovered proxies.
README.md Updates platform requirements/docs (Python 3.7+, Windows invocation, proxy note, Windows compile check).
CHANGELOG.md Records Windows Python detection change, proxy discovery change, and UTF-8 no-BOM write fix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/gh-api.py
Comment on lines +15 to +21
def _install_proxy_handler():
"""Install urllib's standard proxy handler when proxy env vars are set."""
proxies = urllib.request.getproxies()
if proxies:
handler = urllib.request.ProxyHandler(proxies)
opener = urllib.request.build_opener(handler)
urllib.request.install_opener(opener)
Comment thread scripts/linux/gh-api.py
Comment on lines +15 to +21
def _install_proxy_handler():
"""Install urllib's standard proxy handler when proxy env vars are set."""
proxies = urllib.request.getproxies()
if proxies:
handler = urllib.request.ProxyHandler(proxies)
opener = urllib.request.build_opener(handler)
urllib.request.install_opener(opener)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants