Skip to content

.2059806565729300:529a2d613dd49bb01dfe10425f7ff39b_69e3499ab13256d09ece8088.69e4dcd0b13256d09ece8592.69e4dcd07afba9383018b93e:Trae CN.T(2026/4/19 21:46:56)#1278

Closed
Wpc-121 wants to merge 6 commits intoActivityWatch:masterfrom
Wpc-121:bug_rep_08
Closed

.2059806565729300:529a2d613dd49bb01dfe10425f7ff39b_69e3499ab13256d09ece8088.69e4dcd0b13256d09ece8592.69e4dcd07afba9383018b93e:Trae CN.T(2026/4/19 21:46:56)#1278
Wpc-121 wants to merge 6 commits intoActivityWatch:masterfrom
Wpc-121:bug_rep_08

Conversation

@Wpc-121
Copy link
Copy Markdown
Contributor

@Wpc-121 Wpc-121 commented Apr 19, 2026

为Windows构建添加严格验证模式

添加WINDOWS_VERIFY_STRICT环境变量以严格验证Windows构建产物

在CI工作流中为Windows平台启用严格验证模式,确保zip内容与源码目录一致

更新Makefile和package-all.sh脚本以支持新的验证功能

Wpc-121 added 6 commits April 19, 2026 00:11
重构集成测试脚本,增加健康检查轮询机制替代固定等待时间
添加详细的测试帮助文档和环境变量配置说明
改进日志记录和错误检测功能
添加全局和单测超时控制,改进错误分类和诊断信息
添加 pytest-timeout 依赖,支持进程树清理
优化日志输出和错误处理流程
添加 GitHub Actions 集成测试工作流,支持通过输入参数跳过构建步骤直接下载二进制文件
设置全局测试超时和分步超时控制
支持多平台(ubuntu/macos)测试环境
在构建流程中添加 doctor 检查步骤,确保环境依赖正确
新增严格模式验证脚本,检查打包内容和版本一致性
优化构建日志输出,增加详细步骤和错误处理
添加用于验证Windows安装包和zip压缩包内容一致性的脚本
添加WINDOWS_VERIFY_STRICT环境变量以严格验证Windows构建产物
在CI工作流中为Windows平台启用严格验证模式,确保zip内容与源码目录一致
更新Makefile和package-all.sh脚本以支持新的验证功能
import json
import signal
import atexit
import sys


GLOBAL_SERVER_PROCESS: Optional['ServerProcess'] = None
GLOBAL_TEST_TIMEOUT: int = 120
for child in children:
try:
child.kill()
except psutil.NoSuchProcess:

try:
parent.kill()
except psutil.NoSuchProcess:
for p in still_alive:
try:
p.kill()
except psutil.NoSuchProcess:
p.kill()
except psutil.NoSuchProcess:
pass
except psutil.NoSuchProcess:
else:
try:
os.killpg(os.getpgid(pid), signal.SIGKILL)
except ProcessLookupError:
except OSError:
try:
os.kill(pid, signal.SIGKILL)
except ProcessLookupError:

start_time = time.time()
poll_count = 0
last_exit_check = 0
if path and os.path.exists(path):
try:
os.remove(path)
except Exception:
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 19, 2026

Greptile Summary

This PR introduces Windows build artifact strict verification (WINDOWS_VERIFY_STRICT), a new package-all.sh/verify-package.sh infrastructure, an upgraded integration test suite, and make doctor pre-build checks across both CI workflows. There are two P1 bugs that prevent the strict-mode enforcement from working end-to-end: a local keyword used outside a function in verify-windows-artifacts.sh (which causes the script to exit with code 1 instead of reaching the strict-mode logic under set -e), and invalid $(VAR:-default) Make syntax in the Makefile that silently expands to empty string, stripping the WINDOWS_VERIFY_STRICT=true env var before it reaches package-all.sh.

  • verify-windows-artifacts.sh line 487: local diff_count=$? outside any function — under set -euo pipefail, bash 5.1+ exits with code 1 here, short-circuiting the strict-mode exit 2 path.
  • Makefile lines 546–547, 671: $(WINDOWS_VERIFY_STRICT:-false) is not valid GNU Make syntax — the variable expands to empty, overriding the CI-supplied WINDOWS_VERIFY_STRICT=true.

Confidence Score: 3/5

Not safe to merge — two P1 bugs cause the headline feature (strict Windows artifact verification) to silently no-op in CI.

Both P1 issues affect the same core feature: WINDOWS_VERIFY_STRICT strict mode is announced but never actually activates in any execution path. The local-outside-function bug causes an early exit under set -e, and the GNU Make :- syntax bug strips the env var before the shell script sees it. These two bugs compound to make the entire new verification gate ineffective.

scripts/package/verify-windows-artifacts.sh (line 487) and Makefile (lines 546–547, 671)

Important Files Changed

Filename Overview
scripts/package/verify-windows-artifacts.sh New Windows artifact verification script; contains a critical bug where local is used outside a function (line 487) causing early script exit under set -e, making strict mode non-functional.
Makefile Adds doctor check, build-pre-check, and verbose integration test help; uses invalid $(VAR:-default) GNU Make syntax on lines 546, 547, 671 that silently expands to empty string, defeating strict-mode env var forwarding.
scripts/package/package-all.sh New script orchestrating zip/installer creation with Windows artifact verification; logic is sound but depends on upstream fixes to the Makefile and verify-windows-artifacts.sh for strict mode to work.
scripts/package/verify-package.sh New package verification script checking executables, directories, version consistency; logic is clean and structurally sound.
scripts/tests/integration_tests.py Complete rewrite of integration tests with health polling, error classification, and process cleanup; a FileNotFoundError can occur in check_for_errors() if log files are deleted before cleanup is called.
.github/workflows/build.yml Adds doctor check before build and PACKAGE_STRICT/WINDOWS_VERIFY_STRICT env vars for packaging; CI intent is correct but effectiveness depends on fixing the Makefile syntax bug.
.github/workflows/build-tauri.yml Same pattern as build.yml: adds doctor check and strict-mode packaging with TAURI_BUILD=true; same dependency on the Makefile fix.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    CI["CI: WINDOWS_VERIFY_STRICT=true make package"] --> MK["Makefile package target"]
    MK --> MK2["Line 671: WINDOWS_VERIFY_STRICT=$(WINDOWS_VERIFY_STRICT:-false)\n⚠ expands to empty string in GNU Make"]
    MK2 --> PAS["package-all.sh\nWINDOWS_VERIFY_STRICT=''"]
    PAS --> BA["{WINDOWS_VERIFY_STRICT:-false} == false\nNo --strict flag added"]
    BA --> VWA["verify-windows-artifacts.sh\nSTRICT_MODE=false"]
    VWA --> CM["compare_manifests()"]
    CM -- "differences found\n(returns non-zero)" --> ELSE["else branch: local diff_count=$?\n⚠ local outside function → exit 1 under set -e"]
    ELSE --> EXIT1["Script exits with code 1"]
    EXIT1 --> PAHandler["package-all.sh:\nVERIFY_EXIT_CODE=1 → log_warn only\nBuild continues!"]
    CM -- "no differences\n(returns 0)" --> OK["exit 0 → Build continues"]
Loading

Reviews (1): Last reviewed commit: "ci(workflows): 为Windows构建添加严格验证模式" | Re-trigger Greptile

exit 0
else
# Differences found
local diff_count=$?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 local keyword used outside a function

local diff_count=$? is used in the top-level script body, not inside any function. In bash 5.1+ (the version on GitHub Actions Ubuntu 22.04+ runners), local outside a function exits with code 1. Because the script has set -euo pipefail at line 26, this causes the script to immediately exit with code 1 — before the strict-mode check at line 497 is ever reached.

package-all.sh treats exit code 1 as a warning (not an error), so the entire strict-mode enforcement silently no-ops when differences are found.

Fix: remove local and just assign the variable directly.

Suggested change
local diff_count=$?
diff_count=$?

Comment thread Makefile
echo " Use WINDOWS_VERIFY_STRICT=true to fail on differences (CI recommended)."; \
fi
@echo ""
WINDOWS_VERIFY_STRICT=$(WINDOWS_VERIFY_STRICT:-false) TAURI_BUILD=$(TAURI_BUILD) bash scripts/package/package-all.sh
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 $(VAR:-default) is not valid GNU Make syntax

$(WINDOWS_VERIFY_STRICT:-false) uses the bash shell's :- default-value expansion, but this is a GNU Make recipe, not a shell expression. GNU Make interprets $(WINDOWS_VERIFY_STRICT:-false) as a reference to a variable literally named WINDOWS_VERIFY_STRICT:-false, which is always empty. The recipe therefore runs:

WINDOWS_VERIFY_STRICT= bash scripts/package/package-all.sh

This explicitly overrides any WINDOWS_VERIFY_STRICT=true that CI set in the outer environment before calling make package. Inside package-all.sh, ${WINDOWS_VERIFY_STRICT:-false} then falls back to false, so strict mode is never activated. The same issue exists on lines 546–547 ($(PACKAGE_STRICT:-false)).

Use the $(or …,false) Make idiom instead:

WINDOWS_VERIFY_STRICT=$(or $(WINDOWS_VERIFY_STRICT),false) TAURI_BUILD=$(TAURI_BUILD) bash scripts/package/package-all.sh

Comment on lines +383 to +394
with open(self.stdout_path, "r", encoding="utf-8", errors="replace") as f:
stdout = f.read()
for indicator in error_indicators:
if indicator in stdout:
found_errors.append(f"stdout contains '{indicator}'")

if self.stderr_path:
with open(self.stderr_path, "r", encoding="utf-8", errors="replace") as f:
stderr = f.read()
for indicator in error_indicators:
if indicator in stderr:
found_errors.append(f"stderr contains '{indicator}'")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 open() unguarded after potential _cleanup_logs() call

If start() raises TestFailure (e.g. binary not found), it calls _cleanup_logs() which deletes the temp files, then re-raises. The fixture's finally block then calls cleanup(), which calls check_for_errors(). At that point self.stdout_path / self.stderr_path are still set (not None), but the files have already been deleted, so open() raises FileNotFoundError and the cleanup crashes.

Add an os.path.exists guard:

if self.stdout_path and os.path.exists(self.stdout_path):
    with open(self.stdout_path, "r", encoding="utf-8", errors="replace") as f:
        ...

@ErikBjare ErikBjare closed this Apr 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants