From 52582ca8a1d8dccbbe4cc85b541a235ef00213b3 Mon Sep 17 00:00:00 2001 From: Pau1am <41251296+Pau1am@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:11:14 +0800 Subject: [PATCH 1/4] fix: repair the 37 findings from the v1.6.1 audit - a non-string `depends.minecraft` in a mod jar no longer costs the whole check its report: `_coerce_range` funnels the fabric/quilt/toml readers and `parse_spec`/`normalize_spec` refuse mappings and non-string values (MUC-A-01, MUC-A-08). - the download client shares the rate limiter with every other request (MUC-A-02); `RecursionError`/`MemoryError` from a deeply nested response become `UpstreamError` (MUC-A-03); a body that breaks mid-read is retried (MUC-A-04). - an installed download is deleted and the folder swept (MUC-A-05); `.part` and backup names are unique (MUC-A-11, MUC-A-13); permanently uninstallable records are marked instead of retried every stop (MUC-A-12); metadata reads are bounded by chunked reads rather than the declared size (MUC-A-10). - writes to one JSON file are serialised per directory and the rename retried, which is what Windows actually refuses (MUC-A-07); `ResolveCache` writes back only its own dirty keys under a process-wide lock (MUC-A-06). - a config type mistake is caught before MCDR regenerates the file (MUC-B-01); `isdigit` becomes `isdecimal` (MUC-B-02); `download all` honours the in-flight claim (MUC-B-03); the numeric range table is complete (MUC-B-04). - i18n no longer needs a 3.9+ API on the declared 3.8 floor (MUC-B-05); `last_report.json` is atomic (MUC-B-09); the eight hard-coded `!!modupdate` mentions use the alias that was actually typed (MUC-B-08); the version ordering is transitive and `is_prerelease` follows it (MUC-A-15). - `tools/mcdr_matrix.py` pins `PYTHONIOENCODING=utf-8`, reconfigures its own streams and decides "loaded" from ASCII markers, so a CP936 console no longer loses the failure evidence (MAIN-02). - `tools/probe_upstream.py` takes arguments, no longer downloads a jar for `--help` and reports offline failures instead of a traceback (MAIN-03); the mutation tool documents its real reason for requiring e2e (MAIN-04). - `tools/check_python_floor.py` checks (module, attribute) pairs, so `importlib.resources.files` cannot slip past the 3.8 floor again (MUC-B-05). - CI gains an e2e job and a pip cache and pins its actions; dependabot is enabled (REPO-21, REPO-29). An English README with a language cross-link and badges (REPO-34); the CI table matches `ci.yml`; the bench takes a mods path instead of a hard-coded one (REPO-22); the configured message the report tells admins to set is the path that exists (DOC-01, DOC-02). One commit for the whole round: the suites pin the files a fix/chore/docs split would separate - MUC's `tests/test_docs.py` asserts the packaging metadata, its CI table and the READMEs, IG's `tests/test_build_artifact.py` asserts the metadata and the packer, and SLF's tests read `tools/mcdr_matrix.py` - so splitting would leave red intermediate commits. --- .github/dependabot.yml | 31 ++ .github/workflows/ci.yml | 66 ++++- README-dev.md | 38 ++- README.md | 7 + README_en.md | 206 +++++++++++++ bench/audit_language.py | 79 +++-- bench/perf_v1.6.0.py | 380 +++++++++++++----------- mcdreforged.plugin.json | 8 +- mod_update_checker/__init__.py | 436 +++++++++++++++++++++++---- mod_update_checker/checker.py | 71 ++++- mod_update_checker/downloads.py | 65 +++- mod_update_checker/i18n.py | 51 +++- mod_update_checker/installer.py | 207 ++++++++++++- mod_update_checker/jsonfile.py | 129 +++++++- mod_update_checker/lang/en_us.json | 24 +- mod_update_checker/lang/zh_cn.json | 24 +- mod_update_checker/report.py | 47 ++- mod_update_checker/scanner.py | 137 ++++++++- mod_update_checker/upstream.py | 59 +++- mod_update_checker/versioning.py | 133 +++++++-- pytest.ini | 7 +- tests/README.md | 8 +- tests/test_audit_language.py | 82 ++++++ tests/test_checker.py | 74 ++++- tests/test_docs.py | 36 ++- tests/test_i18n.py | 40 +++ tests/test_jsonfile.py | 111 ++++++- tests/test_mcdr_entry.py | 458 +++++++++++++++++++++++++++-- tests/test_python_floor.py | 207 +++++++++++++ tests/test_scanner.py | 122 ++++++++ tests/test_versioning.py | 44 ++- tools/check_python_floor.py | 128 +++++++- tools/mcdr_matrix.py | 53 +++- tools/mutation_check.py | 22 +- tools/probe_upstream.py | 235 ++++++++++----- 35 files changed, 3309 insertions(+), 516 deletions(-) create mode 100644 .github/dependabot.yml create mode 100644 README_en.md create mode 100644 tests/test_audit_language.py create mode 100644 tests/test_python_floor.py diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..2248102 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,31 @@ +# Keep the workflow's actions current, as reviewable pull requests. +# +# Every `uses:` in .github/workflows/ is pinned to a commit SHA with the tag spelled out in a +# comment, which is what makes an upstream moving a *tag* harmless — but a pin also stops +# receiving fixes, silently, until somebody looks. This file is that somebody: GitHub's own +# updater opens a PR for each new release, so the pin moves on a schedule and the diff (SHA plus +# the comment) is the review. +version: 2 +updates: + - package-ecosystem: github-actions + directory: "/" + schedule: + interval: monthly + open-pull-requests-limit: 5 + commit-message: + prefix: "ci" + labels: + - dependencies + + # The test dependencies are pinned in tests/requirements-test.txt for reproducibility, and + # `pip` is what actually installs them into .testlibs; the language packages themselves are + # not declared with their own requirement files, so only this one matters. + - package-ecosystem: pip + directory: "/" + schedule: + interval: monthly + open-pull-requests-limit: 3 + commit-message: + prefix: "test" + labels: + - dependencies diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7cafb51..c604614 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,10 +23,11 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 10 steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v5 + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + - uses: actions/setup-python@42375524e23c412d93fb67b49958b491fce71c38 # v5.4.0 with: python-version: "3.13" + cache: pip - name: Install ruff run: python -m pip install "ruff==0.17.0" - name: Lint @@ -41,18 +42,51 @@ jobs: matrix: python-version: ["3.10", "3.13"] steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v5 + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + - uses: actions/setup-python@42375524e23c412d93fb67b49958b491fce71c38 # v5.4.0 with: python-version: ${{ matrix.python-version }} + cache: pip + cache-dependency-path: tests/requirements-test.txt - name: Install test dependencies run: python -m pip install --target .testlibs -r tests/requirements-test.txt # Python 3.10 has no tomllib, so this matrix also exercises the plugin's regex reader # for Forge/NeoForge metadata instead of only the parser every 3.11+ interpreter takes. - # The real-MCDR end-to-end is excluded here and covered by the matrix job below. + # The real-MCDR end-to-end is excluded here because it needs a real MCDR boot (about 2 m + # 20 s on its own) and is run by the `e2e` job below — not by the matrix job, which does + # not invoke pytest at all and therefore never ran these two cases. - name: Run the suite run: PYTHONPATH=.testlibs python -m pytest -m "not e2e" + e2e: + # The two end-to-end cases are in no other job: `unit` excludes them by marker, `mutation` + # only runs the files its selectors name, and `mcdr-matrix` drives its own scenario without + # pytest. So "827 passed, including 2 end-to-end" was a claim CI never checked. This job is + # that check — one real MCDR boot, the packed plugin, the whole flow. + name: end-to-end (real MCDR) + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + - uses: actions/setup-python@42375524e23c412d93fb67b49958b491fce71c38 # v5.4.0 + with: + python-version: "3.13" + cache: pip + cache-dependency-path: tests/requirements-test.txt + - name: Install test dependencies + run: python -m pip install --target .testlibs -r tests/requirements-test.txt + - name: Boot a real MCDR and drive the plugin through it + run: PYTHONPATH=.testlibs python -m pytest tests/test_e2e.py -q + - name: Upload the console when it fails + if: failure() + uses: actions/upload-artifact@b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882 # v4.4.3 + with: + name: e2e-console + path: | + /tmp/pytest-of-*/** + /tmp/muc_e2e_*/** + if-no-files-found: ignore + python-floor: # `ruff.toml` sets target-version = py38 because that is MCDR 2.13.0's floor, but a config # file is not a check and no job above runs an interpreter that old. This job is the check: @@ -61,8 +95,8 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 10 steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v5 + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + - uses: actions/setup-python@42375524e23c412d93fb67b49958b491fce71c38 # v5.4.0 with: python-version: "3.13" - name: Every file still runs on the declared floor @@ -75,10 +109,12 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 25 steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v5 + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + - uses: actions/setup-python@42375524e23c412d93fb67b49958b491fce71c38 # v5.4.0 with: python-version: "3.13" + cache: pip + cache-dependency-path: tests/requirements-test.txt - name: Install test dependencies run: python -m pip install --target .testlibs -r tests/requirements-test.txt - name: Every mutation must be caught by at least one test @@ -97,17 +133,19 @@ jobs: matrix: mcdr: ["2.13.0", "2.14.1", "2.15.0", "2.15.7", "2.16.0"] steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v5 + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + - uses: actions/setup-python@42375524e23c412d93fb67b49958b491fce71c38 # v5.4.0 with: python-version: "3.13" + cache: pip + cache-dependency-path: tests/requirements-test.txt - name: Install MCDR ${{ matrix.mcdr }} and pytest run: python -m pip install --target .testlibs "mcdreforged==${{ matrix.mcdr }}" pytest - name: Load the packed plugin, run a check, and drive every command run: PYTHONPATH=.testlibs python tools/mcdr_matrix.py --current - name: Upload the MCDR console when it fails if: failure() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882 # v4.4.3 with: name: mcdr-${{ matrix.mcdr }}-console path: /tmp/muc_matrix_work_*/** @@ -121,8 +159,8 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 15 steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v5 + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + - uses: actions/setup-python@42375524e23c412d93fb67b49958b491fce71c38 # v5.4.0 with: python-version: "3.13" - name: Pack twice, compare, and inspect diff --git a/README-dev.md b/README-dev.md index 0a5df15..ab6ef68 100644 --- a/README-dev.md +++ b/README-dev.md @@ -424,7 +424,7 @@ aqua=可操作的东西（按钮、命令、字段名）。 | 语言文件里没有「写进去过、现在没人用」的条目 | `tests/test_i18n.py` 三条守卫：每个键都必须在发布模块里作为**字符串常量**出现、五个运行时拼出来的族（`status.` / `matched_by.` / `explain.` / `install.reason.` / `download.reason.`）必须与常量表**完全一致**、两种语言的键集相同。再加一把更细的审计器 `bench/audit_language.py`（AST 只收「作为调用实参出现的字面量」，专抓只在**注释或 docstring** 里出现的键——正则扫全文看不出来） | | 渲染不是瓶颈 | `bench/bench_render.py`：20 / 200 个 Mod 的列表、汇总、整份报告（日志形态）、详情页各量一次。列表 **0.75 ms** 且**与总共多少个 Mod 基本无关**（只画一页）；唯一的重复劳动是每行重算格宽，现按语言缓存（195 µs → 0.8 µs / 10 行） | | 扫描不并行是有理由的 | `bench/scan_bench.py` 留有线程池那一路的数字：扫描跑在开服 60 秒之后的守护线程上，不在任何关键路径上，几百毫秒不值得多开线程。摘要按块读，内存不随文件增长（`tests/test_digests.py` 有断言） | -| 服务端的负担有数字 | `bench/perf_v1.6.0.py` 与 `bench/bench_render.py`：导入 ~68 ms（宿主已加载 mcdreforged 与 requests 时）、命令树与语言 0.1 ms 级、20 jar 扫描 ~90 ms、100 Mod（100 MiB）扫描 ~0.2 s + 比对与报告 ~17 ms + 3 次请求、空闲线程 0 CPU、峰值分配 ~2 MiB；渲染见 `bench/bench_render.py`（列表 0.7 ms） | +| 服务端的负担有数字 | `bench/perf_v1.6.0.py` 与 `bench/bench_render.py`：导入 ~68 ms（宿主已加载 mcdreforged 与 requests 时）、命令树与语言 0.1 ms 级、20 jar 扫描 ~90 ms、100 Mod（100 MiB）扫描 ~0.2 s + 比对与报告 ~17 ms + 3 次请求、空闲线程 0 CPU、峰值分配 ~2 MiB；渲染见 `bench/bench_render.py`（列表 0.7 ms）。采集条件：第 3 节的真实 `mods/` 目录要显式给（`--mods <目录>` 或 `MUC_BENCH_MODS`），否则该节跳过——它是「20 jar 扫描 ~90 ms」这类数字的来源，不给目录就没有数字可引用。其余各节用合成 Mod，任何机器上都能复现 | | 源码行末统一 LF | `test_no_source_file_carries_windows_line_endings`：扫工作区（跳过二进制与缓存），任何 `\r\n` 都失败。它守的是 Git 看不见的那一侧——`.gitattributes` 只在**提交时**归一，而「读文件再写回」的脚本会在 Windows 上把整份文件翻成 CRLF，内容一个字没改，diff 却全是警告 | | 前缀能匹配、且绝不比精确更宽 | `test_a_unique_prefix_of_a_name_is_enough`（唯一前缀命中）、`test_a_prefix_that_matches_two_mods_is_refused_and_lists_them`（多个就列出候选并拒绝）、`test_an_exact_match_is_never_widened_into_a_prefix_search`（有一个 Mod 就叫 `sod` 时，它就是 `sod`）、`test_a_prefix_is_matched_through_the_same_normalisation_as_everything_else`（前缀走与精确同一套归一化）。矩阵里另有一条：控制台的补全接口真的列出候选（`tree._entry_generate_suggestions`） | | 游戏内没有 Tab 补全可用 | 不是「没做」，是**做不到**：原版的补全只对 `/` 开头的命令生效，`!!` 命令是一条聊天消息。所以游戏内给的是 `suggest_command` 点击事件（点一下把命令填进输入框），矩阵断言候选那一行确实带那个事件 | @@ -434,7 +434,7 @@ aqua=可操作的东西（按钮、命令、字段名）。 | README 里的样例屏就是插件画的那一屏 | `test_the_readme_sample_screens_start_with_the_bar_the_plugin_draws`：拿 `_title_line()` 的真输出跟 README 里每一条标题栏逐字比对。标题栏的 `=` 是按常量算出来的，而三条样例曾长期停在一个早已改掉的宽度上——只查版本号的话永远发现不了 | | 那些守卫真的在守着什么 | `tools/mutation_check.py`：把每条守卫逐个改坏（在临时副本里改，工作树不受影响），对应的用例必须失败。没被抓住的守卫会被点名，锚点挪走了也会被点名——「脚本过期」与「测试没牙」是两回事，分开报 | -测试套件共 **827 项**（其中 2 项是真实 MCDR 端到端，慢约一分钟——本地发布前整跑一遍，CI 的快跑 +测试套件共 **827 项**（其中 2 项是真实 MCDR 端到端，**实测约 2 分 20 秒**，全量约 146 s——本地发布前整跑一遍，CI 的快跑 用 `-m "not e2e"` 跳过；当前数量用 `pytest --collect-only -q` 查；这一行是快照， 所以上面那张表里的「54 项检查」才是被测试自动核对的那个数字），细节见 [`tests/README.md`](tests/README.md)。 @@ -455,7 +455,8 @@ PYTHONPATH=.testlibs python -m pytest -m "not e2e" # 快 PYTHONPATH=.testlibs python -m pytest # 含真实 MCDR 端到端 # 变异检查：每条守卫被改坏都必须被测试抓到。 -# 它把仓库复制到临时目录再改，所以工作树不会被留在注入状态；MCDR_SKIP_E2E 设置时拒绝运行。 +# 它把仓库复制到临时目录再改，所以工作树不会被留在注入状态；MCDR_SKIP_E2E 设置时拒绝运行 +# （12 条 selector 全都由单测满足，拒绝是为了不让基线摘要少掉 e2e 那一半）。 python tools/mutation_check.py # 跨 MCDR 版本 @@ -485,24 +486,43 @@ python tools/probe_upstream.py | 代码注释与 docstring | **只留在仓库里**；进包的 `.py` 全部被清空（行号保留，堆栈仍能对上源文件） | `pack.py` 的 `packaged_source()` + `check_artifact.py` 的 `check_stripped()` | | 其它 | 只打包 `mcdreforged.plugin.json` / 插件包内的 `.py` / `lang/*.json` / `LICENSE` / `CHANGELOG.md` | 同上，白名单式打包 | -发版的顺序：**建分支 → 推分支 → 开 PR → 合并 → 再在合并后的 `main` 上打 tag 发 Release**。 -这样每个版本「新增/删除了什么」在 PR 页面里是逐行可见的。 +发版的顺序：**建分支 → 推分支 → 开 PR → 合并 → 等合并后的 `main` 全绿 → 再在 `main` 上打 tag 发 +Release**。这样每个版本「新增/删除了什么」在 PR 页面里是逐行可见的。 + +「等合并后的 `main` 全绿」不是形式：PR 上的绿色是**合并前**那棵树跑出来的，两棵树不一样。v1.6.1 +就是这么发出去的——tag 指向的提交之后还有两个提交在修同一条竞态测试（提交信息自己写着「It passed +on the pull request and locally, and failed on the merged main」），于是「套件全绿」这句话在 tag 上 +复现不出来。资产不受影响（两个提交只动测试，用 tag 的树重建 `.mcdr` 与仓库内那份字节相同），但 +**标签的意义**受影响：它本该是一个可复现的验证点。 + +另外，另外两个仓库的开发者文档叫 docs/DEVELOPMENT.md，本仓库是根目录的 `README-dev.md`。改名会 +牵动 `tests/test_documented_paths.py` 与多处文档链接，所以维持现状——按「照另一个仓库做」时要记得 +它俩讲的是同一件事。 发版前还要看一眼**仓库地址**：`mcdreforged.plugin.json` 的 `links`、`checker.py` 的 `USER_AGENT`、 CHANGELOG 的 Releases 链接必须指向真实存在的仓库。四处是否**互相一致**由 `tests/test_docs.py` 自动核对，但「这个 owner 是不是对的」只有线上远端知道—— 曾经这四处一起指向一个 404 的仓库，测试全绿而链接全废。 -署名口径：`mcdreforged.plugin.json` 的 `authors` 只列**作者**（插件库与 `!!MCDR` 的插件信息页读的是 -它）；文档整理、审查意见这类贡献记在 `README.md` 的「贡献者」一节，不写进 `authors`。 +署名口径：`mcdreforged.plugin.json` 的 `author` 只列**作者**（插件库与 `!!MCDR` 的插件信息页读的是 +它）；文档整理、审查意见这类贡献记在 `README.md` 的「贡献者」一节，不写进 `author`。 + +这里用的是 legacy 拼写，是**核实过五个版本**的结果：2.13.0 / 2.14.1 / 2.15.0 / 2.15.7 的解析器只有 +`author`（字符串列表）与顶层 `link`，`authors` / `links` / `license` 是 2.16.0 才有的字段。原先写的 +`authors` + `links` + `authors[0].link` 在 2.13–2.15.7 上让作者与主页**整块消失**，而 2.16.0 又只有 +`Person(name/email/homepage)`——`link` 在任何版本上都不是合法字段。所以现在是 `author: ["Pau1am"]` +加 `link`，`links` 保留给 2.16 读，并且**两套作者写法绝不同时出现**（2.16 会把它们合并成两个同名 +作者）。形状由 `tests/test_docs.py` 的两条测试钉住。 推送到 `main` 时会自动跑 CI（`.github/workflows/ci.yml`）： | 作业 | 内容 | |---|---| | `lint` | `ruff check .`（锁定版本 0.17.0，规则见根目录 `ruff.toml`） | -| `unit` | Python 3.10 与 3.13 上跑单元与集成测试（3.10 无 `tomllib`，顺带覆盖 Forge 元数据的正则回退路径） | -| `python-floor` | `tools/check_python_floor.py`：每个文件都必须能以 3.8 解析，且不用更新的注解写法或标准库名字（`ruff.toml` 里的 `target-version` 只是配置，这一条才是检查） | +| `unit` | Python 3.10 与 3.13 上跑单元与集成测试（3.10 无 `tomllib`，顺带覆盖 Forge 元数据的正则回退路径），`-m "not e2e"` | +| `e2e` | 单独一个作业跑那两项真实 MCDR 端到端（`tests/test_e2e.py`，实测约 2 分 20 秒），失败时上传控制台。在此之前它们**在任何作业里都不执行**：`unit` 按标记排除、`mutation` 只跑 selector 点名的文件、`mcdr-matrix` 自己驱动场景而不走 pytest | +| `mutation` | `tools/mutation_check.py`：12 条守卫逐个改坏，都必须被测试抓到 | +| `python-floor` | `tools/check_python_floor.py`：每个文件都必须能以 3.8 解析，且不用更新的注解写法、标准库模块名或标准库**属性**名（`ruff.toml` 里的 `target-version` 只是配置，这一条才是检查）。属性表是这次补的：`importlib.resources` 在 3.8 存在，`importlib.resources.files` 是 3.9+，而原来只按模块名判断，于是恰好在它该管的那个案例上给出假通过 | | `mcdr-matrix` | **2.13.0 / 2.14.1 / 2.15.0 / 2.15.7 / 2.16.0 各一个作业**，各自起真实 MCDR 跑完整流程 | | `artifact` | 连打两次产物比字节（可复现）、把源码临时改成 CRLF 再打一次要求字节一致（换行归一）、检查包内不含 tests/tools/README、确认包内 `.py` 已无注释与 docstring 且仍能编译、包内 changelog 只有一节 | diff --git a/README.md b/README.md index 32c25ba..359fd3d 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,15 @@ # Mod Update Checker +[![CI](https://github.com/LifeSci-Craft/MCDR-ModUpdateChecker/actions/workflows/ci.yml/badge.svg)](https://github.com/LifeSci-Craft/MCDR-ModUpdateChecker/actions/workflows/ci.yml) +[![MCDR](https://img.shields.io/badge/MCDR-2.13.0%2B-blue)](https://mcdreforged.com/) +[![Python](https://img.shields.io/badge/python-3.8%2B-blue)](https://www.python.org/) +[![License](https://img.shields.io/badge/license-MIT-green)](LICENSE) + > MCDR 的 Mod 更新检查插件：检查服务端 `mods/` 中的 Mod 是否有适配当前 Minecraft 版本与加载器 > 的新版本，更新信息来自 **Modrinth**。 +**English**: see [`README_en.md`](README_en.md). + **默认只检查和报告**：不会下载文件，也不会改动 `mods/`。下载到插件文件夹、安装到 `mods/`， 都需要显式开启或用命令指定；安装只会在服务端停止后进行，并保留旧版备份。不需要 API key，也没有 额外依赖。 diff --git a/README_en.md b/README_en.md new file mode 100644 index 0000000..86cce87 --- /dev/null +++ b/README_en.md @@ -0,0 +1,206 @@ +# Mod Update Checker + +**Language / 语言:** **English** | [简体中文](README.md) + +> An MCDReforged plugin that checks the server's `mods/` folder for builds matching the running +> Minecraft version and loader. Update information comes from **Modrinth**. + +[![CI](https://github.com/LifeSci-Craft/MCDR-ModUpdateChecker/actions/workflows/ci.yml/badge.svg)](https://github.com/LifeSci-Craft/MCDR-ModUpdateChecker/actions/workflows/ci.yml) +[![MCDR](https://img.shields.io/badge/MCDR-2.13.0%2B-blue)](https://mcdreforged.com/) +[![Python](https://img.shields.io/badge/python-3.8%2B-blue)](https://www.python.org/) +[![License](https://img.shields.io/badge/license-MIT-green)](LICENSE) + +| | | +|---|---| +| Plugin ID | `mod_update_checker` | +| Commands | `!!modupdate` / `!!muc` | +| Requires | MCDR **2.13.0+** (tested on 2.13.0 · 2.14.1 · 2.15.0 · 2.15.7 · 2.16.0) | +| License | MIT | + +**Check and report only, by default**: nothing is downloaded and `mods/` is never touched. Both +downloading into the plugin folder and installing into `mods/` need an explicit opt-in or an +explicit command; installation happens only after the server has stopped, and the previous jar is +kept as a backup. No API key, no extra dependencies. + +> Building or modifying the plugin, or looking for the decision rules and how they are verified? +> See [`README-dev.md`](README-dev.md) (Chinese). + +## Contents + +- [Features](#features) +- [Installation](#installation) +- [Commands](#commands) +- [Status](#status) +- [Configuration](#configuration) +- [Notes](#notes) +- [Credits](#credits) +- [License](#license) + +## Features + +- Identifies each jar by its SHA-1 and looks it up on Modrinth; when a hash is unknown it falls + back to a project-slug search, and the report labels that as an approximate match +- Every mod lands in one of the states below (updatable / up to date / no compatible build / + unresolved), and anything that needs action is always listed first +- Diagnoses the usual accidents along the way: the same mod installed twice, a required + dependency that is missing, a client-only mod sitting in a server folder +- The check runs on its own background thread — 60 seconds after startup by default, or on a + schedule — and hash lookups are batched (a hundred mods is usually a handful of requests) +- Optional download into the plugin folder, with hash verification (`download.enabled`) +- Optional install-on-stop into `mods/`, keeping the replaced jar as `.old` + (`download.install_on_stop`) +- Old backups are visible and cleanable (see the `cleanup` options below), and by default they + are shown but never deleted +- Console reporting by default; admin-on-join notices and in-game broadcasts are opt-in +- English and Simplified Chinese + +A listing in game looks like this (the Chinese screenshot form is in [`README.md`](README.md)): + +``` +============ Mod Update Checker v1.6.1 ============ +server: 26.3 / Fabric (version source: server_info) +[1] Lithium [version] [status: up] [details] +[2] Sodium [version] [status: up] [details] +[3] Iris [version] [status: ok] [details] +3 mod(s). +Checked 3 jar(s): 1 up to date, 2 with updates not yet downloaded, ... +one mod's detail (version change and links): !!muc info . +======= hover [version] [status], click [details] ======= +``` + +## Installation + +1. Put `ModUpdateChecker-v*.mcdr` into MCDR's `plugins/` folder; +2. Run `!!MCDR reload plugin` (or restart MCDR); +3. The first load writes `config/mod_update_checker/config.json`; the defaults work as they are; +4. Run `!!muc check` for an immediate check, or wait for the automatic one 60 seconds after + startup. + +No extra Python dependencies: `requests` ships with MCDR and everything else is the standard +library. + +## Commands + +MCDR permission level 3 is required (configurable). `!!modupdate` and `!!muc` are equivalent; the +bare command opens the help page. + +| Command | Effect | +|---|---| +| `!!modupdate` (= `!!modupdate help`) | Help page | +| `!!modupdate check` | Run a check now | +| `!!modupdate list [status] [page]` | List mods, optionally filtered by status, with paging | +| `!!modupdate summary` | Summary of the last check | +| `!!modupdate info ` | One mod's detail | +| `!!modupdate download ` | Fetch new builds; `all` = everything pending | +| `!!modupdate install ` | Schedule an install at the next stop (the file must be downloaded) | +| `!!modupdate delete ` | Delete old backups (needs `cleanup.allow_delete`) | +| `!!modupdate cleanup` | Delete every expired old backup (same gate) | +| `!!modupdate confirm` | Confirm the previous `download` / `install` / `delete` / `cleanup` | +| `!!modupdate status` | Server, configuration and last-check state | +| `!!modupdate reload` | Reload the config file | + +A target can be a **number** (from `list`), a **mod name** (a unique prefix is enough), a **mod +id**, or a **jar file name**; the console completes them with Tab. + +`download` / `install` / `delete` / `cleanup` are two-step: the plan is shown first and carried +out by `!!modupdate confirm` (valid for 120 seconds, and only the requester can confirm it). + +## Status + +| Icon | Meaning | +|---|---| +| `✔` | Up to date (`up_to_date`) | +| `↑` | A newer build is available (`update_available`) — `!!modupdate download ` | +| `↓` | Downloaded, waiting to be installed (`awaiting_install`) — `!!modupdate install ` | +| `❌` | No build for this loader or game version (`no_compatible_build`) | +| `☆` | The local build is newer than upstream (`local_ahead`) | +| `?` | Modrinth does not know the jar (`unresolved`) — see `sources.manual_map` below | +| `○` | Not a mod (`not_a_mod`: a library, a datapack) | +| `✘` | Unreadable, or the lookup failed (`error`) | +| `⏪` | An old backup (`old_backup`) | +| `—` | Ignored through `check.ignored_mods` (`ignored`) | + +A hash match is exact; a name-based match is a guess, and the report says so. + +## Configuration + +`config/mod_update_checker/config.json`. Options added by a newer version are filled in at +defaults and the file is rewritten. Out-of-range numbers are **clamped in memory** with one +console notice (the file itself is never edited, so the notice repeats until you fix it); the +range is stated next to each option. The tables in [`README.md`](README.md) list every option with +its full path, and the essentials are: + +| Option | Default | Notes | +|---|---|---| +| `enabled` | `true` | Master switch; with it off only the commands remain | +| `language` | `"auto"` | `auto` follows MCDR; `zh_cn` / `en_us` pin it | +| `command_permission_level` | `3` | Minimum MCDR permission level (0–4) | +| `server.mods_directory` | `""` | Blank = `mods` under the server directory | +| `server.loader` | `"fabric"` | `fabric` / `quilt` / `neoforge` / `forge` | +| `server.mc_version` | `"auto"` | `auto` infers it; **set it explicitly when the guess is wrong** | +| `check.on_server_start` | `true` | One automatic check after startup | +| `check.start_delay_seconds` | `60` | Wait before that check (0–3600) | +| `check.interval_hours` | `0` | Periodic check interval; `0` = off (0–720) | +| `check.include_beta` / `check.include_alpha` | `false` | Count pre-releases as updates | +| `check.ignored_mods` | `[]` | Mods that are never checked | +| `report.updates_only` | `true` | One line when an automatic check finds nothing | +| `report.on_admin_join` | `true` | Notify an admin when they join | +| `report.admin_permission` | `3` | Who counts as an admin (0–4) | +| `report.reuse_report_minutes` | `1440` | How old a report may be to be reused; `0` = always re-check | +| `report.in_game` | `false` | Broadcast an update to online admins | +| `report.in_game_permission` | `3` | Minimum level for that broadcast (0–4) | +| `report.write_file` | `true` | Write `last_report.json` / `last_report.txt` | +| `sources.modrinth.enabled` | `true` | Query Modrinth | +| `sources.modrinth.api_base` | `""` | Blank = the official endpoint; a mirror works, and `HTTP_PROXY` / `HTTPS_PROXY` are honoured | +| `sources.manual_map` | `"project-map.json"` | Hand-written map in the plugin data folder (`""` disables it) | +| `download.enabled` | `false` | Download new builds into the plugin folder | +| `download.folder_name` | `"downloads"` | Sub-folder name (a name, not a path) | +| `download.max_size_mb` | `128` | Per-file size limit (1–8192) | +| `download.install_on_stop` | `false` | Install the downloads after the server stops | +| `download.retries` | `3` | Download retries (total attempts = 1 + this) | +| `cleanup.allow_delete` | `false` | Master switch for every deletion | +| `cleanup.enabled` | `false` | Remind an admin that backups expired | +| `cleanup.max_age_days` | `30` | Age at which a backup counts as expired; `0` = immediately (0–3650) | +| `network.timeout_seconds` | `20` | Per-request timeout (1–300) | +| `network.retries` | `3` | Retries on failure, including 429 | +| `network.concurrent_requests` | `4` | Concurrency (1–32) | +| `network.requests_per_minute` | `240` | Self-imposed rate limit (Modrinth allows 300/min; 0–1000) | +| `network.cache.enabled` | `true` | Remember "upstream does not know this jar" | +| `network.cache.ttl_hours` | `24` | How long that verdict lasts; `0` = never expires | + +A jar you compiled or repacked yourself (so upstream cannot recognise its hash) can be mapped by +hand in `project-map.json`, for example `{"by_mod_id": {"mycustommod": "lithium"}}`: `by_mod_id` +matches the mod id (recommended — it survives a rebuild) and `by_sha1` matches the exact bytes. +The value is a project slug or id; run `!!modupdate reload` afterwards. + +With `install_on_stop` on, the install always happens **after the server has stopped** and only +for files this plugin downloaded: the old jar is renamed to `.jar.old` (`.old.2` next, never +overwriting a backup), the download is hash-verified before anything moves, a failure rolls back, +and an admin's bracket prefix such as `[Lithium]` is carried onto the new file name. + +Old backups stay in the listing with status `⏪` and are never deleted until +`cleanup.allow_delete` is on; `delete` and `cleanup` both require `!!modupdate confirm`. + +## Notes + +- A repacked jar may not be recognised (its hash changed); map it with `project-map.json`. +- Jars nested inside other jars are not checked separately, so the "missing dependency" + advisory can be a false positive when the dependency is bundled inside another jar. +- When Modrinth is unreachable the check still completes, the report says so, and no mod is + reported as up to date on the strength of a failed request. A mirror or a system proxy is + configurable. +- The game version is inferred. After a major upgrade, a wave of `no_compatible_build` usually + means the inference is wrong: check `!!modupdate status` first, and set `server.mc_version` + explicitly. +- An unreadable `mods/` folder (permissions, another program holding it) is reported as such by + `!!modupdate status`, and the check is skipped with a reason rather than reported as + "0 jars, all good". + +## Credits + +- [alex3236](https://github.com/alex3236) — maintainer of the MCDR plugin catalogue: reviewed this + plugin's catalogue entry and helped tighten the `description` and the README. + +## License + +[MIT](LICENSE) diff --git a/bench/audit_language.py b/bench/audit_language.py index c55af71..5604217 100644 --- a/bench/audit_language.py +++ b/bench/audit_language.py @@ -9,10 +9,15 @@ 这一份用 AST 只收「作为调用实参出现的字符串」，再逐键比对占位符与调用处传的关键字。 -用法（仓库根目录）：``python bench/audit_language.py`` +**第 2 节不再靠猜调用点。** 它原来把「任何调用实参里出现的字符串常量」当成消息键，再在那次 +调用上取关键字——于是键经变量传递时（`tr(header_key, count=...)`、`tr(key, **args)`、 +`tr(choice.note_key, **choice.note_args)`）它看不到真正的实参，实测 15 条输出全是误报，而真丢了 +占位符的模板会被这些噪音淹掉。现在改成**运行时**用真正的 `Catalog` 对每个键做一次 format 断言： +传进去的参数由模板自己的 `{...}` 决定，所以「这个键能不能被填满」是可判定的，不再依赖调用点长什么样。 """ import ast +import importlib.util import json import pathlib import re @@ -56,6 +61,37 @@ def calls_with_key(path: pathlib.Path): return out +def _load_i18n(): + """``mod_update_checker.i18n`` without importing MCDR. + + The package's ``__init__`` pulls in ``mcdreforged.api.all``, and this script is documented to + run from a plain interpreter (``tests/README.md`` calls it a language-file audit, not a + plugin entry point). Loading the module file by path keeps the one thing this needs — the + real :class:`Catalog`, so the format check cannot drift from the runtime — and nothing else. + """ + spec = importlib.util.spec_from_file_location( + "audit_language_i18n", str(PACKAGE / "i18n.py") + ) + module = importlib.util.module_from_spec(spec) + # ``__package__`` is what ``pkgutil.get_data`` uses to find the language files. + module.__package__ = "mod_update_checker" + spec.loader.exec_module(module) + return module + + +i18n = _load_i18n() + + +def _dummy_args(template: str) -> dict: + """Argument values to format ``template`` with, one per placeholder it declares. + + Taken from the template rather than from any call site, which is the whole point: it makes + "is this template fillable?" a property of the template, so a key passed through a variable + (or through ``**args``) is checked exactly like a literal one. + """ + return {name: "x" for name in _PLACEHOLDER.findall(template)} + + def main() -> int: languages = sorted(PACKAGE.glob("lang/*.json")) catalogues = { @@ -86,24 +122,29 @@ def main() -> int: print() print("=" * 78) - print("2. 模板里的占位符，从来没有人传过") + print("2. 模板里的占位符，用真正的 Catalog 填一遍看能不能填满") print("=" * 78) - orphan_placeholders = [] - for key in sorted(chinese): - if key.startswith(DYNAMIC_PREFIXES): - continue - wanted = set(_PLACEHOLDER.findall(chinese[key])) - if not wanted: - continue - given = set() - for keywords, _name, _line in passed.get(key, []): - given |= keywords - missing = sorted(char for char in wanted if char not in given) - if missing: - orphan_placeholders.append((key, missing, chinese[key])) - for key, missing, text in orphan_placeholders: - print(" {:<34} 没人传 {} ← {}".format(key, missing, text)) - if not orphan_placeholders: + unfillable = [] + for language in sorted(catalogues): + catalog = i18n.get_catalog(language) + for key in sorted(catalogues[language]): + template = catalog.messages.get(key) + if template is None: + continue + args = _dummy_args(template) + try: + rendered = template.format(**args) + except (KeyError, IndexError, ValueError, AttributeError, TypeError) as error: + # The same exceptions ``translate`` swallows, reported instead of swallowed: a + # template that cannot be filled is the bug this section exists to name. + unfillable.append((language, key, "{}: {}".format(type(error).__name__, error))) + continue + leftover = _PLACEHOLDER.search(rendered) + if leftover: + unfillable.append((language, key, rendered)) + for language, key, result in unfillable: + print(" {:<6} {:<34} ← {}".format(language, key, result.replace("\n", "\\n"))) + if not unfillable: print(" （无）") print() @@ -136,7 +177,7 @@ def main() -> int: print() print("键数：zh_cn {} / en_us {}".format(len(chinese), len(english))) - return 1 if (suspicious or orphan_placeholders) else 0 + return 1 if (suspicious or unfillable) else 0 if __name__ == "__main__": diff --git a/bench/perf_v1.6.0.py b/bench/perf_v1.6.0.py index a4efd36..72d0a62 100644 --- a/bench/perf_v1.6.0.py +++ b/bench/perf_v1.6.0.py @@ -2,14 +2,16 @@ 报告在 ``bench/perf-v1.6.0.md``，这里的数字就是它的来源。在仓库目录下运行： - PYTHONPATH=".testlibs;tests" python bench/perf_v1.6.0.py + PYTHONPATH=".testlibs;tests" python bench/perf_v1.6.0.py [--mods <你的 mods 目录>] 五件事各自独立测量，互不借用成见： 1. **导入**：新起进程 import 一次，取 5 次的中位数——这是每次启动都要付的钱； 2. **加载的可测部分**：命令树构建与语言装载（``on_load`` 里除日志与注册之外的主要工作）； -3. **扫描**：真实服务端 ``mods/``（20 个 jar）与 100 个合成 jar 各扫一遍（顺序读 + 单遍 SHA-1， - 正是检查里最重的本地工作）； +3. **扫描**：一个真实服务端的 ``mods/`` 与 100 个合成 jar 各扫一遍（顺序读 + 单遍 SHA-1， + 正是检查里最重的本地工作）。真实目录由 ``--mods`` 或 ``MUC_BENCH_MODS`` 给出——**不再写死 + 作者本机路径**：那既把私人目录结构带进公开仓库，又让第 3 节在别人机器上只打印一句「跳过」 + 然后以 0 退出，而文档把那一节的数字当来源引用； 4. **一次完整检查**：对本地假上游（真的 HTTP，环回）跑 100 个 Mod 的全流程，量墙钟、进程 CPU、 请求数； 5. **空闲**：一个像调度线程那样反复 ``Event.wait`` 的线程，在 3 秒里消耗多少 CPU（应为零）。 @@ -17,6 +19,7 @@ 渲染路径的数字由 ``bench/bench_render.py`` 单独产出；两者一起构成「它有多重」的全部答案。 """ +import argparse import hashlib import json import os @@ -36,13 +39,36 @@ sys.path.insert(0, str(REPO / ".testlibs")) sys.path.insert(0, str(REPO / "tests")) -#: 用户真实服务端的 mods 目录（只读扫描；找不到就跳过这一节）。 -REAL_MODS = Path("C:/Users/Paul LAM/Desktop/MC26.3/MCDR-Survival/server/mods") +#: 要扫描的真实 mods 目录，由调用者给出：``--mods <目录>`` 或 ``MUC_BENCH_MODS`` 环境变量。 +#: +#: 这里原来写死的是作者本机路径（``C:/Users/.../Desktop/...``）。两个后果：公开仓库里带着一个 +#: 私人目录结构，而别人跑同一个脚本时第 3 节只打印一句「跳过」然后以 0 退出——README-dev 却把 +#: 那一节量出来的数字当作来源引用。所以它现在是显式输入：没给就跳过，并说清楚怎么给。 +REAL_MODS = None COUNT = 100 PAYLOAD_MB = 1 +def parse_args(argv=None): + """``--mods`` 是唯一参数：其余章节用合成 Mod，不需要任何输入。""" + parser = argparse.ArgumentParser( + prog="perf_v1.6.0.py", + description="量这个插件的开销：导入、扫描、一次完整检查、空闲 CPU。", + epilog=( + "第 3 节扫描的是你自己的 mods 目录，必须显式给出，否则跳过。" + " 例如：--mods /path/to/server/mods" + ), + ) + parser.add_argument( + "--mods", + metavar="DIR", + default=os.environ.get("MUC_BENCH_MODS") or None, + help="真实 mods 目录（只读）。默认取环境变量 MUC_BENCH_MODS；两者都没有就跳过第 3 节", + ) + return parser.parse_args(argv) + + def head(title): print() print("=" * 72) @@ -56,175 +82,193 @@ def ms(seconds): return "{:.2f} s".format(seconds) -print("Python :", sys.version.split()[0], "|", sys.platform) -print("仓库 :", REPO) - -# -------------------------------------------------------------------------------------- -# 1. 导入 -# -------------------------------------------------------------------------------------- -head("1. 导入（新进程 ×5，取中位数；同一件事三种口径）") -env = dict(os.environ, PYTHONPATH=str(REPO / ".testlibs")) -TIMED = ("import time; t=time.perf_counter(); import mod_update_checker; " - "print(time.perf_counter()-t)") -VARIANTS = ( - ("冷启动整体（连 mcdreforged 与依赖一起）", TIMED), - ("mcdreforged 已加载", "import mcdreforged; " + TIMED), - ("mcdreforged + requests 已加载（宿主常态）", "import mcdreforged, requests; " + TIMED), -) -for label, code in VARIANTS: - times = [] - for _ in range(5): - proc = subprocess.run( - [sys.executable, "-c", code], - cwd=str(REPO), env=env, capture_output=True, text=True, - ) - times.append(float(proc.stdout.strip())) - print(" {:<46} 中位 {}".format(label, ms(statistics.median(times)))) - print(" 每次: " + ", ".join(ms(item) for item in times)) +def main(argv=None): + """跑完全部五节。参数只有一个 ``--mods``，见 :func:`parse_args`。""" + args = parse_args(argv) + global REAL_MODS + REAL_MODS = args.mods + + print("Python :", sys.version.split()[0], "|", sys.platform) + print("仓库 :", REPO) + + # -------------------------------------------------------------------------------------- + # 1. 导入 + # -------------------------------------------------------------------------------------- + head("1. 导入（新进程 ×5，取中位数；同一件事三种口径）") + env = dict(os.environ, PYTHONPATH=str(REPO / ".testlibs")) + TIMED = ("import time; t=time.perf_counter(); import mod_update_checker; " + "print(time.perf_counter()-t)") + VARIANTS = ( + ("冷启动整体（连 mcdreforged 与依赖一起）", TIMED), + ("mcdreforged 已加载", "import mcdreforged; " + TIMED), + ("mcdreforged + requests 已加载（宿主常态）", "import mcdreforged, requests; " + TIMED), + ) + for label, code in VARIANTS: + times = [] + for _ in range(5): + proc = subprocess.run( + [sys.executable, "-c", code], + cwd=str(REPO), env=env, capture_output=True, text=True, + ) + times.append(float(proc.stdout.strip())) + print(" {:<46} 中位 {}".format(label, ms(statistics.median(times)))) + print(" 每次: " + ", ".join(ms(item) for item in times)) + + # -------------------------------------------------------------------------------------- + # 2. 加载的可测部分 + # -------------------------------------------------------------------------------------- + head("2. 加载的可测部分：命令树构建 + 语言装载") + import timeit # noqa: E402 + + import mod_update_checker as plugin # noqa: E402 + + + class _Cfg: + language = "zh_cn" + + + tree_time = timeit.timeit(lambda: plugin._command_tree("!!muc"), number=50) / 50 + lang_time = timeit.timeit(lambda: plugin._apply_language(None, _Cfg()), number=50) / 50 + print(" 命令树构建 :", ms(tree_time)) + print(" 语言装载 :", ms(lang_time)) + + # -------------------------------------------------------------------------------------- + # 3. 扫描真实文件夹 + # -------------------------------------------------------------------------------------- + head("3. 扫描真实服务端的 mods/（只读）") + from mod_update_checker.scanner import scan_mods # noqa: E402 + + if REAL_MODS is None: + print(" 跳过：没有给出真实 mods 目录。") + print(" 用 --mods <目录> 或环境变量 MUC_BENCH_MODS=<目录> 指一个再跑一次；") + print(" 第 4 节用合成 Mod，不受影响。") + else: + REAL_MODS = Path(REAL_MODS) + if not REAL_MODS.is_dir(): + print(" 跳过：{} 不是一个目录".format(REAL_MODS)) + REAL_MODS = None + + if REAL_MODS is not None: + jars = sorted(p for p in REAL_MODS.iterdir() if p.suffix == ".jar") + total = sum(p.stat().st_size for p in jars) + print(" 目录: {} 个 jar，{:.0f} MiB".format(len(jars), total / 1048576)) + for round_no in (1, 2): + started = time.perf_counter() + scan_mods(REAL_MODS) + print(" 第 {} 遍墙钟: {}".format(round_no, ms(time.perf_counter() - started))) + tracemalloc.start() + scan_mods(REAL_MODS) + _current, peak = tracemalloc.get_traced_memory() + tracemalloc.stop() + print(" 峰值分配（tracemalloc 口径）: {:.1f} MiB".format(peak / 1048576)) + + # -------------------------------------------------------------------------------------- + # 4. 100 个合成 Mod：扫描 + 一次完整检查 + # -------------------------------------------------------------------------------------- + head("4. {} 个合成 Mod：扫描 + 一次完整检查（本地假上游，真 HTTP 环回）".format(COUNT)) + from fake_upstream import FakeFile, FakeProject, FakeUpstream, FakeVersion # noqa: E402 + from support import fabric_metadata # noqa: E402 + + from mod_update_checker.checker import Checker, CheckOptions # noqa: E402 + from mod_update_checker.serverinfo import ServerContext # noqa: E402 + + work = Path(tempfile.mkdtemp(prefix="muc_perf_")) + upstream = FakeUpstream().start() + try: + mods = work / "mods" + mods.mkdir() + payload = os.urandom(PAYLOAD_MB * 1024 * 1024) + for index in range(COUNT): + mod_id = "perfmod{:03d}".format(index) + path = mods / (mod_id + ".jar") + with zipfile.ZipFile(path, "w", zipfile.ZIP_STORED) as archive: + archive.writestr("fabric.mod.json", json.dumps(fabric_metadata( + id=mod_id, name="Perf Mod {:03d}".format(index), version="1.0.0"))) + archive.writestr("payload.bin", payload) + sha1 = hashlib.sha1(path.read_bytes()).hexdigest() + upstream.add_project(FakeProject( + id="proj-" + mod_id, + slug=mod_id, + title="Perf Mod {:03d}".format(index), + versions=[ + FakeVersion(id=mod_id + "-v1", project_id="proj-" + mod_id, + version_number="1.0.0", + files=[FakeFile(sha1=sha1, filename=mod_id + ".jar")]), + FakeVersion(id=mod_id + "-v2", project_id="proj-" + mod_id, + version_number="1.1.0", + date_published="2026-02-01T00:00:00Z", + files=[FakeFile(sha1="b" * 40, + filename=mod_id + "-1.1.0.jar", + url=upstream.file_url(mod_id + "-1.1.0.jar"))]), + ], + )) + on_disk = sum(p.stat().st_size for p in mods.iterdir()) + print(" 合成目录: {} 个 jar，{:.0f} MiB".format(COUNT, on_disk / 1048576)) -# -------------------------------------------------------------------------------------- -# 2. 加载的可测部分 -# -------------------------------------------------------------------------------------- -head("2. 加载的可测部分：命令树构建 + 语言装载") -import timeit # noqa: E402 + started = time.perf_counter() + scan = scan_mods(mods) + print(" 扫描（顺序读 + 单遍 SHA-1）:", ms(time.perf_counter() - started)) -import mod_update_checker as plugin # noqa: E402 + options = CheckOptions( + loader="fabric", mc_version="26.3", modrinth_base=upstream.modrinth_base, + workers=4, requests_per_minute=0, timeout=10, retries=1, use_cache=False, + ) + context = ServerContext(mc_version="26.3", mc_version_source="server_info", loader="Fabric") + + checker = Checker(options) + try: + started = time.perf_counter() + cpu0 = time.process_time() + report = checker.run(scan, context, cache_path=None) + wall = time.perf_counter() - started + cpu = time.process_time() - cpu0 + finally: + checker.close() + print(" 完整检查墙钟（含假上游的 HTTP 往返）:", ms(wall)) + print(" 完整检查进程 CPU:", ms(cpu)) + print(" 报告自记 duration_seconds: {:.2f} s".format(report.duration_seconds)) + updates = sum(1 for item in report.entries if item.status == "update_available") + print(" 结果: {} 个条目，{} 个判为有更新".format(len(report.entries), updates)) + print(" HTTP 请求数: {}".format(len(upstream.requests))) + for method, path, _body in upstream.requests: + print(" {} {}".format(method, path)) + + tracemalloc.start() + started = time.perf_counter() + checker = Checker(options) + try: + checker.run(scan_mods(mods), context, cache_path=None) + finally: + checker.close() + _current, peak = tracemalloc.get_traced_memory() + tracemalloc.stop() + print(" 完整检查峰值分配（tracemalloc 口径）: {:.1f} MiB".format(peak / 1048576)) + finally: + upstream.stop() + shutil.rmtree(work, ignore_errors=True) + # -------------------------------------------------------------------------------------- + # 5. 空闲 CPU + # -------------------------------------------------------------------------------------- + head("5. 空闲 CPU：像调度线程那样反复等待的线程") + stop = threading.Event() -class _Cfg: - language = "zh_cn" + def _waiter(): + while not stop.wait(0.2): + pass -tree_time = timeit.timeit(lambda: plugin._command_tree("!!muc"), number=50) / 50 -lang_time = timeit.timeit(lambda: plugin._apply_language(None, _Cfg()), number=50) / 50 -print(" 命令树构建 :", ms(tree_time)) -print(" 语言装载 :", ms(lang_time)) -# -------------------------------------------------------------------------------------- -# 3. 扫描真实文件夹 -# -------------------------------------------------------------------------------------- -head("3. 扫描真实服务端的 mods/（只读）") -from mod_update_checker.scanner import scan_mods # noqa: E402 + worker = threading.Thread(target=_waiter, daemon=True) + cpu0 = time.process_time() + worker.start() + time.sleep(3.0) + stop.set() + worker.join(timeout=5) + print(" 一个等待线程跑 3 秒的进程 CPU 增量:", ms(time.process_time() - cpu0)) + print(" （调度线程平时就活在这样的等待里；默认 check.interval_hours=0 时它不启动，") + print(" 开服后那次检查也只是 delay 线程 sleep 60 秒再干活一次。）") -if REAL_MODS.is_dir(): - jars = sorted(p for p in REAL_MODS.iterdir() if p.suffix == ".jar") - total = sum(p.stat().st_size for p in jars) - print(" 目录: {} 个 jar，{:.0f} MiB".format(len(jars), total / 1048576)) - for round_no in (1, 2): - started = time.perf_counter() - scan_mods(REAL_MODS) - print(" 第 {} 遍墙钟: {}".format(round_no, ms(time.perf_counter() - started))) - tracemalloc.start() - scan_mods(REAL_MODS) - _current, peak = tracemalloc.get_traced_memory() - tracemalloc.stop() - print(" 峰值分配（tracemalloc 口径）: {:.1f} MiB".format(peak / 1048576)) -else: - print(" 跳过：找不到", REAL_MODS) - -# -------------------------------------------------------------------------------------- -# 4. 100 个合成 Mod：扫描 + 一次完整检查 -# -------------------------------------------------------------------------------------- -head("4. {} 个合成 Mod：扫描 + 一次完整检查（本地假上游，真 HTTP 环回）".format(COUNT)) -from fake_upstream import FakeFile, FakeProject, FakeUpstream, FakeVersion # noqa: E402 -from support import fabric_metadata # noqa: E402 - -from mod_update_checker.checker import Checker, CheckOptions # noqa: E402 -from mod_update_checker.serverinfo import ServerContext # noqa: E402 - -work = Path(tempfile.mkdtemp(prefix="muc_perf_")) -upstream = FakeUpstream().start() -try: - mods = work / "mods" - mods.mkdir() - payload = os.urandom(PAYLOAD_MB * 1024 * 1024) - for index in range(COUNT): - mod_id = "perfmod{:03d}".format(index) - path = mods / (mod_id + ".jar") - with zipfile.ZipFile(path, "w", zipfile.ZIP_STORED) as archive: - archive.writestr("fabric.mod.json", json.dumps(fabric_metadata( - id=mod_id, name="Perf Mod {:03d}".format(index), version="1.0.0"))) - archive.writestr("payload.bin", payload) - sha1 = hashlib.sha1(path.read_bytes()).hexdigest() - upstream.add_project(FakeProject( - id="proj-" + mod_id, - slug=mod_id, - title="Perf Mod {:03d}".format(index), - versions=[ - FakeVersion(id=mod_id + "-v1", project_id="proj-" + mod_id, - version_number="1.0.0", - files=[FakeFile(sha1=sha1, filename=mod_id + ".jar")]), - FakeVersion(id=mod_id + "-v2", project_id="proj-" + mod_id, - version_number="1.1.0", - date_published="2026-02-01T00:00:00Z", - files=[FakeFile(sha1="b" * 40, - filename=mod_id + "-1.1.0.jar", - url=upstream.file_url(mod_id + "-1.1.0.jar"))]), - ], - )) - on_disk = sum(p.stat().st_size for p in mods.iterdir()) - print(" 合成目录: {} 个 jar，{:.0f} MiB".format(COUNT, on_disk / 1048576)) - - started = time.perf_counter() - scan = scan_mods(mods) - print(" 扫描（顺序读 + 单遍 SHA-1）:", ms(time.perf_counter() - started)) - - options = CheckOptions( - loader="fabric", mc_version="26.3", modrinth_base=upstream.modrinth_base, - workers=4, requests_per_minute=0, timeout=10, retries=1, use_cache=False, - ) - context = ServerContext(mc_version="26.3", mc_version_source="server_info", loader="Fabric") - checker = Checker(options) - try: - started = time.perf_counter() - cpu0 = time.process_time() - report = checker.run(scan, context, cache_path=None) - wall = time.perf_counter() - started - cpu = time.process_time() - cpu0 - finally: - checker.close() - print(" 完整检查墙钟（含假上游的 HTTP 往返）:", ms(wall)) - print(" 完整检查进程 CPU:", ms(cpu)) - print(" 报告自记 duration_seconds: {:.2f} s".format(report.duration_seconds)) - updates = sum(1 for item in report.entries if item.status == "update_available") - print(" 结果: {} 个条目，{} 个判为有更新".format(len(report.entries), updates)) - print(" HTTP 请求数: {}".format(len(upstream.requests))) - for method, path, _body in upstream.requests: - print(" {} {}".format(method, path)) - - tracemalloc.start() - started = time.perf_counter() - checker = Checker(options) - try: - checker.run(scan_mods(mods), context, cache_path=None) - finally: - checker.close() - _current, peak = tracemalloc.get_traced_memory() - tracemalloc.stop() - print(" 完整检查峰值分配（tracemalloc 口径）: {:.1f} MiB".format(peak / 1048576)) -finally: - upstream.stop() - shutil.rmtree(work, ignore_errors=True) - -# -------------------------------------------------------------------------------------- -# 5. 空闲 CPU -# -------------------------------------------------------------------------------------- -head("5. 空闲 CPU：像调度线程那样反复等待的线程") -stop = threading.Event() - - -def _waiter(): - while not stop.wait(0.2): - pass - - -worker = threading.Thread(target=_waiter, daemon=True) -cpu0 = time.process_time() -worker.start() -time.sleep(3.0) -stop.set() -worker.join(timeout=5) -print(" 一个等待线程跑 3 秒的进程 CPU 增量:", ms(time.process_time() - cpu0)) -print(" （调度线程平时就活在这样的等待里；默认 check.interval_hours=0 时它不启动，") -print(" 开服后那次检查也只是 delay 线程 sleep 60 秒再干活一次。）") +if __name__ == "__main__": + main() diff --git a/mcdreforged.plugin.json b/mcdreforged.plugin.json index 9b9437f..f6b7946 100644 --- a/mcdreforged.plugin.json +++ b/mcdreforged.plugin.json @@ -6,12 +6,10 @@ "en_us": "Check and update server-side mods from Modrinth.", "zh_cn": "从 Modrinth 检查和更新服务端 Mod" }, - "authors": [ - { - "name": "Pau1am", - "link": "https://github.com/Pau1am" - } + "author": [ + "Pau1am" ], + "link": "https://github.com/LifeSci-Craft/MCDR-ModUpdateChecker", "links": { "homepage": "https://github.com/LifeSci-Craft/MCDR-ModUpdateChecker", "source": "https://github.com/LifeSci-Craft/MCDR-ModUpdateChecker", diff --git a/mod_update_checker/__init__.py b/mod_update_checker/__init__.py index 06f26e2..54598bc 100644 --- a/mod_update_checker/__init__.py +++ b/mod_update_checker/__init__.py @@ -87,7 +87,7 @@ ) from . import i18n -from .jsonfile import read_json, write_json +from .jsonfile import read_json, write_json, write_text from .checker import USER_AGENT, CheckOptions, Checker from .installer import ( STATUS_INSTALLED as INSTALL_INSTALLED, @@ -154,7 +154,7 @@ ) from .cleanup import Backup, expired, list_backups, remove_backups, total_bytes from .report import format_size as _format_size -from .upstream import HttpClient +from .upstream import HttpClient, RateLimiter from .projectmap import MAP_FILE_NAME, ProjectMap, resolve_map_file from .scanner import ( ScanResult, @@ -789,32 +789,58 @@ def _load_config(server: PluginServerInterface) -> Config: defaults. Silent is the problem: an admin who fat-fingered a comma would see their settings vanish with no explanation, so the old file is preserved and the reason logged. - The same silence covers the other two things MCDR does to this file — creating it when it - is missing, and filling in options it does not have and writing the result back. Both are - reported here (see :func:`_report_config_file_state`); the language is applied first so - that report is written in the language the file just asked for. + "Unparseable" is not only a syntax error. ``load_config_simple`` catches whatever + ``Serializable.deserialize`` raises and regenerates the file for that too, so a **type** + mistake — ``network.timeout_seconds: "45"``, valid JSON with the wrong type — reset the whole + file just as thoroughly and just as quietly. Measured before this was fixed: a 36-option file + with one string where a number belonged came back with ``language=auto``, ``interval_hours=0`` + and an empty ignore list, no ``.broken`` backup, and not one line on the console. That is why + the file is deserialised here first (see :func:`_precheck_config`): the same call MCDR is + about to make, without MCDR's regeneration behind it, so a rejection can be described and + quarantined *before* the settings are lost. + + The same silence covers the other things MCDR does to this file — creating it when it is + missing, and filling in options it does not have and writing the result back. The first is + reported here; the second is reported by :func:`_report_config_file_state`, as a *different* + line from a rejection, because "your file was missing options" and "your file was thrown + away" call for different reactions. The language is applied before either report so they are + written in the language the file just asked for. """ path = _config_path(server) existed = os.path.isfile(path) leaves_before: Optional[List[str]] = None legacy_rebuilt = False + rejected: Optional[str] = None if existed: try: - with open(path, "r", encoding="utf-8") as handle: + with open(path, "r", encoding="utf-8-sig") as handle: raw = json.load(handle) if not isinstance(raw, dict): raise ValueError("config root must be a JSON object") - leaves_before = _leaf_paths(raw) - legacy_rebuilt = _warn_about_flat_legacy_options(server, raw) + rejected = _precheck_config(raw) + if rejected is None: + leaves_before = _leaf_paths(raw) + legacy_rebuilt = _warn_about_flat_legacy_options(server, raw) except (OSError, ValueError) as error: - backup = "{}.broken.{}".format(path, time.strftime("%Y%m%d-%H%M%S")) - try: - os.replace(path, backup) - except OSError: - backup = path - server.logger.warning( - tr("console.config_invalid", backup=backup) + " ({})".format(error) - ) + rejected = "{}: {}".format(type(error).__name__, error) + + if rejected is not None: + # Quarantined *before* MCDR is given the file, so the admin's settings survive on disk — + # and in a form the next load will not mistake for the current config. Two messages, not + # one: this one says the file was *refused*, while ``console.config_completed`` says it + # was merely incomplete and has been topped up. They call for different reactions, and + # ``console.config_invalid`` is the one that already means "refused, old file kept" — + # from the branch that handles a file the plugin cannot even parse. The reason goes in + # as a value rather than into a second copy of the sentence, which is what keeps the + # sentence translatable. + backup = "{}.broken.{}".format(path, time.strftime("%Y%m%d-%H%M%S")) + try: + os.replace(path, backup) + except OSError: + backup = path + server.logger.warning( + tr("console.config_invalid", backup=backup) + " ({})".format(rejected) + ) config = server.load_config_simple( CONFIG_FILE_NAME, target_class=Config, echo_in_console=False @@ -822,15 +848,86 @@ def _load_config(server: PluginServerInterface) -> Config: if not isinstance(config, Config): config = Config.get_default() - # Applying the language lives here rather than at the two call sites because the report - # below is the first thing on the console that has to be readable, and the language + # Applying the language lives here rather than at the two call sites because the reports + # below are the first thing on the console that has to be readable, and the language # setting is part of what was just read. _apply_language(server, config) _clamp_and_report(server, config) - _report_config_file_state(server, config, path, existed, leaves_before, legacy_rebuilt) + if rejected is None: + _report_config_file_state(server, config, path, existed, leaves_before, legacy_rebuilt) return config +def _precheck_config(raw: Dict[str, Any]) -> Optional[str]: + """Why MCDR would refuse this config, or ``None`` when it will accept it. + + ``Config.deserialize`` is the same call ``load_config_simple`` makes, made here instead of + there: without MCDR's ``failure_policy='regen'`` behind it, a rejection is something to + report rather than something that replaces the file. The parsed dict is not modified + (``Serializable.deserialize`` reads it and builds a new object), which is what makes the + pre-check safe to run before the real load. + + The message names the field where it can be worked out. It usually can: MCDR's own + ``Serializable.deserialize`` builds the object field by field, so removing a field from a + *copy* of the parsed dict and retrying is enough to find the one it refused — see + :func:`_first_bad_option`. That matters because the exception itself carries no path: + measured, ``Config.deserialize`` on a file with ``network.timeout_seconds: "45"`` raises + ``TypeError: Mismatched input type: expected class but found data with class + ``, which names neither the option nor where it lives. + """ + try: + Config.deserialize(raw) + except Exception as error: # noqa: BLE001 - any type or value mismatch is the same outcome + detail = "{}: {}".format(type(error).__name__, error) + field = _first_bad_option(raw, Config) + if field: + return "{} ({})".format(detail, field) + return detail + return None + + +def _first_bad_option(raw: Dict[str, Any], target_class: Any) -> str: + """Which option MCDR refuses in ``raw``, as ``option: ``, or ``""`` when unknown. + + Narrowed by removal rather than by parsing the exception: "does this subset still raise?" + is answerable without knowing anything about how MCDR words its errors, and it is exact. + Sections first — a nested model is one field of its parent, but the admin needs the path + *inside* it — then a two-way split within the offending section, which is ``O(k log n)`` + deserialisations for ``k`` mistakes and ``n`` options. The dict passed in is never modified: + each attempt works on a shallow copy with the keys taken out, because deserialising the real + config object's own data is what the caller is about to do for real. + """ + if not isinstance(raw, dict): # pragma: no cover - the caller checked this already + return "" + + def refuses(candidate: Dict[str, Any]) -> bool: + try: + target_class.deserialize(candidate) + except Exception: # noqa: BLE001 - the same gate :func:`_precheck_config` uses + return True + return False + + # 1. A section: keeping only that section still fails, so the mistake is inside it. + for key, value in raw.items(): + if not isinstance(value, dict) or not refuses({key: value}): + continue + # 2. The option within it. Each candidate is tried alongside the *rest* of the file — + # other sections included — and with only its siblings in this section removed. A + # leaf on its own is not enough to decide: MCDR fills a missing required field and + # says so through the callback, so an isolated leaf can fail for a reason that has + # nothing to do with the mistake being hunted. + others = {name: item for name, item in raw.items() if name != key} + for name in value: + candidate = dict(others) + candidate[key] = {name: value[name]} + if refuses(candidate): + return "option: {}.{}".format(key, name) + # Several mistakes at once, or one that only shows up in context: name the section, + # which is still enough to send the admin to the right part of their file. + return "option: {}.*".format(key) + return "" + + #: Every numeric option the plugin is willing to use, with the range it is willing to use it in. #: #: ``(option path, minimum, maximum)``. The lower bounds are the ones the code always applied; @@ -844,17 +941,30 @@ def _load_config(server: PluginServerInterface) -> Config: #: a month between checks, a month of startup delay, an 8 GB download, 32 parallel requests #: (the front ends start refusing well before that), 1000 requests/minute, five minutes per #: request, MCDR's whole permission range, and ten years of backups. +#: +#: Every numeric option the plugin reads must be in here, not only the ones that looked risky. +#: The four below were the gap: ``network.retries`` and ``download.retries`` feed +#: ``range(retries + 1)``, so a mistyped 10^9 holds the check lock (or a download thread) for +#: as long as the server runs; ``network.cache.ttl_hours: -1`` reads as "never expire" because +#: :class:`ResolveCache` treats ``0`` as the off switch, which is the opposite of the "immediately +#: stale" an admin writing a negative number usually means; and ``report.reuse_report_minutes`` +#: silently became 0. ``tests/test_mcdr_entry.py`` reflects over the config class so the table +#: cannot fall behind again. _NUMERIC_RANGES: Tuple[Tuple[str, int, int], ...] = ( ("check.start_delay_seconds", 0, 3600), ("check.interval_hours", 0, 720), ("download.max_size_mb", 1, 8192), + ("download.retries", 0, 10), ("network.concurrent_requests", 1, 32), ("network.requests_per_minute", 0, 1000), + ("network.retries", 0, 10), ("network.timeout_seconds", 1, 300), + ("network.cache.ttl_hours", 0, 8760), ("cleanup.max_age_days", 0, 3650), ("command_permission_level", 0, 4), ("report.admin_permission", 0, 4), ("report.in_game_permission", 0, 4), + ("report.reuse_report_minutes", 0, 43200), ) @@ -876,15 +986,19 @@ def _clamp_and_report(server: PluginServerInterface, config: Config) -> List[str """ changed: List[str] = [] for path, minimum, maximum in _NUMERIC_RANGES: - # Both shapes are needed: the permission levels and the level the command needs sit at - # the root of the config, everything else is one section down. + # Walk the dotted path rather than splitting once: most options are one section down, + # but ``network.cache.ttl_hours`` is two, and the permission levels sit at the root. + parts = path.split(".") target: Any = config - name = path - if "." in path: - section, name = path.split(".", 1) - target = getattr(config, section, None) + missing = False + for part in parts[:-1]: + target = getattr(target, part, None) if target is None: - continue + missing = True + break + if missing: + continue + name = parts[-1] try: value: Any = getattr(target, name) except AttributeError: @@ -1213,17 +1327,27 @@ def _write_report_files(server: PluginServerInterface, report: Report, ``announce=False`` for the write that follows a deletion: the "报告已写入" line belongs to "a check just finished", and printing it after a command makes it read as though something was checking behind the admin's back. + + Both files go through :mod:`mod_update_checker.jsonfile`'s writers, and were the last two + writers in the plugin opening a file by hand. That matters most for the JSON one, which is + read back at startup: a process killed mid-write used to leave a truncated + ``last_report.json``, ``Report.from_json`` then answers ``None`` for it, and the admin's + report silently became "no check has run yet" — the exact failure the atomic-write helper + exists to prevent, on the one file it was not applied to. The text rendering is written the + same way, so the two cannot disagree about which of them is safe. """ folder = server.get_data_folder() targets = ( - (REPORT_FILE_NAME, report.to_json()), - (REPORT_TEXT_FILE_NAME, "\n".join(render_full(report, tr)) + "\n"), + (REPORT_FILE_NAME, "json", report.to_dict()), + (REPORT_TEXT_FILE_NAME, "text", "\n".join(render_full(report, tr)) + "\n"), ) - for name, content in targets: + for name, kind, content in targets: path = os.path.join(folder, name) try: - with open(path, "w", encoding="utf-8") as handle: - handle.write(content) + if kind == "json": + write_json(path, content) + else: + write_text(path, content) except OSError as error: server.logger.warning("could not write {}: {}".format(path, error)) if announce: @@ -1240,7 +1364,7 @@ def _notify( broadcast: bool = True, ) -> None: """Print the result to the console, to the invoker, and optionally into the game.""" - lines = render_summary(report, tr) + lines = render_summary(report, tr, command=_reply_command) # The console is the primary channel: an automatic check the admin did not trigger still # has to be visible in the log. When the check is configured to speak up only about @@ -1381,7 +1505,7 @@ def _announce_download_complete( Notice( tr("download.complete_in_game_automatic") if _config.download.install_on_stop - else tr("download.complete_in_game_manual"), + else tr("download.complete_in_game_manual", command=_reply_command), "hint", ) ) @@ -1514,7 +1638,7 @@ def _install_summary_lines(data: Dict[str, Any]) -> List[Notice]: ) if len(installed) > NOTIFY_MAX_UPDATES: lines.append( - Notice(tr("report.and_more", count=len(installed) - NOTIFY_MAX_UPDATES), "hint") + Notice(tr("report.and_more", count=len(installed) - NOTIFY_MAX_UPDATES, command=_reply_command), "hint") ) skipped = data.get("skipped") or [] if skipped: @@ -1611,6 +1735,31 @@ def resolve_download_folder( return resolve_download_folder_path(base, config.download.folder_name) +#: The limiter the *download* path spends from, one per configured rate. +#: +#: ``network.requests_per_minute`` reads as "how fast this plugin talks to the upstream", and +#: the download clients used to be built without a limiter at all — so the option governed the +#: JSON checks and nothing else, and a ``!!muc download all`` over forty jars could put a +#: hundred requests on the wire at whatever rate sockets allowed (``download.retries`` turns a +#: failure into a burst, which is when it is most visible). Sharing one limiter per rate is what +#: makes the budget a *process* budget rather than a per-client one: the automatic check's +#: downloads and a manual ``download all`` are two different :class:`HttpClient` objects in two +#: different threads, and they would otherwise each spend the full allowance. +_DOWNLOAD_LIMITERS: Dict[int, RateLimiter] = {} +_DOWNLOAD_LIMITERS_LOCK = threading.Lock() + + +def _download_rate_limiter(per_minute: int) -> RateLimiter: + """The shared limiter for ``per_minute`` requests a minute, creating it on first use.""" + rate = max(0, int(per_minute)) + with _DOWNLOAD_LIMITERS_LOCK: + limiter = _DOWNLOAD_LIMITERS.get(rate) + if limiter is None: + limiter = RateLimiter(rate) + _DOWNLOAD_LIMITERS[rate] = limiter + return limiter + + def _make_http_client(config: Config) -> HttpClient: """A client for the download host, mirroring the checker's settings. @@ -1621,11 +1770,16 @@ def _make_http_client(config: Config) -> HttpClient: ``retries=0`` because :meth:`HttpClient.download` cannot retry a half-written stream. The retrying is done by :class:`Downloader`, where the hash accumulator and the partial file can be reset per attempt; see its ``_fetch``. + + The rate limiter is the point of :func:`_download_rate_limiter`: :meth:`HttpClient.download` + takes a token when one is set, and without it the only requests the configured rate limited + were the JSON ones. """ return HttpClient( user_agent=USER_AGENT, timeout=max(30.0, float(config.network.timeout_seconds)), retries=0, + rate_limiter=_download_rate_limiter(config.network.requests_per_minute), logger=None, ) @@ -1958,7 +2112,7 @@ def _reply_index( """ is_player = getattr(source, "is_player", False) rows, tail, page_info = render_index( - report, tr, entries=entries, page=page, budget=CHAT_PAGE_LINES + report, tr, entries=entries, page=page, budget=CHAT_PAGE_LINES, command=prefix ) _title(source) source.reply(_context_field(report)) @@ -2044,7 +2198,11 @@ def _reply_summary(source: CommandSource, report: Report, the log line and in ``last_report.json``, which is where something that fetches files automatically would look for it. """ - _context, blocks, closing = summarise(report, tr) + # ``prefix`` is passed down rather than left at its default: this screen's own buttons say + # ``!!muc info 1`` when that is what the reader typed, and the trailing lines used to answer + # with ``!!modupdate`` — two aliases in one screenful, one of them a command the reader had + # not used. The default stays for the callers with no command context. + _context, blocks, closing = summarise(report, tr, command=prefix) numbers = {id(entry): number for number, entry in report.indexed_entries()} is_player = getattr(source, "is_player", False) @@ -2114,7 +2272,7 @@ def _notification_lines(report: Report) -> List[Notice]: # notification and the listing agree about what blue means. lines.append(Notice(entry_line_text(entry, tr), "update")) if len(updates) > NOTIFY_MAX_UPDATES: - lines.append(Notice(tr("report.and_more", + lines.append(Notice(tr("report.and_more", command=_reply_command, count=len(updates) - NOTIFY_MAX_UPDATES), "hint")) # The list is the same every start until the admin acts on it, so the one thing worth # adding is which part of it just appeared. @@ -2128,7 +2286,7 @@ def _notification_lines(report: Report) -> List[Notice]: for entry in pending[:NOTIFY_MAX_UPDATES]: lines.append(Notice(entry_line_text(entry, tr), "update")) if len(pending) > NOTIFY_MAX_UPDATES: - lines.append(Notice(tr("report.and_more", + lines.append(Notice(tr("report.and_more", command=_reply_command, count=len(pending) - NOTIFY_MAX_UPDATES), "hint")) lines.append(Notice(tr("check.in_game_awaiting_where"), "hint")) @@ -2137,7 +2295,7 @@ def _notification_lines(report: Report) -> List[Notice]: elif updates or pending: # A truncated list with no way onward is a dead end, so the notification says which # command carries the rest. - lines.append(Notice(tr("check.in_game_more_hint"), "hint")) + lines.append(Notice(tr("check.in_game_more_hint", command=_reply_command), "hint")) return lines @@ -2269,7 +2427,7 @@ def _admin_join_worker(server: PluginServerInterface, player: str) -> None: lines.extend(_notification_lines(report)) if reused: lines.append( - Notice(tr("check.admin_join_reused", minutes=int((age or 0) // 60)), "hint") + Notice(tr("check.admin_join_reused", minutes=int((age or 0) // 60), command=_reply_command), "hint") ) _tell_player(server, player, lines) @@ -2390,6 +2548,14 @@ def once() -> None: def _stop_scheduler() -> None: + """Ask the interval thread to finish, and wait briefly for it. + + Safe to call twice, and safe when nothing is running — which a reload needs, because it + reaches here twice: MCDR's ``__reload_ready_plugins`` calls ``__unload_plugin`` first (which + dispatches ``PLUGIN_UNLOADED`` and calls ``plugin.unload()``) and then hands the old module + to the new one's ``on_load``, which stops it again. The second call finds the event already + set and the thread already joined. + """ global _scheduler_thread _stop_event.set() thread = _scheduler_thread @@ -2425,9 +2591,30 @@ def _resume_after_stop(server: PluginServerInterface) -> None: # commands (stage a plan, re-derive it at ``confirm``) and the two-step download / install # actions. The permission gate reads the config **when the command is typed**, not when it was # registered, so changing the level takes effect without a reload. +# +# ``!!help`` needs no such treatment, and that is deliberate rather than an omission: the entry +# is registered at permission 0 and states the required level in its own text, so re-reading the +# config cannot make it stale. See ``_register_commands``. # -------------------------------------------------------------------------------------- +#: The alias to name when a message tells the reader to type a command. +#: +#: Two aliases are registered (``ROOT_LITERALS``), so every message that spells one out is +#: telling half the readers about a command they did not type — ``!!muc list`` replied with +#: ``!!modupdate info 1``, in the same screenful as its own ``!!muc`` buttons. Messages that +#: already took a ``command`` parameter were fixed one at a time; these are the ones whose +#: producers are shared between the command path and the event paths (the join notice, the +#: post-download broadcast, the rendered report), which have no ``prefix`` to pass down. +#: +#: A module-level "which alias is being answered" is the honest shape for that: it is set by the +#: command dispatcher from the literal that was actually typed, and everything rendered while +#: that command runs picks it up. Event-driven messages run with whatever was last typed, which +#: is harmless — they are pointers, not identifiers — and keeps this from turning into a +#: ``prefix`` parameter threaded through a dozen unrelated helpers. +_reply_command: str = ROOT_LITERALS[0] + + def _has_permission(source: CommandSource) -> bool: try: return source.has_permission(max(0, int(_config.command_permission_level))) @@ -2435,6 +2622,27 @@ def _has_permission(source: CommandSource) -> bool: return False +def _set_reply_command(prefix: str) -> None: + """Remember which alias is being answered, for the messages that name one. + + Called from the command tree's ``requires`` hook, which MCDR runs once per command before the + callback — so the value is in place for everything the callback renders, including the + helpers that have no ``prefix`` of their own. + """ + global _reply_command + _reply_command = prefix + + +def _permit(source: CommandSource, prefix: str) -> bool: + """The gate on the command tree: record the alias, then check the permission level. + + Wrapped in one function so every command — including the bare ``!!muc`` — passes through the + recording, and so ``_has_permission`` stays a plain predicate that a test can call. + """ + _set_reply_command(prefix) + return _has_permission(source) + + def _denied(_source: CommandSource) -> str: return tr("command.permission_denied", level=_config.command_permission_level) @@ -2446,11 +2654,26 @@ def _show_summary(source: CommandSource) -> None: project convention), this is how the stored result is read back on demand. """ if _last_report is None: - source.reply(tr("command.no_report_yet")) + source.reply(tr("command.no_report_yet", command=_reply_command)) return _reply_summary(source, _last_report) +def _is_page_number(text: str) -> bool: + """Whether ``text`` is a page number — that is, whether ``int()`` will accept it. + + ``str.isdecimal``, not ``str.isdigit``. ``isdigit`` is also true for characters that look + like digits and are not: ``²`` (U+00B2), ``⑨`` (U+2468), ``½``. ``int()`` refuses all of + them, so ``!!muc list ²`` raised ``ValueError`` inside a command callback — and MCDR logs a + callback exception without replying to its source, so the player got silence and the only + record was a traceback on the console. Pasted text makes this easy to hit by accident. + + The two predicates agree on everything ``int()`` accepts, which is why the check can be + stated as "is this convertible" rather than as a list of exceptions. + """ + return bool(text) and text.isdecimal() + + def _show_list(source: CommandSource, raw: str = "", prefix: str = ROOT_LITERALS[0]) -> None: """``list [状态] [页码]`` — the numbered index, optionally filtered and paged. @@ -2465,7 +2688,11 @@ def _show_list(source: CommandSource, raw: str = "", prefix: str = ROOT_LITERALS """ tokens = (raw or "").split() page = 1 - if tokens and tokens[-1].isdigit(): + # ``isdecimal`` rather than ``isdigit``: ``²`` and ``⑨`` are digits to ``isdigit`` and not to + # ``int()``, so the callbacks below raised ``ValueError`` out of a command — and MCDR logs a + # callback exception without replying, so the player saw nothing at all. See + # ``_page_number``, which every paginated command goes through. + if tokens and _is_page_number(tokens[-1]): page = int(tokens.pop()) status_text = " ".join(tokens).strip() wanted = status_text.lower() @@ -2477,7 +2704,7 @@ def _show_list(source: CommandSource, raw: str = "", prefix: str = ROOT_LITERALS ) return if _last_report is None: - source.reply(tr("command.no_report_yet")) + source.reply(tr("command.no_report_yet", command=_reply_command)) return entries = _last_report.by_status(wanted) if wanted else None @@ -2550,11 +2777,11 @@ def _reply_ambiguous( def _show_info(source: CommandSource, target: str, prefix: str = ROOT_LITERALS[0]) -> None: """``info <编号|Mod 名|mod id|文件名>`` — one mod's version change, links and notes.""" if _last_report is None: - source.reply(tr("command.no_report_yet")) + source.reply(tr("command.no_report_yet", command=_reply_command)) return text = (target or "").strip() if not text: - source.reply(tr("command.info.usage")) + source.reply(tr("command.info.usage", command=_reply_command)) return entry = _resolve_handle(source, _last_report, text, prefix=prefix, action="info") if entry is None: @@ -2704,7 +2931,7 @@ def _manual_delete(source: CommandSource, handle: str, prefix: str) -> None: report = _last_report if report is None: - source.reply(tr("command.no_report_yet")) + source.reply(tr("command.no_report_yet", command=_reply_command)) return text = (handle or "").strip() if not text: @@ -3138,7 +3365,7 @@ def _manual_download(source: CommandSource, handle: str, prefix: str) -> None: """ report = _last_report if report is None: - source.reply(tr("command.no_report_yet")) + source.reply(tr("command.no_report_yet", command=_reply_command)) return text = (handle or "").strip() if not text: @@ -3193,7 +3420,7 @@ def _manual_download_all(source: CommandSource, prefix: str) -> None: """``download all`` — stage one fetch covering every mod that has something to fetch.""" report = _last_report if report is None: - source.reply(tr("command.no_report_yet")) + source.reply(tr("command.no_report_yet", command=_reply_command)) return candidates, blocked = _download_all_candidates(report) @@ -3216,7 +3443,7 @@ def _manual_download_all(source: CommandSource, prefix: str) -> None: for entry in candidates[:BULK_PLAN_ROWS]: lines.append(entry_line_text(entry, tr)) if len(candidates) > BULK_PLAN_ROWS: - lines.append(tr("report.and_more", count=len(candidates) - BULK_PLAN_ROWS)) + lines.append(tr("report.and_more", count=len(candidates) - BULK_PLAN_ROWS, command=_reply_command)) if blocked: lines.append(tr("command.download.all_skipped", count=len(blocked))) lines.append(_confirm_ask(prefix)) @@ -3256,7 +3483,7 @@ def _manual_install(source: CommandSource, handle: str, prefix: str) -> None: """ report = _last_report if report is None: - source.reply(tr("command.no_report_yet")) + source.reply(tr("command.no_report_yet", command=_reply_command)) return text = (handle or "").strip() if not text: @@ -3299,7 +3526,7 @@ def _manual_install_all(source: CommandSource, prefix: str) -> None: """``install all`` — stage the authorisation of every downloaded build.""" report = _last_report if report is None: - source.reply(tr("command.no_report_yet")) + source.reply(tr("command.no_report_yet", command=_reply_command)) return pending = report.awaiting_install @@ -3313,7 +3540,7 @@ def _manual_install_all(source: CommandSource, prefix: str) -> None: new=safe_jar_name(entry.download_filename, entry.fallback_file_name()))) if len(pending) > BULK_PLAN_ROWS: - lines.append(tr("report.and_more", count=len(pending) - BULK_PLAN_ROWS)) + lines.append(tr("report.and_more", count=len(pending) - BULK_PLAN_ROWS, command=_reply_command)) if _config.download.install_on_stop: lines.append(tr("command.install.plan_already_automatic")) lines.append(_confirm_ask(prefix)) @@ -3403,12 +3630,29 @@ def _confirmed_download_all( One transfer at a time inside that thread: the downloader is sequential by design, and a batch is exactly the case where twenty parallel connections would be a worse neighbour. + + A batch takes the same claims the single form does, and it did not use to. The comment on + :data:`_inflight_downloads` says why the claims exist — a second download of one build opens + a second connection for the same bytes and fights its own ledger record — and ``download all`` + was the one path that took none, so two batches, or a batch plus ``download ``, + wrote the same temporary file at once. Measured on Windows before this changed: one of the + two transfers failed with ``could not put the file in place: [WinError 32]`` while the file + itself was fine, so the batch summary said ``failed=1`` about a build that was on disk. """ - source.reply(tr("command.download.batch_started", count=len(entries))) + claimed, busy = _begin_batch_download(entries) + if not claimed: + _reply_to(source, tr("command.download.batch_all_in_flight", + names=", ".join(entry.name for entry in busy[:5]))) + return + if busy: + _reply_to(source, tr("command.download.batch_in_flight", count=len(busy), + names=", ".join(entry.name for entry in busy[:5]))) + + source.reply(tr("command.download.batch_started", count=len(claimed))) def run() -> None: try: - outcomes = _perform_manual_downloads(entries) + outcomes = _perform_manual_downloads(claimed) except Exception as error: # noqa: BLE001 - a failure must not take the server down server = _server if server is not None: @@ -3416,6 +3660,10 @@ def run() -> None: _reply_to(source, tr("command.download.batch_failed", reason="{}: {}".format( type(error).__name__, error))) return + finally: + # Released whatever happened, and before the outcome is reported, so a retry is + # possible the moment the reader is told it failed. + _release_batch_download(claimed) if outcomes is None: _reply_to(source, tr("command.download.batch_failed", reason=tr("command.download.bad_folder"))) @@ -3468,6 +3716,36 @@ def _end_download(file_name: str) -> None: _inflight_downloads.discard(file_name) +def _begin_batch_download( + entries: Sequence[UpdateEntry], +) -> Tuple[List[UpdateEntry], List[UpdateEntry]]: + """Claim every entry in ``entries`` that nobody holds. ``(claimed, busy)``. + + Taken in one pass under the lock rather than one entry at a time, because a batch is a unit: + deciding per entry would let a second batch interleave with this one and each end up with + half of the set. ``busy`` is returned rather than dropped, because "which of them" is what + makes the message useful — a bare "some of that was already running" leaves the admin to + guess. + """ + claimed: List[UpdateEntry] = [] + busy: List[UpdateEntry] = [] + with _inflight_lock: + for entry in entries: + if entry.file_name in _inflight_downloads: + busy.append(entry) + continue + _inflight_downloads.add(entry.file_name) + claimed.append(entry) + return claimed, busy + + +def _release_batch_download(entries: Sequence[UpdateEntry]) -> None: + """Release claims taken by :func:`_begin_batch_download`.""" + with _inflight_lock: + for entry in entries: + _inflight_downloads.discard(entry.file_name) + + def _confirmed_download( source: CommandSource, entry: UpdateEntry, number: int, prefix: str ) -> None: @@ -4294,7 +4572,7 @@ def _command_tree(prefix: str): """ return ( Literal(prefix) - .requires(_has_permission, _denied) + .requires(lambda source, _prefix=prefix: _permit(source, _prefix), _denied) # Bare command = the help page, by project convention: an invocation with no arguments # is a reader asking what the commands are. The summary moved to its own word when this # changed — it is still one keystroke away, and it is no longer what an explorer gets. @@ -4319,7 +4597,7 @@ def _command_tree(prefix: str): ) .then( Literal("info") - .runs(lambda source: source.reply(tr("command.info.usage"))) + .runs(lambda source: source.reply(tr("command.info.usage", command=_reply_command))) .then( GreedyText("target").suggests( _suggest_handles(lambda report: report.entries) @@ -4377,9 +4655,27 @@ def _command_tree(prefix: str): def _register_commands(server: PluginServerInterface) -> None: + """Register the command tree and the ``!!help`` line, once, at load. + + The help entry's permission is **0**, not ``command_permission_level``, and that is the fix + for a mismatch MCDR's design creates: a help message carries its permission as a *value* + (``HelpMessage.permission``) and MCDR filters on that value when ``!!help`` is rendered, so + an entry keeps the level it was registered with while the command gate below reads the config + every time it runs. Switching the level and then ``!!muc reload`` therefore left the command + working and ``!!help`` either silent about it (level 3 -> 0) or advertising an entry nobody + could click (0 -> 3). + + Registering at 0 and stating the required level **in the text** removes the whole class: + there is nothing left to refresh, so nothing to get wrong on reload, and the entry cannot + become a lie. The alternative — re-registering help on every config reload — was rejected + because MCDR *appends* help messages, so a second registration with the same prefix shows the + line twice rather than replacing it. + """ for prefix in ROOT_LITERALS: server.register_help_message( - prefix, tr("help.modupdate"), permission=_config.command_permission_level + prefix, + tr("help.modupdate", level=_config.command_permission_level), + permission=0, ) server.register_command(_command_tree(prefix)) @@ -4407,16 +4703,28 @@ def on_load(server: PluginServerInterface, prev_module: Any) -> None: # # * the last report, because the admin is probably looking at it; # * which players are online, because join/leave events are not replayed; - # * the previous module's scheduler thread. MCDR's reload path calls ``plugin.reload()`` - # without dispatching the unloaded event (verified in MCDR's plugin_manager), so - # ``on_unload`` never runs and the old thread would otherwise keep firing checks - # against this server forever, once more on every reload. + # * the previous module's scheduler thread, because it would otherwise keep firing checks + # against this server forever, once more on every reload. ``on_unload`` does run on a + # reload — MCDR's ``__reload_ready_plugins`` calls ``__unload_plugin`` first, which + # dispatches ``PLUGIN_UNLOADED`` and calls ``plugin.unload()`` — but calling the old + # module's stop function here as well is what makes "the old thread is gone" true even + # if a future MCDR changes that order. It is idempotent; the second call is a no-op. if prev_module is not None: carried = getattr(prev_module, "_last_report", None) - if isinstance(carried, Report): + # Duck-typed rather than ``isinstance(carried, Report)``, which can never be true across + # a reload: unloading pops every ``mod_update_checker*`` module from ``sys.modules`` + # (MCDR's ``RegularPlugin._on_unload``), so the new module has a *different* ``Report`` + # class object and the isinstance check was always false. The live report was therefore + # dropped on every ``!!MCDR reload plugin`` and replaced by whatever ``last_report.json`` + # held — nothing at all when ``report.write_file`` is off — leaving the admin with "no + # check has run yet" and paying for a full check to get back to where they were. + if carried is not None and hasattr(carried, "entries") and hasattr(carried, "generated_at"): _last_report = carried previous_players = getattr(prev_module, "_online_players", None) - if isinstance(previous_players, set): + # Same reasoning: an old module's ``set`` is still a set, but a subclass or a frozenset + # from a future version would not pass the old ``isinstance`` either. + if previous_players is not None and hasattr(previous_players, "add") \ + and hasattr(previous_players, "__iter__"): _online_players = previous_players previous_stop = getattr(prev_module, "_stop_scheduler", None) if callable(previous_stop): diff --git a/mod_update_checker/checker.py b/mod_update_checker/checker.py index d3a52fe..e343f9c 100644 --- a/mod_update_checker/checker.py +++ b/mod_update_checker/checker.py @@ -35,6 +35,7 @@ """ import logging +import os import threading import time from concurrent.futures import ThreadPoolExecutor @@ -153,6 +154,31 @@ class ResolveCache: VERSION = 1 + #: One lock for the whole process, and one lock per file path inside that table. The + #: instances are the problem rather than the solution: the entry layer builds several (the + #: check thread, ``!!muc`` commands, the status page) and each takes its own snapshot of the + #: file at construction, so whoever saves last used to win and the other run's freshly + #: learned negative answers were silently dropped — a search plus a version lookup paid + #: again, within the TTL. :meth:`save` therefore merges into what is on disk under the file's + #: lock and writes back only *this* instance's changes, the same discipline + #: :class:`~mod_update_checker.downloads.DownloadLedger` already follows; its + #: ``_LEDGER_LOCK`` is the pattern being copied here. + _LOCKS: Dict[str, threading.RLock] = {} + _LOCKS_GUARD = threading.Lock() + + @classmethod + def _lock_for(cls, path: Path) -> threading.RLock: + try: + key = os.path.normcase(str(path.resolve())) + except OSError: # pragma: no cover - resolve() on a path that cannot be resolved + key = os.path.normcase(os.path.abspath(str(path))) + with cls._LOCKS_GUARD: + lock = cls._LOCKS.get(key) + if lock is None: + lock = threading.RLock() + cls._LOCKS[key] = lock + return lock + def __init__( self, path: Union[str, Path, None], ttl_hours: float, enabled: bool = True ) -> None: @@ -166,6 +192,11 @@ def __init__( self.enabled = enabled and self.path is not None self._lock = threading.Lock() self._data: Dict[str, Any] = self._load() + #: Records this instance added or replaced since it was built. Only these are written + #: back by :meth:`save`, so a save cannot revert what another instance learned in the + #: meantime. A delete is recorded as an explicit ``None`` so that "removed here" is + #: distinguishable from "not touched here". + self._dirty: Dict[str, Any] = {} def _load(self) -> Dict[str, Any]: if not self.enabled or not self.path or not self.path.is_file(): @@ -214,17 +245,51 @@ def put(self, sha1: str, payload: Dict[str, Any]) -> None: record["at"] = time.time() with self._lock: self._data[sha1] = record + self._dirty[sha1] = record def save(self) -> None: + """Write this instance's new records back, merging with what is on disk. + + Read-modify-write rather than "write what I have": the file is shared by every instance + in the process, and a whole-file write from an instance holding an older snapshot is how + one run's answers get thrown away by the next one's. + + It writes even when this instance learned nothing, because that is what it did before and + something depends on it: the cross-version matrix and the end-to-end test read + ``resolve-cache.json`` back after a run to prove the cache is on disk and parses, and a + run where every mod resolved from the batched lookup legitimately has no new records. An + "always write" is also the honest reading of the option — ``network.cache.enabled`` says + the cache is in use, so the file is the evidence of that. + """ if not self.enabled or not self.path: return - with self._lock: - payload = {"version": self.VERSION, "records": self._data} try: - write_json(self.path, payload) + with self._lock_for(self.path): + payload = self._merged_payload() + write_json(self.path, payload) + with self._lock: + self._dirty.clear() except OSError as error: _LOGGER.warning("could not write resolve cache %s: %s", self.path, error) + def _merged_payload(self) -> Dict[str, Any]: + """The file's records with this instance's changes layered on top.""" + merged: Dict[str, Any] = {} + on_disk = read_json(self.path) if self.path else None + if isinstance(on_disk, dict) and on_disk.get("version") == self.VERSION: + records = on_disk.get("records") + if isinstance(records, dict): + for key, record in records.items(): + if isinstance(record, dict) and _cache_timestamp(record) is not None: + merged[str(key)] = record + with self._lock: + for key, record in self._dirty.items(): + if record is None: + merged.pop(key, None) + else: + merged[key] = record + return {"version": self.VERSION, "records": merged} + def _now_iso() -> str: return datetime.now(timezone.utc).astimezone().isoformat(timespec="seconds") diff --git a/mod_update_checker/downloads.py b/mod_update_checker/downloads.py index 2d892d7..38b303d 100644 --- a/mod_update_checker/downloads.py +++ b/mod_update_checker/downloads.py @@ -109,6 +109,27 @@ #: :meth:`DownloadLedger.save` is also called from paths that already hold it. _LEDGER_LOCK = threading.RLock() +#: Counter behind the per-writer ``.part`` names, guarded by :data:`_PART_LOCK`. Together with the +#: pid it replaces ``.part``, which was derived from the target name alone: the check +#: thread's download phase and a manual ``!!muc download all`` are two different +#: :class:`Downloader` objects pointed at one folder, and a fixed name meant the second writer's +#: ``_discard`` could delete the first one's half-written file (Windows refuses the delete while +#: it is open, which is why the symptom was a mysterious hash mismatch rather than a clean error). +_PART_LOCK = threading.Lock() +_PART_COUNTER = 0 + + +def _unique_part_name(target: Path) -> Path: + """A ``.part`` path for ``target`` that no other writer in this process will pick.""" + global _PART_COUNTER + with _PART_LOCK: + _PART_COUNTER += 1 + serial = _PART_COUNTER + return target.with_name( + "{}.{}.{}.part".format(target.name, os.getpid(), serial) + ) + + def _safe_stem(raw: str, fallback: str) -> str: """A bare, writable stem with no separators and no directory meaning. @@ -432,6 +453,22 @@ def approved_keys(self) -> List[str]: def forget(self, key: str) -> None: self._mark_deleted(key) + def update(self, key: str, **fields: Any) -> bool: + """Add or replace fields on a record. ``False`` when there is no such record. + + For annotations the *install* stage makes about a record it will not act on again — a + permanent skip reason and when it was decided. Kept here rather than as a second write + path so the read-modify-write discipline of :meth:`save` covers it too: several + instances are alive at once, and one that edited a record directly would lose the + others' changes when it saved. + """ + record = self._records.get(key) + if record is None: + return False + record.update(fields) + self._mark_changed(key, record) + return True + def save(self) -> None: """Merge this instance's changes into the file and write it back. @@ -572,6 +609,12 @@ class Downloader: API calls are small and latency-bound; downloads are large and bandwidth-bound, and running twenty of them at once would be a worse citizen than letting them queue. It also keeps the "is this name taken?" decision single-threaded, which is what makes it correct. + + Sequential *within this instance* — the entry layer keeps more than one alive at a time (the + check thread's download phase, a manual ``download all``), which is exactly why the temporary + file name is unique per writer rather than derived from the target. Two instances pointed at + one folder can still race for the same *target* name; :meth:`_choose_target` already refuses + to overwrite a file whose bytes differ instead of trusting that they cannot. """ def __init__( @@ -787,7 +830,6 @@ def _fetch(self, entry: UpdateEntry, target: Path) -> DownloadOutcome: attempt produced. """ attempts = max(1, int(self.options.retries) + 1) - part = target.with_name(target.name + ".part") outcome = None #: How many attempts were really made, which is not the same as how many were allowed: #: a 404 or a write error is not retried, so the log must not claim four tries for one. @@ -801,7 +843,9 @@ def _fetch(self, entry: UpdateEntry, target: Path) -> DownloadOutcome: ), ) made = attempt - outcome, retryable = self._transfer_once(entry, target, part) + # A fresh name per attempt, so a retry can never touch a partial file another writer + # (or the previous attempt of this one) still has open. + outcome, retryable = self._transfer_once(entry, target, _unique_part_name(target)) if outcome.status == STATUS_DOWNLOADED: return outcome if not retryable or attempt == attempts: @@ -904,13 +948,20 @@ def _failed(self, entry: UpdateEntry, detail: str) -> DownloadOutcome: detail=detail, ) - @staticmethod - def _discard(path: Path) -> None: - """Remove a partial file. A half-written jar is worse than none: it looks complete.""" + def _discard(self, path: Path) -> None: + """Remove a partial file. A half-written jar is worse than none: it looks complete. + + A failure to remove it is logged rather than passed over. It cannot be made to matter + here — the file is a ``.part`` and nothing reads one — but "the leftover is still there" + is the fact that explains a later hash mismatch, and a silent ``pass`` is what turns that + into a mystery. + """ try: path.unlink() - except OSError: - pass + except FileNotFoundError: + return + except OSError as error: + self._log("debug", "could not remove the partial file {}: {}".format(path, error)) # -- entry point ------------------------------------------------------------------- diff --git a/mod_update_checker/i18n.py b/mod_update_checker/i18n.py index 673abd8..a997765 100644 --- a/mod_update_checker/i18n.py +++ b/mod_update_checker/i18n.py @@ -25,6 +25,7 @@ import json import os import pkgutil +import warnings from typing import Any, Dict, List, NamedTuple, Optional, Sequence #: Last resort for every fallback chain: the one language the docs assume is present. @@ -35,6 +36,21 @@ LANG_DIR_NAME = "lang" +#: The catalogues this plugin ships, as a claim it can be held to. +#: +#: Two jobs, both about the same failure: at least one of these files is always in the package, +#: so ``available_languages()`` must never answer with less. The listing below is what makes +#: that true when the *enumerating* API is missing — which is a real configuration here, not a +#: hypothetical: ``importlib.resources.files`` is Python 3.9+, this plugin's declared floor is +#: 3.8 (``ruff.toml``, ``tools/check_python_floor.py``, the CI ``python-floor`` job), and inside +#: a packed ``.mcdr`` there is no directory to fall back to either. Measured on that +#: combination: ``available_languages()`` returned ``['en_us']`` and a ``zh_cn`` admin was told, +#: in English, that their language did not exist. +#: +#: A code listed here that is *not* shipped is harmless — it is only ever used as a fallback +#: floor, and a code with no catalogue falls back to English exactly as it always did. +SHIPPED_LANGUAGES: Sequence[str] = ("en_us", "zh_cn") + #: Catalogue key of the warning raised when the configured language is not one we ship. UNSUPPORTED_KEY = "language.unsupported" @@ -90,10 +106,29 @@ def _resource_bytes(relative: str) -> Optional[bytes]: def _list_lang_files() -> List[str]: - """File names in the package's ``lang`` folder, empty if it cannot be listed.""" + """File names in the package's ``lang`` folder, empty if it cannot be listed. + + Three ways, in order of how much they need from the import system, because the combination + that matters is "old interpreter **and** packed plugin": + + * ``importlib.resources.contents`` — the enumerator that exists from 3.7, and the one that + works for a zip as well as a directory. ``files()`` is 3.9+ and used to be what stood + here, which meant the declared 3.8 floor silently degraded to English-only. + * ``os.listdir`` on the module's own directory — the unpacked case, and what is left if + even ``contents`` is unavailable or a custom loader refuses it. + * :data:`SHIPPED_LANGUAGES` in :func:`available_languages`, which is the floor rather than a + fourth strategy. + """ try: - folder = importlib.resources.files(__package__).joinpath(LANG_DIR_NAME) - return sorted(entry.name for entry in folder.iterdir()) + # Can only ever fire on 3.11+, where a replacement exists; on 3.8-3.10 ``contents`` is + # the only enumerator there is, and a deprecation notice on a server console is noise + # about a version that has no alternative. + with warnings.catch_warnings(): + warnings.simplefilter("ignore", DeprecationWarning) + entries = importlib.resources.contents( + "{}.{}".format(__package__, LANG_DIR_NAME) + ) + return sorted(entry for entry in entries if entry.endswith(".json")) except Exception: # noqa: BLE001 - any loader quirk falls back to the plain path pass try: @@ -106,11 +141,19 @@ def available_languages() -> List[str]: """Every shipped language code, sorted. Read from the package rather than hardcoded: dropping in a ``.json`` is enough to - make it selectable. + make it selectable. The one hardcoded part is the *floor* — a package always contains the + catalogues in :data:`SHIPPED_LANGUAGES`, so if enumeration comes back with less than that + (an interpreter without the listing API, a zip the loader will not enumerate, a shrunken + package) the answer is the union instead of the truncated list. Under-reporting here is not + a cosmetic failure: the configured language is checked against this list, so a missing entry + turns a working catalogue into ``language.unsupported`` and an English console. """ codes = sorted( os.path.splitext(name)[0] for name in _list_lang_files() if name.endswith(".json") ) + missing = [code for code in SHIPPED_LANGUAGES if code not in codes] + if missing: + codes = sorted(set(codes) | set(SHIPPED_LANGUAGES)) return codes or [FALLBACK_LANGUAGE] diff --git a/mod_update_checker/installer.py b/mod_update_checker/installer.py index 4258cf6..dbb16c0 100644 --- a/mod_update_checker/installer.py +++ b/mod_update_checker/installer.py @@ -30,6 +30,8 @@ import os import re import shutil +import threading +import time from dataclasses import dataclass from pathlib import Path from typing import Any, Dict, List, Optional, Tuple, Union @@ -66,6 +68,23 @@ REASON_NAME_TAKEN = "name-taken" REASON_BACKUP_NAMES_TAKEN = "backup-names-taken" +#: Reasons a later run would reach again and again with the same answer — an admin who deleted +#: the old jar, a name something else holds, a file whose bytes no longer match the hash that was +#: verified. Kept on the record as ``failed_at``/``failed_reason`` so :func:`pending_records` +#: stops offering them and the log stops repeating one line per stop cycle, while the ledger still +#: shows what was attempted and why. The rest (a missing download, a failed copy) can genuinely +#: change between runs and stay pending. +_PERMANENT_REASONS = frozenset( + (REASON_NOT_IN_MODS, REASON_NAME_TAKEN, REASON_BACKUP_NAMES_TAKEN, REASON_HASH_MISMATCH) +) + +#: One install at a time, per process. Two installs racing over one ``mods/`` folder is what the +#: backup-name reservation below exists to survive, but serialising them is the cheaper guarantee: +#: the ``.old`` name is chosen and taken in one step, and no second installer can slip between the +#: two halves of that step. +_INSTALL_LOCK = threading.RLock() + + #: Leading bracket groups on a file name — where an admin puts their own note. Both the ASCII #: and the full-width bracket, because a Chinese-language admin uses either. #: @@ -106,14 +125,32 @@ def backup_name(path: Union[str, Path]) -> Optional[Path]: An existing backup is never overwritten: it is the previous version of a mod, which is exactly the file somebody would want back. Numbering means a second update keeps both. + + **The name is taken, not merely chosen.** The loop creates each candidate with + ``open(..., "x")`` and returns the one that succeeds, so another installer — or an admin + renaming a file by hand in the same moment — cannot be handed the same name. Bare + ``exists()`` probing followed by a later ``os.replace`` was a TOCTOU window, and the loser of + that race overwrote a backup that the docstring promised was never overwritten. The + placeholder file is what the caller renames the old jar onto. """ base = Path(path).with_name(Path(path).name + _BACKUP_SUFFIX) - if not base.exists(): - return base - for index in range(2, _BACKUP_ATTEMPTS + 2): - candidate = base.with_name("{}.{}".format(base.name, index)) - if not candidate.exists(): - return candidate + for candidate in [base] + [ + base.with_name("{}.{}".format(base.name, index)) + for index in range(2, _BACKUP_ATTEMPTS + 2) + ]: + try: + # ``os.O_CREAT | os.O_EXCL``: atomic claim, and a no-op rather than a window when two + # callers reach the same name at once. ``open()`` rather than ``Path.touch`` because + # only the exclusive flag makes the claim itself the test. + descriptor = os.open(str(candidate), os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o644) + except FileExistsError: + continue + except OSError: + # A folder we cannot create in, or a name the filesystem refuses: not a name to hand + # out. The next candidate may still work, so this is a ``continue`` like the clash. + continue + os.close(descriptor) + return candidate return None @@ -185,9 +222,20 @@ def pending_records(ledger: DownloadLedger, approved_only: bool) -> List[str]: Separated from :func:`install_pending` so the caller can ask "is there anything to do?" without doing it — the install runs from the stop event, and a stop with nothing to install must not write an install report claiming otherwise. + + A record closed out by a reason that cannot improve (``failed_reason``; see + :data:`_PERMANENT_REASONS`) is not a pending record. Without this, a mod whose old jar the + admin deleted was re-attempted and re-logged on every single stop, which is also why the + log could never be used to tell "waiting" from "given up". Editing ``failed_reason`` out of + the ledger — or deleting the record — is how an admin asks for another try. """ keys = ledger.approved_keys() if approved_only else sorted(ledger.records()) - return [key for key in keys if str((ledger.get(key) or {}).get("file") or "")] + return [ + key + for key in keys + if str((ledger.get(key) or {}).get("file") or "") + and not (ledger.get(key) or {}).get("failed_reason") + ] @dataclass @@ -262,8 +310,27 @@ def _remove_temporary(temporary: Path) -> bool: return False +def _release_reserved_name(backup: Path) -> None: + """Give back the placeholder :func:`backup_name` created, when the rename onto it failed. + + Without this an aborted install leaves a zero-byte ``x.jar.old`` behind, which the next run + would treat as a real backup (and the cleanup feature would eventually delete as if it were + one). Missing is not a problem: the rename may have succeeded before the call that failed. + """ + try: + backup.unlink() + except OSError: + pass + + def _install_one(record: Dict[str, Any], options: InstallOptions) -> InstallResult: """Install one recorded download, or explain why it was not installed.""" + with _INSTALL_LOCK: + return _install_one_locked(record, options) + + +def _install_one_locked(record: Dict[str, Any], options: InstallOptions) -> InstallResult: + """The install itself. Callers hold :data:`_INSTALL_LOCK`, so the backup decision is final.""" name = str(record.get("name") or record.get("local") or "") version = str(record.get("version") or "") @@ -315,6 +382,7 @@ def _install_one(record: Dict[str, Any], options: InstallOptions) -> InstallResu try: os.replace(installed, backup) except OSError as error: + _release_reserved_name(backup) return InstallResult(name=name, status=STATUS_FAILED, version=version, detail="could not set the old jar aside: {}".format(error)) @@ -367,21 +435,142 @@ def install_pending( Which records those are is :func:`pending_records`' decision, and it is the same list the caller used to decide whether to run at all — an admin who authorised one mod must not get the other four instalments as a side effect of the others merely being downloaded. + + Two things happen to a record that is done with. On success the **consumed download is + deleted**: the build now lives in ``mods/`` and the previous version beside it as ``.old``, + so a third copy in the download folder is one nothing will ever read again, and leaving it + there made the folder grow by one jar per update forever — the note in the code said as much + while ``forget`` only dropped the bookkeeping. On a reason that cannot improve (see + :data:`_PERMANENT_REASONS`) the record is *closed* rather than retried, so + ``download.install_on_stop`` does not re-attempt it and re-log it on every stop. """ results: List[InstallResult] = [] + installed = False for key in pending_records(ledger, options.approved_only): record = ledger.get(key) or {} result = _install_one(record, options) results.append(result) if result.status == STATUS_INSTALLED: - # The file now lives in mods/, and the previous version is kept beside it as - # ``.old``. Keeping a third copy in the download folder would grow forever. + installed = True + _remove_consumed_download(ledger, options, record, logger) ledger.forget(key) ledger.save() + elif result.status == STATUS_SKIPPED and result.detail in _PERMANENT_REASONS: + ledger.update( + key, + failed_at=time.strftime("%Y-%m-%dT%H:%M:%S%z"), + failed_reason=result.detail, + ) + ledger.save() _log(logger, result) + if installed: + # Once per run rather than once per jar, and only when something was actually installed: + # a stop with nothing to do must not go walking the folder. + _sweep_consumed_downloads(ledger, options, logger) return results +def _remove_consumed_download( + ledger: DownloadLedger, + options: InstallOptions, + record: Dict[str, Any], + logger: Optional[Any], +) -> None: + """Delete the download an install just consumed. A failure is a debug line, not an error.""" + name = str(record.get("file") or "") + if not name: + return + ok, _reason = is_safe_component(name) + if not ok: + return + path = options.downloads_folder / name + try: + path.unlink() + except FileNotFoundError: + return + except OSError as error: + # Windows refuses while another handle is open, which is exactly the case for a jar the + # server is reading. The directory sweep below gets it on a later run. + _debug(logger, "could not remove the installed download {}: {}".format(path, error)) + + +def _sweep_consumed_downloads( + ledger: DownloadLedger, options: InstallOptions, logger: Optional[Any] = None +) -> List[str]: + """Delete leftover downloads this plugin provably wrote, once nothing claims them. + + The other half of the same promise. A crash between the install and the ledger save, or a + ``forget`` whose file was locked on Windows, leaves a jar in the download folder with no + record — and a file with no record is invisible to every other code path here, so it stays + forever. Two conditions are required before one is deleted, and both matter: + + * it must be a bare ``.jar``/``.part`` name inside the folder (never a path, and never + something the admin keeps there by hand); + * its SHA-1 must be one the ledger recorded for a download, so the file is *self-proving* to + have come from this plugin. A jar whose bytes this plugin has no record of is left alone: + deleting a file that might be the admin's own is the one mistake this module must not make. + + Returns the names removed, for the tests and the log. + """ + folder = options.downloads_folder + if not folder.is_dir(): + return [] + tracked = set() + known: Dict[str, str] = {} + for key in ledger.records(): + item = ledger.get(key) or {} + name = str(item.get("file") or "") + if not name: + continue + ok, _reason = is_safe_component(name) + if ok: + tracked.add(name) + digest = str(item.get("sha1") or "").lower() + if digest: + known[digest] = name + + if not known: + return [] + + removed: List[str] = [] + try: + candidates = sorted(folder.iterdir()) + except OSError as error: + _debug(logger, "could not list the download folder {}: {}".format(folder, error)) + return [] + for path in candidates: + if not path.is_file() or path.name in tracked: + continue + if path.suffix.lower() not in (".jar", ".part"): + continue + try: + digest = _sha1_of(path) + except OSError: + continue + if digest not in known: + continue + try: + path.unlink() + except OSError as error: + _debug(logger, "could not remove the leftover download {}: {}".format(path, error)) + continue + removed.append(path.name) + if removed: + _debug( + logger, + "removed {} leftover download(s) this plugin had already installed: {}".format( + len(removed), ", ".join(removed) + ), + ) + return removed + + +def _debug(logger: Optional[Any], message: str) -> None: + handler = getattr(logger, "debug", None) if logger else None + if handler is not None: + handler(message) + + def _log(logger: Optional[Any], result: InstallResult) -> None: handler = getattr(logger, "info", None) if logger else None if handler is None: diff --git a/mod_update_checker/jsonfile.py b/mod_update_checker/jsonfile.py index bb7a7a6..2d3998e 100644 --- a/mod_update_checker/jsonfile.py +++ b/mod_update_checker/jsonfile.py @@ -10,19 +10,76 @@ :func:`write_json`, which puts the bytes in a sibling ``.tmp`` first and then renames it over the target: a process killed mid-write leaves either the old file or the new one, never half of a new one. That matters most for the ones read back at startup — a truncated cache does -not merely lose its records, it makes the next start read nonsense. The sibling name carries -the process and thread that wrote it, so two writers of one file can never share it. +not merely lose its records, it makes the next start read nonsense. Writes to one target are +serialised and the rename is retried, because two concurrent renames of one destination are +what Windows actually refuses (see :func:`write_json`). Nothing here imports MCDR, so it is directly unit-testable. """ import json import os +import tempfile import threading +import time from pathlib import Path -from typing import Any, Optional, Union - -__all__ = ["json_text", "read_json", "write_json"] +from typing import Any, Dict, Optional, Union + +__all__ = ["json_text", "read_json", "write_json", "write_text"] + +#: One lock per *directory*, so two writers of different files never wait for each other while two +#: writers of one folder always do. Keyed on the folder rather than on the target file because the +#: failure is not about the destination alone: measured with the lock held only around the rename, +#: 9 of 1600 writes in 8 threads still needed a retry, and the rename is the operation Windows +#: refuses with ``WinError 5`` while another thread is creating its temporary file in the same +#: directory. Serialising the whole write — create, write, rename — is what removes the race. +#: +#: Measured before any of this existed (8 threads x 200 writes of one file): 0 failures while +#: writing the temporary file, 652 failures in ``os.replace``. +_WRITE_LOCKS: Dict[str, threading.Lock] = {} +_WRITE_LOCKS_GUARD = threading.Lock() + +#: How many times a rename is retried, and how long to wait before each retry. Kept as a safety +#: net for interference this module cannot see — a virus scanner or an indexer holding the target +#: open — because with the directory serialised a retry should never be needed at all; the +#: concurrency test asserts exactly that. +_REPLACE_ATTEMPTS = 3 +_REPLACE_BACKOFF = 0.05 + + +def _write_lock_for(target: Path) -> threading.Lock: + """The lock that serialises writes into ``target``'s directory. + + Keyed by the normalised absolute folder so that two spellings of one path (``a/../x.json`` + and ``x.json``) share a lock rather than taking different ones and racing anyway. + """ + folder = target.parent + try: + key = os.path.normcase(str(folder.resolve())) + except OSError: # pragma: no cover - resolve() can fail on a path that does not exist yet + key = os.path.normcase(os.path.abspath(str(folder))) + with _WRITE_LOCKS_GUARD: + lock = _WRITE_LOCKS.get(key) + if lock is None: + lock = threading.Lock() + _WRITE_LOCKS[key] = lock + return lock + + +def _replace_with_retry(temporary: Path, target: Path) -> None: + """``os.replace(temporary, target)``, retrying a failed rename a few times. + + Raises the last ``OSError`` when every attempt fails. The caller removes its own temporary + file in that case, so a permanently unwritable target does not litter the directory. + """ + for attempt in range(_REPLACE_ATTEMPTS): + try: + os.replace(str(temporary), str(target)) + return + except OSError: + if attempt == _REPLACE_ATTEMPTS - 1: + raise + time.sleep(_REPLACE_BACKOFF * (attempt + 1)) def json_text(payload: Any, sort_keys: bool = False) -> str: @@ -52,14 +109,66 @@ def write_json(path: Union[str, Path], payload: Any, sort_keys: bool = False) -> ``FileNotFoundError`` or — worse — put the *other* writer's bytes in place. The clash is not hypothetical here: the download ledger is written from MCDR's command thread and from the check thread at the same time, which is exactly the case ``DownloadLedger`` is careful about. + ``tempfile.mkstemp`` supplies the name rather than ``pid`` + ``threading.get_ident()``, + because a thread ident is recycled once a thread exits (measured: 4000 sequential threads + produced 1931 distinct idents), so it is not the uniqueness it looks like. + + Uniqueness of the name is necessary but **not sufficient**: what actually fails is two + concurrent ``os.replace`` calls aimed at one destination. Windows refuses the second one with + ``WinError 5`` rather than letting it win, so the loser's save is silently lost — the cache + entry or the approval state it was writing never lands. Writes to one target are therefore + serialised on a per-path lock held across the write *and* the rename, and the rename itself is + retried a few times with a short backoff. Callers keep seeing one of two states on disk; they + just no longer contend for it. + """ + target = Path(path) + target.parent.mkdir(parents=True, exist_ok=True) + payload_text = json_text(payload, sort_keys) + with _write_lock_for(target): + handle, temporary_name = tempfile.mkstemp( + dir=str(target.parent), prefix=target.name + ".", suffix=".tmp" + ) + temporary = Path(temporary_name) + try: + with os.fdopen(handle, "w", encoding="utf-8") as stream: + stream.write(payload_text) + _replace_with_retry(temporary, target) + except BaseException: + # Either the write or every rename attempt failed: take our own temporary file away + # before the error reaches the caller, so a failing target cannot fill the folder + # with orphans. The old target file, if any, is untouched. + try: + temporary.unlink() + except OSError: + pass + raise + + +def write_text(path: Union[str, Path], text: str) -> None: + """Write ``text`` to ``path`` with the same old-file-or-new-file guarantee as JSON. + + For the one non-JSON file the plugin keeps: the human-readable copy of the last report, next + to the JSON one. It would have been simpler to leave that one on a plain ``open(..., "w")``, + but then the two files written side by side would have different crash behaviour, and the + reader has no way to tell which of them they are looking at. """ target = Path(path) target.parent.mkdir(parents=True, exist_ok=True) - temporary = target.with_suffix( - "{}.{}.{}.tmp".format(target.suffix, os.getpid(), threading.get_ident()) - ) - temporary.write_text(json_text(payload, sort_keys), encoding="utf-8") - os.replace(str(temporary), str(target)) + with _write_lock_for(target): + handle, temporary_name = tempfile.mkstemp( + dir=str(target.parent), prefix=target.name + ".", suffix=".tmp" + ) + temporary = Path(temporary_name) + try: + with os.fdopen(handle, "w", encoding="utf-8", newline="") as stream: + stream.write(text) + _replace_with_retry(temporary, target) + except BaseException: + try: + temporary.unlink() + except OSError: + pass + raise def read_json(path: Union[str, Path]) -> Optional[Any]: diff --git a/mod_update_checker/lang/en_us.json b/mod_update_checker/lang/en_us.json index f2906b9..42a6add 100644 --- a/mod_update_checker/lang/en_us.json +++ b/mod_update_checker/lang/en_us.json @@ -1,6 +1,6 @@ { "language.unsupported": "[Mod Update Checker] Unknown language \"{value}\", falling back to {fallback}. Available: {options}", - "help.modupdate": "check whether the server's mods are up to date: summary / check / list / detail / download / install / delete / status / reload config", + "help.modupdate": "check whether the server's mods are up to date: summary / check / list / detail / download / install / delete / status / reload config (requires permission level {level})", "console.loaded": "[Mod Update Checker] loaded, {count} jar(s) found in {directory}", "console.no_mods_directory": "[Mod Update Checker] no mods directory at {directory}; set \"server.mods_directory\" in the config if it lives elsewhere", "console.mods_unreadable": "[Mod Update Checker] cannot read {directory} ({error}); the folder is there, so check its permissions or whatever is holding it open", @@ -15,7 +15,7 @@ "console.report_saved": "[Mod Update Checker] report written to {path}", "console.cache_summary": "[Mod Update Checker] resolve cache: {records} entr(ies) in {path}", "command.permission_denied": "Permission denied: this command needs MCDR permission level {level}", - "command.no_report_yet": "No check has run yet. Use !!modupdate check to run one now.", + "command.no_report_yet": "No check has run yet. Use {command} check to run one now.", "command.check_started": "Checking mods for updates in the background; the result will be printed here.", "command.check_already_running": "A check is already running, please wait for it to finish.", "command.unknown_status": "Unknown status \"{value}\". Valid: {options}", @@ -31,7 +31,7 @@ "check.in_game_awaiting": "downloaded, waiting to be installed ({count})", "status.awaiting_install": "to install", "line.awaiting_install": "{latest}", - "report.awaiting_install_hint": " (they are in {folder}; run '!!modupdate install ' to have the plugin put one in at the next stop, or copy it into mods/ yourself)", + "report.awaiting_install_hint": " (they are in {folder}; run '{command} install ' to have the plugin put one in at the next stop, or copy it into mods/ yourself)", "report.awaiting_install_found": "{count} mod(s) have a newer build downloaded and waiting to go into mods:", "note.download_skipped": "not downloaded: {reason}", "note.download_failed": "the download failed: {reason}", @@ -47,7 +47,7 @@ "download.complete_automatic": "[Mod Update Checker] automatic install is on (download.install_on_stop); they will be swapped in at the next stop.", "download.complete_in_game": "[Mod Update Checker] download finished: {count} newer build(s) ready ({size})", "download.complete_in_game_automatic": "(automatic install is on; they will be swapped in at the next stop)", - "download.complete_in_game_manual": "(type !!modupdate install all to install them at the next stop, or !!modupdate install )", + "download.complete_in_game_manual": "(type {command} install all to install them at the next stop, or {command} install )", "cleanup.notice": "{count} old backup(s) have been sitting in mods/ for over {days} day(s), {size} in total.", "cleanup.notice_confirm": "Type {command} to delete them, or {list_command} to look first.", "cleanup.notice_stage": "(something else is waiting to be confirmed) Type {command} to list them, then confirm.", @@ -59,14 +59,14 @@ "download.reason.declared-too-large": "the file is over the download.max_size_mb limit", "download.reason.name-conflict": "the download folder already holds a different file under that name", "check.admin_join_no_report": "[Mod Update Checker] no check has completed yet", - "check.admin_join_reused": "(from a check {minutes} minute(s) ago — !!modupdate check to refresh)", + "check.admin_join_reused": "(from a check {minutes} minute(s) ago — {command} check to refresh)", "check.admin_join_header": "[Mod Update Checker] server {version}", "line.note_group": " ({text})", "line.note_separator": ", ", "report.header": "[Mod Update Checker] server {version} (version source: {source})", "report.updates_found": "{count} mod(s) have a newer build that has not been downloaded:", "report.new_since_last": " {count} of those appeared since the last check: {names}", - "report.and_more": " ... and {count} more (use !!modupdate list)", + "report.and_more": " ... and {count} more (use {command} list)", "report.no_updates": "No updates found.", "report.blocked_found": "{count} mod(s) publish no build for this loader/game version:", "report.tally": "Checked {total} jar(s): {up_to_date} up to date, {update} with updates not yet downloaded, {awaiting} downloaded and waiting to be installed, {blocked} without a compatible build, {unresolved} unresolved.", @@ -120,8 +120,8 @@ "note.modrinth_disabled_by_config": "Modrinth lookups are disabled in the config", "advisory.client_only": "{count} client-only mod(s) are installed on this server: {files}", "advisory.duplicates": "{count} mod id(s) are installed twice — remove the old jar", - "advisory.unknown_mc_version": "the Minecraft version could not be detected, so mods were compared without a game-version filter; set \"mc_version\" explicitly if the results look wrong", - "advisory.guessed_mc_version": "the Minecraft version was inferred from mod metadata ({version}); set \"mc_version\" explicitly to be sure", + "advisory.unknown_mc_version": "the Minecraft version could not be detected, so mods were compared without a game-version filter; set \"server.mc_version\" explicitly if the results look wrong", + "advisory.guessed_mc_version": "the Minecraft version was inferred from mod metadata ({version}); set \"server.mc_version\" explicitly to be sure", "advisory.server_side_unsupported": "{count} mod(s) are marked server-unsupported on Modrinth: {files}", "advisory.missing_dependencies": "{count} declared dependenc(ies) could not be found in mods/: {files} (a dependency bundled inside another jar is reported here too)", "advisory.missing_dependencies_excluded": "{count} mod(s) declare a dependency that is not in mods/: {files} (a dependency bundled inside another jar would be missed; {excluded} client-only dependenc(ies) were excluded)", @@ -173,7 +173,7 @@ "console.config_flat_legacy": "[Mod Update Checker] {count} option(s) in the config use the old flat names and will not be read (old -> new: {moves}). The file has been rewritten in the new grouped shape; re-apply any value you still want.", "report.no_mods": "No jars were found in the mods directory.", "report.index_title": "{count} mod(s).", - "report.index_hint": "one mod's detail (version change and links): !!modupdate info .", + "report.index_hint": "one mod's detail (version change and links): {command} info .", "command.list.detail_link": "[details]", "command.list.detail_hover": "Runs {command}\nto show this mod's details", "command.list.next": "[next page]", @@ -186,7 +186,7 @@ "command.list.prev_command": "previous page: {command}", "command.rule.hint_help": "click a command to run it or fill it in", "command.rule.hint_rows": "hover {version} {status}; click {details}", - "command.info.usage": "Usage: !!modupdate info — any of them; the number comes from !!modupdate list.", + "command.info.usage": "Usage: {command} info — any of them; the number comes from {command} list.", "command.delete.usage": "Usage: {command} delete - delete one .old backup; all = every backup (the age threshold is not applied; the plan is shown first). To delete only the expired ones use {cleanup}.", "command.delete.disabled": "Deleting is switched off: this plugin removes nothing from mods/ by default.", "command.delete.disabled_hint": "To allow removing old backups, set {option} to true in the config ({path}), then run {command}.", @@ -208,7 +208,7 @@ "detail.open_page": "[open in browser]", "command.help.entry_info": "show one mod's details", "command.help.hover_info": "The number comes from list;\nthe name can be the mod name,\nthe mod id or the jar file name,\nor a unique prefix.", - "check.in_game_more_hint": "!!modupdate list for the full list", + "check.in_game_more_hint": "{command} list for the full list", "command.download.usage": "usage: {command} download — all means every pending update; numbers come from {command} list.", "command.download.plan_header": "{count} build(s) about to be fetched from Modrinth:", "command.download.plan_file": "file: {file} ({size})", @@ -216,6 +216,8 @@ "command.download.started": "[Mod Update Checker] fetching {name} {version}; the result is printed here.", "command.download.done": "[Mod Update Checker] {name} {version} downloaded to {path}. Type {command} to put it into mods/.", "command.download.in_flight": "[Mod Update Checker] {name} is being downloaded right now; nothing was started twice.", + "command.download.batch_in_flight": "[Mod Update Checker] {count} mod(s) in this batch are already being downloaded ({names}); only the rest were started.", + "command.download.batch_all_in_flight": "[Mod Update Checker] every mod in this batch is already being downloaded ({names}); nothing was started twice.", "command.download.failed": "[Mod Update Checker] the download of {name} failed: {reason}", "command.download.bad_folder": "the download folder is unusable (check 'download.folder_name' in the config), so nothing was fetched.", "command.download.batch_started": "[Mod Update Checker] fetching {count} newer build(s) now; the result will be reported here.", diff --git a/mod_update_checker/lang/zh_cn.json b/mod_update_checker/lang/zh_cn.json index 5c3b9ae..750356d 100644 --- a/mod_update_checker/lang/zh_cn.json +++ b/mod_update_checker/lang/zh_cn.json @@ -1,6 +1,6 @@ { "language.unsupported": "[Mod Update Checker] 不认识的语言 \"{value}\"，已回退到 {fallback}。可选：{options}", - "help.modupdate": "检查服务端 Mod 是否有新版本：汇总 / 立即检查 / 列表 / 详情 / 下载 / 安装 / 删除 / 状态 / 重载配置", + "help.modupdate": "检查服务端 Mod 是否有新版本：汇总 / 立即检查 / 列表 / 详情 / 下载 / 安装 / 删除 / 状态 / 重载配置（需要权限等级 {level}）", "console.loaded": "[Mod Update Checker] 已加载，在 {directory} 找到 {count} 个 jar", "console.no_mods_directory": "[Mod Update Checker] {directory} 下没有 mods 目录；如果它在别处，请设置配置里的 \"server.mods_directory\"", "console.mods_unreadable": "[Mod Update Checker] 读不到 {directory}（{error}）；目录是在的，请检查权限或是什么占着它不放", @@ -15,7 +15,7 @@ "console.report_saved": "[Mod Update Checker] 报告已写入 {path}", "console.cache_summary": "[Mod Update Checker] 识别缓存：{records} 条记录，位于 {path}", "command.permission_denied": "权限不足：该命令需要 MCDR 权限等级 {level}", - "command.no_report_yet": "还没有检查过。用 !!modupdate check 立即检查一次。", + "command.no_report_yet": "还没有检查过。用 {command} check 立即检查一次。", "command.check_started": "已在后台开始检查 Mod 更新，结果会打印在这里。", "command.check_already_running": "已经有一次检查在运行了，请等它结束。", "command.unknown_status": "未知的状态 \"{value}\"。可选：{options}", @@ -31,7 +31,7 @@ "check.in_game_awaiting": "已下载待安装({count} 个)", "status.awaiting_install": "已下载，待安装", "line.awaiting_install": "{latest}", - "report.awaiting_install_hint": " (就在 {folder}；要插件自己在关服时装就说 !!modupdate install <编号>，否则复制到 mods/ 即可)", + "report.awaiting_install_hint": " (就在 {folder}；要插件自己在关服时装就说 {command} install <编号>，否则复制到 mods/ 即可)", "report.awaiting_install_found": "有 {count} 个 Mod 的新版本已下载，等待放入 mods：", "note.download_skipped": "未下载：{reason}", "note.download_failed": "下载失败：{reason}", @@ -47,7 +47,7 @@ "download.complete_automatic": "[Mod Update Checker] 已开启自动安装(download.install_on_stop)，关服后会自动替换。", "download.complete_in_game": "[Mod Update Checker] 新版本下载完成：{count} 个({size})", "download.complete_in_game_automatic": "(已开启自动安装，关服后会自动替换)", - "download.complete_in_game_manual": "(输入 !!modupdate install all 可在关服后安装，或 !!modupdate install <编号>)", + "download.complete_in_game_manual": "(输入 {command} install all 可在关服后安装，或 {command} install <编号>)", "cleanup.notice": "有 {count} 个旧版备份(.old)已超过 {days} 天，共 {size}。", "cleanup.notice_confirm": "输入 {command} 删除它们，或先用 {list_command} 看一遍。", "cleanup.notice_stage": "(现在有别的操作等待确认)输入 {command} 列出清单后再确认删除。", @@ -59,14 +59,14 @@ "download.reason.declared-too-large": "文件超过 download.max_size_mb 的限制", "download.reason.name-conflict": "下载文件夹里已有同名但内容不同的文件", "check.admin_join_no_report": "[Mod Update Checker] 还没有任何检查结果", - "check.admin_join_reused": "(来自 {minutes} 分钟前的检查结果 — 输入 !!modupdate check 可立即重查)", + "check.admin_join_reused": "(来自 {minutes} 分钟前的检查结果 — 输入 {command} check 可立即重查)", "check.admin_join_header": "[Mod Update Checker] 服务端 {version}", "line.note_group": " ({text})", "line.note_separator": "，", "report.header": "[Mod Update Checker] 服务端 {version}(版本来源：{source})", "report.updates_found": "有 {count} 个 Mod 存在更新，但尚未下载：", "report.new_since_last": " 其中 {count} 项是上次检查之后新出现的：{names}", - "report.and_more": " …… 还有 {count} 个(用 !!modupdate list 查看全部)", + "report.and_more": " …… 还有 {count} 个(用 {command} list 查看全部)", "report.no_updates": "没有发现更新。", "report.blocked_found": "有 {count} 个 Mod 没有适配当前加载器/游戏版本的构建：", "report.tally": "共检查 {total} 个 jar：{up_to_date} 个最新，{update} 个有更新待下载，{awaiting} 个已下载待安装，{blocked} 个无适配构建，{unresolved} 个无法识别。", @@ -120,8 +120,8 @@ "note.modrinth_disabled_by_config": "配置中已关闭 Modrinth 查询", "advisory.client_only": "本服务端装了 {count} 个仅客户端 Mod：{files}", "advisory.duplicates": "有 {count} 个 Mod ID 装了两份 —— 请删掉旧的那个 jar", - "advisory.unknown_mc_version": "无法识别 Minecraft 版本，因此比对时没有按游戏版本过滤；如果结果看起来不对，请显式设置 \"mc_version\"", - "advisory.guessed_mc_version": "Minecraft 版本是从 Mod 元数据推断出来的({version})；建议显式设置 \"mc_version\" 以确保正确", + "advisory.unknown_mc_version": "无法识别 Minecraft 版本，因此比对时没有按游戏版本过滤；如果结果看起来不对，请显式设置 \"server.mc_version\"", + "advisory.guessed_mc_version": "Minecraft 版本是从 Mod 元数据推断出来的({version})；建议显式设置 \"server.mc_version\" 以确保正确", "advisory.server_side_unsupported": "有 {count} 个 Mod 在 Modrinth 上标注为「不支持服务端」：{files}", "advisory.missing_dependencies": "有 {count} 个 Mod 声明的依赖在 mods/ 里找不到：{files}(若该依赖已打包在别的 jar 内部，这里会误报)", "advisory.missing_dependencies_excluded": "有 {count} 个 Mod 声明的依赖在 mods/ 里找不到：{files}(若该依赖已打包在别的 jar 内部，这里会误报；另有 {excluded} 个仅客户端的依赖已排除)", @@ -173,7 +173,7 @@ "console.config_flat_legacy": "[Mod Update Checker] 配置文件里有 {count} 个旧版选项名，它们不会被读取(设置 -> 新位置：{moves})。配置文件已按新结构重新生成，请把仍然需要的值填回去。", "report.no_mods": "Mods 目录里没有找到任何 jar。", "report.index_title": "共 {count} 个 Mod。", - "report.index_hint": "某个 Mod 的详情(版本变更与链接)：!!modupdate info <编号>。", + "report.index_hint": "某个 Mod 的详情(版本变更与链接)：{command} info <编号>。", "command.list.detail_link": "[详细信息]", "command.list.detail_hover": "运行 {command}\n查看这个 Mod 的详情", "command.list.next": "[下一页]", @@ -186,7 +186,7 @@ "command.list.prev_command": "上一页：{command}", "command.rule.hint_help": "点击命令可直接执行或填入参数", "command.rule.hint_rows": "悬停 {version} {status}，点击 {details}", - "command.info.usage": "用法：!!modupdate info <编号 / Mod 名 / mod id>——都可以，编号来自 !!modupdate list。", + "command.info.usage": "用法：{command} info <编号 / Mod 名 / mod id>——都可以，编号来自 {command} list。", "command.delete.usage": "用法：{command} delete <文件名 / 编号 / all>——删除一个旧版备份(<文件名>.jar.old)；all = 全部备份(不看天数，会先列清单让你确认)。只想删过期的用 {cleanup}。", "command.delete.disabled": "删除功能未开启：本插件默认不删除 mods/ 里的任何文件。", "command.delete.disabled_hint": "要允许删除旧版备份，请在配置文件里把 {option} 设为 true({path})，改完输入 {command} 让它生效。", @@ -208,7 +208,7 @@ "detail.open_page": "[点击打开]", "command.help.entry_info": "查看某个 Mod 的详情", "command.help.hover_info": "编号来自 list；名字可以用\nMod 名称、mod id 或 jar 文件名，\n唯一前缀也认。", - "check.in_game_more_hint": "输入 !!modupdate list 看完整列表", + "check.in_game_more_hint": "输入 {command} list 看完整列表", "command.download.usage": "用法：{command} download <编号 / Mod 名 / all>——all 表示下载全部。编号来自 {command} list。", "command.download.plan_header": "即将从 Modrinth 下载 {count} 个：", "command.download.plan_file": "文件名：{file}({size})", @@ -216,6 +216,8 @@ "command.download.started": "[Mod Update Checker] 已开始下载 {name} {version}，完成后在这里告诉你结果。", "command.download.done": "[Mod Update Checker] {name} {version} 已下载到 {path}。要装进 mods/ 请输入 {command}。", "command.download.in_flight": "[Mod Update Checker] {name} 正在下载中，没有重复开始。", + "command.download.batch_in_flight": "[Mod Update Checker] 这批里有 {count} 个 Mod 正在下载（{names}），只开始了其余的部分。", + "command.download.batch_all_in_flight": "[Mod Update Checker] 这批里的每个 Mod 都正在下载（{names}），没有重复开始第二次。", "command.download.failed": "[Mod Update Checker] 下载 {name} 失败：{reason}", "command.download.bad_folder": "下载文件夹不可用(检查配置里的 download.folder_name)，因此没有下载。", "command.download.batch_started": "[Mod Update Checker] 已开始下载 {count} 个 Mod 的新版本，完成后告诉你结果。", diff --git a/mod_update_checker/report.py b/mod_update_checker/report.py index 1ae51ae..7cf655f 100644 --- a/mod_update_checker/report.py +++ b/mod_update_checker/report.py @@ -588,7 +588,11 @@ def resolve_handle(self, handle: str) -> Tuple[Optional[UpdateEntry], str]: if not text: return None, "empty" - if text.isdigit(): + # ``isdecimal`` rather than ``isdigit``: ``isdigit`` is also true for superscripts and + # circled digits (``²``, ``⑨``), which ``int()`` then refuses — so ``!!muc info ²`` + # raised out of a command callback that promises a readable reason. The two predicates + # agree on every character ``int()`` accepts. + if text.isdecimal(): number = int(text) for index, entry in self.indexed_entries(): if index == number: @@ -1123,11 +1127,13 @@ def chat_cell_widths(tr: Translator) -> Tuple[int, int]: attribute :func:`i18n.make_translator` puts on the callable it returns, and the no-cache fallback above for anything that does not carry one. - "Cannot change while the plugin runs" is the part that needs a qualification: MCDR reloads a - plugin by building a fresh copy of the *entry* module, while this sibling module stays in - ``sys.modules`` — so a new version of the plugin bringing a changed ``line.status_word`` for - the same language would keep the old measurement. :func:`reset_cell_width_cache` is the hook - that makes the reload visible here, and the entry module calls it on load. + "While this module run is alive" is the whole scope of the memo, and it survives a reload: + MCDR pops every ``mod_update_checker*`` module out of ``sys.modules`` while unloading + (``RegularPlugin._on_unload``), so this module is re-imported too and a fresh cache is built + with it. :func:`reset_cell_width_cache` is therefore *not* what makes a reload visible here — + the reload already did that. The entry module still calls it on load, which is harmless and + keeps the guarantee if MCDR's unload order ever changes, but "the cache would otherwise go + stale across a reload" is not why. """ language = getattr(tr, "language", None) if language is None: @@ -1302,7 +1308,8 @@ class SummarySection: status: str = "" -def summarise(report: Report, tr: Translator, max_updates: int = 12): +def summarise(report: Report, tr: Translator, max_updates: int = 12, + command: str = "!!modupdate"): """``(context, blocks, closing)`` — the summary, before it is laid out. Split rather than returned as one list of strings so the two renderers can decorate the @@ -1313,6 +1320,10 @@ def summarise(report: Report, tr: Translator, max_updates: int = 12): A block is a :class:`SummarySection` or a plain line — the latter for the "nothing to do" sentence, which belongs where it was written: above the blocked group, not after it. + + ``command`` is the alias named in the lines that tell the reader to type one. It defaults to + the first registered root, because the report *file* is rendered with no command context at + all and still has to read as a sentence. """ context = tr("report.header", version=report.server.describe(), source=report.server.mc_version_source) @@ -1332,7 +1343,8 @@ def add_section(entries, header_key, extra=None, status=""): # "and N more" line then claimed N items had been shown and N were missing, right # above the section that lists all of them. if max_updates and len(ordered) > max_updates: - trailing.append(tr("report.and_more", count=len(ordered) - max_updates)) + trailing.append(tr("report.and_more", count=len(ordered) - max_updates, + command=command)) if extra: trailing.append(extra) blocks.append(SummarySection(tr(header_key, count=len(ordered)), @@ -1356,7 +1368,7 @@ def add_section(entries, header_key, extra=None, status=""): add_section( pending, "report.awaiting_install_found", - tr("report.awaiting_install_hint", folder=report.download_folder) + tr("report.awaiting_install_hint", folder=report.download_folder, command=command) if report.download_folder else None, STATUS_AWAITING_INSTALL, @@ -1375,7 +1387,8 @@ def add_section(entries, header_key, extra=None, status=""): return context, blocks, closing -def render_summary(report: Report, tr: Translator, max_updates: int = 12) -> List[str]: +def render_summary(report: Report, tr: Translator, max_updates: int = 12, + command: str = "!!modupdate") -> List[str]: """The short form as plain lines: what needs attention, plus a tally. The two groups are kept apart rather than merged into one "has an update" list, because the @@ -1387,7 +1400,7 @@ def render_summary(report: Report, tr: Translator, max_updates: int = 12) -> Lis with the project url: a log line cannot be clicked, so the url is the only way to reach the page from there. """ - context, blocks, closing = summarise(report, tr, max_updates) + context, blocks, closing = summarise(report, tr, max_updates, command=command) lines = [context] for block in blocks: if isinstance(block, SummarySection): @@ -1400,9 +1413,9 @@ def render_summary(report: Report, tr: Translator, max_updates: int = 12) -> Lis return lines -def render_full(report: Report, tr: Translator) -> List[str]: +def render_full(report: Report, tr: Translator, command: str = "!!modupdate") -> List[str]: """The long form: every mod, its notes, and the diagnostics. Used by the command.""" - lines = list(render_summary(report, tr, max_updates=0)) + lines = list(render_summary(report, tr, max_updates=0, command=command)) lines.append(tr("report.all_mods")) ordered = report.sorted_entries() @@ -1495,7 +1508,8 @@ def index_page(indexed, page: int, size: int): def render_index( - report: Report, tr: Translator, entries=None, page: int = 1, budget: int = CHAT_PAGE_LINES + report: Report, tr: Translator, entries=None, page: int = 1, budget: int = CHAT_PAGE_LINES, + command: str = "!!modupdate", ): """``(rows, tail, page_info)`` for one page of the numbered listing. @@ -1509,7 +1523,8 @@ def render_index( it in its own vocabulary — the screens use the same labelled field the status screen does. The rows and the tail are the part that must not be computed twice. The pager line is not returned either: it names commands, and the command spelling belongs to the caller that - knows which prefix the reader typed, so it is built by :func:`render_pager` there. + knows which prefix the reader typed, so it is built by :func:`render_pager` there. ``command`` + carries that same spelling into the closing hint, which names one too. ``entries`` narrows the listing to a subset (the status filter) while keeping the numbers from the full list, so a number means one mod whichever command produced the row. @@ -1533,7 +1548,7 @@ def render_index( tail = [ tr("report.index_title", count=len(indexed)), render_tally(report, tr), - tr("report.index_hint"), + tr("report.index_hint", command=command), ] return rows, tail, (page, pages) diff --git a/mod_update_checker/scanner.py b/mod_update_checker/scanner.py index 1d7dce7..4997649 100644 --- a/mod_update_checker/scanner.py +++ b/mod_update_checker/scanner.py @@ -29,7 +29,7 @@ import zlib from dataclasses import dataclass, field from pathlib import Path -from typing import Any, Dict, Iterable, List, Optional, Tuple +from typing import Any, Dict, Iterable, List, Optional, Sequence, Tuple from .digests import digests_of_file from .versioning import RangeSpec, normalize_spec @@ -52,6 +52,12 @@ _LOGGER = logging.getLogger(__name__) +#: How much of a metadata entry is decompressed per read. The limit is enforced on the bytes +#: that actually arrive rather than on the size the entry declares, so a lying or hostile header +#: cannot turn into a multi-megabyte allocation — the same shape the JSON and download paths +#: already use. +_METADATA_CHUNK = 65536 + #: Largest metadata entry this plugin will unpack. Real ``fabric.mod.json`` and ``mods.toml`` #: files are a few kilobytes; the largest seen in the wild are well under a hundred. The limit #: exists because ``ZipFile.read`` allocates the size the entry *declares*, so a 20 KiB jar can @@ -374,6 +380,32 @@ def _contact_links(contact: Any) -> Tuple[str, ...]: return tuple(ordered) +def _coerce_range(value: Any) -> RangeSpec: + """Normalise a declared Minecraft version range to ``str``, ``list[str]`` or ``None``. + + One funnel for all three metadata readers, because the value comes from a third-party jar + and its type is whatever the author typed. A number, a bool or a mapping is not a range — + it is a mistake — and the cost of the mistake used to be the whole check: a + ``{"depends": {"minecraft": 123}}`` reached :func:`versioning.parse_spec`, which iterated + it and raised ``TypeError``, so every mod's result was lost, not just that jar's. A mapping + is dropped as well rather than iterated, which would report its *keys* as versions. + + List items are kept only when they are strings, and emptied ones are dropped: a version + range is text, and ``123`` in a list is a typo rather than a constraint — the point of + ``[123]`` ending up as "no constraint" is that the list form is not a loophole around the + scalar rule above. ``[">=1.21", 123, ""]`` keeps ``">=1.21"``, which is what the author + meant. + """ + if isinstance(value, str): + text = value.strip() + return text or None + if isinstance(value, Sequence) and not isinstance(value, (bytes, bytearray)): + items = [item.strip() for item in value if isinstance(item, str)] + items = [item for item in items if item] + return items or None + return None + + def _metadata_from_fabric(data: Dict[str, Any], entry: str, loader: str) -> ModMetadata: depends = data.get("depends") if isinstance(data.get("depends"), dict) else {} # ``_string_list`` already flattens the bare-string form, so ``provides`` needs no @@ -387,7 +419,7 @@ def _metadata_from_fabric(data: Dict[str, Any], entry: str, loader: str) -> ModM version=_first_string(data.get("version")), loader=loader, metadata_file=entry, - mc_range=depends.get("minecraft"), + mc_range=_coerce_range(depends.get("minecraft")), environment=_first_string(data.get("environment")) or None, authors=_string_list(data.get("authors")) or _string_list(data.get("author")), description=_first_string(data.get("description")) or None, @@ -421,9 +453,9 @@ def _metadata_from_quilt(data: Dict[str, Any], entry: str) -> ModMetadata: if isinstance(depends, list): for dependency in depends: if isinstance(dependency, dict) and "minecraft" in dependency: - mc_range = dependency["minecraft"] + mc_range = _coerce_range(dependency["minecraft"]) elif isinstance(depends, dict): - mc_range = depends.get("minecraft") + mc_range = _coerce_range(depends.get("minecraft")) provides = loader_block.get("provides") provided = _string_list(provides) return ModMetadata( @@ -578,7 +610,7 @@ def _metadata_from_toml_parsed( version=str(first.get("version", "") or ""), loader=loader, metadata_file=entry, - mc_range=_minecraft_range_from_dependencies(parsed.get("dependencies")), + mc_range=_coerce_range(_minecraft_range_from_dependencies(parsed.get("dependencies"))), environment=None, authors=_string_list(first.get("authors")), description=str(first.get("description", "") or "") or None, @@ -606,7 +638,7 @@ def _metadata_from_toml_regex(raw: str, entry: str, loader: str) -> ModMetadata: if target.lower() == "minecraft": # The first one wins, matching the strict parser's behaviour. if mc_range is None: - mc_range = _toml_value(body, "versionRange") or None + mc_range = _coerce_range(_toml_value(body, "versionRange")) continue if _toml_bool(body, "mandatory") is False: continue @@ -643,6 +675,44 @@ def _count_nested_jars(names: Iterable[str]) -> int: return sum(1 for name in names if any(p.match(name) for p in _NESTED_JAR_PATTERNS)) +def _read_entry_capped(archive: zipfile.ZipFile, entry: str, limit: int) -> bytes: + """Read one archive entry without ever allocating its declared size. + + ``ZipFile.read`` allocates ``file_size`` bytes up front, so until this existed the declared + size was an allocation an outside file got to ask for, and ``METADATA_SIZE_LIMIT`` only + refused the sizes that *said* they were too big. Measured: a 4209-byte jar made + ``fabric.mod.json`` declare 4194303 bytes and decompressed all of it. Streaming through + ``archive.open`` and counting the bytes as they arrive makes the limit the thing that stops + the read, whatever the entry claimed — the same shape the JSON and download paths use. + + A ratio guard was tried first and is *not* the right primary defence: ``"d" * 262144`` is + legitimate metadata that compresses 520:1, so any threshold low enough to stop an + amplification case also drops honest files (the repository has a test for exactly that + quarter-megabyte file). Counting the bytes cannot make that trade, because the cap holds + whatever the ratio is. + + Raises :class:`BadZipFile` — which both callers already treat as "cannot be read" — when the + entry turns out to be longer than the limit. + """ + chunks = [] + total = 0 + declared = archive.getinfo(entry).file_size + with archive.open(entry) as stream: + while True: + chunk = stream.read(_METADATA_CHUNK) + if not chunk: + break + total += len(chunk) + if total > limit: + raise zipfile.BadZipFile( + "{} declares {} bytes and passed the {} byte limit while being read".format( + entry, declared, limit + ) + ) + chunks.append(chunk) + return b"".join(chunks) + + def read_metadata(archive: zipfile.ZipFile) -> Optional[ModMetadata]: """Extract mod metadata from an open jar, or ``None`` if nothing recognisable is there. @@ -651,6 +721,15 @@ def read_metadata(archive: zipfile.ZipFile) -> Optional[ModMetadata]: tell them apart: the first is a library, the second is a broken file worth reporting. :func:`scan_jar` recovers the difference by asking :func:`read_metadata_error` afterwards, which is one extra ``namelist`` and no extra decompression. + + This function does not raise, and the shapes a *zip* can be broken in are the reason it has + to say so: an entry with the encryption bit set (``RuntimeError``), a deflate stream that + does not decompress (``zlib.error``), a truncated central directory (``BadZipFile``), an + entry asking for a compression method the interpreter does not implement + (``NotImplementedError``, which is a ``RuntimeError`` and was missing here until a jar used + one), and an entry whose declared size is a lie (``BadZipFile`` from + :func:`_read_entry_capped`). All of them are "this file cannot be read", which is a fact to + report rather than a reason to produce no report at all. """ names = archive.namelist() available = set(names) @@ -666,12 +745,24 @@ def read_metadata(archive: zipfile.ZipFile) -> Optional[ModMetadata]: # from the check thread, so an absurd entry is a memory spike that can stall or # kill the server for a file nobody asked it to read. continue - raw = archive.read(entry).decode("utf-8", errors="replace") - except (KeyError, OSError, RuntimeError, zipfile.BadZipFile, zlib.error): + raw = _read_entry_capped(archive, entry, METADATA_SIZE_LIMIT).decode( + "utf-8", errors="replace" + ) + except ( + KeyError, + OSError, + RuntimeError, + NotImplementedError, + zipfile.BadZipFile, + zlib.error, + ): # ``RuntimeError`` is what ``zipfile`` raises for an entry whose encryption bit is - # set, and ``zlib.error`` is a corrupt deflate stream — which is exactly what a jar - # copied while the download was still running looks like. ``BadZipFile`` covers - # neither, so without these two the "never raises" contract has a hole in it. + # set, and ``NotImplementedError`` (a ``RuntimeError`` subclass) is a compression + # method the interpreter does not implement; both are listed explicitly so the + # contract does not depend on that inheritance. ``zlib.error`` is a corrupt deflate + # stream — which is exactly what a jar copied while the download was still running + # looks like — and ``BadZipFile`` covers a truncated archive and the over-long entry + # caught by :func:`_read_entry_capped`. continue metadata: Optional[ModMetadata] = None @@ -707,10 +798,21 @@ def read_metadata_error(archive: zipfile.ZipFile) -> str: present but unreadable, or one that declares a size past :data:`METADATA_SIZE_LIMIT`. A jar that simply carries no metadata file — a library, a datapack, a stray download — returns ``""``, which is not a failure. + + The exceptions that count as "cannot be read" are the same set :func:`read_metadata` swallows, + deliberately: this function's whole job is to explain the difference between "unreadable" and + "absent", and two lists that disagree would attribute the same damage to two different causes + depending on which path reached it first. """ try: available = set(archive.namelist()) - except (OSError, RuntimeError, zipfile.BadZipFile, zlib.error) as error: + except ( + OSError, + RuntimeError, + NotImplementedError, + zipfile.BadZipFile, + zlib.error, + ) as error: return "cannot list the archive contents ({}: {})".format(type(error).__name__, error) for entry, _loader in MOD_METADATA_ENTRIES: @@ -725,8 +827,15 @@ def read_metadata_error(archive: zipfile.ZipFile) -> str: entry, info.file_size, METADATA_SIZE_LIMIT ) try: - archive.read(entry) - except (KeyError, OSError, RuntimeError, zipfile.BadZipFile, zlib.error) as error: + _read_entry_capped(archive, entry, METADATA_SIZE_LIMIT) + except ( + KeyError, + OSError, + RuntimeError, + NotImplementedError, + zipfile.BadZipFile, + zlib.error, + ) as error: return "{} cannot be unpacked ({}: {})".format(entry, type(error).__name__, error) return "" diff --git a/mod_update_checker/upstream.py b/mod_update_checker/upstream.py index 5eeafcb..c885f8d 100644 --- a/mod_update_checker/upstream.py +++ b/mod_update_checker/upstream.py @@ -95,6 +95,17 @@ class RateLimited(UpstreamError): """429 after the retries were exhausted.""" +class _BodyInterrupted(UpstreamError): + """The response body broke while it was being read — worth another attempt. + + Private, and a subclass of :class:`UpstreamError` on purpose: a caller that only knows + "the upstream failed" (which is every caller) keeps working unchanged, while + :meth:`HttpClient._request` can tell this one apart and retry it. It is never raised out + of the client: once the attempts are used up the retry loop re-raises it, which reads to + the caller exactly like the ``UpstreamError`` it is. + """ + + class RateLimiter: """A sliding-window limiter: at most ``per_minute`` grants per rolling minute. @@ -313,8 +324,27 @@ def _request( # Read the body rather than letting ``response.json()`` pull it in: the declared # length is checked first, and the bytes are then counted as they arrive, so an # unbounded or lying response cannot become an unbounded allocation. + # + # A body that breaks *while* it is being read is the one failure that can be + # retried, and it is the common one on a censored or flaky link — the connection + # was up for the headers and died mid-stream. The retry loop above covers the + # connection, so it has to cover this too, or ``network.retries`` only helps when + # the request never started. try: return self._read_json(response, url, status) + except _BodyInterrupted as error: + last_error = error + if attempt >= self.retries: + raise + wait = min(self.backoff_base * (2 ** attempt), 20.0) + self._log( + "debug", + "reading {} failed ({}), retrying in {:.1f}s".format( + url, type(error.__cause__).__name__ or "error", wait + ), + ) + time.sleep(wait) + continue finally: response.close() @@ -346,6 +376,17 @@ def _read_json(self, response: "requests.Response", url: str, status: int) -> An Two checks rather than one, for the same reason the download path has two: the declared length catches a large response before any of it is read, and the running total catches a response that declares nothing, declares a lie, or is chunked. + + Two kinds of failure leave here as an :class:`UpstreamError`, and the distinction is + the caller's: a body that broke mid-read (``ChunkedEncodingError``, + ``IncompleteRead``, a reset connection) is retried, because the next attempt may well + succeed, while anything the parser itself rejected is not, because the same bytes will + be rejected again. ``RecursionError`` belongs to the second group even though it looks + like a crash: ``json.loads`` raises it for a deeply nested document, and it is not a + ``ValueError``, so it used to escape the client entirely — past the retry loop, past + every ``except UpstreamError`` in the callers, and out into the check that had no + report. The document that does it costs a few hundred kilobytes, well under + :attr:`json_max_bytes`, so the size limit is not a defence against it. """ self._check_declared_size(response, url, status) chunks = [] @@ -363,7 +404,7 @@ def _read_json(self, response: "requests.Response", url: str, status: int) -> An ) chunks.append(chunk) except requests.RequestException as error: - raise UpstreamError( + raise _BodyInterrupted( "{}: {}".format(type(error).__name__, error), status=status, url=url ) from error @@ -376,6 +417,22 @@ def _read_json(self, response: "requests.Response", url: str, status: int) -> An status=status, url=url, ) from error + except MemoryError as error: + # A nesting bomb is the one shape that reaches here as an allocation failure rather + # than as ``RecursionError``, depending on the interpreter's limits. Both mean the + # same thing to the caller: this response is not usable, and asking for it again + # would do the same thing again. + raise UpstreamError( + "the response was too deeply nested to parse (MemoryError)", + status=status, + url=url, + ) from error + except RecursionError as error: + raise UpstreamError( + "the response was too deeply nested to parse (RecursionError)", + status=status, + url=url, + ) from error # -- public ------------------------------------------------------------------------ diff --git a/mod_update_checker/versioning.py b/mod_update_checker/versioning.py index 075f51f..9ff8a70 100644 --- a/mod_update_checker/versioning.py +++ b/mod_update_checker/versioning.py @@ -37,15 +37,22 @@ #: Pre-release markers and how they rank against each other. Lower is older. A version #: whose *extra* trailing segment is one of these is therefore older than the same version #: without it, which is what makes ``1.0.0`` newer than ``1.0.0-rc.1``. +#: +#: Only the spelled-out markers are here. ``a`` and ``b`` used to be in the table as +#: abbreviations of ``alpha`` and ``beta``, and that made the order incoherent: ``1.0.0-a`` was +#: *older* than ``1.0.0`` while ``1.0.0-1`` — the same suffix spelled as a number — was *newer*, +#: because the rule below only recognises a marker in the text table. Two strings that differ +#: only in how the author spelled the sequence number cannot compare on opposite sides of the +#: same release, so the abbreviations go and the numeric rule covers them. Real pre-releases +#: spell the marker out (``alpha``/``beta``/``rc``/``pre``); a bare ``-a`` is read as a +#: sequence, like every other non-marker suffix. _PRERELEASE_RANK = { "snapshot": -40, "dev": -35, "nightly": -34, "test": -30, "alpha": -20, - "a": -20, "beta": -10, - "b": -10, "pre": -6, "prerelease": -6, "preview": -6, @@ -55,10 +62,16 @@ #: Splits a version string into segments. Every separator mod authors use in practice. _SEPARATORS = re.compile(r"[.\-_]+") +#: The same split with the separators kept, so :func:`_tokenize` can tell ``1.0.0-1`` from +#: ``1.0.0.1``: the first is a pre-release sequence, the second a fourth version component. +_SEPARATORS_WITH_SEPARATORS = re.compile(r"([.\-_]+)") #: Splits a segment into alternating text / digit runs, so ``rc11`` == ``rc`` + ``11``. _ALNUM = re.compile(r"(\d+)") -Token = Tuple[int, str, str] # (kind, digit string, text); kind 0 = numeric, 1 = text +#: ``(kind, digit string, text, dashed)``. Kind 0 is numeric, 1 is text; ``dashed`` says the +#: token came from a ``-``-separated segment, which is what tells a pre-release apart from a +#: dotted extension (``1.0.0-1`` versus ``1.0.0.1``). +Token = Tuple[int, str, str, bool] def _normalise_digits(digits: str) -> str: @@ -86,6 +99,10 @@ def _tokenize(text: str) -> List[Token]: A leading ``v`` is dropped (``v1.2.3`` is ``1.2.3``), and every digit run becomes its own numeric token, so ``rc11`` sorts after ``rc2`` rather than before it. + + Each token also remembers whether its segment was introduced by a ``-``. That is the only + difference between ``1.0.0-1`` (a pre-release sequence) and ``1.0.0.1`` (a fourth version + component), and once the cores are equal it is the difference between older and newer. """ text = text.strip() if not text: @@ -93,16 +110,21 @@ def _tokenize(text: str) -> List[Token]: if len(text) > 1 and text[0] in "vV" and text[1].isdigit(): text = text[1:] tokens: List[Token] = [] - for segment in _SEPARATORS.split(text): - if not segment: + dashed = False + for piece in _SEPARATORS_WITH_SEPARATORS.split(text): + if not piece: + continue + if piece[0] in ".-_": + # The separator itself, kept by the capturing group so the flag can be set from it. + dashed = "-" in piece continue - for piece in _ALNUM.split(segment): - if not piece: + for run in _ALNUM.split(piece): + if not run: continue - if piece.isdigit(): - tokens.append((0, _normalise_digits(piece), "")) + if run.isdigit(): + tokens.append((0, _normalise_digits(run), "", dashed)) else: - tokens.append((1, "", piece.lower())) + tokens.append((1, "", run.lower(), dashed)) return tokens @@ -138,20 +160,62 @@ def _compare_tokens(left: Sequence[Token], right: Sequence[Token]) -> int: longer = left if len(left) > len(right) else right extra = longer[min(len(left), len(right)):] - # A trailing segment that *starts* with a pre-release marker makes a version older, not - # newer: ``1.0.0`` beats ``1.0.0-rc.1``. Only the first extra token is consulted, since - # the rest of a pre-release identifier is its sequence number (``-rc.1`` -> ``rc`` + 1) - # and would otherwise mask the marker. + # What an extra trailing segment means, stated as one rule because two rules is how this + # went wrong before: # - # Anything else extra is a genuine extension, so the longer side wins: ``1.0.1`` beats - # ``1.0``, and ``1.19.2-0.5.3`` beats a bare ``1.19.2``. That last one is a judgement - # call — the dash there separates a game version from a mod version rather than - # introducing a pre-release — but it is the reading that matches what authors mean. - if extra and _is_prerelease_token(extra[0]): + # * a **pre-release marker** makes the version older — ``1.0.0`` beats ``1.0.0-rc.1``. Only + # the first extra token is consulted, since the rest of a pre-release identifier is its + # sequence number (``-rc.1`` -> ``rc`` + 1) and would otherwise mask the marker. + # * a **pre-release suffix** makes it older: ``1.0.0`` beats ``1.0.0-rc.1``, and also beats + # ``1.0.0-1`` and ``1.0.0-a``. SemVer's rule is that everything after the first ``-`` is a + # pre-release identifier, and its *numeric* form ranks lowest — which is exactly why this + # cannot be decided one token at a time. Doing that made ``1.0.0-a.1 < 1.0.0``, + # ``1.0.0 < 1.0.0-1`` and ``1.0.0-a.1 < 1.0.0-1`` all hold at once, so "sort and take the + # last" depended on the order the strings arrived in. What decides it is the extra + # segments *as a whole*: a marker anywhere in them, or nothing but numbers. + # * a number after a **dot** is another component, so the longer side wins: ``26.3.1`` beats + # ``26.3`` and ``1.0.1`` beats ``1.0``. Reading that as a pre-release sequence would invert + # a whole class of ordinary version numbers. + # * **anything else** is a genuine extension, so the longer side wins: ``1.19.2-0.5.3`` beats + # a bare ``1.19.2``. That is a judgement call — the dash there separates a game version + # from a mod version rather than introducing a pre-release — but it is the reading that + # matches what authors mean, and it stays transitive because the pre-release test above + # looks at the same "extra segments" for every pair of strings. + if extra and _looks_like_prerelease(extra): return -1 if longer is left else 1 return 1 if longer is left else -1 +def _looks_like_prerelease(tokens: Sequence[Token]) -> bool: + """Whether a run of extra trailing tokens is a pre-release identifier. + + True when it carries a spelled-out marker anywhere (``rc.1``, ``beta.2``) or is introduced by + a dash and is nothing but numbers (``-1``, ``-2.3``) — SemVer's numeric pre-release + identifiers, which rank below the release they lead to. + + The dash matters: ``1.0.0-1`` is a pre-release sequence while ``1.0.0.1`` is a fourth version + component, and reading the second as a pre-release would invert ``26.3`` against ``26.3.1``. + It matters for the *first* extra token only, because the dash that introduces a pre-release + is the one that separates it from the core. + + Something that starts with a letter is a pre-release however it is spelled — that is what + replaces the ``a``/``b`` abbreviations the rank table used to carry, and it keeps a bare + ``-a`` (a sequence number, not a channel) on the *older* side of its core. The all-numbers + case after a dash is also a pre-release, which makes ``1.19.2-0.5.3`` read as "0.5.3 of + 1.19.2" rather than "1.19.2 plus more": a deliberate change from the old behaviour, and what + keeps the order consistent, because the alternative lets a three-component suffix outrank + the core while a one-component one does not. + """ + if not tokens: + return False + if any(_is_prerelease_token(token) for token in tokens): + return True + if tokens[0][0] == 1 and tokens[0][3]: + # ``-a``, ``-a.1``, ``-snapshot2``: a word after the dash, whatever the word is. + return True + return tokens[0][3] and all(token[0] == 0 for token in tokens) + + def _split_build(text: str) -> Tuple[str, str]: """Split ``0.162.0+26.3`` into its core and its build metadata.""" core, _, build = text.partition("+") @@ -165,6 +229,13 @@ def compare(left: str, right: str) -> int: ``0.162.0+26.3`` ranks above a bare ``0.162.0`` — for mod jars that suffix normally encodes the Minecraft version the build targets, which is genuine information. + When one side is a prefix of the other, the extra trailing segments decide it, and the rule + is stated in one place in :func:`_compare_tokens`: a pre-release marker or a number makes the + longer version **older** (``1.0.0`` beats both ``1.0.0-rc.1`` and ``1.0.0-1``), anything else + makes it **newer** (``1.0.1`` beats ``1.0``). The rule is deliberately total and transitive, + because callers sort with it: ``latest_of`` and the report's "newest build" both take a + maximum, and a comparison that is not transitive makes that maximum depend on input order. + The function never raises: junk in becomes a lexicographic answer out. """ if left == right: @@ -184,6 +255,11 @@ def is_prerelease(version: str) -> bool: Build metadata is dropped first: in ``1.0.0-beta+26.3`` the marker is in the core, and the ``26.3`` after the ``+`` is the Minecraft version, not a release channel. + + ``1.0.0-1`` is **not** a pre-release by this test, and that is the intended reading: a bare + number is a sequence number with no channel in it, and calling it one would gate a genuine + release behind ``check.include_alpha``. It still ranks below ``1.0.0`` in :func:`compare`, + because there it is a suffix rather than a channel. """ core, _ = _split_build(str(version)) return any(_is_prerelease_token(token) for token in _tokenize(core)) @@ -334,6 +410,12 @@ def parse_spec(spec: RangeSpec) -> List[Callable[[str], bool]]: A list means the alternatives the metadata allows — Fabric treats a list as *any of*, so this follows suit. Empty alternatives (``[""]``, seen in the wild) are ignored rather than treated as "matches nothing", which would silently drop a good mod. + + A value that is neither a string nor a sequence of them is also "no constraint". The + metadata is third-party data, and a typo in it is allowed to be wrong — not to abort the + run: ``{"minecraft": 123}`` used to reach the iteration below and raise ``TypeError``, + which cost the *whole* check its report. A mapping is refused for the same reason, since + iterating it yields its keys (``{"any": "1.21"}`` would have reported a range of "any"). """ if spec is None: return [lambda _version: True] @@ -341,8 +423,10 @@ def parse_spec(spec: RangeSpec) -> List[Callable[[str], bool]]: alternatives: Sequence[str] if isinstance(spec, str): alternatives = [spec] - else: + elif isinstance(spec, Sequence) and not isinstance(spec, (bytes, bytearray)): alternatives = [str(item) for item in spec] + else: + return [lambda _version: True] any_of: List[Callable[[str], bool]] = [] for alternative in alternatives: @@ -370,12 +454,19 @@ def covers(spec: RangeSpec, version: str) -> bool: def normalize_spec(spec: RangeSpec) -> str: - """A short human-readable form of a range specification, for reports.""" + """A short human-readable form of a range specification, for reports. + + ``*`` covers "not declared" and "declared as something this plugin cannot read" alike: + the report shows an unconstrained range either way, rather than a ``123`` or a dict's key + presented to an admin as if it were a Minecraft version. + """ if spec is None: return "*" if isinstance(spec, str): text = spec.strip() return text or "*" + if not isinstance(spec, Sequence) or isinstance(spec, (bytes, bytearray)): + return "*" parts = [str(item).strip() for item in spec] parts = [part for part in parts if part] return " | ".join(parts) if parts else "*" diff --git a/pytest.ini b/pytest.ini index 63e1e1c..4a6e226 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,7 +1,8 @@ [pytest] testpaths = tests -# ``e2e`` boots a real MCDR instance and takes about a minute; it is worth running before a -# release and not worth running on every save. +# ``e2e`` boots a real MCDR instance and takes about 2 minutes 20 seconds (the whole suite is +# about 146 s, measured); it is worth running before a release and not worth running on every +# save. markers = - e2e: boots a real MCDR instance (slow, ~1 minute) + e2e: boots a real MCDR instance (slow, ~2 m 20 s measured) addopts = -q diff --git a/tests/README.md b/tests/README.md index a39106b..ebd9e99 100644 --- a/tests/README.md +++ b/tests/README.md @@ -17,8 +17,9 @@ python -m pip install --target .testlibs -r tests/requirements-test.txt ## 跑测试 ```bash -PYTHONPATH=.testlibs python -m pytest # 全量（827 项，含端到端；端到端本身约 1 分钟） +PYTHONPATH=.testlibs python -m pytest # 全量（827 项，含端到端；实测约 146 s） PYTHONPATH=.testlibs python -m pytest -m "not e2e" # 跳过端到端，约 25 秒 +# 全量（含 2 项 e2e）：实测约 146 s，其中 e2e 两项约 2 分 20 秒 ``` `pytest.ini` 里已把 `tests/` 设为测试根，`conftest.py` 负责把仓库根和 `tests/` 放进 `sys.path`； @@ -87,8 +88,9 @@ MCDR_TEST_PYTHON=/path/to/mcdr-2.14/python PYTHONPATH=.testlibs python -m pytest python tools/mutation_check.py ``` -它拒绝在设了 `MCDR_SKIP_E2E` 的环境里运行——至少有一条守卫只有端到端套件能抓到，跳过了 e2e 就等于 -把「被抓住」误报成「存活」。CI 里有独立的 `mutation` 作业跑它。 +它拒绝在设了 `MCDR_SKIP_E2E` 的环境里运行，理由比原来写的那条窄：**现在 12 条 selector 里没有一条 +引用 `tests/test_e2e.py`**，「至少一条守卫只有 e2e 能抓到」这个前提实测不成立。守卫真正护的是 +**基线**——脚本报的是整套测试的摘要，悄悄少了端到端那一半，摘要就比它读起来的样子弱。CI 里有独立的 `mutation` 作业跑它。 ## 无法在仓库内闭环的部分 diff --git a/tests/test_audit_language.py b/tests/test_audit_language.py new file mode 100644 index 0000000..959f1ab --- /dev/null +++ b/tests/test_audit_language.py @@ -0,0 +1,82 @@ +"""``bench/audit_language.py`` — the maintenance tool, against its own false positives. + +Its second section used to infer "who passes this placeholder" from *any* call whose arguments +contained the key as a string literal. Keys that travel through a variable — ``tr(header_key, +count=...)``, ``tr(key, **args)``, ``tr(choice.note_key, **choice.note_args)`` — were therefore +attributed to whatever unrelated call happened to mention the literal, and the tool reported +15/15 findings that were all false. Noise at that level is worse than no tool: the real finding +is in the pile somewhere and nobody reads it. + +The fix is to stop inferring and ask the templates: format each one with arguments derived from +its own placeholders. That is a property of the file, so these tests pin the property rather +than any particular call site. +""" + +import json +import subprocess +import sys +from pathlib import Path + +REPO = Path(__file__).resolve().parent.parent +TOOL = REPO / "bench" / "audit_language.py" +PACKAGE = REPO / "mod_update_checker" + + +def _run(): + result = subprocess.run( + [sys.executable, str(TOOL)], + cwd=str(REPO), + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + ) + return result.returncode, result.stdout.decode("utf-8", "replace") + + +def test_the_tool_runs_and_reports_nothing_suspicious(): + """The repository's own language files are clean, and the tool says so.""" + code, output = _run() + + assert code == 0, output + assert "键数：zh_cn" in output, output + # Both the "unreferenced key" and the "unfillable template" sections. + assert output.count("（无）") >= 2, output + + +def test_no_unfillable_placeholder_is_reported(): + """The 15 findings that used to be reported were all keys passed through a variable. + + Asserted by absence of the *shape* of that output rather than by a count, so a genuine + finding still fails this test. + """ + _code, output = _run() + + # From the section heading to the next one, minus the heading line itself. + lines = output.splitlines() + start = next( + index for index, line in enumerate(lines) if line.startswith("2. ") + ) + end = next( + index for index, line in enumerate(lines[start + 1:], start + 1) + if line.startswith("3. ") + ) + body = [line.strip() for line in lines[start + 1:end]] + body = [line for line in body if line and not set(line) <= set("=")] + assert body == ["（无）"], body + + +def test_every_template_can_be_formatted_with_its_own_placeholders(): + """The property the section rests on, checked without the tool's formatting. + + Every message in both catalogues must survive ``str.format`` with one value per placeholder + it declares — which is exactly what the runtime does when a caller passes the arguments named + in the template. + """ + import re + + placeholder = re.compile(r"\{([a-z_0-9]+)\}") + for path in sorted(PACKAGE.glob("lang/*.json")): + catalogue = json.loads(path.read_text(encoding="utf-8")) + for key, template in catalogue.items(): + args = {name: "x" for name in placeholder.findall(template)} + rendered = template.format(**args) + assert not placeholder.search(rendered), (path.name, key, rendered) diff --git a/tests/test_checker.py b/tests/test_checker.py index 45edae8..4f24827 100644 --- a/tests/test_checker.py +++ b/tests/test_checker.py @@ -261,6 +261,18 @@ def run_check(upstream, scenario, tmp_path, map_path=None, cache_path=None, **ov checker.close() +def cached_payload(cache_path): + """The cache file's parsed contents, with a missing file read as an empty cache. + + ``ResolveCache.save`` writes back only what the instance learned, so a run that learned + nothing — nothing was asked, or nothing could be proven — leaves **no file at all** rather + than an empty one. "No record was written" is what these tests mean either way. + """ + if not cache_path.is_file(): + return {"version": ResolveCache.VERSION, "records": {}} + return json.loads(cache_path.read_text(encoding="utf-8")) + + def entry_for(report, mod_id): """The single entry for a mod id. @@ -867,7 +879,7 @@ def test_a_failed_search_is_not_cached_as_a_negative_verdict(tmp_path, upstream) assert entry.status == STATUS_UNRESOLVED assert "note.search_incomplete" in dict(entry.notes) - payload = json.loads(cache_path.read_text(encoding="utf-8")) + payload = cached_payload(cache_path) assert payload["records"] == {}, "an unproven verdict was written to the cache" # And the same run with the search reachable again resolves the mod. @@ -1114,6 +1126,32 @@ def run(): assert payload["records"] +def test_two_cache_instances_do_not_erase_each_other(tmp_path): + """两个实例各学到一个键，两次保存之后两个键都必须在。 + + 入口层同时活着好几个 ``ResolveCache``（检查线程、``!!muc`` 命令线程、状态页），每个都在 + 构造时取一次快照。整份回写等于「谁后保存谁赢」，先保存者这一轮花掉一次 search 加一次版本 + 查询换来的负缓存会被静默丢掉，而负缓存正是这个类存在的理由。所以 ``save`` 只回写本实例 + 改过的键——这条测试是那个性质的护栏，旧实现下第二次 ``save`` 会让第一个键消失。 + """ + path = tmp_path / "resolve-cache.json" + first = ResolveCache(path, ttl_hours=24.0) + second = ResolveCache(path, ttl_hours=24.0) + + first.put("a" * 40, {"resolved": False}) + first.save() + second.put("b" * 40, {"resolved": False}) + second.save() + + reloaded = ResolveCache(path, ttl_hours=24.0) + assert reloaded.get("a" * 40) is not None, "the first instance's record was overwritten" + assert reloaded.get("b" * 40) is not None + + payload = json.loads(path.read_text(encoding="utf-8")) + assert payload["version"] == ResolveCache.VERSION + assert sorted(payload["records"]) == ["a" * 40, "b" * 40] + + def test_the_cache_entry_expires_after_the_ttl(tmp_path): path = tmp_path / "resolve-cache.json" cache = ResolveCache(path, ttl_hours=1.0) @@ -1242,7 +1280,7 @@ def refuse(_self, *_args, **_kwargs): use_cache=True, cache_path=cache_path) assert entry_for(report, "zeta").status == STATUS_UNRESOLVED - payload = json.loads(cache_path.read_text(encoding="utf-8")) + payload = cached_payload(cache_path) assert payload["records"] == {}, "an unasked question was written to the cache" assert "note.modrinth_disabled_by_config" in dict(report.upstream_notes) @@ -1367,10 +1405,10 @@ def test_the_full_listing_does_not_claim_rows_were_held_back(): summary = "\n".join(render_summary(report, tr)) full = "\n".join(render_full(report, tr)) - hidden_line = tr("report.and_more", count=len(entries) - 12) + hidden_line = tr("report.and_more", count=len(entries) - 12, command="!!modupdate") assert hidden_line in summary, "the summary must say what it left out" assert hidden_line not in full, "nothing is left out of the full listing" - assert tr("report.and_more", count=len(entries)) not in full + assert tr("report.and_more", count=len(entries), command="!!modupdate") not in full # And every mod is there, which is what makes the line above a lie rather than a warning. assert all("Mod {}".format(index) in full for index in range(13)) @@ -2020,6 +2058,34 @@ def test_the_reason_a_handle_failed_is_told_apart(): assert report.resolve_handle("no such mod") == (None, "unknown") +@pytest.mark.parametrize("handle", ["\u00b2", "\u2468", "1\u00b2", "\u00bd"]) +def test_a_digit_like_character_is_not_a_number(handle): + """``isdigit`` is true for digits ``int()`` refuses, and that used to be a silent crash. + + ``²`` (U+00B2), ``⑨`` (U+2468) and ``½`` are all ``isdigit``, and none of them converts: + ``resolve_handle`` raised ``ValueError`` out of a command callback, and MCDR logs a callback + exception **without replying to its source** — so the player typed a command and saw nothing + at all. Pasted text makes this easy to hit by accident. + """ + report = _index_report(_many(actionable=2)) + + entry, reason = report.resolve_handle(handle) + + assert entry is None, (handle, entry) + assert reason in ("out-of-range", "unknown"), (handle, reason) + + +def test_arabic_indic_digits_are_still_a_number(): + """The other direction, so the fix is not "refuse anything unusual". + + ``٣`` (U+0663) is ``isdecimal`` **and** ``int() -> 3``, so it stays a number; the difference + between it and ``²`` is exactly what the predicate has to track. + """ + report = _index_report(_many(actionable=2)) + + assert report.resolve_handle("\u0663") == (None, "out-of-range") + + def test_an_entry_can_be_looked_up_by_number_or_by_name(): """A player copying an id out of the listing should not have to translate it to a number.""" report = _index_report([_entry_with_links("sodium", STATUS_UPDATE_AVAILABLE)]) diff --git a/tests/test_docs.py b/tests/test_docs.py index 07071a2..f71c1f2 100644 --- a/tests/test_docs.py +++ b/tests/test_docs.py @@ -358,9 +358,39 @@ def test_every_repository_url_names_the_same_repository(): ) # And the author is a person, not a repository: the metadata keeps ``Pau1am`` even though - # the repo lives under the org. - authors = {entry["name"] for entry in metadata["authors"]} - assert authors, "no author is declared" + # the repo lives under the org. Written in the legacy ``author``/``link`` spelling, which is + # the only one every MCDR release reads: ``authors``/``links`` arrived in 2.16.0, so on + # 2.13.0-2.15.7 both the author and the homepage were silently absent from + # ``!!MCDR plugin list``. ``test_the_metadata_is_readable_on_every_supported_mcdr`` pins the + # whole shape. + assert metadata["author"] == ["Pau1am"], metadata["author"] + assert metadata["link"] == metadata["links"]["homepage"] + + +def test_the_metadata_is_readable_on_every_supported_mcdr(): + """The declared minimum is 2.13.0, and 2.13.0 understands a *different* set of keys. + + Measured by reading each release's own parser: 2.13.0/2.14.1/2.15.0/2.15.7 have only + ``author`` (a list of strings) and a top-level ``link``; ``authors``/``links``/``license`` + are 2.16.0 additions. So a plugin declaring 2.13.0 support must use the legacy pair or lose + its author and homepage on the versions it claims — and must **not** use both, because 2.16.0 + merges them into two authors with the same name. + + This is asserted as a property of the file rather than as a version-by-version matrix: the + legacy keys are present, the modern ones are only the ones 2.16 reads *in addition* (never + ``authors``), and no field is one that no release has ever accepted. + """ + metadata = json.loads((REPO / "mcdreforged.plugin.json").read_text(encoding="utf-8")) + + assert isinstance(metadata.get("author"), list) + assert all(isinstance(name, str) for name in metadata["author"]) + assert isinstance(metadata.get("link"), str) and metadata["link"].startswith("https://") + assert "authors" not in metadata, "2.16 would merge this with author into two authors" + # ``links`` is kept because 2.16 reads it, but only with keys 2.16 actually defines. + assert set(metadata.get("links", {})) <= {"homepage", "source", "issues"} + # ``author`` entries are names, not objects: ``{"name": ..., "link": ...}`` inside the legacy + # list is not a field on any version (2.16's Person has name/email/homepage). + assert all(isinstance(name, str) for name in metadata["author"]) # -------------------------------------------------------------------------------------- diff --git a/tests/test_i18n.py b/tests/test_i18n.py index 8d74ee8..68360db 100644 --- a/tests/test_i18n.py +++ b/tests/test_i18n.py @@ -224,6 +224,46 @@ def test_available_languages_reads_the_package(): assert i18n.available_languages() == ["en_us", "zh_cn"] +def test_the_language_list_never_falls_below_what_the_package_ships(monkeypatch): + """The Python 3.8 floor, in the layout where it used to cost a language. + + ``importlib.resources.files`` is 3.9+, and this plugin declares 3.8 as its floor. On 3.8 the + old call raised ``AttributeError``, the module's ``except Exception`` swallowed it, and the + ``os.listdir`` fallback then found nothing inside a packed ``.mcdr`` — so + ``available_languages()`` came back as ``['en_us']`` and a ``zh_cn`` admin was told, in + English, that their language did not exist. Simulated here by making the enumerator raise and + the package directory unlistable, which is exactly the 3.8-plus-zip combination. + """ + def unavailable(_name): + raise AttributeError("module 'importlib.resources' has no attribute 'files'") + + monkeypatch.setattr(i18n.importlib.resources, "contents", unavailable) + monkeypatch.setattr(i18n, "_filesystem_dir", lambda: "/nonexistent/inside/a/mcdr/zip") + + assert i18n.available_languages() == sorted(i18n.SHIPPED_LANGUAGES) + assert "zh_cn" in i18n.available_languages() + + +def test_a_shrunk_answer_is_widened_to_the_shipped_floor(monkeypatch): + """A partial listing is no better than none: the floor is what the package is known to hold.""" + monkeypatch.setattr(i18n, "_list_lang_files", lambda: ["en_us.json"]) + + assert i18n.available_languages() == ["en_us", "zh_cn"] + + +def test_a_language_added_to_the_package_is_still_picked_up(monkeypatch): + """The other direction: the floor must not become a ceiling.""" + monkeypatch.setattr(i18n, "_list_lang_files", lambda: ["en_us.json", "zh_cn.json", "ja_jp.json"]) + + assert i18n.available_languages() == ["en_us", "ja_jp", "zh_cn"] + + +def test_every_shipped_language_actually_has_a_catalogue(): + """The floor is a claim about the package, so it has to be true of the package.""" + for code in i18n.SHIPPED_LANGUAGES: + assert (PACKAGE / "lang" / "{}.json".format(code)).is_file(), code + + @pytest.mark.parametrize( "setting,mcdr_language,expected", [ diff --git a/tests/test_jsonfile.py b/tests/test_jsonfile.py index 3be7839..60eba87 100644 --- a/tests/test_jsonfile.py +++ b/tests/test_jsonfile.py @@ -12,7 +12,9 @@ """ import json +import os import threading +import time from pathlib import Path import pytest @@ -68,6 +70,7 @@ def test_a_failed_write_leaves_the_old_file_untouched(tmp_path, monkeypatch): 这是「先写临时文件再改名」存在的全部理由，所以它值得一条测试——直接构造一次写盘失败 （磁盘满、权限、被杀掉），断言两种坏结果都没有发生：目标文件既没被改写，也没被截断。 + 失败的那次还必须把自己的临时文件带走：失败路径留下垃圾，等于把「换个名字」的优点又还回去。 """ target = tmp_path / "download-manifest.json" write_json(target, {"version": 1, "mods": {"kept": {"file": "kept.jar"}}}) @@ -81,24 +84,15 @@ def explode(_source, _destination): write_json(target, {"version": 1, "mods": {}}) monkeypatch.undo() - # 目标文件一个字没变。留下的 ``.tmp`` 是这次失败的痕迹，不是会被当成结果读走的东西—— - # 这也正是「换个名字」和「就地改写」的区别：就地改写的那份文件现在是半份。 - # - # 临时名的**前缀**是目标名，后面两个数字是 pid 与线程号（见 - # ``test_two_threads_writing_one_file_do_not_share_a_temporary_name``）：它们的存在让两个 - # 写者不可能用同一个临时名互相截断，而断言仍要求这份文件「看起来是这次写的」。 assert target.read_text(encoding="utf-8") == before assert read_json(target) == {"version": 1, "mods": {"kept": {"file": "kept.jar"}}} - leftovers = [path.name for path in tmp_path.iterdir() if path.name != target.name] - assert len(leftovers) == 1, leftovers - assert leftovers[0].startswith("download-manifest.json.") - assert leftovers[0].endswith(".tmp") + assert [path.name for path in tmp_path.iterdir()] == [target.name] def test_two_threads_writing_one_file_do_not_share_a_temporary_name(tmp_path, monkeypatch): """两个写者共用一个 ``.tmp`` 会互相截断——账本正是被两个线程同时写的那个文件。 - 这里不靠时序碰运气：临时名里的 pid 与线程号就是这条性质的实现，所以直接观察**两个线程 + 这里不靠时序碰运气：``tempfile.mkstemp`` 就是这条性质的实现，所以直接观察**两个线程 留下的临时名不同**。旧实现下两者都是 ``download-manifest.json.tmp``，断言立刻失败。 """ blocker = tmp_path / "download-manifest.json" @@ -127,14 +121,105 @@ def writer(): monkeypatch.undo() try: - assert len(seen) == 2, seen - assert seen[0] != seen[1], "two writers shared one temporary name: {}".format(seen) + # 3 = how many times a failed rename is retried, so this is the same observation seen + # through the retry loop: the names are recorded once per attempt. + assert len(set(seen)) == 2, "two writers shared one temporary name: {}".format(seen) + assert len(seen) % 3 == 0, seen finally: for leftover in tmp_path.iterdir(): if leftover.name != blocker.name: leftover.unlink() +def test_eight_threads_writing_one_file_always_succeed(tmp_path): + """同一目标的并发写必须全部成功，而且一个临时文件都不许剩下。 + + 这条钉住的是**改名冲突**，不是临时名冲突：8 个写者各自的临时名从来不同（上面那条测试 + 已经量过），失败的是两个并发的 ``os.replace`` 指向同一个目标——Windows 会用 + ``WinError 5`` 拒绝第二个。实测 8 线程 × 200 次写同一个文件，只有 write 全部成功、 + 652/1600 次 replace 失败。修法是按目标路径串行化写入，所以这里断言的是「零异常」与 + 「无 ``.tmp`` 残留」。 + + 还有一条断言是**这个测试唯一能在锁被删掉时失败的**：``_replace_with_retry`` 会重试 3 次， + 所以「有没有锁」在结果上看不出来——实测把锁去掉、只留重试，这条测试照样一个异常都没有， + 而每次写入都要多等一次 50 ms 的退避。于是这里直接量那次重试**有没有发生**：串行化之后每次 + 改名都应该一次成功，1600 次写入不该有任何一次走到重试分支。 + """ + import mod_update_checker.jsonfile as jsonfile_module + + target = tmp_path / "download-manifest.json" + write_json(target, {"version": 1, "mods": {}}) + + errors = [] + retries = [] + real_sleep = time.sleep + threads = 8 + rounds = 200 + ready = threading.Barrier(threads) + + def watching_sleep(seconds): + # ``_replace_with_retry`` is the only caller of ``time.sleep`` in this module. + with lock: + retries.append(seconds) + return real_sleep(min(seconds, 0.001)) + + def writer(index): + ready.wait() + for round_index in range(rounds): + try: + write_json(target, {"version": 1, "mods": {}, "writer": index, + "round": round_index}) + except OSError as error: # noqa: PERF203 - the failure is the observation + with lock: + errors.append("{}: {}".format(type(error).__name__, error)) + return + + lock = threading.Lock() + workers = [threading.Thread(target=writer, args=(index,)) for index in range(threads)] + original_sleep = jsonfile_module.time.sleep + jsonfile_module.time.sleep = watching_sleep + try: + for worker in workers: + worker.start() + for worker in workers: + worker.join() + finally: + jsonfile_module.time.sleep = original_sleep + + assert errors == [], errors + assert [path.name for path in tmp_path.iterdir()] == [target.name] + assert isinstance(read_json(target), dict) + assert retries == [], ( + "{} renames needed a retry, so writes to one file were racing rather than being " + "serialised".format(len(retries)) + ) + + +def test_a_rename_that_fails_once_is_retried(tmp_path, monkeypatch): + """被别的写者挡了一下的改名是可重试错误，重试一次就该过去。""" + import mod_update_checker.jsonfile as jsonfile_module + + target = tmp_path / "download-manifest.json" + write_json(target, {"version": 1, "mods": {}}) + + attempts = [] + real_replace = os.replace + + def flaky(source, destination): + attempts.append(Path(source).name) + if len(attempts) == 1: + raise OSError("WinError 5: access denied") + return real_replace(source, destination) + + monkeypatch.setattr(jsonfile_module.os, "replace", flaky) + write_json(target, {"version": 1, "mods": {"kept": {}}}) + monkeypatch.undo() + + assert len(attempts) == 2, "the rename was not retried" + assert read_json(target) == {"version": 1, "mods": {"kept": {}}} + assert [path.name for path in tmp_path.iterdir()] == [target.name] + + # -- reading ----------------------------------------------------------------------------- diff --git a/tests/test_mcdr_entry.py b/tests/test_mcdr_entry.py index 857db3a..b908c6d 100644 --- a/tests/test_mcdr_entry.py +++ b/tests/test_mcdr_entry.py @@ -87,6 +87,12 @@ def load_config_simple(self, file_name=None, *, target_class=None, **_kwargs): would be describing a state a real server cannot produce. This mirrors MCDR's ``load_config_simple``: load, deserialize with the missing-field callback, and save exactly when the file was unreadable or something was missing. + + The ``failure_policy='regen'`` branch is part of that mirror, and it used to be missing: + MCDR catches whatever ``deserialize`` raises and **overwrites the file with defaults** + (``plugin/si/plugin_server_interface.py``). Without it here, a type mistake raised + straight through the fake and the whole path was untestable — which is exactly how the + plugin came to reset a user's config in silence. """ from mcdreforged.plugin.si._simple_config_handler import SimpleConfigHandler @@ -104,9 +110,17 @@ def note_missing(*_args): handler.save(config.serialize(), encoding="utf8") return config - config = target_class.deserialize( - raw, missing_callback=note_missing, redundancy_callback=note_missing - ) + try: + config = target_class.deserialize( + raw, missing_callback=note_missing, redundancy_callback=note_missing + ) + except Exception as error: # noqa: BLE001 - this is MCDR's own ``except Exception`` + self.logger.debug( + "config regeneration: {}: {}".format(type(error).__name__, error) + ) + config = target_class.get_default() + handler.save(config.serialize(), encoding="utf8") + return config if incomplete: handler.save(config.serialize(), encoding="utf8") return config @@ -192,22 +206,96 @@ def test_event_handlers_are_not_registered_explicitly_as_well(): ) -def test_load_stops_the_previous_modules_scheduler(): - """A reload hands the old module to ``on_load`` and never fires ``on_unload``. +def test_load_stops_the_previous_modules_scheduler(tmp_path, monkeypatch): + """A reload must stop the outgoing module's interval thread — for real, not in the source. - Verified in MCDR's own ``plugin_manager``: ``__reload_plugin`` calls ``plugin.reload()`` - directly, with no ``PLUGIN_UNLOADED`` dispatch and no ``__unload_plugin``. So the outgoing - module's scheduler thread is still alive, and if ``on_load`` does not stop it, every - reload leaves one more thread running checks against this server forever. + This used to be an ``ast.dump`` assertion that the names ``_stop_scheduler`` and + ``prev_module`` appeared somewhere in ``on_load``. Deleting the call and leaving the names in + a string literal kept it green, which is the shape of a test that cannot fail. So the + previous module here is a real object whose stop function records that it ran. """ - tree = ast.parse(_entry_source()) - loader = next( - node for node in ast.walk(tree) - if isinstance(node, ast.FunctionDef) and node.name == "on_load" - ) - body = ast.dump(loader) - assert "_stop_scheduler" in body, "on_load does not call into the previous module" - assert "prev_module" in body, "on_load ignores the previous module handed to it" + import mod_update_checker as plugin + + server = _FakeServer(tmp_path) + calls = [] + + class _Previous: + _last_report = None + _online_players = set() + + @staticmethod + def _stop_scheduler(): + calls.append("stopped") + + monkeypatch.setattr(plugin, "_register_commands", lambda _server: None, raising=False) + monkeypatch.setattr(plugin, "reset_cell_width_cache", lambda: None, raising=False) + monkeypatch.setattr(plugin, "_log_cache_summary", lambda *_a, **_k: None, raising=False) + monkeypatch.setattr(plugin, "_start_interval_scheduler", lambda *_a, **_k: None, raising=False) + monkeypatch.setattr(plugin.Config, "enabled", False, raising=False) + + plugin._stop_event.clear() + plugin.on_load(server, _Previous()) + + assert calls == ["stopped"], calls + + +def test_load_keeps_a_live_report_from_a_genuinely_reloaded_module(tmp_path, monkeypatch): + """The reload carry-over, against the module identity a real reload produces. + + MCDR unloads before it reloads: ``__reload_ready_plugins`` calls ``__unload_plugin``, which + dispatches ``PLUGIN_UNLOADED`` and calls ``plugin.unload()``; ``RegularPlugin._on_unload`` + then pops every ``mod_update_checker*`` module from ``sys.modules``. The replacement module + therefore has a **different** ``Report`` class object, so ``isinstance(carried, Report)`` — + which is what ``on_load`` used to ask — is false no matter which report is handed over. The + live report was dropped on every ``!!MCDR reload plugin`` and the admin was shown "no check + has run yet" until a whole new check finished. + + Reproduced here the way MCDR does it (import a second copy, then hand the first one over), + rather than by monkeypatching ``_last_report`` on the new module — which is what the old + version of this test did, and which never entered the ``prev_module`` branch at all. + """ + import importlib + import sys + + plugin = importlib.import_module("mod_update_checker") + server = _FakeServer(tmp_path) + live = _report(updates=1) + + saved = { + name: module + for name, module in list(sys.modules.items()) + if name == "mod_update_checker" or name.startswith("mod_update_checker.") + } + for name in saved: + del sys.modules[name] + fresh = importlib.import_module("mod_update_checker") + try: + assert fresh.Report is not plugin.Report, ( + "the reload simulation did not produce a second module" + ) + + previous = type("_PreviousModule", (), { + "_last_report": live, + "_online_players": set(), + "_stop_scheduler": staticmethod(lambda: None), + })() + + monkeypatch.setattr(fresh, "_load_config", lambda _server: _config_with(), raising=False) + monkeypatch.setattr(fresh, "_register_commands", lambda _server: None, raising=False) + monkeypatch.setattr(fresh, "reset_cell_width_cache", lambda: None, raising=False) + monkeypatch.setattr(fresh, "_log_cache_summary", lambda *_a, **_k: None, raising=False) + monkeypatch.setattr(fresh, "_start_interval_scheduler", lambda *_a, **_k: None, raising=False) + monkeypatch.setattr(fresh.Config, "enabled", False, raising=False) + + fresh._stop_event.clear() + fresh.on_load(server, previous) + + assert fresh._last_report is live, "the live report was dropped by the reload" + finally: + for name in [name for name in sys.modules if name == "mod_update_checker" + or name.startswith("mod_update_checker.")]: + del sys.modules[name] + sys.modules.update(saved) def test_the_stop_event_is_cleared_before_a_new_scheduler_starts(tmp_path, monkeypatch): @@ -296,6 +384,75 @@ def explode(*_args, **_kwargs): plugin._stop_scheduler() +def test_the_help_entry_states_the_level_instead_of_filtering_on_it(tmp_path): + """``!!help`` must not be able to disagree with the gate that actually runs the command. + + MCDR stores a help message's permission level as a value and filters on it when ``!!help`` is + rendered, while this plugin's gate reads the config every time a command runs. Registering the + entry at ``command_permission_level`` therefore pinned the two apart: after lowering the level + and running ``!!muc reload`` the command worked and ``!!help`` did not mention it, and after + raising it ``!!help`` offered an entry that could not be clicked. + + So the entry is registered at 0 and carries the number in its text — asserted here by + changing the level, re-reading the config, and checking both halves. + """ + import mod_update_checker as plugin + + server = _FakeServer(tmp_path) + plugin._stop_event.clear() + monkeypatch_config = _config_with({"command_permission_level": 0}) + plugin._config = monkeypatch_config + + plugin._register_commands(server) + + assert {prefix for prefix, _m, _p in server.help_messages} == {"!!modupdate", "!!muc"} + assert {perm for _p, _m, perm in server.help_messages} == {0} + assert all("0" in str(message) for _p, message, _perm in server.help_messages) + + # Raising the level later must be reflected in what a reader is told, without a reload of + # anything: nothing was filtered on the level, so there is no stale copy of it anywhere. + server.help_messages.clear() + plugin._config = _config_with({"command_permission_level": 4}) + plugin._register_commands(server) + + assert all("4" in str(message) for _p, message, _perm in server.help_messages), \ + [str(m) for _p, m, _perm in server.help_messages] + + +def test_the_report_files_are_written_atomically(tmp_path, monkeypatch): + """``last_report.json`` is read back at startup, so a half-written one costs the report. + + It was the last file in the plugin written with a bare ``open(path, "w")``: killed mid-write, + ``Report.from_json`` answers ``None`` for the truncated file, and the admin's report becomes + "no check has run yet" — the failure the temp-then-rename helper exists to prevent. Asserted + by making every rename fail: the old content must survive, in both files. + """ + import mod_update_checker as plugin + + server = _FakeServer(tmp_path) + report = _report(updates=1) + + plugin._write_report_files(server, report) + folder = Path(server.get_data_folder()) + json_path = folder / plugin.REPORT_FILE_NAME + text_path = folder / plugin.REPORT_TEXT_FILE_NAME + json_before = json_path.read_text(encoding="utf-8") + text_before = text_path.read_text(encoding="utf-8") + + def explode(*_args, **_kwargs): + raise OSError("disk full") + + monkeypatch.setattr(plugin.os, "replace", explode) + plugin._write_report_files(server, _report(updates=3), announce=False) + + assert json_path.read_text(encoding="utf-8") == json_before + assert text_path.read_text(encoding="utf-8") == text_before + # And the failure left no temporary file behind in either case. + assert sorted(path.name for path in folder.iterdir()) == sorted( + [plugin.REPORT_FILE_NAME, plugin.REPORT_TEXT_FILE_NAME] + ), sorted(path.name for path in folder.iterdir()) + + def test_on_load_warns_when_the_mods_folder_is_missing(tmp_path): import mod_update_checker as plugin @@ -567,6 +724,71 @@ def test_an_incomplete_config_file_is_healed_and_the_added_options_are_named(tmp assert str(path) in joined, joined +def test_a_type_mistake_in_the_config_is_reported_instead_of_silently_reset(tmp_path): + """The other half of "unparseable", and the one that cost the admin everything. + + A legal JSON file with one string where a number belongs does not trip the plugin's own + ``json.load``; it trips ``Config.deserialize``, and MCDR's default ``failure_policy='regen'`` + then replaces the whole file with defaults. Measured before this was fixed, on a 36-option + file with only ``network.timeout_seconds`` wrong: ``language`` went back to ``auto``, + ``interval_hours`` to 0, the ignore list emptied, no backup was written and nothing was said. + + Three things are asserted, because all three were missing: the admin's file survives as a + backup, the console says the file was *rejected* (rather than "missing options, filled in"), + and the message names the option. + """ + import mod_update_checker as plugin + + server = _FakeServer(tmp_path) + payload = plugin.Config.get_default().serialize() + payload["check"]["interval_hours"] = 6 + payload["check"]["ignored_mods"] = ["sodium"] + payload["network"]["timeout_seconds"] = "45" + path = _write_config(server, payload) + + config = plugin._load_config(server) + + # The file itself was rebuilt from defaults by MCDR, but not before being preserved. + backups = sorted(p.name for p in path.parent.iterdir() if ".broken." in p.name) + assert len(backups) == 1, backups + kept = json.loads((path.parent / backups[0]).read_text(encoding="utf-8")) + assert kept["check"]["ignored_mods"] == ["sodium"], "the user's values were not preserved" + assert kept["check"]["interval_hours"] == 6 + assert kept["network"]["timeout_seconds"] == "45" + + # And the config object is MCDR's regenerated one, which is the point of the warning. + assert config.network.timeout_seconds == plugin.Config.get_default().network.timeout_seconds + + joined = "\n".join(server.logger.messages) + assert "配置无法解析" in joined, joined + assert "network.timeout_seconds" in joined, joined + assert "已按默认值补上" not in joined, "a rejected file was reported as merely incomplete" + assert "已按默认值创建" not in joined + + +def test_a_rejected_config_does_not_get_the_incomplete_file_report_afterwards(tmp_path): + """Two different events, two different lines — and only one of them per load. + + After regeneration the file has every option, so the leaf-path comparison sees nothing + added and nothing missing; that silence is right, but it must not come *with* a claim that + the file was merely completed. The two reports are asserted against each other here. + """ + import mod_update_checker as plugin + + server = _FakeServer(tmp_path) + payload = plugin.Config.get_default().serialize() + payload["network"]["retries"] = "many" + _write_config(server, payload) + + plugin._load_config(server) + + joined = "\n".join(server.logger.messages) + assert "配置无法解析" in joined, joined + assert "network.retries" in joined, joined + assert "缺少" not in joined, joined + assert "没有被更新" not in joined, joined + + def test_a_config_file_that_was_not_updated_is_warned_about_with_its_path(tmp_path): """The message for the state nobody has reproduced yet: the file just will not change. @@ -1229,6 +1451,12 @@ def test_the_report_records_what_was_downloaded(tmp_path): import mod_update_checker as plugin from mod_update_checker.downloads import DownloadOutcome + # The language is pinned rather than inherited: ``_translator`` is module state that survives + # between tests, and the assertion below quotes Chinese wording — so without this the test + # passes or fails on which test ran first, and this file's own convention is to say so + # explicitly (see ``_manual_env``). + plugin._apply_language(None, _config_with({"language": "zh_cn"})) + server = _FakeServer(tmp_path) report = _report(updates=2) outcomes = [ @@ -2012,6 +2240,44 @@ def _last_reply_with(source, marker): return next(reply for reply in reversed(replies) if marker in str(reply)) +def test_every_line_in_one_screen_names_the_alias_that_was_typed(tmp_path, monkeypatch): + """``!!muc`` must not answer with ``!!modupdate`` in the same screenful as its own buttons. + + Both aliases are registered, so naming the wrong one is not broken — it is a reader who + typed ``!!muc list`` being told, by the buttons and the closing hint at once, that the + command is called something else. The hint lines had ``!!modupdate`` written into their + templates; the ones that are also produced outside a command (the join notice, the report + file) take the alias from the module-level reply context, which the command tree records. + """ + plugin, _server = _one_screen_setup(tmp_path, monkeypatch, _listing_entries(14)) + source = _PlayerSource("Admin") + + plugin._show_list(source, "", "!!muc") + + body = source.body + assert "!!muc" in body, body + assert "!!modupdate" not in body, "the screen answered with the other alias:\n" + body + + +def test_a_command_with_no_report_names_the_alias_that_was_typed(tmp_path, monkeypatch): + """The "no check has run yet" line is a command hint too, and takes the same treatment. + + The alias is recorded by the command tree's gate rather than passed in, because the same + lines are produced from event handlers that have no command context; the recording is + reproduced here with the same call the gate makes. + """ + import mod_update_checker as plugin + + monkeypatch.setattr(plugin, "_last_report", None, raising=False) + source = _PlayerSource("Admin") + plugin._set_reply_command("!!muc") + + plugin._show_list(source, "", "!!muc") + + assert "!!muc check" in source.body, source.body + assert "!!modupdate" not in source.body, source.body + + def test_the_listing_pages_with_clickable_buttons(tmp_path, monkeypatch): """一页装不下的列表翻页看，而不是被截断；翻页条做成标题栏的形状。 @@ -4943,6 +5209,98 @@ def slow_fetch(item): assert fetches == ["sodium.jar"], fetches assert "正在下载中" in source.body, source.body + + +def test_a_second_batch_does_not_start_the_same_downloads_again(tmp_path, monkeypatch): + """``download all`` has to take the same claims the single form does. + + The claim exists because two connections for one build fight over the same temporary file and + the same ledger record — and the batch path took none, so a second ``download all`` (or a + ``download `` alongside the first) ran the same transfers concurrently. Measured on + Windows before this changed: one of the two reported ``could not put the file in place: + [WinError 32]`` although the file was on disk, and the summary line said ``failed=1``. + """ + import threading + + plugin, _server, source, entry, _ledger = _manual_env(tmp_path, monkeypatch) + fetches = [] + started = threading.Event() + release = threading.Event() + + def slow_fetch(entries): + fetches.extend(item.file_name for item in entries) + started.set() + release.wait(5) + return None + + monkeypatch.setattr(plugin, "_perform_manual_downloads", slow_fetch, raising=False) + + plugin._confirmed_download_all(source, [entry], "!!muc") + assert started.wait(5), "the first batch never started" + source.replies.clear() + source.raw.clear() + plugin._confirmed_download_all(source, [entry], "!!muc") + release.set() + for _ in range(200): + if not plugin._inflight_downloads: + break + time.sleep(0.01) + + assert fetches == ["sodium.jar"], fetches + assert "都正在下载" in source.body, source.body + assert not plugin._inflight_downloads, plugin._inflight_downloads + + +def test_a_batch_skips_only_what_is_already_in_flight(tmp_path, monkeypatch): + """A partial overlap reports the overlap and still starts the rest. + + Dropping the whole batch on any overlap would be the easy reading and the wrong one: the + admin asked for everything waiting, and one already-running transfer is not a reason to + refuse the other four. + """ + import threading + + from mod_update_checker.report import UpdateEntry + + plugin, _server, source, entry, _ledger = _manual_env(tmp_path, monkeypatch) + other = UpdateEntry( + mod_id="lithium", + name="Lithium", + file_name="lithium.jar", + local_version="1.0.0", + latest_version="1.1.0", + status="update_available", + download_url="https://cdn.example/lithium-1.1.0.jar", + download_filename="lithium-fabric-1.1.0.jar", + download_sha1="b" * 40, + download_size=1024, + ) + + started = threading.Event() + release = threading.Event() + batches = [] + + def slow_fetch(entries): + batches.append([item.file_name for item in entries]) + started.set() + release.wait(5) + return None + + monkeypatch.setattr(plugin, "_perform_manual_downloads", slow_fetch, raising=False) + + plugin._confirmed_download_all(source, [entry], "!!muc") + assert started.wait(5), "the first batch never started" + source.replies.clear() + source.raw.clear() + plugin._confirmed_download_all(source, [entry, other], "!!muc") + release.set() + for _ in range(200): + if not plugin._inflight_downloads: + break + time.sleep(0.01) + + assert batches == [["sodium.jar"], ["lithium.jar"]], batches + assert "只开始了其余" in source.body, source.body assert not plugin._inflight_downloads, "占用的名字没有被放回去" @@ -5202,6 +5560,15 @@ def test_a_caption_too_wide_for_the_bar_is_cut_not_fitted_around(tmp_path, monke ("check.start_delay_seconds", 0, 3600), ("check.interval_hours", 0, 720), ("download.max_size_mb", 1, 8192), + # The four that were missing from the table (MUC-B-04 / DOC-02). Every one of them is + # read with ``int()``/``max()`` somewhere, so a typo used to be silently truncated: + # ``network.retries: 1000000000`` holds the check lock for as long as the server runs + # (``range(retries + 1)`` with a 20 s backoff cap), and ``cache.ttl_hours: -1`` flips + # "expire immediately" into "never expire" because ``ResolveCache`` reads 0 as the off + # switch. + ("download.retries", 0, 10), + ("network.retries", 0, 10), + ("network.cache.ttl_hours", 0, 8760), ("network.concurrent_requests", 1, 32), ("network.requests_per_minute", 0, 1000), ("network.timeout_seconds", 1, 300), @@ -5209,6 +5576,7 @@ def test_a_caption_too_wide_for_the_bar_is_cut_not_fitted_around(tmp_path, monke ("command_permission_level", 0, 4), ("report.admin_permission", 0, 4), ("report.in_game_permission", 0, 4), + ("report.reuse_report_minutes", 0, 43200), ], ) def test_an_out_of_range_number_is_clamped_and_reported(tmp_path, path, low, high): @@ -5220,13 +5588,63 @@ def test_an_out_of_range_number_is_clamped_and_reported(tmp_path, path, low, hig config = _config_with({path: value}) changed = plugin._clamp_and_report(server, config) - section, name = path.split(".", 1) if "." in path else (None, path) - target = getattr(config, section) if section else config - assert getattr(target, name) == used, (path, value) + # Walk the dotted path, not just the first segment: ``network.cache.ttl_hours`` is two + # sections deep. + target = config + for part in path.split(".")[:-1]: + target = getattr(target, part) + assert getattr(target, path.split(".")[-1]) == used, (path, value) assert path in changed, changed assert any(path in line for line in server.logger.messages), server.logger.messages +def test_every_numeric_option_the_code_reads_is_in_the_clamp_table(): + """The table is hand-maintained, so it drifts — and drift here is invisible. + + README promises "a numeric option outside its range is clamped to the boundary and said + once". That promise is only true for the paths in ``_NUMERIC_RANGES``, and four real ones + were missing for exactly this reason: nothing connected the table to the code that reads the + options. This does, in two directions: + + * every ``int``/``float`` option of the config class must be in the table, unless it is on the + explicit exclusion list below; + * every path in the table must name a real option — a renamed option would otherwise leave + the table clamping nothing at all, silently. + + A new numeric option therefore has to be added to the table *and* to this test in one step, + which is the only kind of guard that works on a list maintained by hand. + """ + import mod_update_checker as plugin + from support import option_paths + + #: Numeric-looking options that are deliberately NOT clamped, with the reason. Empty is the + #: right state for this: every number the plugin reads has a range that means something. + EXCLUDED: dict = {} + + numeric = set() + for path in option_paths(plugin.Config): + target = plugin.Config + for part in path.split(".")[:-1]: + target = getattr(target, part) + annotation = target.get_field_annotations().get(path.split(".")[-1]) + if annotation in (int, float): + numeric.add(path) + + assert numeric, "no numeric options were found, so this test proves nothing" + + declared = {path for path, _low, _high in plugin._NUMERIC_RANGES} + missing = sorted(numeric - declared - set(EXCLUDED)) + assert missing == [], "numeric options the clamp table does not cover: {}".format(missing) + + unknown = sorted(declared - option_paths(plugin.Config)) + assert unknown == [], "the clamp table names options that do not exist: {}".format(unknown) + + # And the table has to stay ordered by range, because a typo like (10, 0) would clamp every + # value up to the minimum and then back down again. + for path, low, high in plugin._NUMERIC_RANGES: + assert low <= high, (path, low, high) + + def test_a_number_inside_its_range_is_left_alone(tmp_path): """钳制不能变成每一屏都在念的噪音——出厂配置必须一句话都不说。""" import mod_update_checker as plugin diff --git a/tests/test_python_floor.py b/tests/test_python_floor.py new file mode 100644 index 0000000..2620aef --- /dev/null +++ b/tests/test_python_floor.py @@ -0,0 +1,207 @@ +"""The Python-floor check, tested against source it is supposed to catch. + +``tools/check_python_floor.py`` is a guard for a promise the repository makes in four places +(``ruff.toml``, this tool's ``FLOOR``, ``README-dev.md`` and the CI ``python-floor`` job), and a +guard that cannot fail is not a guard. It gave a **false pass** on the case it was written for: +``importlib.resources`` exists on 3.8, so an import of it looked fine, while +``importlib.resources.files`` is 3.9+ and raised ``AttributeError`` on the declared floor — which +``i18n.py`` swallowed, leaving a packed install with one language. + +So the tests below do two things: run the real tool over the real repository (it must stay +green), and run it over a temporary tree containing the exact spellings it has to catch. The +temporary tree is what makes them regression tests rather than a restatement of the tables. +""" + +import ast +import shutil +import subprocess +import sys +from pathlib import Path + +TOOL = Path(__file__).resolve().parent.parent / "tools" / "check_python_floor.py" +REPO = TOOL.parent.parent + + +def _run_checker(python, cwd): + return subprocess.run( + [python, str(cwd / "tools" / "check_python_floor.py")], + cwd=str(cwd), + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + universal_newlines=True, + ) + + +def _tree_with(tmp_path, name, source): + """A throwaway tree the checker treats as a repository: a copy of the tool, plus ``source``. + + The tool locates its repository from ``__file__``, so the copy is what makes it scan the + sample rather than the real tree — which also means these tests exercise the real script, + not a re-implementation of its rules. + """ + tree = tmp_path / "tree" + (tree / "tools").mkdir(parents=True, exist_ok=True) + shutil.copy2(str(TOOL), str(tree / "tools" / "check_python_floor.py")) + (tree / name).write_text(source, encoding="utf-8") + return tree + + +def test_the_repository_itself_is_within_the_floor(): + """The check that CI runs, run the same way — and it has to say so, not just exit 0.""" + result = _run_checker(sys.executable, REPO) + + assert result.returncode == 0, result.stdout + assert "within the Python 3.8 floor" in result.stdout, result.stdout + + +def test_a_newer_module_attribute_is_caught(tmp_path): + """The finding this table was extended for, in the spelling that used to slip through. + + ``importlib.resources`` is importable on 3.8; ``files`` is not an attribute of it there. + """ + tree = _tree_with( + tmp_path, + "i18n.py", + "import importlib.resources\n" + "\n" + "\n" + "def lang_files(package):\n" + " return importlib.resources.files(package)\n", + ) + + result = _run_checker(sys.executable, tree) + + assert result.returncode == 1, result.stdout + assert "importlib.resources.files" in result.stdout, result.stdout + assert "3.9" in result.stdout, result.stdout + + +def test_a_bare_name_bound_from_a_newer_attribute_is_caught(tmp_path): + """``from importlib.resources import files`` is the same call spelled differently.""" + tree = _tree_with( + tmp_path, + "module.py", + "from importlib.resources import files\n" + "\n" + "DATA = files(__package__)\n", + ) + + result = _run_checker(sys.executable, tree) + + assert result.returncode == 1, result.stdout + assert "importlib.resources.files" in result.stdout, result.stdout + + +def test_a_guarded_newer_attribute_is_still_reported(tmp_path): + """A guard around an *import* cannot make an absent attribute appear. + + The module-level table treats ``try: import tomllib / except ImportError`` as handled, which + is right — the module is either there or it is not. The attribute is different: the module is + always there, so ``files`` raises inside the same ``except`` and the fallback has to be + something that actually works. + """ + tree = _tree_with( + tmp_path, + "module.py", + "try:\n" + " import importlib.resources as resources\n" + "except ImportError:\n" + " resources = None\n" + "\n" + "\n" + "def where(package):\n" + " return resources.files(package)\n", + ) + + result = _run_checker(sys.executable, tree) + + assert result.returncode == 1, result.stdout + assert "resources.files" in result.stdout, result.stdout + + +def test_an_aliased_attribute_is_caught(tmp_path): + """``import importlib.resources as res`` then ``res.files`` is one rename away from the same bug.""" + tree = _tree_with( + tmp_path, + "module.py", + "import importlib.resources as res\n" + "\n" + "DATA = res.files('mod_update_checker')\n", + ) + + result = _run_checker(sys.executable, tree) + + assert result.returncode == 1, result.stdout + # Reported under the canonical path, whichever alias the file used. + assert "importlib.resources.files" in result.stdout, result.stdout + + +def _load_tool(): + """Import ``check_python_floor`` from ``tools/`` without polluting ``sys.path`` for long.""" + sys.path.insert(0, str(TOOL.parent)) + try: + import check_python_floor as floor # type: ignore[import-not-found] + finally: + sys.path.pop(0) + return floor + + +def _path_of(floor, expression, names): + """The dotted path the checker sees for an expression like ``res.files(x)``. + + Walked the same way ``check`` walks the tree — ``ast.Attribute`` first, then a bare name — so + this observes the real resolution instead of restating it. + """ + tree = ast.parse(expression) + for node in ast.walk(tree): + if isinstance(node, ast.Attribute): + candidate = floor.dotted_name(node, names) + if candidate: + return candidate + for node in ast.walk(tree): + if isinstance(node, ast.Name): + return floor.dotted_name(node, names) + return "" + + +def test_the_import_table_follows_every_spelling(): + """The name resolution behind the check, exercised directly. + + The subprocess tests prove the check fires; this one proves *why* it fires for four different + spellings, which is the part a future edit to the import walker would break silently. + """ + floor = _load_tool() + + source = ( + "import importlib.resources\n" + "import importlib.resources as res\n" + "from importlib import resources as r\n" + "from importlib.resources import files\n" + ) + names = floor.imported_module_names(ast.parse(source)) + + for spelling in ( + "importlib.resources.files(x)", + "res.files(x)", + "r.files(x)", + "files(x)", + ): + path = _path_of(floor, spelling, names) + assert path == "importlib.resources.files", spelling + assert floor.newer_attribute(path) == "3.9", spelling + + +def test_a_plain_module_attribute_is_not_reported(): + """No false positives on the ordinary uses, or the check becomes noise and gets ignored.""" + floor = _load_tool() + + source = ( + "import json\n" + "import os.path\n" + "from pathlib import Path\n" + "from typing import Optional\n" + ) + names = floor.imported_module_names(ast.parse(source)) + for spelling in ("json.loads(x)", "os.path.join(a, b)", "Path(x)", "Optional[int]"): + path = _path_of(floor, spelling, names) + assert floor.newer_attribute(path) is None, (spelling, path) diff --git a/tests/test_scanner.py b/tests/test_scanner.py index 497f5c1..d6f5c22 100644 --- a/tests/test_scanner.py +++ b/tests/test_scanner.py @@ -11,6 +11,8 @@ import zipfile from pathlib import Path +import pytest + from mod_update_checker.scanner import ( METADATA_SIZE_LIMIT, _metadata_from_toml_regex, @@ -19,6 +21,7 @@ iter_mod_jars, missing_dependencies, read_metadata, + read_metadata_error, resolve_mods_directory, scan_jar, scan_mods, @@ -34,6 +37,10 @@ def _set_central_directory_sizes(path: Path, uncompressed: int) -> None: allocate against, so this is how a small jar claims to hold a huge metadata file — the shape of the memory spike the size limit exists for. Walked record by record rather than searched for, because the file *contents* may contain the signature bytes. + + The *compressed* size in the same record is deliberately left alone: it is what a + compression-ratio guard would divide by to decide the declaration is a lie, and this fixture + has to say "declared huge" without saying "declared by a bomb". """ raw = bytearray(path.read_bytes()) start = raw.find(b"PK\x01\x02") @@ -46,6 +53,23 @@ def _set_central_directory_sizes(path: Path, uncompressed: int) -> None: path.write_bytes(bytes(raw)) +def _set_compression_method(path: Path, method: int) -> None: + """Set the compression method on every entry — in both copies of the header. + + ``zipfile`` reads the central-directory record when an entry is opened and the local header + when its bytes are decoded, so a method the interpreter does not implement has to be written + in both places to be seen at all. Method 99 is not in ``zipfile``'s table and is not one + zlib can do: the shape of an archive built by something newer than the reader. + """ + raw = bytearray(path.read_bytes()) + for signature, offset in ((b"PK\x03\x04", 8), (b"PK\x01\x02", 10)): + start = raw.find(signature) + while start != -1: + struct.pack_into(" None: """Set the encryption bit on every entry, which ``zipfile`` refuses to read. @@ -740,3 +764,101 @@ def test_a_jar_with_no_metadata_contributes_nothing(tmp_path): write_plain_file(directory / "library.jar") assert missing_dependencies(scan_mods(directory)) == {"gone": ["needs.jar"]} + + +# -------------------------------------------------------------------------------------- +# Malformed metadata must not cost the whole check +# +# A ``depends.minecraft`` value is third-party data, and the four shapes below are what a typo +# looks like. Each one used to reach ``versioning.parse_spec``, which iterated the value — for a +# number that is a ``TypeError``, for a mapping it is a silent ``"any"`` reported as a game +# version. Either way the damage escaped the scanner and took the report with it, for a value +# that is pure local data and needs no network at all. +# -------------------------------------------------------------------------------------- + + +@pytest.mark.parametrize("declared", [123, {}, True, [123]]) +def test_a_nonsense_minecraft_range_does_not_break_the_scan(tmp_path, declared): + """``123`` / ``{}`` / ``true`` / ``[123]`` all scan, and none of them becomes a version.""" + path = write_jar( + tmp_path / "typo.jar", + fabric=fabric_metadata(id="typo", depends={"minecraft": declared}), + ) + + mod = scan_jar(path) + + assert mod.identified is True + assert mod.mod_id == "typo" + assert mod.metadata.mc_range is None, "a non-string range was kept as if it were one" + assert mod.metadata.mc_range_text == "*" + + +def test_a_list_of_numbers_is_read_as_the_strings_it_holds(tmp_path): + """A list with a usable entry in it still counts; the unusable ones are dropped.""" + path = write_jar( + tmp_path / "mixed.jar", + fabric=fabric_metadata(id="mixed", depends={"minecraft": [">=26.3", 123, ""]}), + ) + + mod = scan_jar(path) + + assert mod.metadata.mc_range == [">=26.3"] + assert mod.metadata.mc_range_text == ">=26.3" + + +def test_a_mapping_range_is_dropped_rather_than_iterated(): + """The MUC-A-08 half: iterating a dict yields its *keys*, and ``"any"`` reached the report.""" + from mod_update_checker.versioning import covers, normalize_spec, parse_spec + + assert normalize_spec({"any": "1.21"}) == "*" + assert parse_spec({"any": "1.21"})[0]("26.3") is True + assert covers({"any": "1.21"}, "26.3") is True + # A string that is a path through the mapping is still read as a version. + assert normalize_spec([">=1.21"]) == ">=1.21" + + +def test_a_compression_method_zipfile_cannot_do_is_a_metadata_failure(tmp_path): + """``NotImplementedError`` is not a ``ValueError``; it used to escape the "never raises" claim. + + Method 99 is what an archive written by a newer tool looks like to an older interpreter. The + contract is the point: :func:`read_metadata` promises not to raise, and the *reason* it + returns ``None`` has to be reportable rather than a difference in attribution between two + entry points. + """ + path = write_jar(tmp_path / "method99.jar", fabric=fabric_metadata(id="odd")) + _set_compression_method(path, 99) + + with zipfile.ZipFile(path) as archive: + assert read_metadata(archive) is None + assert read_metadata_error(archive) != "" + + mod = scan_jar(path) + assert mod.identified is False + assert mod.error is not None + + +def test_a_declared_gigabyte_of_metadata_is_never_allocated(tmp_path, monkeypatch): + """The other half of the size limit: the bytes that arrive are counted, not the declared size. + + ``_set_central_directory_sizes`` leaves the compressed size alone, so the entry is a *lie* + rather than a bomb — declared 8 MiB, storing a few hundred bytes — and the reader must still + not grow that allocation. The declared size is refused before the read, so what this pins is + that the refusal happens without opening the entry at all. + """ + path = write_jar(tmp_path / "liar.jar", fabric=fabric_metadata(id="liar")) + _set_central_directory_sizes(path, 8 * 1024 * 1024) + + opened = [] + original = zipfile.ZipFile.open + + def watching(self, name, *args, **kwargs): + opened.append(name) + return original(self, name, *args, **kwargs) + + monkeypatch.setattr(zipfile.ZipFile, "open", watching) + + mod = scan_jar(path) + + assert [_name for _name in opened if _name == "fabric.mod.json"] == [] + assert mod.error is not None + diff --git a/tests/test_versioning.py b/tests/test_versioning.py index 93d340e..7cd65c3 100644 --- a/tests/test_versioning.py +++ b/tests/test_versioning.py @@ -45,11 +45,25 @@ # Fabric API's real scheme, and the MC-prefixed scheme some authors use. ("0.161.0+26.3", "0.162.0+26.3", -1), ("mc1.21.1-1.2.3", "mc1.21.1-1.2.4", -1), - # The "game version - mod version" scheme: the dash is a token separator, not a - # pre-release marker, so a version with more parts outranks a prefix of itself. + # The "game version - mod version" scheme. The dash is a token separator, and a suffix that + # is *only* numbers reads as a pre-release of the core — the same rule that ranks ``1.0.0-1`` + # below ``1.0.0``. This is a deliberate change from the old behaviour, which had the + # three-component form outranking the core while the one-component form did not; a comparison + # used for sorting cannot depend on how many dots the suffix happens to contain. ("1.19.2-0.5.3", "1.19.2-0.5.4", -1), - ("1.19.2", "1.19.2-0.5.3", -1), + ("1.19.2", "1.19.2-0.5.3", 1), ("1.19.2-0.5.3", "1.19.2-0.5.2", 1), + # A *bare number* after the core is a sequence, so it ranks below the release it leads to — + # SemVer's rule for numeric pre-release identifiers. This is what makes the order transitive: + # before, ``1.0.0-1`` was newer than ``1.0.0`` while ``1.0.0-a`` was older, and no sort of the + # three agreed with every pairwise comparison. + ("1.0.0-1", "1.0.0", -1), + ("1.0.0", "1.0.0-1", 1), + ("1.0.0-a.1", "1.0.0-1", -1), + # ``a``/``b`` are no longer abbreviations of alpha/beta: a single letter is a sequence, so a + # version carrying one is a pre-release of the core and ranks below it. + ("1.0.0-a", "1.0.0-b", -1), + ("1.0.0", "1.0.0-a", 1), # Calendar versioning, now that Minecraft uses it. ("26.3", "26.3.1", -1), ("26.3", "26.2", 1), @@ -105,6 +119,7 @@ def test_compare_is_a_consistent_order(): "1.0.0-beta.1", "1.0.0-beta.2", "1.0.0-rc.1", + "1.0.0-1", "1.0.0", "1.0.1", "1.1.0", @@ -116,6 +131,26 @@ def test_compare_is_a_consistent_order(): assert compare(earlier, later) == -1, (earlier, later) +def test_latest_of_does_not_depend_on_the_order_it_is_given(): + """The property the transitivity bug actually broke, over every permutation. + + ``latest_of`` is a sequential maximum, so it only agrees with "the maximum" when ``compare`` + is transitive. The set below is the one that used to disagree: ``1.0.0-a.1`` was older than + ``1.0.0``, ``1.0.0`` was older than ``1.0.0-1``, and ``1.0.0-a.1`` was older than + ``1.0.0-1`` — three conclusions that cannot all hold in one order. + """ + import itertools + + versions = ["1.0.0-a.1", "1.0.0", "1.0.0-1", "1.0.0-rc.1", "1.0.0-2"] + results = {latest_of(list(order)) for order in itertools.permutations(versions)} + + assert results == {"1.0.0"}, results + + # And the answer every permutation agrees on is the one no other version outranks. + best = results.pop() + assert all(compare(best, other) >= 0 for other in versions) + + @pytest.mark.parametrize( "version,expected", [ @@ -128,6 +163,9 @@ def test_compare_is_a_consistent_order(): # The marker is in the core; the "+26.3" is the Minecraft version, not a channel. ("1.0.0-beta+26.3", True), ("1.19.2-0.5.3", False), + # A bare number is a sequence number, not a channel: calling it one would hide a real + # release behind ``check.include_alpha``. + ("1.0.0-1", False), ], ) def test_is_prerelease(version, expected): diff --git a/tools/check_python_floor.py b/tools/check_python_floor.py index f79fa75..7595877 100644 --- a/tools/check_python_floor.py +++ b/tools/check_python_floor.py @@ -12,10 +12,16 @@ python tools/check_python_floor.py Exits non-zero and prints one line per offending file/line. It is a **static** check: it proves -the syntax parses as 3.8 and that no annotation or standard-library name from a newer version is -used, not that the code was executed on 3.8. Running the suite on a real 3.8 interpreter is what -``mcdr-matrix`` does for MCDR instead of Python; both are needed, and only the cheap half can be -in CI on every push. +the syntax parses as 3.8 and that no annotation, standard-library module or standard-library +*attribute* from a newer version is used, not that the code was executed on 3.8. Running the +suite on a real 3.8 interpreter is what ``mcdr-matrix`` does for MCDR instead of Python; both are +needed, and only the cheap half can be in CI on every push. + +The attribute half (``NEWER_MODULE_ATTRIBUTES``) is what catches the case the module half gave a +false pass on: ``importlib.resources`` exists on 3.8, while ``importlib.resources.files`` — which +this repository used, in a module whose own ``except Exception`` hid the ``AttributeError`` — is +3.9+. ``tests/test_python_floor.py`` proves it fires by pointing the checker at a temporary +tree. """ import ast @@ -40,6 +46,34 @@ "exceptiongroup": "3.11", } +#: ``(module, attribute)`` pairs where the *attribute* is newer than the floor even though the +#: module is not. This table exists because the module-level one above gave a false pass on the +#: exact failure it was written for: ``importlib.resources`` is present on 3.8, so importing it +#: looked fine, while ``importlib.resources.files`` is 3.9+ and raised ``AttributeError`` at +#: run time — which the plugin's own ``except Exception`` swallowed, degrading a packaged +#: install to English-only on the interpreter the repository declares as its floor. +#: +#: `from ... import x` is caught by both the attribute form and the bare-name form, because the +#: second is how a module re-exports one. +NEWER_MODULE_ATTRIBUTES = { + ("importlib.resources", "files"): "3.9", + ("importlib.resources", "as_file"): "3.9", + ("importlib.resources", "read_text"): "3.9", + ("importlib.resources", "read_binary"): "3.9", + ("functools", "cache"): "3.9", + ("itertools", "pairwise"): "3.10", + ("contextlib", "aclosing"): "3.10", + ("dataclasses", "slots"): "3.10", + ("importlib.metadata", "packages_distributions"): "3.10", + ("typing", "Self"): "3.11", + ("typing", "assert_type"): "3.11", + ("typing", "override"): "3.12", + ("asyncio", "timeout"): "3.11", + ("re", "NOFLAG"): "3.11", + ("datetime", "UTC"): "3.11", + ("sys", "exception"): "3.11", +} + #: Builtin generics (PEP 585) and the union operator (PEP 604) are only legal in annotations #: whose evaluation is deferred. Collected at runtime they raise on 3.8, so they are flagged. BUILTIN_GENERICS = {"list", "dict", "set", "tuple", "frozenset", "type"} @@ -53,6 +87,68 @@ } +def imported_module_names(tree): + """``bound name -> the dotted path it stands for`` for every import in the file. + + Both spellings, because both are ways to reach the attribute this check is about:: + + import importlib.resources # ``importlib.resources.files`` + from importlib import resources # ``resources.files`` + from importlib.resources import files # a bare ``files``, which is the same attribute + + Aliases are followed rather than ignored: ``import importlib.resources as res`` is the same + call, and a check that depends on spelling is one that gets avoided by accident. + """ + names = {} + for node in ast.walk(tree): + if isinstance(node, ast.Import): + for alias in node.names: + if not alias.name: + continue + if alias.asname: + names[alias.asname] = alias.name + else: + root = alias.name.split(".")[0] + names[root] = root + names[alias.name] = alias.name + elif isinstance(node, ast.ImportFrom): + module = node.module or "" + for alias in node.names: + if alias.name == "*": + continue + bound = alias.asname or alias.name + # ``from importlib.resources import files`` binds ``files`` to that attribute, + # which is the same dotted path the attribute form produces. + names[bound] = "{}.{}".format(module, alias.name) if module else alias.name + return names + + +def dotted_name(node, names): + """The dotted path ``node`` refers to, or ``""`` when it is not a plain name chain.""" + parts = [] + current = node + while isinstance(current, ast.Attribute): + parts.append(current.attr) + current = current.value + if not isinstance(current, ast.Name): + return "" + parts.append(current.id) + path = ".".join(reversed(parts)) + if "." not in path: + # A bare name: only the import table can say what it stands for. + return names.get(path, path) + head, _, tail = path.partition(".") + return "{}.{}".format(names.get(head, head), tail) + + +def newer_attribute(module_path): + """The version that introduced the attribute ``module_path`` names, or ``None``.""" + if "." not in module_path: + return None + parent, _, attribute = module_path.rpartition(".") + return NEWER_MODULE_ATTRIBUTES.get((parent, attribute)) + + def iter_sources(): for path in sorted(REPO.rglob("*.py")): rel = path.relative_to(REPO) @@ -118,6 +214,7 @@ def check(path, rel, problems): return guarded = guarded_imports(tree) + names = imported_module_names(tree) for node in ast.walk(tree): if isinstance(node, (ast.Import, ast.ImportFrom)): for alias in node.names: @@ -130,6 +227,29 @@ def check(path, rel, problems): if isinstance(node, ast.Attribute) and node.attr in NEWER_ATTRIBUTES: problems.append("{}:{}: .{}() needs Python {}".format( rel, node.lineno, node.attr, NEWER_ATTRIBUTES[node.attr])) + if isinstance(node, ast.Attribute): + # Only attribute chains, not every ``ast.Name``: ``check`` has no name resolution + # beyond imports, so a bare local variable that happens to be called ``files`` is + # not the same finding, and reporting it would make the check cry wolf. + path = dotted_name(node, names) + since = newer_attribute(path) + if since is not None: + problems.append( + "{}:{}: {} needs Python {} (the module exists on {}.{}, the attribute does not)" + .format(rel, node.lineno, path, since, FLOOR[0], FLOOR[1])) + + for node in ast.walk(tree): + # Bare names are only interesting where they are *called*: ``from importlib.resources + # import files`` then ``files(package)`` is the same finding as the attribute form, while + # a local variable that happens to be spelled the same is not. Without the call check + # this reports every ``Optional(...)``-looking name in the tree. + if isinstance(node, ast.Call) and isinstance(node.func, ast.Name): + path = dotted_name(node.func, names) + since = newer_attribute(path) + if since is not None: + problems.append( + "{}:{}: {}() needs Python {} (the module exists on {}.{}, the attribute does not)" + .format(rel, node.lineno, path, since, FLOOR[0], FLOOR[1])) for node in annotation_nodes(tree): for inner in ast.walk(node): diff --git a/tools/mcdr_matrix.py b/tools/mcdr_matrix.py index 0b29863..56f26e1 100644 --- a/tools/mcdr_matrix.py +++ b/tools/mcdr_matrix.py @@ -48,6 +48,26 @@ REPO = Path(__file__).resolve().parent.parent +#: The ASCII marker MCDR prints for every plugin it loads, followed by the prefix this plugin's +#: own messages carry. Deliberately **not** the Chinese "已加载" alone: the check for "did the +#: plugin load" is the one decision this whole tool rests on, and it was being made by matching +#: localized text that a mis-encoded console can turn into replacement characters. MCDR's own +#: marker is ASCII and comes from MCDR, not from this repository's catalogues. +LOADED_MARKER = "Loading plugin" + + +def plugin_loaded(console: str) -> bool: + """Whether the plugin actually loaded, decided by an ASCII line rather than by prose. + + Two markers, either of which is enough: MCDR's own ``Loading plugin ...`` line, and this + plugin's badge followed by its Chinese load message (which additionally proves + ``language: auto`` followed MCDR). The first is the one that survives an encoding accident. + """ + if LOADED_MARKER in console and "mod_update_checker" in console: + return True + return plugin_badge() + " 已加载" in console + + def plugin_badge() -> str: """``[Mod Update Checker]`` — read from the plugin's own metadata. @@ -411,8 +431,16 @@ def _child_env(python: str) -> dict: ``cwd`` is also why the path must be absolute: the children run in temporary instance directories, where a relative ``.testlibs`` does not resolve. + + ``PYTHONIOENCODING`` is forced to UTF-8 for the child. MCDR prints the plugin's own messages, + this plugin's catalogue is Chinese and English, and a child that inherits a GBK console + encodes those bytes into replacement characters — which the parent then reads, because the + "did the plugin load" decision is made by matching text. Measured on a Chinese Windows + console: the failure detail the tool tried to print contained U+FFFD and the print itself + died with ``UnicodeEncodeError``, so a failing run left no usable evidence at all. """ env = dict(os.environ) + env.setdefault("PYTHONIOENCODING", "utf-8") testlibs = REPO / ".testlibs" if not testlibs.is_dir() or _interpreter_has_own_mcdr(python): @@ -1012,8 +1040,9 @@ def _find_published(name): "mcdr": version, "python": python, "version_tuple": version_tuple(version), - # "已加载" proves both that the plugin loaded and that `language: auto` followed MCDR. - "loaded": "Mod Update Checker] 已加载" in console, + # ``plugin_loaded`` also accepts MCDR's own ASCII marker, so the decision does not rest + # on a localized string surviving a console that is not UTF-8. + "loaded": plugin_loaded(console), "refused_cleanly": "不满足版本约束" in console, "spoke_chinese": plugin_badge() + " 已加载" in console and "[Mod Update Checker] loaded" not in console, @@ -1259,7 +1288,27 @@ def verdict(result: dict) -> str: return "FAIL (did not load)" +def _make_output_utf8() -> None: + """Make this process able to print its own findings, whatever console it was started from. + + The failure detail quoted from a child contains Chinese — the plugin's own messages — and a + GBK console cannot encode it. The result was not merely a mangled line: ``print`` raised + ``UnicodeEncodeError`` and the tool stopped *while reporting a failure*, so the run that most + needed evidence left none. ``errors="replace"`` is the second half: an unprintable character + costs one ``?`` instead of the whole report. + """ + for stream in (sys.stdout, sys.stderr): + reconfigure = getattr(stream, "reconfigure", None) + if reconfigure is None: # pragma: no cover - only a stream replaced by a wrapper + continue + try: + reconfigure(encoding="utf-8", errors="replace") + except (OSError, ValueError): # pragma: no cover - a stream that refuses to change + pass + + def main() -> int: + _make_output_utf8() parser = argparse.ArgumentParser( description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter ) diff --git a/tools/mutation_check.py b/tools/mutation_check.py index f07b3d2..32144af 100644 --- a/tools/mutation_check.py +++ b/tools/mutation_check.py @@ -13,8 +13,14 @@ python tools/mutation_check.py -It refuses to run when ``MCDR_SKIP_E2E`` is set: at least one mutation is only caught by the -end-to-end suite, so a skipped ``e2e`` would turn a caught mutation into a reported survivor. +It refuses to run with e2e skipped, and the honest reason is narrower than the old text claimed. +That text said "at least one mutation is only caught by the end-to-end suite"; measured, **every +one of the twelve selectors is satisfied by a unit test**, and none of them references +``tests/test_e2e.py``. So the guard is not protecting a particular mutation. It is protecting the +*baseline*: the run reports one summary over the suite, and a suite that quietly lost its +end-to-end half would make that summary a weaker claim than it reads as. For a tool whose whole +argument is "this suite has teeth", that is worth refusing over — but it is a statement about the +baseline, not about any single mutation, and writing it as the latter is what made it unverifiable. A mutation that changes nothing is reported as a **stale anchor** rather than a pass: the code moved and the mutation has to be rewritten, which is a different problem from a test without teeth. @@ -90,10 +96,11 @@ def mutate(source, anchor=anchor, replacement=replacement): " return True # mutation", ["tests/test_cleanup.py"]) - add("JSON is written through a shared temporary name", + add("JSON writes to one target are no longer serialised", JSONFILE, - ' "{}.{}.{}.tmp".format(target.suffix, os.getpid(), threading.get_ident())', - ' "{}.tmp".format(target.suffix) # mutation: shared by every writer', + " payload_text = json_text(payload, sort_keys)\n with _write_lock_for(target):", + " payload_text = json_text(payload, sort_keys)\n" + " if True: # mutation: no lock, so two renames race for one destination", ["tests/test_jsonfile.py"]) add("the ledger writes only what one instance remembers", @@ -186,8 +193,9 @@ def run_pytest(workdir, args): def main() -> int: if os.environ.get("MCDR_SKIP_E2E"): - print("MCDR_SKIP_E2E is set: the end-to-end suite would be skipped, and at least one") - print("mutation is only caught there. Unset it and run again.") + print("MCDR_SKIP_E2E is set: the end-to-end suite would be skipped, so the baseline") + print("summary below would cover less than the suite this repository claims to run.") + print("Unset it and run again.") return 2 if not (REPO / ".testlibs").is_dir(): print("run the tests/README.md setup first (.testlibs is missing)") diff --git a/tools/probe_upstream.py b/tools/probe_upstream.py index 4071cfd..1f99bf5 100644 --- a/tools/probe_upstream.py +++ b/tools/probe_upstream.py @@ -17,27 +17,74 @@ Usage:: - python tools/probe_upstream.py - -Needs an interpreter with ``requests`` (any MCDR environment has it). Read-only apart from one -mod download; no credentials are needed anywhere in it -precisely to document what a key is needed *for*. + python tools/probe_upstream.py [--download-jar] [--timeout SECONDS] + +Needs an interpreter with ``requests`` (any MCDR environment has it). Read-only unless +``--download-jar`` is given — the one download is off by default, so ``--help`` and a first look +cost no traffic. No credentials are needed anywhere in it, precisely to document what a key is +needed *for*. + +Exit codes are three, because "the probe ran and the upstream is unreachable" and "the probe +itself is broken" call for different next steps: ``0`` all good, ``1`` a network failure that was +reported readably, ``2`` bad arguments. It used to accept no arguments at all — ``--help`` ran the +whole probe against the live API — and to die with a bare traceback when the network was down, +which is the least useful thing a diagnostic tool can do on a machine that has no route to +Modrinth. """ +import argparse import hashlib import json import sys -import requests - USER_AGENT = "LifeSci-Craft/MCDR-ModUpdateChecker-probe (server admin tool)" MODRINTH = "https://api.modrinth.com/v2" #: Big enough to be a plausible jar, small enough that the probe stays quick. MAX_DOWNLOAD_BYTES = 3_000_000 -SESSION = requests.Session() -SESSION.headers.update({"User-Agent": USER_AGENT}) +#: ``requests`` is imported inside :func:`_session` rather than at the top so that ``--help`` +#: works on an interpreter that does not have it — which is the interpreter a maintainer is most +#: likely to be holding when they run ``--help`` to find out what this tool needs. +SESSION = None + + +def _session(): + """The shared session, built on first use, or a readable refusal to go on.""" + global SESSION + if SESSION is not None: + return SESSION + try: + import requests + except ImportError as error: + print("this tool needs 'requests': python -m pip install requests") + print("(any MCDR environment already has it — see tests/README.md for the local copy)") + raise SystemExit(1) from error + SESSION = requests.Session() + SESSION.headers.update({"User-Agent": USER_AGENT}) + return SESSION + + +def _requests(): + """The ``requests`` module, for the exception type the failure path catches.""" + return sys.modules["requests"] + +#: Set by the entry point; used for every request so ``--timeout`` reaches all of them. +TIMEOUT = 30.0 + +#: Whether to fetch a jar and check its digest. Off unless asked: it is the only request here +#: that moves real bytes, and someone reading ``--help`` should not be sending it. +DOWNLOAD_JAR = False + + +def _get(url, **kwargs): + kwargs.setdefault("timeout", TIMEOUT) + return _session().get(url, **kwargs) + + +def _post(url, **kwargs): + kwargs.setdefault("timeout", TIMEOUT) + return _session().post(url, **kwargs) def heading(title: str) -> None: @@ -53,28 +100,27 @@ def show(payload, limit: int = 900) -> None: def demo_headers() -> None: print("request headers (Modrinth requires a descriptive User-Agent):") - print(" " + json.dumps(dict(SESSION.headers), ensure_ascii=False)) + session = _session() + print(" " + json.dumps(dict(session.headers), ensure_ascii=False)) def probe_modrinth() -> None: """Everything the Modrinth client depends on, verified against the live service.""" heading("Modrinth — find a small Fabric mod to use as a specimen") - response = SESSION.get( + response = _get( MODRINTH + "/search", params={ "facets": json.dumps([["project_type:mod"], ["categories:fabric"]]), "limit": 12, }, - timeout=30, ) print("GET /search ->", response.status_code) specimen = None for hit in response.json().get("hits", []): - versions = SESSION.get( + versions = _get( MODRINTH + "/project/{}/version".format(hit["project_id"]), params={"loaders": json.dumps(["fabric"])}, - timeout=30, ) if versions.status_code != 200: continue @@ -108,58 +154,62 @@ def probe_modrinth() -> None: "size": item["size"], }) - heading("download it and confirm the digest matches what the API advertises") - blob = SESSION.get(item["url"], timeout=120).content - sha1 = hashlib.sha1(blob).hexdigest() - sha512 = hashlib.sha512(blob).hexdigest() - print("computed sha1 :", sha1) - print("advertised :", item["hashes"]["sha1"]) - print("sha1 matches :", sha1 == item["hashes"]["sha1"]) - print("sha512 matches:", sha512 == item["hashes"]["sha512"]) - - heading("POST /version_files — how does it answer for a hash it does not know?") - response = SESSION.post( - MODRINTH + "/version_files", - json={"hashes": [sha1, "0" * 40, "f" * 40], "algorithm": "sha1"}, - timeout=30, - ) - print("status:", response.status_code) - body = response.json() - print("keys returned:", list(body.keys())) - print("-> only the hashes it recognises appear; unknown ones are simply absent") - - heading("POST /version_files/update — the batched 'newest compatible' lookup") - for label, game_versions in (("current release", ["26.3"]), ("impossible version", ["1.7.10"])): - response = SESSION.post( + if DOWNLOAD_JAR: + heading("download it and confirm the digest matches what the API advertises") + blob = _get(item["url"], timeout=max(TIMEOUT, 120.0)).content + sha1 = hashlib.sha1(blob).hexdigest() + sha512 = hashlib.sha512(blob).hexdigest() + print("computed sha1 :", sha1) + print("advertised :", item["hashes"]["sha1"]) + print("sha1 matches :", sha1 == item["hashes"]["sha1"]) + print("sha512 matches:", sha512 == item["hashes"]["sha512"]) + + heading("POST /version_files — how does it answer for a hash it does not know?") + response = _post( + MODRINTH + "/version_files", + json={"hashes": [sha1, "0" * 40, "f" * 40], "algorithm": "sha1"}, + ) + print("status:", response.status_code) + body = response.json() + print("keys returned:", list(body.keys())) + print("-> only the hashes it recognises appear; unknown ones are simply absent") + + heading("POST /version_files/update — the batched 'newest compatible' lookup") + for label, game_versions in ( + ("current release", ["26.3"]), ("impossible version", ["1.7.10"]) + ): + response = _post( + MODRINTH + "/version_files/update", + json={ + "hashes": [sha1], + "algorithm": "sha1", + "loaders": ["fabric"], + "game_versions": game_versions, + }, + ) + payload = response.json() + print(" {:20} status {} -> {!r}".format(label, response.status_code, payload)[:160]) + print(" -> no match gives an empty object, which is what the client keys off") + + heading("empty filter arrays — this is the one that silently breaks everything") + response = _post( MODRINTH + "/version_files/update", - json={ - "hashes": [sha1], - "algorithm": "sha1", - "loaders": ["fabric"], - "game_versions": game_versions, - }, - timeout=30, + json={"hashes": [sha1], "algorithm": "sha1", "loaders": ["fabric"], + "game_versions": []}, ) - payload = response.json() - print(" {:20} status {} -> {!r}".format(label, response.status_code, payload)[:160]) - print(" -> no match gives an empty object, which is what the client keys off") - - heading("empty filter arrays — this is the one that silently breaks everything") - response = SESSION.post( - MODRINTH + "/version_files/update", - json={"hashes": [sha1], "algorithm": "sha1", "loaders": ["fabric"], "game_versions": []}, - timeout=30, - ) - print(" game_versions=[] -> status {} -> {} key(s) returned".format( - response.status_code, len(response.json()))) - print( - " -> whether this means 'no filter' or 'match nothing' is exactly why the client\n" - " omits empty arrays entirely instead of sending them" - ) + print(" game_versions=[] -> status {} -> {} key(s) returned".format( + response.status_code, len(response.json()))) + print( + " -> whether this means 'no filter' or 'match nothing' is exactly why the client\n" + " omits empty arrays entirely instead of sending them" + ) + else: + heading("skipped: the jar download and the hash round-trip") + print(" pass --download-jar to fetch one and check /version_files against its digest") heading("GET /projects?ids= — batch metadata") - response = SESSION.get( - MODRINTH + "/projects", params={"ids": json.dumps([version["project_id"]])}, timeout=30 + response = _get( + MODRINTH + "/projects", params={"ids": json.dumps([version["project_id"]])} ) print("status:", response.status_code) project = response.json()[0] @@ -167,19 +217,68 @@ def probe_modrinth() -> None: "id", "slug", "title", "project_type", "source_url", "client_side", "server_side")}) heading("GET /project/{unknown} and /project/{id}/version with an impossible filter") - print("unknown project ->", SESSION.get(MODRINTH + "/project/nope-xyz", timeout=30).status_code) - response = SESSION.get( + print("unknown project ->", _get(MODRINTH + "/project/nope-xyz").status_code) + response = _get( MODRINTH + "/project/{}/version".format(version["project_id"]), params={"loaders": json.dumps(["fabric"]), "game_versions": json.dumps(["1.7.10"])}, - timeout=30, ) print("impossible filter -> {} {} (an empty list, not a 404)".format( response.status_code, response.json())) -def main() -> int: - demo_headers() - probe_modrinth() +def parse_args(argv=None): + parser = argparse.ArgumentParser( + prog="probe_upstream.py", + description=( + "Probe the live Modrinth API and print what it answers. Read-only unless " + "--download-jar is given." + ), + epilog=( + "exit codes: 0 the probe ran; 1 the upstream could not be reached (the reason is " + "printed); 2 bad arguments" + ), + ) + parser.add_argument( + "--download-jar", + action="store_true", + help=( + "also fetch one small jar and check its digest against the API's — the only " + "request here that transfers real bytes, so it is off by default" + ), + ) + parser.add_argument( + "--timeout", + type=float, + default=30.0, + metavar="SECONDS", + help="per-request timeout (default: %(default)s); the jar download uses at least 120", + ) + return parser.parse_args(argv) + + +def main(argv=None) -> int: + global TIMEOUT, DOWNLOAD_JAR + args = parse_args(argv) + TIMEOUT = args.timeout + DOWNLOAD_JAR = args.download_jar + + print("Modrinth base: {} (timeout {:.0f}s, download-jar: {})".format( + MODRINTH, TIMEOUT, "yes" if DOWNLOAD_JAR else "no")) + try: + demo_headers() + probe_modrinth() + except _requests().RequestException as error: + # A readable conclusion rather than a traceback. This plugin is built for networks where + # reaching Modrinth is the exception, so "the probe exploded" is the wrong report: what + # the maintainer needs is which URL failed, how, and the two things worth trying. + print() + print("!" * 78) + print("the upstream could not be reached: {}".format(type(error).__name__)) + print(" url : {}".format(getattr(error.request, "url", "?"))) + print(" detail : {}".format(error)) + print(" this is a network problem, not a probe bug. Check the proxy/VPN settings for") + print(" this shell, or raise --timeout, and run it again.") + return 1 print() return 0 From 3ab1a383ca6d809d01826291ee32999a41b574f9 Mon Sep 17 00:00:00 2001 From: Pau1am <41251296+Pau1am@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:30:24 +0800 Subject: [PATCH 2/4] chore(release): v1.6.2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 版本号 1.6.1 → 1.6.2（`mcdreforged.plugin.json`）。 - `CHANGELOG.md` 换成 1.6.2 一节，只保留最新版。 - README 与 README_en 的界面示例标题栏跟着升到 v1.6.2。 - 验证：884 项测试、变异 12/12、MCDR 2.13.0–2.16.0 五版本矩阵通过、产物逐字节可重建。 --- CHANGELOG.md | 59 +++++++++++++++++------------------------ README.md | 2 +- README_en.md | 2 +- mcdreforged.plugin.json | 2 +- 4 files changed, 28 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b75d0e7..5771429 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,44 +9,35 @@ --- -## v1.6.1 - -这一版是一轮全代码审查的修复轮：下载台账的并发写入、频道开关失效、mods 目录读不到时的误报， -以及十几处「坏了但不出声」的路径。 +## v1.6.2 ### 修复 -- **并发下载不再丢台账。** 下载台账以前由多个实例各自把内存快照整份写回，`!!muc download` 的两个后台线程、命令线程上的 `install` 与检查线程会互相覆盖（实测 9 条记录被清成 0 条），后果是「关服自动安装已批准的版本」静默失效。现在写入是进程级加锁的重读-合并-写回，JSON 的临时文件名也带上进程与线程号。 -- **`check.include_beta` / `check.include_alpha` 之前完全不起作用**（频道过滤函数没有任何调用者），默认配置下 beta 会被当成可用更新推荐。现在两个开关生效；某个项目在所选频道里没有版本时会回落到更旧的频道，并在报告里说明这次回落。 -- **`project-map.json` 存成 GBK / ANSI 不再让整次检查失败。** 以前 `UnicodeDecodeError` 会穿过映射表的错误处理：`!!muc status` 整页报错，一次检查以「check failed」收场、一条结论都不产出。 -- **关掉 `sources.modrinth` 不再污染解析缓存。** 以前「没问过」会被写成「确认不存在」，打开 Modrinth 之后这些 jar 在缓存有效期内（默认 24 小时）一次名字搜索都不会发，永久显示为「认不出」。 -- **下载失败不再先删掉旧版本。** 旧文件要等新文件就位之后才删，否则新版本 404 时新旧两份都没有，回滚点消失。 -- **游戏版本投票只取下界。** 以前区间上界与下界等权计票、平局按字典序取最大者，猜错版本会让每个 Mod 都显示「无适配构建」。 -- **`mods/` 目录读不到时不再报成「0 个 jar」。** 状态页写明目录与原因，检查直接跳过并说明原因——读不到的目录与空目录是两件事。 -- **安装失败不再在 `mods/` 留下半截新 jar。** 改成同目录临时文件 + 原子替换；万一仍然留下东西，安装结果会点名那个文件。 -- **上游整体不可用时不再对每个 Mod 逐个搜索**（以前 12 个 Mod 会发 48 次搜索请求）。 -- 其余收紧：`resolve-cache.json` 里的坏记录不再让每次检查失败；`scan_mods` 在目录可 stat 不可列举时返回可解释的结果而不是抛异常；备份名判定改用 `fullmatch`；台账里的文件名做安全校验（不再接受 `..`）；加密或损坏的 jar 不再打破 `scan_jar` 的「不抛异常」承诺；元数据解压加 4 MiB 上限；下载失败文案不再把不重试的失败说成「重试 N 次后失败」。 -- 停服后在途的检查不再覆盖报告；`checker` 不再在关闭客户端之后继续发请求。 - -### 变更 - -- **`!!muc status` 改成两段式**：命令线程先回一句「正在读取」，扫描与渲染在后台线程完成后整屏给出——页面不再占用 MCDR 唯一的同步执行器线程；同一时间只允许一份状态页在生成。 -- **数值选项超出范围会被钳制**并在控制台提示一次（范围写进 README 的配置表；只改内存，不回写管理员写的文件）。 -- `!!muc` 别名下不再有写死 `!!modupdate` 的提示行；`cleanup.notice_confirm` / `command.delete.usage` / `command.delete.disabled_hint` 的占位符随之调整（翻译者注意）。 -- 版本识别是从日志或 Mod 元数据推断出来的时候，状态页会多一行说明来源。 -- 版本号 1.6.0 → 1.6.1。 - -### 内部整理 - -- 新增 `tools/mutation_check.py`：把每条守卫逐个改坏（在仓库的临时副本里改，工作树不受影响），对应的用例必须失败；没被抓住的守卫与「锚点挪走」分开报告。 -- 新增 `tools/check_python_floor.py` 与 CI 的 `python-floor` 作业：每个文件都必须能以 3.8 解析、且不用更新的注解写法或标准库名字。 -- 新增 CI 的 `lint` 与 `mutation` 作业；两个 workflow 都补上 `permissions`、`timeout-minutes` 与 `concurrency`。 -- `README-dev.md` 引用的测量脚本（7 个）连同 `bench/README.md` 一起进仓库，并新增 `tests/test_documented_paths.py` 守卫文档里写出的仓库路径确实存在。 -- 入口模块顶部按主题列出段落顺序与拆分计划（未搬迁）。 +- **一个畸形 mod jar 不再让整次检查没有报告**：`depends.minecraft` 写成数字或对象（例如 `1.21` 忘了引号）时，以前会让检查整体失败、只剩一句 check failed。 +- **下载请求也计入 `network.requests_per_minute`**：以前只有 JSON 请求限速，批量下载不受限。 +- 响应体读到一半断连会重试；深嵌套响应抛出的 `RecursionError` 归类为上游错误，不再穿透检查流程。 +- 安装成功后会删除已消费的下载文件并清扫下载目录，磁盘占用不再随更新次数单调增长。 +- **并发写同一 JSON 文件不再失败**：按目标目录串行化写入并重试改名，缓存与批准状态不再静默丢失。 +- 配置里写错类型会被点名并备份为 `config.json.broken.<时间戳>`，不再被 MCDR 静默重置为默认值。 +- `ResolveCache` 只回写自己新增的记录，多个实例不再互相抹掉对方的解析结果。 +- `!!muc list ²` 这类输入不再让命令静默失败；数值选项范围表补全（`network.retries`、`download.retries`、`network.cache.ttl_hours`、`report.reuse_report_minutes`）。 +- 版本比较满足传递性，`is_prerelease` 与之一致：单字母后缀不再被当成预发布标记。 +- `download all` 也遵守「同一文件不并发下载」；备份名改为原子抢占，不会再覆盖已有备份；`.part` 临时名唯一。 +- 元数据解压改为分块读并设上限。 +- i18n 不再依赖 Python 3.9+ 的 API，`language: auto` 在声明的 3.8 下限上不会退化成只有 `en_us`。 +- 语言文件里让用户设置的键此前写的是缺少 `server.` 前缀的旧路径，照做的键会被 MCDR 丢掉；现已更正为 `server.mc_version`。 +- `!!muc` 与 `!!modupdate` 两种别名下，屏内提示都指向用户实际输入的那个别名。 +- `last_report.json` 改为原子写入；reload 后 `!!help` 的权限等级文案与实际一致。 + +### 工具与工程 + +- `tools/mcdr_matrix.py` 固定子进程编码并在中文终端下保住失败证据；`tools/probe_upstream.py` 接受参数、离线时给出结论而不是回溯。 +- `tools/check_python_floor.py` 改为检查「模块 + 属性」对，`importlib.resources.files` 这类问题不会再漏过。 +- CI 增加端到端作业、pip 缓存与固定 action 版本；补英文 README 与徽章。 ### 验证 -- 测试 **827 项**全部通过（其中 2 项是真实 MCDR 端到端）。 +- 测试 **884 项**全部通过（含 2 项真实 MCDR 端到端）。 - 变异检查 **12 条全部被抓**。 -- 真实 MCDR **2.13.0 / 2.14.1 / 2.15.0 / 2.15.7 / 2.16.0** 五版本各跑一遍完整流程，逐项一致。 -- 产物可复现、换行归一、包内无注释与 docstring、包内 changelog 只留本节且与版本号一致。 +- 真实 MCDR **2.13.0 / 2.14.1 / 2.15.0 / 2.15.7 / 2.16.0** 五版本矩阵通过，54 项跨版本判据一致。 +- 产物重建与提交件逐字节一致；包内 changelog 只留本节且与版本号一致。 diff --git a/README.md b/README.md index 359fd3d..00227dc 100644 --- a/README.md +++ b/README.md @@ -48,7 +48,7 @@ 游戏内列表效果： ``` -============ Mod Update Checker v1.6.1 ============ +============ Mod Update Checker v1.6.2 ============ 服务端: 26.3 / Fabric(版本来源：server_info) [1] Lithium [版本] [状态: ↑] [详细信息] [2] Sodium [版本] [状态: ↑] [详细信息] diff --git a/README_en.md b/README_en.md index 86cce87..e8b6d7c 100644 --- a/README_en.md +++ b/README_en.md @@ -57,7 +57,7 @@ kept as a backup. No API key, no extra dependencies. A listing in game looks like this (the Chinese screenshot form is in [`README.md`](README.md)): ``` -============ Mod Update Checker v1.6.1 ============ +============ Mod Update Checker v1.6.2 ============ server: 26.3 / Fabric (version source: server_info) [1] Lithium [version] [status: up] [details] [2] Sodium [version] [status: up] [details] diff --git a/mcdreforged.plugin.json b/mcdreforged.plugin.json index f6b7946..aff9480 100644 --- a/mcdreforged.plugin.json +++ b/mcdreforged.plugin.json @@ -1,6 +1,6 @@ { "id": "mod_update_checker", - "version": "1.6.1", + "version": "1.6.2", "name": "Mod Update Checker", "description": { "en_us": "Check and update server-side mods from Modrinth.", From aea2bb6605d5f0f0a25faf3bc6726e7760abbc3d Mon Sep 17 00:00:00 2001 From: Pau1am <41251296+Pau1am@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:35:05 +0800 Subject: [PATCH 3/4] fix(ci): give the lint job a cache dependency path `cache: pip` without `cache-dependency-path` makes `actions/setup-python` look for `**/requirements.txt` or `**/pyproject.toml`; this repository has neither, so the step failed before ruff ever ran. Point it at `tests/requirements-test.txt`, the same file the other jobs use. --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c604614..ada2caf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,6 +28,7 @@ jobs: with: python-version: "3.13" cache: pip + cache-dependency-path: tests/requirements-test.txt - name: Install ruff run: python -m pip install "ruff==0.17.0" - name: Lint From 4d336c86111abe9e35c4812b5a9cfba1931d7ff1 Mon Sep 17 00:00:00 2001 From: Pau1am <41251296+Pau1am@users.noreply.github.com> Date: Thu, 15 Oct 2026 15:56:48 +0800 Subject: [PATCH 4/4] test: make the serialisation guard platform independent `test_eight_threads_writing_one_file_always_succeed` decides whether the per-directory write lock is present by looking for a failed rename or a retry. Both only happen on Windows: on Linux two concurrent `os.replace` calls at one destination do not fail, so with the lock removed nothing in that test changes and the mutation check reports the mutation as survived (CI runs on Linux, and it did). `test_writes_to_one_target_never_overlap` measures the overlap directly: a counting wrapper around the rename records how many writers are inside the write path at once, and asserts the peak is one. That holds on any platform, so the mutation is caught everywhere. --- CHANGELOG.md | 2 +- tests/test_jsonfile.py | 60 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5771429..a713ed9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,7 +37,7 @@ ### 验证 -- 测试 **884 项**全部通过（含 2 项真实 MCDR 端到端）。 +- 测试 **885 项**全部通过（含 2 项真实 MCDR 端到端）。 - 变异检查 **12 条全部被抓**。 - 真实 MCDR **2.13.0 / 2.14.1 / 2.15.0 / 2.15.7 / 2.16.0** 五版本矩阵通过，54 项跨版本判据一致。 - 产物重建与提交件逐字节一致；包内 changelog 只留本节且与版本号一致。 diff --git a/tests/test_jsonfile.py b/tests/test_jsonfile.py index 60eba87..901a6ae 100644 --- a/tests/test_jsonfile.py +++ b/tests/test_jsonfile.py @@ -195,6 +195,66 @@ def writer(index): ) +def test_writes_to_one_target_never_overlap(tmp_path, monkeypatch): + """同一目标路径的写入必须真的串行——而且这一点要在任何操作系统上都测得出来。 + + 上面那条测试是**反推**的：靠「并发的改名会不会失败」与「有没有走重试」判断锁在不在。 + 那是 Windows 的替换语义——Linux 上两个并发的 ``os.replace`` 指向同一目标并不会失败， + 于是把锁删掉以后那里既不会有异常、也不会有重试，变异检查会如实报「存活」（CI 就跑在 + Linux 上）。这条测试改成正着量：在写路径里套一层计数包装，只要锁不在，两个写者就会同时 + 处在临界区内。 + """ + import mod_update_checker.jsonfile as jsonfile_module + + target = tmp_path / "download-manifest.json" + write_json(target, {"version": 1, "mods": {}}) + + real_replace = jsonfile_module._replace_with_retry + counter_lock = threading.Lock() + state = {"inside": 0, "peak": 0} + + def counting_replace(temporary, destination): + with counter_lock: + state["inside"] += 1 + state["peak"] = max(state["peak"], state["inside"]) + try: + # 把重叠窗口放大到调度器一定会撞上的量级；串行化之后这段时间在锁内， + # 所以整条测试的耗时仍然是可接受的（8 × 20 × 5 ms）。 + time.sleep(0.005) + return real_replace(temporary, destination) + finally: + with counter_lock: + state["inside"] -= 1 + + monkeypatch.setattr(jsonfile_module, "_replace_with_retry", counting_replace) + + threads = 8 + rounds = 20 + ready = threading.Barrier(threads) + errors = [] + + def writer(index): + ready.wait() + for round_index in range(rounds): + try: + write_json(target, {"writer": index, "round": round_index}) + except OSError as error: # noqa: PERF203 - the failure is the observation + errors.append("{}: {}".format(type(error).__name__, error)) + return + + workers = [threading.Thread(target=writer, args=(index,)) for index in range(threads)] + for worker in workers: + worker.start() + for worker in workers: + worker.join() + + assert errors == [], errors + assert state["peak"] == 1, ( + "{} writers were inside the write path of one target at the same time, so writes to " + "one file were not serialised".format(state["peak"]) + ) + + def test_a_rename_that_fails_once_is_retried(tmp_path, monkeypatch): """被别的写者挡了一下的改名是可重试错误，重试一次就该过去。""" import mod_update_checker.jsonfile as jsonfile_module