From 7e61f6f01cf0ef4ee7d26186af52dab0f44f1483 Mon Sep 17 00:00:00 2001 From: Jeffrey Wall <265409180+jeffwall-curlewlabs@users.noreply.github.com> Date: Mon, 12 Oct 2026 18:27:51 -0700 Subject: [PATCH] Leave the sealed system volume to the OS build instead of watching it Bazel reads every watched file in full when a new server first checks a repository's recorded inputs. On macOS the search path is mostly /bin and /usr/bin, so that check read every system tool: on one CI host the watches covered 1,214 files and 277 MiB, and the first build after a boot spent 33.5 s on the check without running the rule. Those files sit on the sealed system volume, which changes only with the OS build the key already records and watches. An entry whose resolution stays on that volume throughout now contributes its name, mode and link target but not its content, and neither it nor a listing on the volume is watched. An entry that passes through a link or a directory anywhere else stays watched and digested, even when it ends on the volume, because that part can change without an OS update. On the same host the watches drop to 255 files and 2.4 MiB. On a warm macOS host, a new server's check of the repository went from 0.24-0.76 s to 0.02-0.03 s. Linux keys are unchanged; every macOS host's key changes once. --- README.md | 20 +++++++++--- host_key.py | 69 +++++++++++++++++++++++++++++++++++++----- tests/host_key_test.py | 60 +++++++++++++++++++++++++++++++++++- 3 files changed, 136 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 73d6d28..bcd21b3 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,9 @@ build after setup runs every action once under the new key. - **The action search path.** Every entry in `/bin`, `/usr/bin` and `/usr/local/bin`: its name, mode, link target, and the content of the file it resolves to. An unreadable file, such as a setuid `sudo`, contributes its size; - its bytes belong to the OS or a package, which the next items cover. + its bytes belong to the OS or a package, which the next items cover. On macOS, + an entry that resolves entirely within the sealed system volume contributes + no content either: the OS build covers it. - **macOS.** The OS build (`sw_vers`), which stands in for the sealed system volume and its libraries; the selected developer directory and its Xcode or Command Line Tools version; the shell `/bin/sh` runs. @@ -60,6 +62,8 @@ with the same tools get the same key. Not covered: - Files edited in place without going through dpkg or Homebrew. +- A macOS system volume that is not sealed. The OS build stands in for the + volume's contents, and the key does not check the seal. - Tools outside the search path, unless you add their directory to `search_path`. - Environment variables. Use `--incompatible_strict_action_env`, or declare @@ -69,10 +73,16 @@ Not covered: ## When the key changes -The rule asks Bazel to watch everything it read. When any of it changes, Bazel -recomputes the key at its next command, including in a server that is already -running, and every action whose key changed runs again or comes from a cache -entry made under the same key. +The rule asks Bazel to watch everything it read, except search path entries +and listings on the macOS system volume: those change only with the OS build, +which Bazel watches instead. When any of it changes, Bazel recomputes the key at +its next command, including in a server that is already running, and every +action whose key changed runs again or comes from a cache entry made under the +same key. + +Bazel reads each watched file in full when a new server first uses the +repository. Leaving the system volume out spares that check a read of every +system tool, which on a freshly booted host can take tens of seconds. ## Comparing two machines diff --git a/host_key.py b/host_key.py index 6b221c1..9d0f788 100755 --- a/host_key.py +++ b/host_key.py @@ -84,7 +84,48 @@ def command_output(*command: str) -> str: ).stdout.strip() -def entry_identity(entry: Path, digests: dict[Path, str], watches: Watches) -> str: +def on_system_volume(path: Path, system_volume: int | None) -> bool: + """Whether resolving the absolute `path` stays on the system volume, which + changes only with the OS build: every directory it passes through, every + link it follows, and what it reaches. + + A link or directory elsewhere can change without an OS update, so a path + that passes through one is not covered by the build even when it ends on + the volume. + """ + if system_volume is None or not path.exists(): + return False + current = Path(path.anchor) + pending = list(path.parts[1:]) + links = 0 + while pending: + # A link sits in the directory that holds it, so checking each + # directory before looking in it covers the links as well. + if os.statvfs(current).f_fsid != system_volume: + return False + part = pending.pop(0) + if part == "..": + current = current.parent + continue + candidate = current / part + if not candidate.is_symlink(): + current = candidate + continue + # The path exists, so this bounds only a chain changed underneath the + # walk; it is macOS's own MAXSYMLINKS. + links += 1 + if links > 32: + return False + text = Path(os.readlink(candidate)) + if text.is_absolute(): + current = Path(text.anchor) + pending[:0] = text.parts[1:] if text.is_absolute() else text.parts + return os.statvfs(current).f_fsid == system_volume + + +def entry_identity( + entry: Path, digests: dict[Path, str], watches: Watches, system_volume: int | None +) -> str: # Type and mode decide whether a PATH search runs the entry at all. parts = [entry.name, oct(entry.lstat().st_mode)] if entry.is_symlink(): @@ -97,7 +138,12 @@ def entry_identity(entry: Path, digests: dict[Path, str], watches: Watches) -> s return "\0".join(parts) status = target.stat() parts += [str(target), oct(status.st_mode)] - if stat.S_ISREG(status.st_mode): + if on_system_volume(entry, system_volume): + # The OS build keys these bytes. Bazel reads every watched file whole + # when a new server first checks the repository, so watching the + # system's tools would read all of them again at each server start. + parts.append("system-volume") + elif stat.S_ISREG(status.st_mode): if os.access(target, os.R_OK): if target not in digests: digests[target] = content_digest(target) @@ -112,8 +158,13 @@ def entry_identity(entry: Path, digests: dict[Path, str], watches: Watches) -> s return "\0".join(parts) -def search_path_identity(directories: list[Path], watches: Watches) -> str: - """Digest every entry a PATH search or an absolute tool path can reach.""" +def search_path_identity( + directories: list[Path], watches: Watches, system_volume: int | None +) -> str: + """Digest every entry a PATH search or an absolute tool path can reach. + + `system_volume` is the filesystem the OS build keys as a whole, if any. + """ digest = hashlib.sha256() digests: dict[Path, str] = {} listed: set[Path] = set() @@ -122,7 +173,8 @@ def search_path_identity(directories: list[Path], watches: Watches) -> str: digest.update(f"{directory}={resolved}\n".encode()) if directory != resolved: watches.file(directory) - watches.listing(resolved) + if not on_system_volume(resolved, system_volume): + watches.listing(resolved) if resolved in listed: continue listed.add(resolved) @@ -131,7 +183,8 @@ def search_path_identity(directories: list[Path], watches: Watches) -> str: continue # PATH search looks at direct entries only, never into subdirectories. for entry in sorted(resolved.iterdir()): - digest.update(f"{entry_identity(entry, digests, watches)}\n".encode()) + identity = entry_identity(entry, digests, watches, system_volume) + digest.update(f"{identity}\n".encode()) return digest.hexdigest() @@ -331,13 +384,15 @@ def host_fields( "system": system, "machine": platform.machine(), } + system_volume: int | None = None if system == "Darwin": fields.update(macos_fields(watches)) + system_volume = os.statvfs(MACOS_VERSION).f_fsid elif system == "Linux": fields.update(linux_fields(watches)) else: raise UnsupportedHost(f"no host key for {system}") - fields["search_path"] = search_path_identity(search_path, watches) + fields["search_path"] = search_path_identity(search_path, watches, system_volume) homebrew: dict[str, object] = {} for prefix in homebrew_prefixes: identity = homebrew_identity(prefix, watches) diff --git a/tests/host_key_test.py b/tests/host_key_test.py index 06905ae..0ae71e7 100644 --- a/tests/host_key_test.py +++ b/tests/host_key_test.py @@ -13,6 +13,12 @@ import host_key +# The host's real system volume where it has one, so the scratch tools below sit +# off it, as a host's own installs do. +SYSTEM_VOLUME = ( + os.statvfs(host_key.MACOS_VERSION).f_fsid if sys.platform == "darwin" else None +) + def pour( prefix: Path, @@ -144,7 +150,9 @@ def setUp(self) -> None: self.directory = Path(scratch.name) def identity(self) -> str: - return host_key.search_path_identity([self.directory], host_key.Watches()) + return host_key.search_path_identity( + [self.directory], host_key.Watches(), SYSTEM_VOLUME + ) def test_changed_tool_bytes_rekey(self) -> None: tool = self.directory / "tool" @@ -177,5 +185,55 @@ def test_unreadable_file_ignores_timestamps(self) -> None: self.assertEqual(before, self.identity()) +@unittest.skipUnless(sys.platform == "darwin", "only macOS has a sealed system volume") +class SystemVolumeTest(unittest.TestCase): + def setUp(self) -> None: + scratch = tempfile.TemporaryDirectory() + self.addCleanup(scratch.cleanup) + # Watches name resolved paths, and macOS reaches the temporary + # directory through a link. + self.directory = Path(scratch.name).resolve() + + def test_system_tools_are_not_watched_or_digested(self) -> None: + # Watching them makes every new Bazel server read each system tool + # whole, which takes tens of seconds on a freshly booted host; the OS + # build already keys them. + watches = host_key.Watches() + host_key.search_path_identity([Path("/bin")], watches, SYSTEM_VOLUME) + self.assertEqual((watches.files, watches.directories), (set(), set())) + identity = host_key.entry_identity(Path("/bin/sh"), {}, watches, SYSTEM_VOLUME) + self.assertNotIn(host_key.content_digest(Path("/bin/sh")), identity) + + def test_link_within_the_system_volume_is_not_watched(self) -> None: + # A link that resolves in its own directory never leaves the volume, so + # the OS build keys the link as well as the file it reaches. + link = next( + entry + for entry in sorted(Path("/usr/bin").iterdir()) + if entry.is_symlink() and "/" not in os.readlink(entry) + ) + watches = host_key.Watches() + identity = host_key.entry_identity(link, {}, watches, SYSTEM_VOLUME) + self.assertTrue(identity.endswith("\0system-volume"), identity) + self.assertEqual(watches.files, set()) + + def test_link_from_elsewhere_into_the_system_volume_stays_watched(self) -> None: + # Moving a link outside the volume changes which tool runs without an + # OS update, so the build cannot stand in for it. + (self.directory / "sh").symlink_to("/bin/sh") + watches = host_key.Watches() + host_key.search_path_identity([self.directory], watches, SYSTEM_VOLUME) + self.assertIn(str(self.directory / "sh"), watches.files) + self.assertIn(str(self.directory), watches.directories) + + def test_path_through_a_movable_directory_link_is_not_covered(self) -> None: + # A directory link off the volume can be pointed at other tools without + # an OS update, even while it leads onto the volume. + (self.directory / "bin").symlink_to("/bin") + covered = host_key.on_system_volume + self.assertTrue(covered(Path("/bin/sh"), SYSTEM_VOLUME)) + self.assertFalse(covered(self.directory / "bin" / "sh", SYSTEM_VOLUME)) + + if __name__ == "__main__": unittest.main()