buck2 — Engineering Performance

Move starlark_fmt into buck2/tools/ Summary: Relocates the `starlark_fmt` crate (an opinionated ruff-based Starlark/BUCK formatter) from `fbcode/starlark_fmt/` to a new shared `fbcode/buck2/tools/starlark_fmt/` directory as the first step toward open-sourcing it into the public `facebook/buck2` repo, and makes the crate open-source-buildable. Sources are moved with `sl mv` to preserve internal history. Relocation: - New shared `fbcode/buck2/tools/` home for buck2-adjacent tools. - Renamed `mod.rs` files to sibling modules (`autofixes.rs`, `formatting.rs`, `subcommands.rs`) to satisfy the buck2 `rust-no-mod-rs` lint. - No local `PACKAGE`; the crate inherits `fbcode/buck2/PACKAGE`. Target-level `visibility` is kept on `starlark_fmt_lib`. - A forwarding `alias` is left at `fbcode//starlark_fmt:starlark_fmt` so the generated DotSlash manifest (`tools/lint/starlark/starlark_fmt`) keeps resolving; removing it is a follow-up gated on the MSDK schedule repoint. - Updated the sole external Buck consumer `fbcode//autodeps2/autodeps2`, the `README.md` build/test paths, and dropped the now-stale `starlark_fmt` row from `fbcode_enable_pyrefly_targets.csv`. The ~20 path-based consumers of `tools/lint/starlark/starlark_fmt` are unaffected (that path is unchanged). Open-source readiness (so the OSS `cargo`/`buck2-oss` build works). Because this diff registers the crate in the OSS cargo workspace, it must itself be OSS-buildable, so all of the following live here: - Applied the dual MIT/Apache license header to all `.rs` files. - autocargo-generated `Cargo.toml` for the crate and registered it in the OSS workspace (`fbcode/buck2/Cargo.toml` members). - Gated the internal `lint` subcommand out of the OSS build: it uses `//common/rust/lint_message` (not in OSS), so `mod lint`, the `lint_message` import, the `Command::Lint` variant/arm, and the `lint_files` use are `#[cfg(fbcode_build)]`, and the dep is `# oss-disable`. `fmt`/`diff`/`stdin` are unaffected; the internal build is unchanged. - Gated the Meta-internal `quickcheck_arbitrary_derive` (used only by the fbcode_build-gated fuzz tests) out of the OSS build: `mod fuzz_starlark` is `#[cfg(all(test, fbcode_build))]`, and an autocargo `extra_buck_dependencies` removal drops it from the generated `Cargo.toml`. - Pinned `get-size2 = "=0.10.1"` into the OSS `[workspace.dependencies]` (via an autocargo `extra_buck_dependencies` reference) so the OSS cargo build resolves `ruff_python_ast` against `compact_str 0.9.1` (0.10.2 would need compact_str ^0.10 and fail the `CompactString: GetSize` bound). - Fixed lints that `buck2-oss-lint` denies but internal lint doesn't: `clippy::str_to_string` (28x `.to_string()` -> `.to_owned()` on `&str`) and `let_underscore_drop`. Reviewed By: d16r Differential Revision: D113069398 fbshipit-source-id: d724baad1d7b60ea545a335961b6bd36ceca933c

mem: Add mini_vec crate and type Summary: `MiniVec` is a `Vec` but is a single pointer on the stack; the length and capacity are bitpacked into the *top* bits of the pointer if they're small, otherwise allocated in the heap. This does introduce high-bit pointer packing to buck2 which we have not had so far. Claude assures me that modern kernels do not hand out high addresses without a user explicitly asking for them and no one complained when I asked, so I guess the team is on board with this. I'll use this in a couple places, but the first thing we'll be able to do is replace all the `ThinBoxSlice` Reviewed By: jtbraun Differential Revision: D105465807 fbshipit-source-id: 65663fb277c1301be55a3b1e94ef831ae80d4f78

Codemod `ok_or_else(|| internal_error!(...))` to `Option::internal_error` Summary: Mechanical adoption of the `BuckErrorOptionContext` API from the previous diff: 345 call sites across 142 files convert `.ok_or_else(|| internal_error!(...))` into the documented spelling — `.internal_error("...")` for plain messages, `.with_internal_error(|| format!(...))` where the message has format arguments. Message shape (`... (internal error)`), `InternalError` tag, and caller source location are identical before and after. Import fallout handled: 109 now-dead `use buck2_error::internal_error;` lines removed (40 files keep it for remaining non-`Option` macro uses); imports scoped into `mod tests` where the only converted sites are `cfg(test)`-gated, so the lib build stays warning-free. One `#[cfg(windows)]` site (`buck2/src/check_user_allowed.rs`) is deliberately left unconverted since it cannot be compile-verified on Linux. Reviewed By: christolliday Differential Revision: D116804875 fbshipit-source-id: 879fcb502cb449cca4e7e7cf608c08ccff0e1819

Add profile options to all commands Summary: This adds a common set of flags to all commands that support enabling profiling based on regexes that match stringified forms of StarlarkEvalKind. There's more documentation on the flags on the --profile-patterns flag in CommonStarlarkOptions. But basically: --profile-patterns: accepts regex patterns of evaluations to enable profiling --profile-patterns-mode: profiling mode to use --profile-patterns-output: root path to write profiling data. every individual evaluation gets its profiling data written to a separate file. we write to a subdir of this with similar format to buck-out/log (i.e. datetime+uuid) we additionally write an all_keys.list file listing all the evaluation names we profiled. This works by (1) setting up profile configuration that StarlarkEvaluatorProvider uses to enable profiling as appropriate and then (2) adding a ProfileEventListener that gets profile_collected calls as evaluations are finished. We already consistently use StarlarkEvaluatorProvider to get the enabling of profiling. The remaining diffs in the stack ensure that we consistently finalize profiles for all evaluations. Reviewed By: JakobDegen Differential Revision: D64281578 fbshipit-source-id: b09824a94d1a596216fa9d30e761436492e63b39

Model DICE key equality behavior Summary: ## Motivation DICE previously represented value equality only through `Key::equality(old, new)`. Paging therefore could not determine whether an old paged-out value might be reusable without first hydrating it. Some keys always install the recomputed value, so that hydration cannot affect the update decision. ## Equality behavior Replace the equality hooks on `Key`, `ProjectionKey`, and `InjectedKey` with: ```rust enum EqualityBehavior<T> { Compare(fn(&T, &T) -> bool), AlwaysUnequal, } ``` `Compare` carries the comparator used by DICE. `AlwaysUnequal` expresses that no comparison can reuse the old value. Keeping the policy and comparator in one enum prevents separate declarations from diverging. ## Implementation - Migrate existing equality implementations to `EqualityBehavior::Compare`. - Mark keys with unconditionally false equality as `EqualityBehavior::AlwaysUnequal`. - Forward the behavior through injected keys and erased key and projection values. This diff only exposes the equality policy; it does not change page-in behavior. The dependent diff uses the policy to skip hydration whose result cannot affect DICE value selection. Reviewed By: JakobDegen Differential Revision: D115785700 fbshipit-source-id: ff96564d46310f27dc545687a48d7478d9a10ecf

Add errorformat lib Summary: rfc: https://fb.workplace.com/groups/buck2dev/permalink/4110348932586481/ This diff adds the errorformat parsing lib. Since I did this development first locally using git, it is hard for me to split the diffs. However, I added comprehensive tests, it should be ok. Reviewed By: Will-MingLun-Li Differential Revision: D82085318 fbshipit-source-id: 4a5f0cde6bb37a504e126bd70f5002de7d950167

fix: a lot of instances of clippy::needless_lifetimes (#944) Summary: This doesn't get all of them because: 1. there are so many of them 2. the lint is pretty jank Part of https://github.com/facebook/buck2/issues/943 Pull Request resolved: https://github.com/facebook/buck2/pull/944 Reviewed By: JakobDegen Differential Revision: D74271655 Pulled By: dtolnay fbshipit-source-id: c7e41c47248d4be1f5d33a14b676ab35fc5180e2

cli: Allow `--isolation-dir` globally Summary: Let's see what CI says at least? Reviewed By: IanChilds Differential Revision: D87866823 fbshipit-source-id: c04e23ac1dc764a35f58e50286bba436e51740c0

Add `--agent-context` CLI flag with buckconfig-driven validation Summary: RFC: https://fb.workplace.com/groups/buck2dev/permalink/4302138863407486/ Adds a new `--agent-context key=value` CLI flag (modeled after `--client-metadata`) that lets AI agents pass structured context like `intent=fix`, `attempt=2`, or `prior_error=missing_target`. On the server side, a buckconfig-driven schema (`[agent_context]` / `[agent_context#field]` sections) allows enforcing required fields, allowed values, and unknown-key rejection for specific client IDs, while non-enforced clients pass data through without validation. Reviewed By: jtbraun Differential Revision: D99735398 fbshipit-source-id: 1c3e7d0eacae1d92b18068ad5e8fd472fd4d2d39

Replace Dice task internals Summary: This mostly rewrites the management of the per-key dice task state that manages cancellation, dependents, spawning the worker, etc. Chris initially did this because it unblocked an approach to paging that he was considering (described in the next diff). My understanding is that they have since picked a different direction, so this needs new justification. I do think that despite that change, the next diff is a well-motivated simplification on its own. It fixes a small memory issue I encountered elsewhere, but also having just the single map does seem quite a bit easier to reason about. As to whether this diff would be well-motivated on its own even without the next one is a bit less obvious (claude and I talked and interestingly we disagree somewhat). The new data structure is subtler and more complicated than any one thing that existed before, but in exchange for that there's only one thing; previously there were three differently synchronized things (an atomic and two locks) managing information equivalent to what is now the `DiceTaskInternal` contents. I tend to think that's an improvement. The current version has some sketchy things going on, but at least they're all in one place and I think a person has better odds of reasoning correctly about it (claude tells me the previous version of this was actually probably bug-free but idrk if I believe it). In other words, I like this because I think one explicitly complicated component is better than three implicitly complicated ones. There's one somewhat significant wart here in the form of the `AtomicList` type, which is simple but pretty poorly optimized. I may try to revisit that. ...and then after writing all that explanation I ran benchmarks on the next diff and saw it was -2.5% memory. So now this is all easily justified Reviewed By: jtbraun Differential Revision: D103355606 fbshipit-source-id: f8616468c3e8f94ed2c3a8fe8e5d0276a56b7748