feat(tools): 新增 MaaFramework 控制单元下载与解压脚本 - #21
Open
moranfanhua wants to merge 1 commit into
Open
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了两个问题，并提供了一些整体反馈：
detect_host_platform的实现在两个脚本中都被重复使用；建议将其抽取到一个共享的辅助函数中，这样在未来需要更新平台映射时可以避免出现不一致。check_asset_cache/set_asset_cache辅助函数使用extract_dir作为缓存位置，但实际操作的是下载目录；重命名该参数或收紧路径语义可以让意图更清晰，并减少混淆。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `detect_host_platform` implementation is duplicated in both scripts; consider extracting it into a shared helper to avoid divergence if platform mappings need updating later.
- The `check_asset_cache`/`set_asset_cache` helpers use `extract_dir` for the cache location but operate on the download directory; renaming the parameter or tightening the path semantics would make the intent clearer and reduce confusion.
## Individual Comments
### Comment 1
<location path="tools/maafw_download.py" line_range="82-91" />
<code_context>
+ return filename
+
+
+def retry_urlopen(*args, **kwargs):
+ import http.client
+
+ for _ in range(5):
+ try:
+ resp: http.client.HTTPResponse = urllib.request.urlopen(*args, **kwargs)
+ return resp
+ except urllib.error.HTTPError as e:
+ if e.status == 403 and e.headers.get("x-ratelimit-remaining") == "0":
+ # rate limit
+ t0 = time.time()
+ reset_time = t0 + 10
+ try:
+ reset_time = int(e.headers.get("x-ratelimit-reset", 0))
+ except ValueError:
+ pass
+ reset_time = max(reset_time, t0 + 10)
+ print(
+ f"rate limit exceeded, retrying after {reset_time - t0:.1f} seconds"
+ )
+ time.sleep(reset_time - t0)
+ continue
+ raise
+
+
</code_context>
<issue_to_address>
**issue (bug_risk):** retry_urlopen may return None silently if rate limiting persists across all retries.
If all retries hit a 403 rate limit, the loop exits and the function returns None, causing callers like `retry_urlopen(req).read()` to fail with an AttributeError instead of an HTTPError. Add an explicit failure after the loop (e.g., re-raise the last HTTPError or raise a dedicated exception) so callers see a predictable, meaningful error.
</issue_to_address>
### Comment 2
<location path="tools/maafw_download.py" line_range="136-143" />
<code_context>
+ f"no MAA-{platform}-* archive found in release {release['tag_name']}"
+ )
+
+ if cache_asset and check_asset_cache(asset, download_dir):
+ print("using cached asset", asset["name"])
+ return
+ url = asset["browser_download_url"]
+ print("downloading from", url)
+ download_dir.mkdir(parents=True, exist_ok=True)
+ local_file = download_dir / sanitize_filename(asset["name"])
+ if check_local_digest(local_file, asset["digest"]):
+ print("reusing matched digest", asset["digest"])
+ else:
</code_context>
<issue_to_address>
**issue (bug_risk):** Code assumes all assets have a "digest" field, which may raise KeyError for typical GitHub releases.
`main`, `check_asset_cache`, and `set_asset_cache` all index `asset["digest"]`. The standard GitHub release asset JSON does not include this field, so this will raise `KeyError` unless you have a separate process guaranteeing it. If `digest` is optional, use `asset.get("digest")` and handle `None` (e.g., skip digest validation/caching) to avoid hard failures on normal GitHub releases.
</issue_to_address>帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据这些反馈改进后续的代码审查。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- The
detect_host_platformimplementation is duplicated in both scripts; consider extracting it into a shared helper to avoid divergence if platform mappings need updating later. - The
check_asset_cache/set_asset_cachehelpers useextract_dirfor the cache location but operate on the download directory; renaming the parameter or tightening the path semantics would make the intent clearer and reduce confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `detect_host_platform` implementation is duplicated in both scripts; consider extracting it into a shared helper to avoid divergence if platform mappings need updating later.
- The `check_asset_cache`/`set_asset_cache` helpers use `extract_dir` for the cache location but operate on the download directory; renaming the parameter or tightening the path semantics would make the intent clearer and reduce confusion.
## Individual Comments
### Comment 1
<location path="tools/maafw_download.py" line_range="82-91" />
<code_context>
+ return filename
+
+
+def retry_urlopen(*args, **kwargs):
+ import http.client
+
+ for _ in range(5):
+ try:
+ resp: http.client.HTTPResponse = urllib.request.urlopen(*args, **kwargs)
+ return resp
+ except urllib.error.HTTPError as e:
+ if e.status == 403 and e.headers.get("x-ratelimit-remaining") == "0":
+ # rate limit
+ t0 = time.time()
+ reset_time = t0 + 10
+ try:
+ reset_time = int(e.headers.get("x-ratelimit-reset", 0))
+ except ValueError:
+ pass
+ reset_time = max(reset_time, t0 + 10)
+ print(
+ f"rate limit exceeded, retrying after {reset_time - t0:.1f} seconds"
+ )
+ time.sleep(reset_time - t0)
+ continue
+ raise
+
+
</code_context>
<issue_to_address>
**issue (bug_risk):** retry_urlopen may return None silently if rate limiting persists across all retries.
If all retries hit a 403 rate limit, the loop exits and the function returns None, causing callers like `retry_urlopen(req).read()` to fail with an AttributeError instead of an HTTPError. Add an explicit failure after the loop (e.g., re-raise the last HTTPError or raise a dedicated exception) so callers see a predictable, meaningful error.
</issue_to_address>
### Comment 2
<location path="tools/maafw_download.py" line_range="136-143" />
<code_context>
+ f"no MAA-{platform}-* archive found in release {release['tag_name']}"
+ )
+
+ if cache_asset and check_asset_cache(asset, download_dir):
+ print("using cached asset", asset["name"])
+ return
+ url = asset["browser_download_url"]
+ print("downloading from", url)
+ download_dir.mkdir(parents=True, exist_ok=True)
+ local_file = download_dir / sanitize_filename(asset["name"])
+ if check_local_digest(local_file, asset["digest"]):
+ print("reusing matched digest", asset["digest"])
+ else:
</code_context>
<issue_to_address>
**issue (bug_risk):** Code assumes all assets have a "digest" field, which may raise KeyError for typical GitHub releases.
`main`, `check_asset_cache`, and `set_asset_cache` all index `asset["digest"]`. The standard GitHub release asset JSON does not include this field, so this will raise `KeyError` unless you have a separate process guaranteeing it. If `digest` is optional, use `asset.get("digest")` and handle `None` (e.g., skip digest validation/caching) to avoid hard failures on normal GitHub releases.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+82
to
+91
| def retry_urlopen(*args, **kwargs): | ||
| import http.client | ||
|
|
||
| for _ in range(5): | ||
| try: | ||
| resp: http.client.HTTPResponse = urllib.request.urlopen(*args, **kwargs) | ||
| return resp | ||
| except urllib.error.HTTPError as e: | ||
| if e.status == 403 and e.headers.get("x-ratelimit-remaining") == "0": | ||
| # rate limit |
There was a problem hiding this comment.
issue (bug_risk): retry_urlopen 在所有重试都遇到限流时，可能静默返回 None。
如果所有重试都返回 403 限流，循环结束后函数会返回 None，导致调用方像 retry_urlopen(req).read() 这样的代码抛出 AttributeError，而不是 HTTPError。请在循环结束后添加一个显式的失败处理（例如，重新抛出最后一个 HTTPError，或抛出一个专门的异常），以便调用方能收到可预期且更有意义的错误信息。
Original comment in English
issue (bug_risk): retry_urlopen may return None silently if rate limiting persists across all retries.
If all retries hit a 403 rate limit, the loop exits and the function returns None, causing callers like retry_urlopen(req).read() to fail with an AttributeError instead of an HTTPError. Add an explicit failure after the loop (e.g., re-raise the last HTTPError or raise a dedicated exception) so callers see a predictable, meaningful error.
Comment on lines
+136
to
+143
| if cache_asset and check_asset_cache(asset, download_dir): | ||
| print("using cached asset", asset["name"]) | ||
| return | ||
| url = asset["browser_download_url"] | ||
| print("downloading from", url) | ||
| download_dir.mkdir(parents=True, exist_ok=True) | ||
| local_file = download_dir / sanitize_filename(asset["name"]) | ||
| if check_local_digest(local_file, asset["digest"]): |
There was a problem hiding this comment.
issue (bug_risk): 代码假设所有资源都有 "digest" 字段，这在典型的 GitHub Release 上可能导致 KeyError。
main、check_asset_cache 和 set_asset_cache 都直接访问 asset["digest"]。标准的 GitHub Release 资源 JSON 中并不包含这个字段，因此除非有额外的流程保证它存在，否则这里会抛出 KeyError。如果 digest 是可选字段，建议使用 asset.get("digest") 并处理返回的 None（例如跳过摘要校验/缓存），以避免在正常 GitHub Release 上出现硬错误。
Original comment in English
issue (bug_risk): Code assumes all assets have a "digest" field, which may raise KeyError for typical GitHub releases.
main, check_asset_cache, and set_asset_cache all index asset["digest"]. The standard GitHub release asset JSON does not include this field, so this will raise KeyError unless you have a separate process guaranteeing it. If digest is optional, use asset.get("digest") and handle None (e.g., skip digest validation/caching) to avoid hard failures on normal GitHub releases.
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
1 participant
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.
feat(tools): 新增 MaaFramework 控制单元下载与解压脚本
背景
MaaAssistantArknights 的 Win32Controller / MaaFwAdbController 功能需要 MaaFramework
控制单元 DLL（
MaaWin32ControlUnit.dll/MaaAdbControlUnit.dll），但 MAA 的构建产物不包含它们，开发文档此前要求开发者手动从 MaaFramework Releases 下载。
MAA 侧配套 PR
（新增
tools/maafw-download.py薄封装）需要子模块提供下载与解压实现。
本次在 MaaUtils 新增两个脚本，与现有
maadeps_download.py/maadeps-extract.py完全同构，供 MAA 侧封装调用。设计概要
maafw_download.py对应maadeps_download.py（
main(platform, repo, version, cache_asset)由封装按位置传参，下载/解压/安装逻辑内联在 main 中）；maafw-extract.py对应maadeps-extract.py（独立可运行，sys.argv手写解析，重复detect_host_platform）target_dir = <MAA仓库根>/build/bin/Release，download_dir/archive_dir=<MAA仓库根>/build/MaaFramework（脚本定位在 MAA checkout 的
src/MaaUtils/tools/下，向上 4 级取 MAA 仓库根）macos-aarch64，资产按 MaaFramework 实际命名匹配（
MAA-<platform>-v<version>.zip）build/MaaFramework/，与 GitHub API 返回的sha256:digest一致即跳过下载；
--cache-asset追加.cache_digest.json记录已安装资产，再次运行整体跳过build/MaaFramework/<资产名>/后仅复制bin/下*ControlUnit*到目标目录，不污染构建产物
新增文件（2 个）
tools/maafw_download.pymaadeps_download.py一致tools/maafw-extract.pymaadeps-extract.py一致用法
python tools/maafw-download.py
局限