Files
ElyOffice/tasks/reviews/C00.4-r2.md
T

6.5 KiB

FAIL

Round-1 defects:

  • R-01 upstream claim: fixed. Line 22 cites bd2d0bec67. At the pin, vcl/quartz/salvd.cxx has no SetSizeUsingBuffer override; only the base in vcl/inc/salvd.hxx:46 exists.
  • R-03 libetpan blocker: fixed. libetpan is now pinned at 4d47862. I built it from scratch in a scratch directory and it builds and installs both headers. All 76 mailgmail_*/mailactivesync_* functions that MailCore 2 calls are defined in libetpan.a.
  • Spikes not in the repo: fixed. tests/engines/lok_probe.c and mupdf_probe.c are in the repo, the sample documents are generated, and the tile is pre-filled with 0xAB.

What I ran:

  • tests/engines/lok.sh exits 1 twice in a row. See the first entry.
  • tests/engines/mupdf.sh passes. The library sizes match, and the pkcs7 archive imports 17 EVP/PKCS7 symbols.
  • tests/engines/libetpan.sh passes.
  • tests/scripts/*.sh and agents_md.py exit 0.
  • The patch is byte-identical to the engines/lo-src diff. It applies with git apply --check to the pin.
  • The init.cxx:4277/4348 and configure.ac:5950-5952 references match.

tests/engines/lok.sh:12 — The cleanup runs hdiutil detach "$mount" -quiet || true. The detach fails with "Resource busy", and || true swallows the error. rm -rf "$work" then walks into the still-mounted read-only dmg. Scenario: on this machine, two runs in a row printed "lok: load, PDF export, thread rule pass; tile untouched". Then came about 3 MB of rm: ... Read-only file system lines, and the script exited 1. The dmg stayed mounted under /var/folders/.../tmp.*/mnt until I detached it by hand a few seconds later. The evidence script for R-01 never exits 0, and every run leaks a mount. Fix: make cleanup wait until the image is released, for example by retrying the detach until it succeeds or a deadline passes. A detach failure must fail the script. Never run rm -rf over a path that is still mounted.

docs/engineering-risks.md:25 (status row :7) — The doc says "构建关闭 Skia" and "构建已在 engines/lo-src 进行". Neither is true. No build process is running, and engines/lo-build.log (17:34) ends in make: *** [Makefile:302: build] Error 2. The cause is six errors in vcl/osx/salframeview.mm:802-847 (SkiaHelper and CAMetalLayer undeclared). That code is not guarded by HAVE_FEATURE_SKIA and asserts "macos requires skia". config_host.mk shows ENABLE_SKIA= because --disable-skia is in autogen.lastrun. Scenario: C07.1 follows the documented plan of a self-build with Skia off, and it cannot compile LibreOffice 26.2.6.3 on macOS. With Skia on (which is required), the patch's reasoning about the Quartz AquaSalGraphics drawing into a foreign CGContext has not been examined. C00.4 requires blockers to be written down; this one is missing. The agents.md decision depends on it too. Fix: record the failed Skia-off build as a blocker, citing the log and salframeview.mm:802. Correct the build status. Either show that a Skia-on build still sends the patched paintTile path through Quartz into pBuffer, or record that as unverified and change the R-01 row.

docs/engineering-risks.md:42; scripts/build-libetpan.sh:21 — The doc says md5.c fails to compile under the default C standard because of K&R definitions. At the pinned 4d47862, md5.c uses ANSI prototypes (MD5Init(MD5_CTX * context) at :92). A clean ./autogen.sh with no CFLAGS, run with Apple clang 21, builds and installs with no errors. Setting CFLAGS="-std=gnu17" replaces autoconf's default -g -O2, so the generated Makefiles have CFLAGS = -std=gnu17 -W -Wall. Scenario: the shipped libetpan is built at -O0 with no debug info, to work around a failure that does not happen at this pin. The doc records a false reason. Fix: drop the CFLAGS override, or keep the default optimization flags. Delete the md5.c sentence, or confine it to 6dc099a.

scripts/build-libetpan.sh:16, :21-22 — These lines are silent fallbacks.

  • brew --prefix openssl@3 ... || echo /usr points OpenSSL at /usr, which has no OpenSSL headers on macOS. libetpan's configure.ac:227-243 then sets with_openssl=no without an error.
  • json-c is auto-detected (configure.ac:841-852, with_json=auto). Without json-c ≥ 0.14, the gmail subdirectory and mailgmail.h are left out (src/low-level/Makefile.am:40-46). This build found Homebrew json-c 0.19. The doc's dependency list (:42) does not mention json-c. Scenario: on a machine without Homebrew or with no OPENSSL_DIR, libetpan builds without TLS. tests/engines/libetpan.sh only checks headers, so it still passes, and PRD §23 requires TLS for MailCore 2. On a machine without json-c, the test fails with "pinned libetpan lacks libetpan/mailgmail.h", which hides the real cause. Fix: require OPENSSL_DIR or the brew prefix and fail if it is missing. Pass --with-json, so configure errors out when json-c is absent. Assert HAVE_OPENSSL in the generated config.h in the test. List json-c in R-03.

tests/engines/mupdf.sh:44 — The only check on the signing helper is [ -s libmupdf-pkcs7.a ]. When pkg-config libcrypto fails, MuPDF's Makerules:238 leaves HAVE_LIBCRYPTO unset. pkcs7-openssl.c:28 then compiles stub versions of the same three symbols, and the archive is still non-empty. Scenario: on a machine without OpenSSL's pkg-config file, the test prints "signing helper pass". The R-02 claim "用 OpenSSL 3.6.4 构建成功" is false there. Fix: pass HAVE_LIBCRYPTO=yes in scripts/build-mupdf.sh, and assert that nm -u libmupdf-pkcs7.a lists EVP/PKCS7 symbols.

docs/engineering-risks.md:16 — The doc says lok.sh asserts each fact listed below ("逐条断言下列事实"). It does not. The script never checks:

  • the dmg size or the three exported symbols (fact 1);
  • that no svp plugin is present (fact 2);
  • the init.cxx line numbers (fact 4);
  • bd2d0bec67 or salvd.cxx (fact 5);
  • configure.ac (fact 6). Scenario: a different dmg that drops lok_preinit, or ships an svp plugin, still passes lok.sh, and the doc claims those facts were asserted. Fix: assert the binary facts (nm -gU, the plugin list, the dmg size) in lok.sh, and give the source facts as plain citations.

docs/engineering-risks.md:22 — "与补丁第二处相同" and "需要补丁第三处" point to the wrong hunks. The patch's second hunk (@@ -174) is the vcl/sysdata.hxx include. The guard change is hunk 3 (@@ -4274), and the CoreGraphics path is hunk 4 (@@ -4283). Scenario: a reader checking the claim against the patch compares the wrong hunk. Fix: cite the hunks by their header lines, or by the code they change.