Repository navigation
Recent Changes - #4
Merged
Merged
Conversation
Summary: `#[starlark_value]` decides whether to emit a restrictive `attr_ty(_) -> None` by scanning the impl block for an explicit `get_attr`. Attribute macros run before item macros expand, so `starlark_attrs!()` is still an unexpanded `syn::ImplItem::Macro` at that point and the `get_attr` it will produce is invisible. The generated `attr_ty(_) -> None` then claims the type has no attributes at all, and `example.hello` fails to typecheck even though it resolves fine at runtime. Fix: if the impl block contains any item macro, fall back to the permissive default (`Some(Ty::any())`) instead of asserting the attribute does not exist. We cannot see what the macro will define, so we must not claim it defines nothing. This only loosens typechecking, and only for impls that use an item macro. Exact per-field types would be better still, but that needs `StarlarkAttrs` to register the fields in `Methods`, which is a larger change. Pre-existing bug: today it is mostly hidden because `TypecheckMode::Compiler` only walks top-level `def` bodies, but the new test shows it already fires for anything inside a `def`. Reviewed By: JakobDegen Differential Revision: D114815951 fbshipit-source-id: e6c2a4a3301ae778bbd5e369269662a62141c684
Summary: Anon target and bxl outputs are written to sibling directories rather than under `art`: their `make_hashed_path` impls append `-anon`/`-bxl` to the path prefix. The scan root list named only `gen` and `art`, so the walk never reached them and artifacts there were never cleaned, either as stale or as untracked. Pull Request resolved: #1447 Reviewed By: 8Keep Differential Revision: D114949943 Pulled By: scottcao fbshipit-source-id: 6a7d07d5c35a8a6b08979611f2129787fcf7eaa2
Summary:
D114682223 routed the Rust binary link through `cxx_link_into` with a hardcoded `LinkExecutionPreference("any")`, so the link moved to RE. That silently dropped build-info stamping.
The fbcode linker wrapper (`fbcode/tools/build/buck/wrappers/ld.py`) generates a C stub defining the `BuildInfo_k*` symbols and adds it to the link. Under `--build-info=full` with no explicit `--build-info-rev`, it fills the revision in by shelling out to `hg log` (`lib/build_info.py:get_revision_info`). On RE there is no repo, so `get_revision_info` returns empty, the symbols are empty, and `cli_version::get_version()` falls back to `buck-dev`.
Before the migration this was handled for us: the rustc action *was* the link, and it ran with `prefer_local = link_cxx_binary_locally(ctx)`. Restore that for the cxx link, matching `cxx_executable`, which resolves the same toolchain bit into `LinkExecutionPreference("local")`. Dylibs keep `any`, as `rust_link_shared` does.
`link_cxx_binary_locally` rather than `get_resolved_cxx_binary_link_execution_preference`: the latter falls through to `get_link_execution_preference`, which `fail()`s on rules without a `link_execution_preference` attribute, and the Rust rules do not have one.
Note that the stale RE action-cache entries produced while the link ran remotely can still be served after this lands, since this does not change the action's key. They will age out or be invalidated by the next change to the crate's inputs.
Reviewed By: capickett
Differential Revision: D115067153
fbshipit-source-id: ce8c420b9656a080e9a2bcbac9211c228174bf31
Summary: A restored heap materializes values in demand order, which can reverse their physical arena order relative to the stored recipe. Add an expected-failure regression that constructs this reversal and checks both directions of the mapping: an old parent must resolve a recipe index to the correct restored value, and a new parent pointing into the restored heap must serialize that pointer back to the original recipe index rather than its current physical index. The test uses one narrow panic marker for the known index corruption, so unrelated setup or deserialization failures still fail CI. The marker remains until the reverse mapping is implemented near the top of the stack. Reviewed By: christolliday Differential Revision: D114793216 fbshipit-source-id: 670f168da8da13842c6cc89b66d5c1233adf099b
Summary: Add a DICE/Starlark integration regression with two independently stored roots whose dependency heaps have the same logical `HeapRefId` but different contents. The test pauses hydration of the old root, hydrates the new root, and then resumes the old root to demonstrate that storage-global Starlark state can replace H0 with H1 and resolve B0 through the wrong heap. The unfixed revision uses a narrow expected panic at the H0/H1 assertion. The marker is removed when Starlark heap bindings move into the root's `PageInScope`. Reviewed By: christolliday Differential Revision: D114793226 fbshipit-source-id: 791f5377c9b10a58ea2f71261342051b348a073f
Summary: Add a storage-backed regression with two simultaneously live frozen heaps that share one logical `HeapRefId` but contain different values. Serializing both through one storage-owned Starlark state must register both exact allocations; treating the logical name as resident identity causes the second heap's pointer to be absent from the chunk index. The test then drops both native roots and round-trips the second stored root to verify that it retained its own content. This isolates the serialization-side collision from the separate root-scoped deserialization problem. The unfixed revision uses a narrow expected panic for the missing chunk index, which is removed when serialization state is keyed by exact allocation. Reviewed By: christolliday Differential Revision: D114793218 fbshipit-source-id: 93f7bd3d4256c09fc05e305a79fe62ec3e95d1f2
Summary: A restored `FrozenHeap` may be only partially deserialized in memory even though its complete canonical representation already exists in storage. Serializing the partial in-memory heap directly is difficult because unmaterialized values are represented only by deferred recipe state, not ordinary resident values that the serializer can traverse. Record the stored `DataKey` on erased Arcs during deserialization, and make the generic, in-memory, and Sled recursive page-out paths reuse that key instead of invoking the inner serializer. This establishes the storage-identity contract required by `PartialPagableArc` in the next commit. Reviewed By: christolliday Differential Revision: D114793219 fbshipit-source-id: 83e68eb68d3d46759d97b5adc0c55e0a37c77fb8
Summary: Introduce `PartialPagableArc<T>` for values whose restored in-memory form can remain incomplete. It provides a stable Arc allocation, a write-once canonical `DataKey`, shared identity across clones, and a weak counterpart that does not extend the value's lifetime. Before a key is assigned, the Arc can serialize a new value normally. After restoration, page-out must reuse the stored key and direct inner serialization is rejected. Migrate `FrozenHeapRef` to this representation so a lazy frozen-heap skeleton can materialize values on demand without ever replacing its complete stored representation with a partial one. Reviewed By: christolliday Differential Revision: D114793220 fbshipit-source-id: 22f97722137cbf7a1f9bb58b4dd065d79f0c8424
Summary: `HeapRefId` is a logical name and cannot identify one resident allocation. Multiple DICE roots can legitimately retain different heap incarnations with the same name: one root may recompute against an updated dependency while another root that is outside the current computation path still holds the older heap. Both allocations can therefore remain live at the same time. Introduce `FrozenHeapPtr` and key serialization registrations, chunk ownership, lookup, and drop cleanup by that exact process-local allocation instead. This lets multiple same-name heaps coexist in one `StarlarkSerState` without overwriting each other's chunk indices. The old `HeapRefId` resident candidate remains temporarily for the page-in path and is removed after root-scoped resolution is established. Reviewed By: christolliday Differential Revision: D114793214 fbshipit-source-id: 7a58d4ae728ddd06274180ed3c8b4d137c63a753
Summary: Page-in should reuse an exact allocation that survived page-out, but storage must not keep that allocation alive. Add a weak resident Arc registry keyed by exact `(TypeId, DataKey)` and route key association through one storage API that both records canonical identity and registers the weak entry. Only Arc types that expose stored identity and weak ownership, such as `PartialPagableArc`, enter this path. A cache hit returns the original allocation without deserializing it again; after the last external owner drops, the weak entry can no longer upgrade and normal deserialization resumes. Reviewed By: christolliday Differential Revision: D114793228 fbshipit-source-id: 6893d3c601964d32801d3d8fe677ce4528d7c8a7
Summary: The former `SessionContext` name hid the fact that its state survives individual serialization and deserialization operations. Rename it to `StorageContext`, make storage-owned extensions opt in through `StorageState`, and return shared typed `Arc`s from the registry. Update every backend and user to obtain this state from the storage instance, including making the in-memory owner and handle share one context. This establishes a clear storage-lifetime boundary before introducing the separate root page-in lifetime. Reviewed By: christolliday Differential Revision: D114793222 fbshipit-source-id: 5045510474865c5ad58b42af80fabe3731f1f02a
Summary: In deserialization, Starlark must resolve serialized heap references through a `HeapRefId -> exact heap` binding. That logical ID is expected to be unambiguous within one stored DICE root's reachable heap graph, but it is not unique across independent roots or different incarnations retained by DICE. The binding state therefore needs the lifetime and identity of one top-level page-in, rather than the whole storage backend. Add `PageInScope`, created exactly once by `PagableStorageHandle::root_deserializer` for each top-level hydration. Nested Arc deserializers and deferred recipes inherit the same scope, while a separate root deserializer receives a distinct scope even when it reads the same `DataKey`. Make concrete deserializer construction internal so Pagable users define only the root boundary and cannot accidentally substitute a new scope for nested work. The scope carries the root key and provides the lifetime anchor for root-specific state added next. Reviewed By: christolliday Differential Revision: D114793227 fbshipit-source-id: 3bdfe1c243c7f7aa0dd857e02185a06cdb9ad7a4
Summary: Page-in users need typed state shared by every nested deserializer and deferred recipe originating from one root, without leaking that state across independent roots. Add a `TypeId`-keyed registry to `PageInScope` and strongly retain one `Arc` per state type for the scope lifetime. `get()` and `get_or_init()` return typed `Arc`s, giving the following Starlark migration a stable root-local `StarlarkDeserScope`. That state will index heap allocations weakly, so retaining the state for the root lifetime does not retain the heaps or create an ownership cycle. Reviewed By: christolliday Differential Revision: D114793224 fbshipit-source-id: 5c72858efa36c61c7eb08005e8e86b89a0a7f9a5
Summary: Move Starlark's `HeapRefId` bindings out of storage-lifetime state and into a scope-owned `StarlarkDeserScope`. Each restored `FrozenFrozenHeap` retains its exact `HeapDeserializationState`, recipe, and owning scope; the scope indexes heaps weakly and upgrades the exact owner before accessing arena-backed state. Nested Starlark deserializers now recover their scope structurally from the Pagable recipe instead of accepting arbitrary shared state. Keep only cross-thread wait coordination storage-scoped, because an Arc-cached restored heap can be shared across roots, and key that graph by exact `FrozenHeapPtr` plus `value_index`. This is the commit that makes the DICE cross-root regression pass normally. Reviewed By: christolliday Differential Revision: D114793221 fbshipit-source-id: d17014591ba929b28ebd62b21457eb0c68a7051a
Summary: `deserialize_arc` may construct a restored heap, return a cached restored heap, or reuse a stored native heap. Register every non-empty result in the current `StarlarkDeserScope` by logical `HeapRefId` and exact weak allocation, rather than relying on an optional deserialization state that native heaps do not have. Within one stored root, a logical heap ID must identify at most one live allocation. Exact repeat registration is idempotent, expired bindings can be replaced, and two different live allocations are rejected with `ConflictingHeapBinding` rather than allowing a serialized index to resolve against an arbitrary arena. Reviewed By: christolliday Differential Revision: D114793217 fbshipit-source-id: 86bd2b4c54484aecdbaa1accbea76a213300b7a2
Summary: Stop resolving a serialized heap reference through a storage-global `HeapRefId` candidate. First obtain the exact heap bound in the current root scope; restored heaps resolve through their retained `HeapDeserializationState`, while native resident heaps resolve through their exact `FrozenHeapPtr` registration in `StarlarkSerState`. This makes the root scope authoritative and prevents a same-name heap from another root or incarnation from supplying the value. Split the former generic missing-bases error into diagnostics for a missing scope binding and a missing exact native value registration. Reviewed By: christolliday Differential Revision: D114793230 fbshipit-source-id: 5344fbd7fb547f8fccb532419821726efbff0c3a
Summary: Resident Arc reuse already happens in the generic Pagable cache by exact `(TypeId, DataKey)` before Starlark opens the child blob or invokes its heap-deserialization callback. The Starlark-specific `HeapRefId -> FrozenHeapPtr` candidate lookup is therefore both redundant and less precise. Remove that candidate index, its drop bookkeeping, the duplicate callback lookup, and the body-skipping path required only by that lookup. A generic cache hit returns the exact resident Arc; a cache miss now proceeds directly to constructing a lazy heap skeleton. Reviewed By: christolliday Differential Revision: D114793223 fbshipit-source-id: 448b3d185e6f246197e85c9a43e185f3be350524
Summary: Lazy page-in can allocate restored values in a different physical order from their original recipe, so arena order cannot be used as the wire `value_index`. Record a reverse mapping from each finalized payload address to its original recipe index and mark restored-heap chunk entries as using that mapping. Pointer serialization now uses physical indices for native heaps and original recipe indices for restored heaps. Refresh restored heaps' physical chunk coverage as lazy values appear, while retaining recipe indices as their stable wire identity. This removes the expected-failure marker from the recipe-index regression and validates both serialization and deserialization directions. Reviewed By: christolliday Differential Revision: D114793215 fbshipit-source-id: a10f43d302731a1eb4c9e882f59bd858ab26b9b6
Summary: The previous commit conservatively rebuilt a restored heap's physical chunk index on every serialization registration. Track a dirty bit beside the arena pointer under the same mutex used for lazy allocation: materializing a value marks the index dirty, and a successful refresh clears it. Clean restored heaps reuse their existing chunk entries. Dirty heaps rebuild once while holding the arena lock, so index construction cannot race with allocation. Add coverage for both stable clean registrations and refresh after a later lazy value materializes. Reviewed By: christolliday Differential Revision: D114793225 fbshipit-source-id: eca99328428b4e5aad9d91d3e15a2be27dc66df7
Summary: `TyCustomFunction::attribute` rejected every attribute unconditionally, so `foo.type` was reported as "The attribute `type` is not available on the type `def(x: int) -> int`". But `NativeFunction::get_attr` does serve `.type` for functions declared with `#[starlark(as_type = ...)]`, and `NativeFunction::dir_attr` lists it. So resolve `.type` to `str` when the function is a type constructor, and keep rejecting it (and everything else) otherwise. Ordinary `def`s are unaffected: they have `type_attr: None` and no runtime `.type` either. The predicate for "is a type constructor" is `TyCustomFunctionImpl::is_type()`. That was only accurate for `TyFunction` (which derives it from `type_attr`) and `ListType`. `struct` and `namespace` have hand-written `TyCustomFunctionImpl`s that never overrode `is_type()`, so they inherited the `false` default despite their globals declaring `as_type = FrozenStruct` / `as_type = FrozenNamespace`. That left `struct.type` and `namespace.type` rejected even though both work at runtime. Override `is_type()` on both, matching what `ListType` already does. Overriding `is_type()` also makes `struct | None` and `namespace | None` valid type expressions. That matches runtime, which already accepts both (verified with static typechecking disabled), and matches `list | None`. Nothing in the type system ties "this global declares `as_type`" to "its `TyCustomFunctionImpl` reports `is_type()`", so this class of bug is invisible until someone writes `X.type` inside a `def`. Rather than leave the next type constructor to rediscover it, add a test that walks every global, finds the ones with `as_type` set, and asserts `.type` typechecks for each. Adding a fifth constructor without overriding `is_type()` now fails in starlark's own test suite instead of in a user's build file. Pre-existing bug: today it is mostly hidden because `TypecheckMode::Compiler` only walks top-level `def` bodies, but the new tests show it already fires for anything inside a `def`. Reviewed By: ndmitchell Differential Revision: D114817292 fbshipit-source-id: 60fe5b46af4762056ad2c5b8145489fd2b6ecff8
Summary: Bug found while trying to update our buck2 version: apparently we screwed it up while implementing #1004. Error in production: ``` 3: error: Error coercing attribute `srcs_deps` of `root//src/Foo:Foo` Traceback (most recent call last): * src/Foo/BUCK:28, in <module> mercury_haskell_library( * haskell/mercury_haskell.bzl:236, in mercury_haskell_library haskell_library( 4: Error coercing attribute `srcs_deps` of type `attrs.dict(attrs.source(), attrs.list(attrs.source()), sorted=False, default={})` 5: Error coercing {"FooBar.hs": ["//data/SomeMercuryThing"]} 6: Error coercing ["//data/SomeMercuryThing"] 7: Error coercing "//data/SomeMercuryThing" 8: Coercing `//data/SomeMercuryThing` as a source 9: expected a relative path but got an absolute path instead: `//data/SomeMercuryThing` ``` Assisted-By: Claude Opus 5 Pull Request resolved: #1437 Reviewed By: ndmitchell, scottcao Differential Revision: D114635584 Pulled By: 8Keep fbshipit-source-id: 753e7320bf1e855b30c2191ee583429e6afaca6b
Summary: D114793226 added a Buck2/Starlark paging regression test that requires the real Starlark pagable vtable registry. Buck builds enable pagable support, but OSS Cargo builds intentionally leave the `starlark/pagable` feature disabled. Gate the integration-test module on `buck_build` as well as `test`. This keeps the regression running in Buck while preventing OSS `cargo test` from invoking the feature-disabled registry stub. ___ Differential Revision: D115140989 fbshipit-source-id: bfaa416825103c48dbf8bff7492462e229d2c14f
Summary: `test_daemon_crash` was flaky: OOM detection converted a wall clock time into `dmesg --since`, so a stale matching OOM record left by an earlier daemon could relabel a deliberate daemon crash as an OOM kill. This captures the daemon start as an `Instant` from the uptime already returned by the bootstrap `status()` call and keeps attribution entirely in the kernel monotonic clock domain. `dmesg --raw` supplies the complete bounded log; the lower bound is the daemon start, falling back to five minutes before disconnect only when the daemon start is unavailable, floored to the microsecond precision `dmesg` exposes. Records are scanned newest-to-oldest and the scan stops at the first older timestamp, the raw `<priority>[seconds.microseconds]` prefix is parsed explicitly, and detection fails closed if the monotonic clock cannot be read. Attribution itself is still a cgroup match inside that window; D113982859 on top of this adds daemon-identity (PID and task comm) matching. Reviewed By: scottcao Differential Revision: D113907322 fbshipit-source-id: 3f86d14339a3650c254236fe3ca85035cf155b2c
Summary: OOM attribution rested on a cgroup match inside a time window, so an OOM kill of an unrelated process could still be attributed to this daemon — raised in review on D113907322. This carries the daemon PID already recorded in `buckd.info` into the client OOM detector and returns structured evidence instead of a Boolean. A kernel `Killed process <pid> (buck2)` record must match both the daemon PID and the task comm; that pair identifies the daemon on its own, so it is accepted whether or not the daemon start time was available. oomd and systemd-oomd attribution now accepts only the daemon's own cgroup or an ancestor containing it and rejects descendant kills, and the client error message names the evidence source. The lower bound is unchanged from D113907322 (daemon start, falling back to five minutes before disconnect), but the `dmesg` snapshot supplies the upper boundary because the kernel can emit its victim record after the daemon connection closes. Ancestor-cgroup kills remain accepted for oomd — a parent-cgroup OOM does kill the daemon — so that path is still time-window based. Reviewed By: scottcao Differential Revision: D113982859 fbshipit-source-id: ad34ece72dbf0f36b21faa3b8723c3dea1da13a8
Summary: Fish can redraw the interactive command line on another terminal row while rendering its completion menu. The verifier previously discarded only the first command line and split every remaining line into candidates, so a redraw was reported as spurious `%`, `buck2`, and input tokens. This made `test_zero`, `test_one`, `test_two`, and `test_long_completion` flaky together. Ignore rows beginning with the configured `% ` test prompt when parsing a multiline completion menu. Add a regression test covering both an identical redraw and a redraw after fish updates the command text. Failing example: https://www.internalfb.com/intern/test/281475240191589 Related Test Infra flaky issue: 234533950 ___ Differential Revision: D115183932 fbshipit-source-id: 083174567c29730448fbd8361efbe47175c41c47
Summary: simplify Reviewed By: d16r Differential Revision: D115065927 fbshipit-source-id: 37c68f83692df97dd5d808b7729212020aa5ce3f
Summary: align it with comment in code, shorter the better Reviewed By: d16r Differential Revision: D115067714 fbshipit-source-id: 5a34efc04528a09b85d031c706ab0acdf1244bc7
Reviewed By: d16r Differential Revision: D115069543 fbshipit-source-id: 52692c4f65ed14226bac7a3a02853ab050cdd254
Summary: Two optimizations to the persisted dep-file cache's SQLite table: - Use `prepare_cached` for every statement - Release the connection before deserializing in `read_filtered` This matters because a single `Arc<Mutex<Connection>>` serializes all access, and with the persisted cache enabled a cold daemon performs one lookup per locally-executed run action. Reviewed By: jtbraun Differential Revision: D115045270 fbshipit-source-id: d6e767356a1fec59360fd67d8e4ce508494a4881
Summary: Add the write path: `DepFileState::to_stored` (serialize to the config-independent `StoredDepFileState`), the forward-only key encoders, and the `DEP_FILE_STORE` write hooks (insert/delete/clear). Write-only and inert until the daemon installs the store, so no behavior change. Reviewed By: jtbraun Differential Revision: D113402618 fbshipit-source-id: 13f7ae9cff1e397cb0f5c733e432908ff8cab586
…ed types Summary: Branded downcasts match frozen and unfrozen instances alike, so plain `ValueTyped` unpacks work and the `.unpack()`-to-`Either` dances go away. Also collapses `freeze_worker` into the generic `ValueTyped` freeze. Reviewed By: jtbraun Differential Revision: D115252889 fbshipit-source-id: 01bb43db8c1477fde30ef94032ebe44f8f83caed
Summary: Same as the previous diff, for the storage field; the freeze goes through the transitive_sets pattern. Reviewed By: jtbraun Differential Revision: D115252865 fbshipit-source-id: 7f57c9c45482a2721d39ace0a56f1aa42b9d8dfc
Summary: `TransitiveSet.definition` is a `FrozenValueTyped`, so the unfrozen matcher arm was dead. Reviewed By: jtbraun Differential Revision: D115252873 fbshipit-source-id: c3c8368cddbc94e6dfbe0f09cb28a1c5acdeb981
…s brand Summary: Same shape as `StarlarkOutputArtifactUnpack`; replaces the `ValueTypedComplex` dance in the `LocalResourceInfo` validator. Reviewed By: jtbraun Differential Revision: D115252868 fbshipit-source-id: 994ef425fdc2ac2327626ee9cb375789231ae88a
Summary: All their users are gone. `StarlarkOutputArtifact`'s stays: the dynamic-lambda-params storage still freezes it through `derive(Freeze)` containers. Reviewed By: jtbraun Differential Revision: D115252879 fbshipit-source-id: 87dc098738688871c01ed5e2ade584b131f1d948
…value_branded!` Summary: With the last containers migrated, allocating a branded type and then freezing it via the legacy path no longer compiles; `alloc_complex` on a branded type is now a type error. One hand-written interop impl remains (`StarlarkOutputArtifact` in buck2), so `freeze_via_branded` stays. Reviewed By: jtbraun Differential Revision: D115252887 fbshipit-source-id: b326edbe1e026d0c328e9c340b165729f25b74a1
Summary: `OwnedFrozenRef` is its sound replacement and the last production caller went through the bridge, which is itself now caller-less. Its solver-coverage test is duplicated by `test_owned_frozen_ref`. Also resolves the name-proximity hazard between the two types. Reviewed By: jtbraun Differential Revision: D115252895 fbshipit-source-id: f9ce1d5b2c1ced4279a6004cb7c0b3cade08cab8
Summary: Deletes the brand-shrink transmute; the two test-helper exercisers of the relaxation build their literals at the heap brand instead. Reviewed By: jtbraun Differential Revision: D115252882 fbshipit-source-id: eb4f17953cc32e4dfe5a744ddbbfdc12cc8c93b9
Summary: The transition caller's unsafe keep-alive-at-a-distance dance becomes an `add_heap_ref` into the module heap, so the keep-alive is structural. Reviewed By: jtbraun Differential Revision: D115252891 fbshipit-source-id: 71f4db835b14646248950ff0af0df51873eb45b2
Summary: The `FreezeBranded` doc example now actually shows the branded derive, the `internal_provider` doc drops the deleted `freeze=` argument, and DESIGN/STATE record the batch (notably: prefer `ValueTyped` over `ValueTypedComplex` for branded types — the latter no longer compiles for them). Reviewed By: jtbraun Differential Revision: D115252890 fbshipit-source-id: cd58d27607883794bb0e528b3dfd60cf87667a0f
Summary: Post-land review feedback on D115028981: the mix in `ExhaustivenessHash` was computed with `BuckHasher`, but mixed hashes are embedded in pagable-persisted directory data, so the mix algorithm is a persisted format — a routine `buck2_hash` algorithm change would silently split the identities of persisted trees against freshly-rebuilt ones. Switch to blake3 via the strong hash infra, whose algorithm never changes, and pin the encoding with a golden-value test following the `StarlarkHashValue` precedent (D115452993). Measured cost (directory microbenchmarks, opt, 3 interleaved rounds, `--content exhaustive`): blake3's per-call floor on tiny inputs adds ~150ns per mixed node, which lands entirely on the marking-boundary spine: `assemble_inputs` 2.6ms → 3.1ms, `assemble_premerged` 34µs → 37µs; `fingerprint_wide` and all of `--content scaffold` are unchanged (the sentinel paths never hash). In absolute terms that is one extra tiny-input blake3 next to the content fingerprint's existing one per spine node, bounded by path depth × artifact count — which should be far below the abtest resolution the stack was gated on. If it ever shows up e2e, the dominant single-dir-child spine shape could propagate the child's hash as the mix without hashing at all, at the price of a subtler soundness argument. Reviewed By: 8Keep Differential Revision: D115547175 fbshipit-source-id: 5e480ee6b65e25ca2bf3b40a1755a28f99ed62a5
Summary: Make the `bolt` action inherit the resolved link execution attributes and `link_weight` from both regular and distributed ThinLTO links. Similar to {D104074890}
Reviewed By: skrueger
Differential Revision: D115621843
fbshipit-source-id: 07ec4f2b0c4a75033b9e32bd4fd84715d180f98c
Summary: The two inplace par actions never got `allow_cache_upload`, so unlike the standalone branch right above them there was no way to control cache upload for them at all. Now that the bootstrapper is a function of its inputs (see the diff below this one), it can be cached. **Reviewers please read: this is not a no-op internally.** `python_binary` resolves `allow_cache_upload` as `cxx_attrs_get_allow_cache_upload(ctx.attrs, cxx_compiler_info.allow_cache_upload)`, and the rule attr defaults to `None`, so it falls through to the cxx toolchain. The only toolchains in fbsource that set `cxx_compiler_allow_cache_upload = True` are ARVR's (`arvr/mode/linux/toolchain/clang*.buckconfig.inc`, `arvr/mode/win/toolchain/*`, `arvr/mode/toolchains.py`); fbcode leaves it unset, so it resolves to `False`. So this flips these two actions from never-upload to upload for ARVR configs, and only those. That leaves one thing worth an ARVR-savvy opinion before this lands: the `modules` action's output is a symlink tree whose links deliberately point outside the output directory (`make_py_package_modules.py`, `os.path.relpath(realpath(src), realpath(dest.parent))`). Whether that is sound to upload is the open question here. The `bootstrap` action's output is a plain file and is the safe half. The 13 failing signals on D114816218 — the upstream import that made this same change — are **not** evidence against it. Of the 5 I was able to read, covering both job families (`automation_twilight_*`, `automation_oculus-social-platform_*`) and the one standalone target (`fbandroid/javatests/.../commcore/ipc:ipc_colada`), every one dies on the same unrelated action: ``` Action failed: fbsource//xplat/tools/logginginfra/id_name_mapping:events_validation_genrule (genrule) Local command returned non-zero exit code 1 Stderr: The following events: social_super_agent_client_event are not in TAL_EVENTS_TO_ENUMS. Make sure you have ran xplat/messenger/tal/tool/sync/sync.sh ``` That is a stale TAL sync in the base revision, not cache-upload fallout. Every ARVR-touching job builds that genrule, which is why the correlation with this change looked exact. Note that fbcode's default `python_binary` doesn't reach these actions at all (`use_rust_make_par` sends it to `lpar_gen_bootstrap` instead), which is why fbcode CI is unlikely to tell us anything either way. Inspired by #1444 (imported as D114816218). Reviewed By: itamaro Differential Revision: D114911004 fbshipit-source-id: c29d4a791eab72c7d5868ab520db3046526e7840
Summary: ## Stack context `targets --streaming` command: speeding it up and making it use less memory ## This diff Bound the full streaming pipeline, not just target loading. Previously, `--num-threads` limited active loads, but up to one million packages could still queue for formatting, hashing, and output. This caps that outer queue at `2 * --num-threads`, so memory no longer grows with `-j` (about 10.7 GiB from `-j 45` through `-j 360` in the stack benchmark). It also rejects `-j 0` instead of hanging and extends the streaming keep-going test to cover `--no-cache`. Full-stack benchmarks and output validation: D114013678. Reviewed By: JakobDegen Differential Revision: D114013686 fbshipit-source-id: f2da2ceeee36e92c1042e92e9a41bb8eea4e8dec
…name Summary: The ingress tailer records each distinct configuration it sees into the `buck2_platforms` XDB table, deduplicating through a per-task in-memory cache. Because these events are sharded like everything else - by trace id - every one of the ~1200 tailer tasks sees every popular configuration, and re-attempts the `INSERT` for all of them whenever a push wipes its cache (which currently happens fleet-wide, daily). That amounts to ~300M duplicate-key `INSERT IGNORE` attempts per day against the table's uniqueness index (~99.9% of all insert attempts - measurable as the gap between the table's `AUTO_INCREMENT` burn rate and its actual row growth), all of it point-reads and lock traffic on the master. It scales with fleet size times configuration churn, and it was a major contributor to the xdb.buck2 overload during the Aug 10 ingress rollout. Keying these messages by configuration name instead gives each configuration a single owning consumer shard, which makes the existing per-task dedupe globally effective: fleet-wide insert attempts drop by ~1000x, to roughly the distinct-configuration rate. The win phases in as clients pick up the change; old clients keep keying by trace id, which stays correct - the tailer's per-task dedupe and `INSERT IGNORE` remain the correctness floor, this only determines how much duplicate traffic reaches the database. Reviewed By: 8Keep Differential Revision: D115527221 fbshipit-source-id: b44bbdaa46719f7f719d1a9a61152351ec24de1a
Summary: version: austin's fork, occurred on version 98bf97c # what was i doing i was building a small pile of golang on macOS in my [dotfiles](https://github.com/lf-/dotfiles) on rev 165b13169df097572d80f1b368358ce99d9dae83. this was basically `configs/nix/packages2/buck2-austin/buck2 build //...` on a mac with buildbuddy set up in the buckconfig.local. ``` [buck2_re_client] # local -- ignore RE entirely # cached -- run everything here, read and write BuildBuddy's cache # remote -- also dispatch this machine's own actions, which needs BuildBuddy # to have workers for this machine's OS (its hosted pool is Linux # only, so on a Mac this wants to stay "cached") default_mode = cached http_headers = x-buildbuddy-api-key:.... ``` symptom: ``` 2026-08-05T06:32:25.438240Z WARN buck2_execute::execute::cache_uploader: Cache upload for `a8fcdf6cb485bcf81f19a5f3b373cb5145f4a55a4a809cd366663afbfe6d7b47:141` failed: Error uploading outputs: Remote Execution Error on upload_files_and_directories for ReSession GRPC-SESSION-ID Error: (Batch upload failed: ["Unable to upload blob 'd8da673f46b4e0012afabdfa5a8b10947e253de4c07f88ec6c00b29efc68d9f7', rpc status code: 3, message: \"Uploaded bytes checksum (\"dc25da770d7cd25f0aa6d9e30944beda7a755c40d1ad0a75629ba10aa06572ec\") did not match digest (\"d8da673f46b4e0012afabdfa5a8b10947e253de4c07f88ec6c00b29efc68d9f7\").\""]) ``` # cause uhh. it appears that there are multiple actions using `__action_0_239__` in the `output_artifacts` unbound dir that execute close together in time in my log. that is a really suspicious combination with the offending hashes appearing in the same spot in the output directory. it vaguely implies that these got switcheroo'd. ``` $ sha256sum buck-out/v2/art/prelude/go/tools/__stdlib__/__action___0_239__/*/go_compile_out_non-shared.x d8da673f46b4e0012afabdfa5a8b10947e253de4c07f88ec6c00b29efc68d9f7 buck-out/v2/art/prelude/go/tools/__stdlib__/__action___0_239__/d8da673f46b4e001/go_compile_out_non-shared.x dc25da770d7cd25f0aa6d9e30944beda7a755c40d1ad0a75629ba10aa06572ec buck-out/v2/art/prelude/go/tools/__stdlib__/__action___0_239__/dc25da770d7cd25f/go_compile_out_non-shared.x dc25da770d7cd25f0aa6d9e30944beda7a755c40d1ad0a75629ba10aa06572ec buck-out/v2/art/prelude/go/tools/__stdlib__/__action___0_239__/output_artifacts/go_compile_out_non-shared.x ``` ``` build prelude//go/tools:stdlib (root//platforms:arm64-linux#ce8c0bf00c0f8d33) (go_compile go/constant [non-shared]) local env -C "$(buck2 root --kind project)" -- 'TMPDIR=/Users/jade/.dotfiles/buck-out/v2/tmp/prelude/76143a280e33f102/go_compile/_buck_4b3f4d306503fdba' 'BUCK_SCRATCH_PATH=buck-out/v2/tmp/prelude/76143a280e33f102/go_compile/_buck_4b3f4d306503fdba' 'GOARCH=arm64' 'GOEXPERIMENT=' 'GOOS=linux' 'BUCK2_DAEMON_UUID=fa36071c-d237-4b01-93b9-414c650f0944' 'BUCK_BUILD_ID=6bbcea1a-8092-4757-9c26-19249495dd64' buck-out/v2/art/prelude/go/tools/__go_wrapper__/7d251197f13eaba5/go_wrapper --go buck-out/v2/art/toolchains/23970406e468f12c/__go__archive_darwin_arm64__/go__archive_darwin_arm64/pkg/tool/darwin_arm64/compile -- '-buildid=' -nolocalimports -pack -trimpath '%cwd%' -p go/constant -importcfg buck-out/v2/art/prelude/go/tools/__stdlib__/__action___0_239__/6c4954525cd52d22/non_shared.importcfg -o buck-out/v2/art/prelude/go/tools/__stdlib__/__action___0_239__/output_artifacts/go_compile_out_non-shared.x -linkobj buck-out/v2/art/prelude/go/tools/__stdlib__/__action___0_239__/output_artifacts/go_compile_out_non-shared.a -complete -std buck-out/v2/art/prelude/go/tools/__stdlib__/__action___0_239__/41e842d4be79d281/non-shared_srcs.go_package_argsfile build prelude//go/tools:stdlib (root//platforms:arm64-macos#a8115d6e81a1e6cd) (go_compile go/scanner [non-shared]) local env -C "$(buck2 root --kind project)" -- 'TMPDIR=/Users/jade/.dotfiles/buck-out/v2/tmp/prelude/76143a280e33f102/go_compile/_buck_fe74aecdf98c69d8' 'BUCK_SCRATCH_PATH=buck-out/v2/tmp/prelude/76143a280e33f102/go_compile/_buck_fe74aecdf98c69d8' 'GOARCH=arm64' 'GOEXPERIMENT=' 'GOOS=darwin' 'BUCK2_DAEMON_UUID=fa36071c-d237-4b01-93b9-414c650f0944' 'BUCK_BUILD_ID=6bbcea1a-8092-4757-9c26-19249495dd64' buck-out/v2/art/prelude/go/tools/__go_wrapper__/7d251197f13eaba5/go_wrapper --go buck-out/v2/art/toolchains/23970406e468f12c/__go__archive_darwin_arm64__/go__archive_darwin_arm64/pkg/tool/darwin_arm64/compile -- '-buildid=' -nolocalimports -pack -trimpath '%cwd%' -p go/scanner -importcfg buck-out/v2/art/prelude/go/tools/__stdlib__/__action___0_239__/815d7a8bbe1475fb/non_shared.importcfg -o buck-out/v2/art/prelude/go/tools/__stdlib__/__action___0_239__/output_artifacts/go_compile_out_non-shared.x -linkobj buck-out/v2/art/prelude/go/tools/__stdlib__/__action___0_239__/output_artifacts/go_compile_out_non-shared.a -complete -std buck-out/v2/art/prelude/go/tools/__stdlib__/__action___0_239__/2d5820e224d4e2a5/non-shared_srcs.go_package_argsfile ``` # commentary and diagnosis here's where it gets a path: https://github.com/facebook/buck2/blob/42ed7efc3e366bba83297840bfc7c00cff19d966/app/buck2_execute_impl/src/executors/caching.rs#L366-L373 here's how the path is formed: https://github.com/facebook/buck2/blob/065afcb59b519dc0e766d3a72d02969d7c045cdc/app/buck2_execute/src/execute/request.rs#L870-L875 and: https://github.com/facebook/buck2/blob/d57d17ccc18b2b427bb4fe7fb390428bfa357770/app/buck2_core/src/fs/buck_out_path.rs#L289-L307 my thoughts: - hm, these are two actions in two different configurations (note the different targets) being executed locally at the same time. i would suspect this happens at fb; it happened for me bc of a mac->linux config transition. hypothesis as to why y'all didn't notice: maybe it happens at fb but cas doesn't do secondary verification of the hash and there's corruption? that sounds scary and unlikely, but idk maybe! or, there's not enough obervability of the log to notice the errors? [it looks like the logs get trashed so maybe there's something to that](https://github.com/facebook/buck2/blob/01b3d9aaddeddca76413ea91bb41db58f9e5bbb7/app/buck2_execute/src/re/client.rs#L989-L990). - it seems to be a regression in fff2bbf (D75866194). that diff isn't *wrong*, either: it's correct to name the uploaded action after the output_artifacts path since that's what action execution actually did. however, consuming the placeholder path in the uploader *while not holding appropriate locks on the path* is busted, since the `output_artifacts` path could get overwritten by concurrently-executing actions. there are locks (see NamedSemaphores) but they only affect run actions and are [relinquished after the run action finishes](https://github.com/facebook/buck2/blob/2f411eba848cf31899ecc89a0e101f5dd349a27a/app/buck2_execute_impl/src/executors/local.rs#L1293-L1294) since they're only held locally in exec_cmd. thus, we have a race. this is very #1309 shaped: it's the same bug class where the host sharing permits are only used for run actions and aren't held elsewhere. in this case, it's that the *uploader* was relying on the invariants of that lock without actually holding it: it was using the `output_artifacts` paths for both picking what to upload and actually pulling it off disk. proposed fix: uploading should fetch off disk from the content-based path while still telling the remote server about the placeholder path as before. this avoids needing to hold the lock for no good reason, while still being correct. i wrote this fix with claude but i rewrote all its comments and i am pretty confident in my understanding of the bug based on my own inspection of the code. i am not 100% convinced about the need to ask the materializer to ensure all the content-based paths, but it's almost certainly harmless since they are almost certainly already on disk. Pull Request resolved: #1454 Test Plan: i changed my go toolchain version to cause an invalidation then tried running the command that got *several* failures again on the fresh build. seems to not happen again. Reviewed By: 8Keep Differential Revision: D115404825 fbshipit-source-id: bd239cd9d9188dfe5f91d8212a97190936628c48
Differential Revision: D115578518 fbshipit-source-id: 272990318cb4f11c160354f9214bafa253edff53
Reviewed By: sriramramani Differential Revision: D114983545 fbshipit-source-id: 72edccbc37a09cff32b6e10174b5c9197189d966
Summary: Original commit changeset: 66fd22811cc7 Original Phabricator Diff: D115078736 Too many tests hardcode the resource load path to fbsource root, fix forward requires fixing test code and would take too long. Revert for now. Differential Revision: D115648995 fbshipit-source-id: e76d1998d805f75883c9a5afe69713ef6fcaad87
Summary:
## Stack context
This substack follows up on JakobDegen's comment in D114815951.
### Why retire `StarlarkAttrs`?
`StarlarkAttrs` uses a derive and a helper macro to expose Rust fields to Starlark:
```rust
#[derive(StarlarkAttrs)]
struct Foo {
name: String,
count: i64,
#[starlark(skip)]
Reviewed By: JakobDegen
Differential Revision: D115230112
fbshipit-source-id: 50a8e81536b3b4e2baaa21d4c93df4da0e895646
Summary: Remove the public `StarlarkAttrs` derive and `starlark_attrs!` helper. They implicitly exposed every unskipped Rust field, cloned every non-`String` field, and allocated the result on every attribute lookup without supplying exact type metadata or documentation. This is a breaking API change for the next 0.15.0 release. Callers should define a `#[starlark_module]` initializer with explicit `#[starlark(attribute)]` functions and return its generated `<FUNCTION>_STATICS` table from `StarlarkValue::get_methods`. The public `get_methods` rustdoc now includes this migration pattern. Expensive values should be preallocated and stored when getters would otherwise allocate. Keep D114815951 independently covered with a synthetic item macro that expands to consistent `get_attr` and `dir_attr` implementations; attribute access from a statically checked `def` still verifies generic macro-item handling. Stack context: This is the final API removal in the four-diff `StarlarkAttrs` retirement substack. See D115230112 for the motivation, benefits, and full substack overview. Reviewed By: JakobDegen Differential Revision: D115115594 fbshipit-source-id: f684e48f1197a7dda5e3580480c1b24f22ef9cf6
Summary: ## Motivation `ConflictingHeapBinding` reported only the `HeapRefId`: ``` Heap HeapRefId(4165443088957135852) is already bound to a different heap in this page-in scope ``` `HeapRefId` is a blake3 hash of the heap's `FrozenHeapName` (`HeapRefId::from_heap_name`), so it cannot be inverted. A production report therefore says that *some* heap has two live allocations under one identity, but not which one — and that is the single most useful fact, because it distinguishes a `.bzl` module heap (`StarlarkEvalKind::Load(..)`) from the globals heap (`globals(..::global_env)`) from a singleton such as `host_info`. Identifying the heap behind a `dice_hydration_failed` report currently requires recomputing candidate name hashes offline. The two allocations are also worth reporting: they are what makes the binding ambiguous, and their addresses distinguish "two generations of one name" from a repeated registration of the same allocation. ## Implementation - `PagableError::ConflictingHeapBinding` gains `heap_name`, `bound_heap_ptr` and `conflicting_heap_ptr`. The pre-existing message text is kept as a prefix so existing matchers on it keep working. - Both raise sites (`StarlarkDeserScope::register_heap` and `is_heap_bound`) already upgrade the conflicting weak ref to test liveness, so the name is recovered from that upgraded `FrozenHeapRef` with no extra work and no signature changes. - The error is constructed in a `#[cold]` helper: naming the heap allocates a `String`, which the non-conflicting path must not pay for. - `FrozenHeapPtr::addr` exposes the address for diagnostics only. This is a diagnostic-only change; no binding behaviour changes. Reviewed By: christolliday Differential Revision: D115657361 fbshipit-source-id: b6d07a8b8fd8a94573c833060e6e2dc66af83831
Summary: Add `LinkerInfo.runtime_library_files` (optional, defaults to `[]`): the `LinkInfo`s of runtime libraries a toolchain provides. The prelude links each into every cxx link via the shared `link_info_to_args`, toward the end of the link line, matching the Clang driver's placement of compiler-rt. `link_info_to_args` handles loose objects (`--start-lib`/`--end-lib`) or an archive uniformly, so this reuses the same conversion every other library link uses instead of re-implementing it. Populated inline from the toolchain's deps (each dep's `pic_archive` LinkInfo). Empty by default, so toolchains that don't set it are unaffected. The consumer is responsible for stopping the driver from also injecting these (e.g. `-nodefaultlibs`); the prelude only links them. gnu-only -- fails for other linker types and for distributed thin-lto (no consumer today). Reviewed By: smeenai, bcardosolopes Differential Revision: D114532036 fbshipit-source-id: 4e4effc8281874f9c215d7baa9cc8275e39669bb
Summary: Absolute filepaths can be included into the binary unless `-trimpath` provided Reviewed By: echistyakov Differential Revision: D115659891 fbshipit-source-id: b8d04959ed5e83158836fcbb2af89508a8a99946
… classes Summary: Kosabi generates source-only ABI with JVMConfigurationKeys.SKIP_BODIES=true, so fir2ir drops constructor bodies (including the delegating constructor call). Under Kotlin 2.3 this breaks the JVM backend that Kosabi's ABI pipeline runs: - Enum primary constructors arrive body-less -> ExpressionCodegen throws "Function has no body". - The synthetic default-parameter constructor (FUNCTION_FOR_DEFAULT_PARAMETER) still carries Kosabi's skipBodies IrErrorExpression placeholders that codegen cannot emit. - Inner-class constructors run through InnerClassesLowering, which asserts the body contains an IrDelegatingConstructorCall (it rewrites that call to thread the outer this). A body-less inner constructor is skipped by the lowering but then fails codegen; an empty stub body trips the "Delegating constructor call expected" assertion. IrNonAbiStripper.visitConstructor now: - stubs body-less / FUNCTION_FOR_DEFAULT_PARAMETER constructors with an empty block for non-inner classes (ABI jars are compile-classpath-only, only signatures matter), and - for inner-class constructors missing a delegating call, synthesizes a trivial delegation to the superclass (defaulting to kotlin.Any) via DeclarationIrBuilder.irDelegatingConstructorCall so InnerClassesLowering has a call to rewrite. ``` java.lang.RuntimeException: Exception while generating code for: CONSTRUCTOR visibility:public returnType:com.whatsapp.infra.protocol.peerwatermark.ReadSelfWatermark [primary] VALUE_PARAMETER kind:Regular name:watermarkMap index:0 type:kotlin.collections.Map<com.whatsapp.infra.core.jid.ChatJid, com.whatsapp.infra.protocol.peerwatermark.WatermarkEntry> at org.jetbrains.kotlin.backend.jvm.codegen.FunctionCodegen.generate(FunctionCodegen.kt:57) at org.jetbrains.kotlin.backend.jvm.codegen.FunctionCodegen.generate$default(FunctionCodegen.kt:50) at org.jetbrains.kotlin.backend.jvm.codegen.ClassCodegen.generateMethodNode(ClassCodegen.kt:428) at org.jetbrains.kotlin.backend.jvm.codegen.ClassCodegen.generateMethod(ClassCodegen.kt:445) at org.jetbrains.kotlin.backend.jvm.codegen.ClassCodegen.generate(ClassCodegen.kt:168) at org.jetbrains.kotlin.backend.jvm.JvmIrCodegenFactory.generateFile(JvmIrCodegenFactory.kt:467) at org.jetbrains.kotlin.backend.jvm.JvmIrCodegenFactory.invokeCodegen(JvmIrCodegenFactory.kt:433) at org.jetbrains.kotlin.cli.jvm.compiler.legacy.pipeline.JvmCompilerPipelineKt.generateCodeFromIr(jvmCompilerPipeline.kt:153) at com.facebook.PipelineCompatKt.generateCodeFromIrCompat(PipelineCompat.kt:68) at com.facebook.K2JvmAbiFirAnalysisHandlerExtension.runBackendForKosabi-c8A5__o(K2JvmAbiFirAnalysisHandlerExtension.kt:575) at com.facebook.K2JvmAbiFirAnalysisHandlerExtension.doAnalysis(K2JvmAbiFirAnalysisHandlerExtension.kt:170) at org.jetbrains.kotlin.fir.extensions.FirAnalysisHandlerExtension$Companion.analyze(FirAnalysisHandlerExtension.kt:29) at org.jetbrains.kotlin.cli.pipeline.jvm.JvmFrontendPipelinePhase.executePhase(JvmFrontendPipelinePhase.kt:83) at org.jetbrains.kotlin.cli.pipeline.jvm.JvmFrontendPipelinePhase.executePhase(JvmFrontendPipelinePhase.kt:55) at org.jetbrains.kotlin.cli.pipeline.PipelinePhase.phaseBody(PipelinePhase.kt:68) at org.jetbrains.kotlin.cli.pipeline.PipelinePhase.phaseBody(PipelinePhase.kt:58) at org.jetbrains.kotlin.config.phaser.NamedCompilerPhase.invoke(CompilerPhase.kt:102) at org.jetbrains.kotlin.backend.common.phaser.CompositePhase.invoke(PhaseBuilders.kt:22) at org.jetbrains.kotlin.config.phaser.CompilerPhaseKt.invokeToplevel(CompilerPhase.kt:53) at org.jetbrains.kotlin.cli.pipeline.AbstractCliPipeline.runPhasedPipeline(AbstractCliPipeline.kt:109) at org.jetbrains.kotlin.cli.pipeline.AbstractCliPipeline.execute(AbstractCliPipeline.kt:68) at org.jetbrains.kotlin.cli.jvm.K2JVMCompiler.doExecutePhased(K2JVMCompiler.kt:51) at org.jetbrains.kotlin.cli.jvm.K2JVMCompiler.doExecutePhased(K2JVMCompiler.kt:42) at org.jetbrains.kotlin.cli.common.CLICompiler.execImpl(CLICompiler.kt:91) at org.jetbrains.kotlin.cli.common.CLICompiler.exec(CLICompiler.kt:363) at org.jetbrains.kotlin.buildtools.internal.CompilationServiceImpl.compileInProcess(CompilationServiceImpl.kt:227) at org.jetbrains.kotlin.buildtools.internal.CompilationServiceImpl.compileJvm(CompilationServiceImpl.kt:113) at org.jetbrains.kotlin.buildtools.internal.CompilationServiceProxy.compileJvm(CompilationServiceImpl.kt) at com.facebook.buck.jvm.kotlin.buildtools.KotlinCompilationService.compile(KotlinCompilationService.kt:46) at com.facebook.buck.jvm.kotlin.buildtools.BuildToolsKotlinc.buildWithClasspath(BuildToolsKotlinc.kt:93) at com.facebook.buck.jvm.kotlin.KotlincStep.executeIsolatedStep(KotlincStep.java:159) at com.facebook.buck.step.isolatedsteps.IsolatedStepsRunner.runStep(IsolatedStepsRunner.java:70) at com.facebook.buck.step.isolatedsteps.IsolatedStepsRunner.execute(IsolatedStepsRunner.java:37) at com.facebook.buck.step.isolatedsteps.IsolatedStepsRunner.executeWithDefaultExceptionHandling(IsolatedStepsRunner.java:57) at com.facebook.buck.jvm.cd.CompilerDaemonRunner.execute(CompilerDaemonRunner.java:83) at com.facebook.buck.jvm.cd.CompilerDaemonRunner.run(CompilerDaemonRunner.java:102) at com.facebook.buck.jvm.kotlin.cd.workertool.KotlinCDMain.main(KotlinCDMain.java:46) at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104) at java.base/java.lang.reflect.Method.invoke(Method.java:565) at com.facebook.buck.cli.bootstrapper.ClassLoaderBootstrapperUtils.invokeMainMethod(ClassLoaderBootstrapperUtils.java:24) at com.facebook.buck.cli.bootstrapper.ClassLoaderBootstrapper.main(ClassLoaderBootstrapper.java:47) Caused by: java.lang.IllegalStateException: Function has no body: CONSTRUCTOR visibility:public <> (watermarkMap:kotlin.collections.Map<com.whatsapp.infra.core.jid.ChatJid, com.whatsapp.infra.protocol.peerwatermark.WatermarkEntry>) returnType:com.whatsapp.infra.protocol.peerwatermark.ReadSelfWatermark [primary] at org.jetbrains.kotlin.backend.jvm.codegen.ExpressionCodegen.generate(ExpressionCodegen.kt:227) at org.jetbrains.kotlin.backend.jvm.codegen.FunctionCodegen.doGenerate(FunctionCodegen.kt:135) at org.jetbrains.kotlin.backend.jvm.codegen.FunctionCodegen.generate(FunctionCodegen.kt:54) ... 40 more ``` Reviewed By: siaojiecai Differential Revision: D111225994 fbshipit-source-id: b1ceb78ffa5593b867d1a7df130b3f00c11dc440
Summary: Summary hand-written by me: Note D114768660 fixed a bug where some synthetic classes were not packaged in the apk. We still have an issue with dev-cov runs of instrumentation tests on javac `-target 11` (D114791392) which I need to fix. The specific issue is: - with java11 bytecode, Jacoco instruments using a new `ConstantDynamic` instruction - for compatibility with Android, D8 desugars this into a synthetic - BUT our primary dex pattern matching tends to only match explicit classes (Note we have a handful of explicit matches on Synthetic naming e.g. https://fburl.com/code/vczv3ntu, but this is not scalable) - so we may wrongly package a synthetic class into a secondary dex which can break startup Other solutions I considered: - Update primary_dex_patterns to accommodate all possible synthetic names for instrumented classes (this seems stupid and not scalable) - Update primary_dex_patterns to make matchers more lenient to match `com/example/Foo.*` instead of `^com/example/Foo$` for example, which would also fix the problem but could lead to undesirable over-including - Update `filter_dex.py` to guess whether a synthetic class name should match I didn't really like any of these - they don't feel robust so this is what I'm proposing: - Have D8 report all synthetic names from their context via the `SyntheticInfoConsumer` visitor API - `filter_dex` consumes this mapping to determine whether a class should go into primary dex without guessing at synthetic class names. ___________ Note No behavior change in this diff - see next diff for consuming. I just split this for easier reviewing. Reviewed By: agampe Differential Revision: D115448400 fbshipit-source-id: ca0795d3d3ff5c9005bf72f92ca5008cce259059
…ntext map Summary: Summary by me: See previous diff for explanation. This diff consumes the map so we correctly package all `com/example/Foo` and its synthetic classes (which can have various names) without putting the burden on primary_dex_patterns to encode that information. Reviewed By: wsanville Differential Revision: D115250365 fbshipit-source-id: 5e7dd89344c16a45ea54fc80e52af92ec2fd9a3a
Summary: Starlark call-stack frames and compiler spans can, rarely, carry a `Span` that does not lie within their own `CodeMap`. Resolving one goes through `CodeMap::find_line` (an `assert!`) or `CodeMap::source_span` (a raw slice), and either panics — killing the buck2 daemon mid-command: in production while compiling BUCK files (`Compiler::eval_module`), and locally while formatting an unrelated PACKAGE-file error. P2457607755 catalogs the production occurrences and the open root-cause leads. The root cause is unknown, so this diff has three goals: stop the crashes, make each occurrence visible and auditable, and record enough detail to be a clue later. Stop the crashes. `CodeMap::find_line`, `find_line_col`, and `source_span` clamp instead of panicking: a position past the end resolves to the last line, and a span that is not a character range grows to the enclosing character boundaries rather than yielding nothing. Make them visible. Each clamp reports through a new reporter hook in `starlark_syntax`, which `starlark` bridges to a process-global `SoftErrorHandler` slot (`set_global_soft_error_handler`), which buck2 installs in `init_late_bindings` as the existing `Buck2StarlarkSoftErrorHandler` — so occurrences land in Scuba rather than vanishing. Contexts holding an `Evaluator` keep using the evaluator-scoped handler; the global slot exists for span resolution and freeze-time optimization, where none is in reach. `FrozenFileSpan::new` reports at construction too, catching corruption at the moment a bad pairing is built rather than when it is next read, and `Frame::write_two_lines` prints a self-describing line in place of the source snippet. Record clues. Reports name the resolution path, the offending position or span, the file length, and whether the `CodeMap` is a real source file or a native one — that last distinction matters, because a span resolved against a native codemap it cannot belong to is what a pointer landing on unrelated memory looks like, `<native>` being a fixed 8-byte source. Reporting is capped at eight occurrences per process, since resolution is hot and one corrupt span is typically resolved many times, but is not capped at one, so several distinct corruptions stay visible. Debug builds still fail fast at `FrozenFileSpan::new`. This intentionally does not fix the corruption itself. This is a mitigation, not a feature: a `TODO(jtbraun)` at the reporting machinery in `codemap.rs` marks the whole diff for back-out once the invalid spans are root caused. That is also why resolution clamps rather than surfacing "invalid position" through the resolvers' return types — sentinel-bearing signatures would thread through every consumer and make this hard to remove. Reviewed By: JakobDegen Differential Revision: D115579787 fbshipit-source-id: ec52e040c65e842e31fc91154823eb65c53f1a43
Summary: `fileReady` is documented as "installs an artifact from a given path", but it was not called until *every* artifact for the install had been received -- all of them were then dispatched at once. So an implementation could not tell when any particular artifact turned up, and anything it wanted to do per artifact was deferred until there was nothing left to overlap it with. In this change: - Hand each artifact over as it arrives, and run the install once every artifact has arrived and none of them failed. - Anything thrown while handing an artifact over becomes a tagged error in the response, rather than escaping as a bare gRPC failure that loses the `InstallErrorTag`. - Whether the install runs no longer depends on which artifact happened to arrive last. It runs when all of them are in and none failed; before, per-artifact errors were collected and the install ran regardless, so a failed ABI compatibility check did not stop it. Reviewed By: siaojiecai Differential Revision: D115225435 fbshipit-source-id: 4a04b1f77208c3d76da662088d8baeb7b9e29c08
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.