Navid Qaragozlou

Require rules with execution groups to declare `DefaultExecGroup` to use it Summary: A rule with named execution groups uses `DefaultExecGroup` only if it lists it in `rule(exec_groups = [...])`. Otherwise a forgotten `exec_group = ...` on an action or test silently falls back to the target's platform. **What this means** - Rules without named groups keep the implicit default. Nothing existing changes. - Exec deps and toolchain deps must name their group: in a rule with named groups, every `attrs.exec_dep` and `attrs.toolchain_dep` (nested ones included) needs `exec_group`, even `exec_group = DefaultExecGroup`. Checked when the `.bzl` file loads. - An `attrs.arg` without `exec_group` may hold flags (`["-O2"]`) but no tools. A `$(exe)`, `$(location_exec)` or `$(location_toolchain)` in it fails when the target is created (every `select()` branch included), or when the `.bzl` file loads if it is in the default. The error names the target, the attribute and the macro, and says the fix is an `exec_group` on the rule's `attrs.arg`, not a BUCK change. - The check walks the value with its attribute type, so in an attribute that mixes grouped and ungrouped args (tuple positions, the matched `one_of` alternative, dict keys and values) only the ungrouped args are checked. - Actions (`run`, `write`), test infos and anon targets keep the rule-level opt-in. If the rule lists `DefaultExecGroup`, they use the default group when no `exec_group` is given. **Error for an unmarked tool attribute (any rule with named groups)** > Attribute `{attr}` contains an `{function}` without `exec_group`. A rule that declares named execution groups must give every exec dep and toolchain dep an execution group: add `exec_group = DefaultExecGroup` (and list `DefaultExecGroup` in `exec_groups`) or a named execution group **Errors when a rule has named groups but does not list `DefaultExecGroup`** - `exec_group = DefaultExecGroup` on a dep or arg: when the `.bzl` file loads. - `uses_plugins` (plugin deps are default-group exec deps): when the `.bzl` file loads. - `attrs.exec_compatible_with(exec_group = DefaultExecGroup)`: when the `.bzl` file loads. - `ctx.actions.run()`, `write()`, `write_json()`, `_write_hmap()`, `anon_target()` or `anon_targets()` without a named group: during analysis. - A test info with `exec_group = DefaultExecGroup`: when it is created. - A test info with neither `exec_group` nor `default_executor`: when it is created, so a rule that returns it, such as an alias, fails too. `executor_overrides` does not exempt it, since the tests run in the default group whenever no override is picked. This depends only on the rule's declared groups, not on the host's platforms. - `buck2 test` keeps the same check against the tested target's rule, for test infos created outside rule analysis or by another rule. Its error says the test info may come from another rule and gives both remedies: set `exec_group` or `default_executor` where the test info is created, or add `DefaultExecGroup` to the tested target's rule. **Not affected** - Actions that do not depend on the platform (`copy_file`, `symlinked_dir`, ...) and cquery's `buck.execution_platform` still use the default platform. **How it works** - The rule's `default_execution_group` flag is copied to the execution platform resolution, where the action, anon target and test checks read it. - Each attribute records which of its tool constructors lack `exec_group` (a small bitset on `StarlarkAttribute`). Every attribute function passes its marks to `Attribute::attr`: containers the union of their inner attributes' marks, leaves `ToolMarks::NONE`. `rule()` reads them at load time, and keeps the attributes with ungrouped args for the target-creation check. Attribute types, and so DICE keys, are unchanged. Reviewed By: jtbraun Differential Revision: D126123551 fbshipit-source-id: 1bf7bb29d61947a531982bd862a273eba4dcc2fc

Measure how much of an install was spent waiting for artifacts Summary: Exopackage native libs and resources are ready long before the apk, but the installer waits for every artifact before it touches the device. Publish enough timing to size that idle window across the fleet before changing any install behaviour. The report is one timeline in seconds.milliseconds, measured from the first artifact arriving. Each class of artifact gets two numbers -- when buck finished delivering it, and how long moving it to the device took -- emitted in arrival order, so the shape of an install is readable top to bottom: ``` native_arrival_s 0.000 native_transfer_s 26.324 dex_arrival_s 0.001 dex_transfer_s 27.143 ... critical_path_s 34.926 total_s 37.214 potential_saving_s 2.288 ``` `critical_path_s` replays the install against these same measured durations, but starts each payload the moment its own artifacts were there rather than waiting for all of them; `potential_saving_s` is `total_s` minus that. Also adds `transport` (`tcp`/`emulator`/`usb`), because throughput is not comparable across transports and today we cannot tell how much installing happens against remote devices versus emulators. Differential Revision: D115107906 fbshipit-source-id: fb507fb97830d68bc3e942de2965d3a9ab2d8329

Kill a timed-out test's detached processes on hosts without cgroups Summary: When a local test is killed, buck2 kills its process group and, with resource control on, everything left in its action cgroup. Without an action cgroup (all Mac, about 12% of Linux CI), a process the test detached into its own session survives, such as a daemon or an e2e test's inner buck2 daemon. Windows kills the whole job object, so it is unaffected. This adds an opt-in, `[buck2] kill_detached_test_processes` (default false), for local tests on Linux without an action cgroup: - The test's main process becomes a child subreaper (`PR_SET_CHILD_SUBREAPER`), so double-forked processes stay its descendants instead of moving to init. The process group decides this at spawn and acts on it at kill, from the same flag. Best effort: if `prctl` fails, the test still runs. - On kill, before any signal, buck2 lists the main process's descendants from `/proc` (off the async runtime). After the grace period, it lists again the descendants of the main process and of the processes it found, to catch what they started meanwhile. Then it kills the group as before and `SIGKILL`s each listed process that is still the same process (pid and start time, signalled through a pidfd so a reused pid can't be hit). - If the kill is abandoned midway, all of this still happens. Abandoned while listing, the listing kills the group once it is done: killing the main process earlier would move descendants it had not listed yet to init. **Limits.** - Not covered: processes left behind by a main process that exits, on its own or during the grace period. Its orphans move to init the moment it exits, and can no longer be found. - Linux only; Mac has no subreaper. - The main process adopts orphaned descendants and does not reap them unless it waits for all its children, so they stay zombies until it exits. A test that waits for one of its daemons to exit by polling its pid then waits until it is killed, and a daemon that checks for parent death with `getppid() == 1` does not see it. This is why it is opt-in. Reviewed By: cbnilrem Differential Revision: D128649806 fbshipit-source-id: 4906f5e806a9e4714156c7e398e6d4979dbbc595

Add `exec_group` to `anon_target()` and `anon_targets()` Summary: Lets an anon target run in an execution group. Without it, anon targets always use the creating target's default platform, so work split into anon targets cannot follow a group. **For rule authors** - `ctx.actions.anon_target(rule, attrs, exec_group = G)` and `anon_targets(..., exec_group = G)` create the anon target with `G`'s execution platform. - Its actions run there. - Its exec-dep platform check compares against `G`, so a tool from a `G` attribute can be passed in. - `exec_group` is typed `ExecGroup | None`. **Args** - Each `$(exe ...)`, `$(location_exec ...)` and `$(location_toolchain ...)` tool in an arg passed to the anon target must be built for the anon target's platform. - The check runs only when execution groups are involved: the creating rule declares named groups, the anon target is created with `exec_group`, or the tool is in a grouped arg. Otherwise args are not checked, as before this stack. - When it runs it is exact: each anon-compatible arg value that holds a tool records the execution platform resolution of the target that configured it. A grouped tool is checked against that target's group, an ungrouped one against that target's platform. - Values without tools (plain flags, `$(location)` of a regular dep) record nothing: they share one static handle and allocate nothing. The field is one pointer in the slot of the old `anon_target_compatible` bool, so `ConfiguredAttr` keeps its size. - The recorded resolution is not part of the arg's equality or hashes, so creators with different resolutions that pass the same arg share one anon target. - So an arg from a `G` attribute can go to an anon target created in `G`, and a default-group tool cannot. An arg from a toolchain in `G` (for example its `attrs.arg` flags with `$(exe)`) is checked against `G`'s platform too. - This replaces the blanket rejection of grouped args from D126123548. **Errors** - Anon targets have no groups of their own. `anon_target()` of a rule that declares execution groups fails with an error that says so, instead of a confusing "not declared" error later. - `anon_target(..., exec_group = G)` and `anon_targets(..., exec_group = G)` fail at the call when `G` has no compatible execution platform, with an error naming the anon rule and `G`. Before, the anon target's analysis failed later with an error that named neither. - In a node that resolves no execution platforms (an unbound node, such as a `select()` key), they fail at the call with the "Execution platform resolution was not performed" error that an action in `G` gets there. Reviewed By: jtbraun Differential Revision: D126123550 fbshipit-source-id: 71c12e3e3fa7f2c397f5ff07f156c46d0365b9c3

Let timed-out tests report what they were doing Summary: **Problem.** When a test times out, buck2 kills it with `SIGKILL` at once. The test can't dump stacks, flush logs or say what it was waiting on, so the timeout result holds only what it printed earlier, and flaky timeouts can't be triaged without reproducing them. **Proposal.** On timeout, buck2 sends `SIGTERM` to the test's main process, waits up to `N` seconds (`[buck2] test_timeout_grace_period_s`, default 0 = today's behaviour), then kills the process group and cgroup as today. - What the test prints, or writes to its artifacts directory, in that window reaches the test runner. The result stays a timeout. - A test that doesn't handle `SIGTERM` dies at once; one that ignores it or hangs is killed after `N`. - Only timeouts get the grace period. Cancellations (ctrl-c, resource control reclaiming memory, hybrid execution) are killed at once. - Only the main process gets `SIGTERM`, so the test can still query what it started, such as an inner buck2 (which has no `SIGTERM` handler). **Changes.** - Forkserver `timeout_grace_period_s`, applied only on timeout; `graceful_shutdown_timeout_s` (persistent workers) still applies to every kill. Linux and Mac; Windows has no `SIGTERM`. - One kill routine, in three steps (also documented on `ProcessGroupImpl::kill`): 1. `SIGTERM` the leader alone, so it can report what it was doing and still query the processes it started. 2. Wait up to the grace period for it to exit, without reaping it: an unreaped leader's pid, which is also the group id, can't be reused, so it still names this group. 3. `SIGKILL` the whole group, whether or not the leader exited. This also applies to workers' `graceful_shutdown_timeout_s`, so their final kill now reaches their whole group. If the kill is abandoned before this step, the group is still killed. - `miniperf`, which wraps tests when resource control is on, forwards `SIGTERM` to the command it wraps. Its child unblocks `SIGTERM` in `pre_exec`, so std spawns it with fork and exec rather than `posix_spawn`; the child is `miniperf`'s own small command, so this costs little. **Cost.** Only on timeout, at most `N`: 15 s is at most 2.5% of a 600 s timeout. **Alternative considered.** A timer in each test harness that fires before the runner's timeout. Rejected: it shortens every test's budget, must be built per harness, and turns timeouts into failures unless the runner learns to keep them as timeouts. **Rollout.** `N = 15` for buck2's tests, then all Linux, then Mac, a week each, while timeout wall-clock stays within budget + `N`. Rollback: `N = 0`. Follow-ups: `faulthandler` on `SIGTERM` (`chain=True`) in the Python test main, and a `SIGTERM` handler in the e2e harness that dumps inner buck2 state. Reviewed By: cbnilrem Differential Revision: D128649800 fbshipit-source-id: 953aecd2f532cbb785b7b8467cf112253a343f35

Resolve an execution platform per execution group Summary: This diff makes execution groups do something for exec deps. Each group a rule declares gets its own execution platform, and exec deps marked with a group are configured for it. **For rule authors** - `attrs.exec_dep(exec_group = G)` puts an exec dep in group `G`. - An exec dep without `exec_group` is in `DefaultExecGroup`. D126123551 requires the explicit `exec_group = DefaultExecGroup` in rules with named groups. - Rules without named groups are unchanged. ```python Link = exec_group() binary = rule( impl = _impl, attrs = { "_compiler": attrs.exec_dep(default = "//:cc", exec_group = DefaultExecGroup), "_linker": attrs.exec_dep(default = "//:ld", exec_group = Link), }, exec_groups = [DefaultExecGroup, Link], ) ``` **How it works** - A group's platform is resolved from the target's `exec_compatible_with` plus only the exec deps in that group, with the existing cached resolution key. - Every declared group is resolved, even one with no deps. - The groups resolve concurrently, alongside the configuration of the default group's deps (one `compute_join` over the groups). Results are merged in declaration order, so the node and its first error are deterministic. - Resolution and `buck2 audit execution-platform-resolution` build a group's constraints with one helper, `ExecutionPlatformConstraints::for_execution_group`. - Exec deps in named groups no longer constrain the default group. - A tool configured identically in several groups is one exec dep of the node (a linear check; group dep lists are small, so nothing is allocated for it). - An unbound node (for example a `select()` key, which is configured without an execution platform) resolves no platforms. Its grouped exec deps are configured like its default-group exec deps, so they stay deps of the node. A dep in both the default and a named group keeps the stricter visibility check. **Errors** - At load time: an attribute uses a group the rule does not declare. - In analysis: a lookup of a group the rule does not declare. **Later diffs** - Toolchain deps in groups, the cquery attribute, grouped exec deps passed to anon targets, and the defining-file note in errors. Reviewed By: jtbraun Differential Revision: D124430400 fbshipit-source-id: 17f56e68ad34c68e3a45497fbdbcebe1f83e572a

Report per-command dep-file cache work on the invocation record Summary: The dep-file cache counters live on `Snapshot`, which reaches `InvocationRecord` only as the nested `first_snapshot`/`last_snapshot`. Nested messages do not become `buck2_builds` columns -- only flat scalars on the record do -- so none of the cache counters are queryable today. This adds the flat fields, computed client-side as the growth between a session's first and last snapshot, which is also what makes them per-command rather than per-daemon. Columns are not automatic. `buck2_builds` is fed by a hand-written mapping in `fbcode/buck2/facebook/ingress/src/processors/dual/builds_processor.rs` and a column in the Buck2Builds logger config. This commit is the emitter half: the values are in the record, and two follow-ups are needed before they appear in the table. Reviewed By: jtbraun Differential Revision: D117419033 fbshipit-source-id: 76107956749e60e8a84d3a24877042197ae462a1

Share process kill and wait helpers in buck2_process_kill Summary: Process management code was spread across buck2, and each copy had its own unstated rule on whether it reaps, and so on whether a later signal to the same pid is safe. This moves the pieces into one small crate, `buck2_process_kill`, whose docs state that rule for each helper: - `child_has_exited` / `wait_for_child_exit`: wait for a child without reaping it (`waitid(WNOWAIT)`). - `process_is_gone`: for a process that is not our child; a zombie counts as gone. - `terminate` (`SIGTERM` one process) and `kill_group` (`SIGKILL` a process group); an already-gone target is not an error. - `KillGroupOnDrop`: kills a process group if a kill is abandoned before its final `SIGKILL`. - Linux: `set_child_subreaper`, `descendants` and `ProcessId` (pid plus start time, so a reused pid is never killed). Users: - `buck2_execute_local`: the kill path and the detached-descendants guard use it; `unix/descendants.rs` is gone. - `buck2_miniperf`: waits for its child with `wait_for_child_exit`. - `buck2_common::kill_util`: uses `process_is_gone` instead of its own zombie check. The crate depends on `libc` only, because `buck2_miniperf` depends on it and every change to miniperf's dependencies re-embeds it into `buck2`. So `buck2_execute_local` no longer needs `rustix`, and `rustix` comes back out of the workspace and the open source shim. Reviewed By: cbnilrem Differential Revision: D129768793 fbshipit-source-id: 0b5c3f3eb0d793ade9af664247ab8038215dfca3

Fix content-based path write race on Windows Summary: `test_shared_content_hash_race` has never passed on Windows: 544 runs, 544 failures, 0 passes. It was added skipped in D101867568 and unskipped in D101944928 once `fs_util::remove_all` became idempotent under concurrent removes, but that only fixed the Unix failure mode. `immediate::write_to_disk` runs `cleanup_path` followed by `write_file` with no synchronization. A target analyzed under N configurations submits N writes for one shared content-based path, and on Windows the blocking executor is `DirectIoExecutor` — a bare `block_in_place` with no semaphore — so all N run at once. Windows deletes differ from Unix here. `DeleteFile` against a name whose handle is still open (Rust's `File::create` opens with `FILE_SHARE_READ|WRITE|DELETE`) does not remove the name, it marks it delete-pending, and a delete-pending name can be neither opened nor stat'd. Both errors in the failing run follow from that: `write_file` writing `...\artifacts.json`: write_with_executable_bit(...): Access is denied. (os error 5) Binary being executed, please close the process first: symlink_metadata(...\artifacts.json): Access is denied. (os error 5) The first is a second writer's `File::create`, the second is `remove_all`'s stat. On Unix the same interleaving is benign, which is why the earlier fix looked complete. This diff serializes writers per path over a table of striped mutexes sized to `STRIPES_PER_THREAD` times the host's parallelism, so `cleanup_path` never runs while another writer holds the file open. Collisions between unrelated paths fall as roughly `1 / (2 * STRIPES_PER_THREAD)`; at 16 that is about 3% of concurrent writers, measured by `test_unrelated_paths_rarely_share_a_stripe`, which also pins the bound so the constant or the hash cannot silently regress. All three write paths share `locked_write`: `write_to_disk`, `WriteIoRequest::execute_inner`, and the `ArtifactMaterializationMethod::Write` arm of `materialize_entry_span`. The latter two are unreachable today because `defer_write_actions` defaults to `never`, so flipping that rollout would have silently reintroduced this bug; the second carried a byte-for-byte copy of the unsynchronized sequence and the third wrote with no serialization at all. `write_file` now has exactly one non-test caller. Decompression stays outside the lock, so a decompress failure no longer removes the previous file at that path before erroring. The action still fails, and any later write to the path cleans it first. Reviewed By: jtbraun Differential Revision: D116656537 fbshipit-source-id: 3fad7c019120a2030206dc613f3a568d8dd79a91

Add `exec_group()` and `rule(exec_groups = ...)` Summary: First diff of the execution groups stack. It adds the way to declare execution groups; they have no effect yet. Today every action of a target runs on one execution platform. Rules that need two kinds of machine (for example Apple resources on macOS, packaging on Linux) split one logical target into several in macros. **For rule authors** - `exec_group()` creates a group. It is named by the global it is assigned to, like `plugins.kind()`. - `rule(exec_groups = [...])` lists the groups a rule uses. - `DefaultExecGroup` is the built-in group whose platform is the target's execution platform. A rule may list it. ```python Resources = exec_group() bundle = rule( impl = _impl, attrs = {...}, exec_groups = [DefaultExecGroup, Resources], ) ``` **Errors (at load time)** - A group listed twice, including `DefaultExecGroup`. - A group that was never assigned to a global. - `exec_groups` on a toolchain rule or a configuration rule, even `[DefaultExecGroup]`. A toolchain's exec deps belong to the group of the rule that depends on it, and a configuration rule's targets are configured without an execution platform. **How it works** - `Rule` stores the groups and records whether the rule lists `DefaultExecGroup` or declares no named groups. Later diffs read this. - A group's identity is the interned `{ name, defining file }` that `plugins.kind()` uses (`BzlGlobalId`, D127214746). - The `ExecGroup` Starlark type lives in `buck2_interpreter`, so later diffs can take `exec_group: ExecGroup | None` as a typed parameter in crates that cannot depend on `buck2_interpreter_for_build`. **Not in this diff** - Resolving a platform per group, and letting deps, actions and tests use a group. Later diffs in the stack do that. Reviewed By: jtbraun Differential Revision: D124370253 fbshipit-source-id: 7664b131ed6dafb6cbdedd8b488f576de379a142