From 948955cb74cab2205817244faa762c1e16fd7cba Mon Sep 17 00:00:00 2001 From: Long Ho Date: Sun, 20 Sep 2026 01:39:56 +0000 Subject: [PATCH] fix: replace procps-ng with Toybox process probes --- MODULE.bazel | 2 +- internal/remote.bzl | 5 +- internal/test_tools/BUILD.bazel | 18 +++-- internal/test_tools/README.md | 17 +++-- internal/test_tools/defs.bzl | 26 +++++-- internal/test_tools/patches/BUILD.bazel | 5 +- internal/test_tools/patches/procps-ng.patch | 76 --------------------- internal/test_tools/patches/toybox.patch | 38 +++++++++++ internal/test_tools/repositories.bzl | 14 ---- internal/test_tools/tools_test.py | 35 +++++++--- 10 files changed, 114 insertions(+), 122 deletions(-) delete mode 100644 internal/test_tools/patches/procps-ng.patch diff --git a/MODULE.bazel b/MODULE.bazel index 1129980..8ecf417 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -48,4 +48,4 @@ bazel_dep(name = "bazel_skylib", version = "1.9.0") bazel_dep(name = "ncurses", version = "6.4.20221231.bcr.4") test_tool_sources = use_extension("//internal/test_tools:repositories.bzl", "sources") -use_repo(test_tool_sources, "test_tools_libmagic", "test_tools_procps_ng", "test_tools_toybox") +use_repo(test_tool_sources, "test_tools_libmagic", "test_tools_toybox") diff --git a/internal/remote.bzl b/internal/remote.bzl index 94e96d4..bc30912 100644 --- a/internal/remote.bzl +++ b/internal/remote.bzl @@ -43,10 +43,7 @@ def _native_test(ctx, root, descriptor, files, job): "unset BASH_ENV", 'if [[ -e /bin/sh || -e /lib64/ld-linux-x86-64.so.2 ]]; then echo "Browser tests require isolated actiond execution" >&2; exit 1; fi', 'case "$TEST_SRCDIR" in /*) ;; *) export TEST_SRCDIR="$PWD/$TEST_SRCDIR" ;; esac', - 'export MAGIC="$TEST_SRCDIR/%s"' % runfile(tools.magic), - ] + [ - '%s() { "$TEST_SRCDIR/%s" %s "$@"; }; export -f %s' % (command, runfile(binary), " ".join(args), command) - for command, (binary, args) in tools.commands.items() + 'source "$TEST_SRCDIR/%s"' % runfile(tools.shell_setup), ]) + "\n") ctx.actions.write(executable, "\n".join([ "#!/bin/bash", diff --git a/internal/test_tools/BUILD.bazel b/internal/test_tools/BUILD.bazel index cf6ecff..b87f2e7 100644 --- a/internal/test_tools/BUILD.bazel +++ b/internal/test_tools/BUILD.bazel @@ -5,10 +5,9 @@ test_tools( name = "tools", file = "@test_tools_libmagic//:file", magic = "@test_tools_libmagic//:magic.mgc", - pgrep = "@test_tools_procps_ng//:pgrep", toybox = "@test_tools_toybox//:toybox", - zip = "@zip//:zip", visibility = ["//:__subpackages__"], + zip = "@zip//:zip", ) py_binary( @@ -23,18 +22,27 @@ py_test( srcs = ["tools_test.py"], args = ["$(rlocationpaths :tools)"], data = [":tools"], + target_compatible_with = [ + "@platforms//os:linux", + "@platforms//cpu:x86_64", + ], deps = ["@rules_python//python/runfiles"], - target_compatible_with = ["@platforms//os:linux", "@platforms//cpu:x86_64"], ) config_setting( name = "linux_x86_64", - constraint_values = ["@platforms//os:linux", "@platforms//cpu:x86_64"], + constraint_values = [ + "@platforms//os:linux", + "@platforms//cpu:x86_64", + ], visibility = ["//visibility:public"], ) config_setting( name = "linux_aarch64", - constraint_values = ["@platforms//os:linux", "@platforms//cpu:aarch64"], + constraint_values = [ + "@platforms//os:linux", + "@platforms//cpu:aarch64", + ], visibility = ["//visibility:public"], ) diff --git a/internal/test_tools/README.md b/internal/test_tools/README.md index c5ba665..b2f8ad2 100644 --- a/internal/test_tools/README.md +++ b/internal/test_tools/README.md @@ -3,28 +3,31 @@ Bazel's `test-setup.sh` executes before our native browser test. Its utilities are declared test runfiles, built for Linux x86-64 with hermetic LLVM and musl: -- Toybox: filesystem operations, find, grep, sed, and ps. -- procps-ng: pgrep, including Bazel's `-a -g` process-group query. +- Toybox: filesystem operations, find, grep, sed, ps, and pgrep. - libmagic: file and its compiled MIME database. - Info-ZIP: packaging undeclared test outputs. -All four executables are static. The launcher calls their runfile paths directly +All three executables are static. The launcher calls their runfile paths directly through `BASH_ENV` functions and sets `MAGIC` to the declared database. actiond still supplies the pinned static Bash requested by `requires-bash`. Browser libraries and fonts remain separate, caller-selectable runtime inputs. +The private launcher adapter maps Bazel's exact `pgrep -a -g PGID` probe to +Toybox's `pgrep -g PGID`. Bazel only checks for nonempty output; it does not +consume the full command line requested by procps's `-a` flag. Other invocations +retain Toybox's native argument handling. + The tools' platform transition selects the pinned LLVM toolchain only for this bundle, without replacing the caller's C/C++ toolchains globally. The libmagic database compiler runs on the build execution platform and also uses musl on Linux; cross-compiling test tools from macOS does not execute Linux binaries. -`repositories.bzl` pins source archives and BCR overlays. Three small local +`repositories.bzl` pins source archives and BCR overlays. Two small local patches are applied through repository rules so they also apply in consuming modules (root-only module overrides would not): -- Toybox: use musl's syslog-name definitions in the compilation unit using them. -- procps-ng: remove unavailable glibc/gettext feature claims and generate the - empty config header with a Bazel write action. +- Toybox: use musl's syslog-name definitions in the compilation unit using them, + and make `pgrep -g` select process groups instead of the Unix groups used by `ps -g`. - libmagic: compile its database using declared Python/compiler inputs instead of host cat/mv/rm commands, with a static Linux build-time compiler. diff --git a/internal/test_tools/defs.bzl b/internal/test_tools/defs.bzl index 1aba9c5..363a682 100644 --- a/internal/test_tools/defs.bzl +++ b/internal/test_tools/defs.bzl @@ -1,6 +1,8 @@ """Declared static Linux utilities used by Bazel's native test launcher.""" -TestToolsInfo = provider(fields = ["commands", "magic"]) +load("//playwright:defs.bzl", "runfile") + +TestToolsInfo = provider(fields = ["shell_setup"]) _MULTICALL_COMMANDS = ["cat", "date", "dirname", "find", "grep", "ln", "mkdir", "ps", "rm", "sed", "sleep", "sort", "stat", "touch"] @@ -18,15 +20,29 @@ linux_tools = transition( def _tools_impl(ctx): commands = {name: (ctx.executable.toybox, [name]) for name in _MULTICALL_COMMANDS} - commands.update({name: (getattr(ctx.executable, name), []) for name in ["file", "pgrep", "zip"]}) - files = depset([ctx.executable.toybox, ctx.executable.file, ctx.executable.pgrep, ctx.executable.zip, ctx.file.magic]) - return [DefaultInfo(files = files, runfiles = ctx.runfiles(transitive_files = files)), TestToolsInfo(commands = commands, magic = ctx.file.magic)] + commands.update({name: (getattr(ctx.executable, name), []) for name in ["file", "zip"]}) + setup = ctx.actions.declare_file(ctx.label.name + ".bash-env") + ctx.actions.write(setup, "\n".join([ + 'export MAGIC="$TEST_SRCDIR/%s"' % runfile(ctx.file.magic), + ] + [ + '%s() { "$TEST_SRCDIR/%s" %s "$@"; }; export -f %s' % (command, runfile(binary), " ".join(args), command) + for command, (binary, args) in commands.items() + ] + [ + # Bazel checks only whether this exact process-group probe has output. + # Toybox has -g but not procps's -a (print full command line). + "pgrep() {", + " if [[ $# == 3 && $1 == -a && $2 == -g ]]; then shift; fi", + ' "$TEST_SRCDIR/%s" pgrep "$@"' % runfile(ctx.executable.toybox), + "}; export -f pgrep", + ]) + "\n") + files = depset([ctx.executable.toybox, ctx.executable.file, ctx.executable.zip, ctx.file.magic, setup]) + return [DefaultInfo(files = files, runfiles = ctx.runfiles(transitive_files = files)), TestToolsInfo(shell_setup = setup)] test_tools = rule( implementation = _tools_impl, cfg = linux_tools, attrs = dict( - {name: attr.label(executable = True, cfg = "target", mandatory = True) for name in ["toybox", "file", "pgrep", "zip"]}, + {name: attr.label(executable = True, cfg = "target", mandatory = True) for name in ["toybox", "file", "zip"]}, _allowlist_function_transition = attr.label(default = "@bazel_tools//tools/allowlists/function_transition_allowlist"), magic = attr.label(allow_single_file = True, mandatory = True), ), diff --git a/internal/test_tools/patches/BUILD.bazel b/internal/test_tools/patches/BUILD.bazel index 196944a..a60405a 100644 --- a/internal/test_tools/patches/BUILD.bazel +++ b/internal/test_tools/patches/BUILD.bazel @@ -1 +1,4 @@ -exports_files(["libmagic.patch", "procps-ng.patch", "toybox.patch"]) +exports_files([ + "libmagic.patch", + "toybox.patch", +]) diff --git a/internal/test_tools/patches/procps-ng.patch b/internal/test_tools/patches/procps-ng.patch deleted file mode 100644 index 4328fc4..0000000 --- a/internal/test_tools/patches/procps-ng.patch +++ /dev/null @@ -1,76 +0,0 @@ ---- a/BUILD -+++ b/BUILD -@@ -1,3 +1,4 @@ -+load("@bazel_skylib//rules:write_file.bzl", "write_file") - load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library") - - PROGRAMS = [ -@@ -21,28 +22,23 @@ - - LOCAL_DEFINES = [ - "BUILD_WITH_WHINE=1", -- "ENABLE_NLS=1", - "ENABLE_PIDWAIT=1", - "HAVE_ALARM=1", - "HAVE_ARPA_INET_H=1", - "HAVE_ATEXIT=1", - "HAVE_CLOCK_GETTIME=1", -- "HAVE_DCGETTEXT=1", - "HAVE_DECL___UT_HOSTSIZE=1", - "HAVE_DLFCN_H=1", - "HAVE_DUP2=1", -- "HAVE_ERROR_H=1", - "HAVE_FCNTL_H=1", - "HAVE_FLOAT_H=1", - "HAVE_FORK=1", - "HAVE_GETHOSTNAME=1", - "HAVE_GETPAGESIZE=1", -- "HAVE_GETTEXT=1", - "HAVE_GETTIMEOFDAY=1", - "HAVE_INTTYPES_H=1", - "HAVE_ISWPRINT=1", - "HAVE_LANGINFO_H=1", -- "HAVE_LIBINTL_H=1", - "HAVE_LIMITS_H=1", - "HAVE_LOCALE_H=1", - "HAVE_MALLOC=1", -@@ -63,7 +59,6 @@ - "HAVE_RPMATCH=1", - "HAVE_SELECT=1", - "HAVE_SETLOCALE=1", -- "HAVE_SIGABBREV_NP=1", - "HAVE_SIGINFO_T_SI_INT=1", - "HAVE_STDINT_H=1", - "HAVE_STDIO_EXT_H=1", -@@ -111,22 +106,22 @@ - "HAVE___PROGNAME=1", - 'LT_OBJDIR=\\".libs/\\"', - "MAJOR_IN_SYSMACROS=1", -- 'PACKAGE=\\"' + module_name() + '\\"', -+ 'PACKAGE=\\"' + "procps-ng" + '\\"', - 'PACKAGE_BUGREPORT=\\"procps@freelists.org\\"', -- 'PACKAGE_NAME=\\"' + module_name() + '\\"', -- 'PACKAGE_STRING=\\"' + module_name() + "=" + module_version() + '\\"', -- 'PACKAGE_TARNAME=\\"' + module_name() + '\\"', -+ 'PACKAGE_NAME=\\"' + "procps-ng" + '\\"', -+ 'PACKAGE_STRING=\\"' + "procps-ng" + "=" + "4.0.5" + '\\"', -+ 'PACKAGE_TARNAME=\\"' + "procps-ng" + '\\"', - 'PACKAGE_URL=\\"https://gitlab.com/procps-ng/procps\\"', -- 'PACKAGE_VERSION=\\"' + module_version() + '\\"', -+ 'PACKAGE_VERSION=\\"' + "4.0.5" + '\\"', - "STDC_HEADERS=1", -- 'VERSION="' + module_version() + '"', -+ 'VERSION="' + "4.0.5" + '"', - "_GNU_SOURCE", - ] - --genrule( -+write_file( - name = "gen_config_h", -- outs = ["config.h"], -- cmd = "touch $@", -+ out = "config.h", -+ content = [], - ) - - cc_library( diff --git a/internal/test_tools/patches/toybox.patch b/internal/test_tools/patches/toybox.patch index 1779877..701cc6a 100644 --- a/internal/test_tools/patches/toybox.patch +++ b/internal/test_tools/patches/toybox.patch @@ -1,3 +1,4 @@ +diff --git a/toys/pending/syslogd.c b/toys/pending/syslogd.c --- a/toys/pending/syslogd.c +++ b/toys/pending/syslogd.c @@ -31,6 +31,12 @@ @@ -13,3 +14,40 @@ #define FOR_syslogd #include "toys.h" +diff --git a/toys/posix/ps.c b/toys/posix/ps.c +--- a/toys/posix/ps.c ++++ b/toys/posix/ps.c +@@ -463,10 +463,10 @@ + } + + // process match filter for top/ps/pgrep: Return 0 to discard, nonzero to keep +-static int shared_match_process(long long *slot) ++static int match_process_with_group(long long *slot, int group_slot) + { + struct ps_ptr_len match[] = { +- {&TT.gg, SLOT_gid}, {&TT.GG, SLOT_rgid}, {&TT.pp, SLOT_pid}, ++ {&TT.gg, group_slot}, {&TT.GG, SLOT_rgid}, {&TT.pp, SLOT_pid}, + {&TT.PP, SLOT_ppid}, {&TT.ss, SLOT_sid}, {&TT.tt, SLOT_ttynr}, + {&TT.uu, SLOT_uid}, {&TT.UU, SLOT_ruid} + }; +@@ -484,6 +484,11 @@ + } + + return ll ? 0 : -1; ++} ++ ++static int shared_match_process(long long *slot) ++{ ++ return match_process_with_group(slot, SLOT_gid); + } + + // process match filter for ps: Return 0 to discard, nonzero to keep +@@ -1945,7 +1950,7 @@ + + static int pgrep_match_process(long long *slot) + { +- return !FLAG(v) == !!shared_match_process(slot); ++ return !FLAG(v) == !!match_process_with_group(slot, SLOT_pgrp); + } + + void pgrep_main(void) diff --git a/internal/test_tools/repositories.bzl b/internal/test_tools/repositories.bzl index f11868b..8e1b210 100644 --- a/internal/test_tools/repositories.bzl +++ b/internal/test_tools/repositories.bzl @@ -33,20 +33,6 @@ def _sources_impl(_ctx): patch_args = ["-p1"], patches = ["//internal/test_tools/patches:toybox.patch"], ) - http_archive( - name = "test_tools_procps_ng", - urls = ["https://gitlab.com/procps-ng/procps/-/archive/v4.0.5/procps-v4.0.5.tar.gz"], - integrity = "sha256-LG1+2fKs3h1N1GAsYXL+Vu/4aVP+hjm9Yz29IswY9ds=", - strip_prefix = "procps-v4.0.5", - remote_file_urls = { - "BUILD": [_BCR + "/procps-ng/4.0.5/overlay/BUILD"], - }, - remote_file_integrity = { - "BUILD": "sha256-g/zOMIvfZisLKcYxUGyco/iglnwU+Iz4vnXkvEKPw3g=", - }, - patch_args = ["-p1"], - patches = ["//internal/test_tools/patches:procps-ng.patch"], - ) http_archive( name = "test_tools_libmagic", urls = ["https://github.com/file/file/archive/refs/tags/FILE5_47.tar.gz"], diff --git a/internal/test_tools/tools_test.py b/internal/test_tools/tools_test.py index f2d4326..b5b3c43 100644 --- a/internal/test_tools/tools_test.py +++ b/internal/test_tools/tools_test.py @@ -3,7 +3,6 @@ import base64 import os from pathlib import Path -import signal import struct import subprocess import sys @@ -25,11 +24,13 @@ def setUp(self): self.env = dict(os.environ, PATH="", MAGIC=_FILES["magic.mgc"], LC_ALL="C") def run_tool(self, command, *args): - prefix = [_FILES[command]] if command in ("file", "pgrep", "zip") else [_FILES["toybox"], command] - return subprocess.check_output(prefix + list(args), cwd=self.root, env=self.env, text=True) + return subprocess.check_output( + ["/bin/bash", "-c", 'source "$1"; shift; "$@"', "tools", _FILES["tools.bash-env"], command, *args], + cwd=self.root, env=self.env, text=True, + ) def test_static_elf(self): - for name in ("toybox", "file", "pgrep", "zip"): + for name in ("toybox", "file", "zip"): with self.subTest(name=name): data = Path(_FILES[name]).read_bytes() self.assertEqual(data[:6], b"\x7fELF\x02\x01") @@ -69,13 +70,29 @@ def test_manifest_and_signal_parsing(self): self.assertTrue(self.run_tool("date", "+%F %T %Z").strip()) def test_process_group_monitoring(self): - process = subprocess.Popen([_FILES["toybox"], "sleep", "60"], env=self.env, start_new_session=True) + leader = subprocess.Popen([_FILES["toybox"], "sleep", "60"], env=self.env, process_group=0) + member = None try: - self.assertIn(str(process.pid), self.run_tool("ps", "-p", str(process.pid))) - self.assertIn(str(process.pid), self.run_tool("pgrep", "-a", "-g", str(process.pid))) + member = subprocess.Popen([_FILES["toybox"], "sleep", "60"], env=self.env, process_group=leader.pid) + group = str(leader.pid) + self.assertIn(group, self.run_tool("ps", "-p", group, "-o", "PID=").split()) + self.assertIn(group, self.run_tool("ps", "-g", str(os.getgid()), "-o", "PID=").split()) + self.assertEqual(set(self.run_tool("pgrep", "-a", "-g", group).split()), {group, str(member.pid)}) + leader.terminate() + leader.wait(timeout=5) + # The group is still alive after its leader exits: cleanup must wait. + self.assertEqual(self.run_tool("pgrep", "-a", "-g", group).strip(), str(member.pid)) + member.terminate() + member.wait(timeout=5) + with self.assertRaises(subprocess.CalledProcessError) as failure: + self.run_tool("pgrep", "-a", "-g", group) + self.assertEqual(failure.exception.returncode, 1) + self.assertEqual(failure.exception.output, "") finally: - os.killpg(process.pid, signal.SIGTERM) - process.wait(timeout=5) + for process in (leader, member): + if process is not None and process.poll() is None: + process.terminate() + process.wait(timeout=5) def test_screenshot_mime_and_zip(self): png = base64.b64decode("iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+jZ1kAAAAASUVORK5CYII=")