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()