buck2_test: add an allowlist for env-vars in buckconfig - #28
Open
lf- wants to merge 29 commits into
Open
Conversation
I have a highly trivial Manifold implementation internally at Mercury (https://github.com/mercurytechnologies/locally-euclidean) for which I would like to set up for buck2 to receive rage and all that at. This allows for fully implementing the build logs feature mentioned in facebook#441. There's still a buck2_core::facebook_only() or two that need inspected/removed and some more tidying.
> Prefer using the short name when referring to an eponymous target (`//x` instead of `//x:x`). If > you are in the same package, prefer the local reference (`:x` instead of `//x`). See: https://bazel.build/build/style-guide#target-naming This syntax is supported on the command-line, but not in BUCK files. Unfortunately, the only(?) Starlark formatter (buildifier: https://github.com/bazelbuild/buildtools) automatically abbreviates targets on the assumption that this syntax is valid, which causes errors when using Buck2: ... 4: Error coercing "//src/Foo" 5: Invalid absolute target pattern `//src/Foo` is not allowed 6: Expected a `:`, a trailing `/...` or the literal `...`. We can adjust the parsing rules to allow this syntax in more places.
Only show test output if errors.
Can be overridden with:
buck test --test-output=(all|none|error) ...
Avoids re-running passed tests. Only compatible with DICE (local only).
…e-commit-due-to-buck-starlark-lint-failure-concurrency make buck external cells more reliable facebook#1259
Avoids needing to rebase the entire patch of facebook#1100, since we don't care about supporting cell segmentation at Mercury anyway.
facebook#1221 Note: modified during rebase to reconcile with prior patches (pr1076) which partially overlapped with this PR. Also adds execution_concurrency_limit and grpc_timeout config fields which were assumed present by the upstream diff.
experimental patch by joseph which we may try to upstream later if it works
use BlockingExecuter = BuckBlockingExecutor::default_concurrency(fs.dupe()) for macos
Patch hyper with arianvp's fork and tonic/tonic-prost/tonic-prost-build with edef1c's fork until these changes make it upstream.
) * Add debugging output for RE actions * Print method for RPC retries * Track and retry message received from stream During stream reading, there was no retry mechanism in place, so seeing many `ResourceExhausted` errors would lead h2 issuing a GOWAWY to the server, effectively closing the connection. Now, stream responses are also retried by wrapping both, stream establishment and stream reading in a retry loop. * Do not set a timeout on the gRPC message itself This is otherwise enforced by the server which cancels the action, which wo don't want. * Do not enforce a timeout for Execute requests
If you run `buck2` in a `pre-commit` hook, then the inherited `GIT_DIR` and similar can cause `buck2` to run destructive commands like `git reset --hard` in the wrong repository!
* Merge pull request #13 from MercuryTechnologies/hdgarrood/push-rlpkyxwyzskk Allow source labels to omit eponymous target names * buck2_server: Implement Git revision data What it says on the tin, closes a long-standing TODO and helps with our telemetry. TODO: Should we just reuse the `hg_revision` field for Git revisions? They _are_ also 40-character hashes... Upstream: - facebook#1346 --------- Co-authored-by: Harry Garrood <harry@garrood.me>
Unnecessary given validation in TransitiveSet:new. Already upstreamed.
The OpenTelemetry exporter we're using was added with `default-features = false`, which adds `reqwest` without a TLS stack, which ends up breaking OpenTelemetry export in our current CI setup. Follow-up to #14 `cargo tree` confirms that we're using the system store: ``` $ cargo tree --edges=normal --invert=rustls-platform-verifier --package=buck2_core rustls-platform-verifier v0.7.0 └── reqwest v0.13.4 ├── opentelemetry-http v0.32.0 │ └── opentelemetry-otlp v0.32.0 │ └── buck2_core v0.1.0 (/Users/wiggles/buck2-2/app/buck2_core) └── opentelemetry-otlp v0.32.0 (*) ``` See: - https://github.com/rustls/rustls-platform-verifier - "A certificate verification library for rustls that uses the operating system's verifier" <details><summary><code>Cargo.lock</code> diff</summary> ```diff --- /Users/wiggles/mwb/nix/packages/buck2-source/Cargo.lock 2026-07-06 10:44:58.618882289 -0700 +++ Cargo.lock 2026-07-06 11:50:41.817123538 -0700 @@ -3270,6 +3270,17 @@ checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" [[package]] +name = "chacha20" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d524456ba66e72eb8b115ff89e01e497f8e6d11d78b70b1aa13c0fbd97540a81" +dependencies = [ + "cfg-if", + "cpufeatures 0.3.0", + "rand_core 0.10.1", +] + +[[package]] name = "chrono" version = "0.4.44" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -3376,6 +3387,16 @@ checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570" [[package]] +name = "combine" +version = "4.6.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba5a308b75df32fe02788e748662718f03fde005016435c444eea572398219fd" +dependencies = [ + "bytes", + "memchr", +] + +[[package]] name = "common-path" version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -4642,8 +4663,10 @@ checksum = "ff2abc00be7fca6ebc474524697ae276ad847ad0a6b3faa4bcb027e9a4614ad0" dependencies = [ "cfg-if", + "js-sys", "libc", "wasi", + "wasm-bindgen", ] [[package]] @@ -4665,11 +4688,13 @@ checksum = "0de51e6874e94e7bf76d726fc5d13ba782deca734ff60d5bb2fb2607c7406555" dependencies = [ "cfg-if", + "js-sys", "libc", "r-efi 6.0.0", "rand_core 0.10.1", "wasip2", "wasip3", + "wasm-bindgen", ] [[package]] @@ -5379,6 +5404,55 @@ ] [[package]] +name = "jni" +version = "0.22.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5efd9a482cf3a427f00d6b35f14332adc7902ce91efb778580e180ff90fa3498" +dependencies = [ + "cfg-if", + "combine", + "jni-macros", + "jni-sys", + "log", + "simd_cesu8", + "thiserror 2.0.18", + "walkdir", + "windows-link 0.2.1", +] + +[[package]] +name = "jni-macros" +version = "0.22.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a00109accc170f0bdb141fed3e393c565b6f5e072365c3bd58f5b062591560a3" +dependencies = [ + "proc-macro2", + "quote", + "rustc_version", + "simd_cesu8", + "syn 2.0.117", +] + +[[package]] +name = "jni-sys" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c6377a88cb3910bee9b0fa88d4f42e1d2da8e79915598f65fb0c7ee14c878af2" +dependencies = [ + "jni-sys-macros", +] + +[[package]] +name = "jni-sys-macros" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "38c0b942f458fe50cdac086d2f946512305e5631e720728f2a61aabcd47a6264" +dependencies = [ + "quote", + "syn 2.0.117", +] + +[[package]] name = "jobserver" version = "0.1.34" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -5677,6 +5751,12 @@ ] [[package]] +name = "lru-slab" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "112b39cec0b298b6c1999fee3e31427f74f676e4cb9879ed1a121b43661a4154" + +[[package]] name = "lsp-server" version = "0.7.9" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -6935,6 +7015,63 @@ ] [[package]] +name = "quinn" +version = "0.11.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0c1a41e437b6bbd489372cd4971de128e85c855f56c57f283d20ff016cf7c0a8" +dependencies = [ + "bytes", + "cfg_aliases 0.2.1", + "pin-project-lite", + "quinn-proto", + "quinn-udp", + "rustc-hash", + "rustls", + "socket2", + "thiserror 2.0.18", + "tokio", + "tracing", + "web-time", +] + +[[package]] +name = "quinn-proto" +version = "0.11.16" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2f4bfc015262b9df63c8845072ce59068853ff5872180c2ce2f13038b970e560" +dependencies = [ + "aws-lc-rs", + "bytes", + "getrandom 0.4.2", + "lru-slab", + "rand 0.10.1", + "rand_pcg", + "ring", + "rustc-hash", + "rustls", + "rustls-pki-types", + "slab", + "thiserror 2.0.18", + "tinyvec", + "tracing", + "web-time", +] + +[[package]] +name = "quinn-udp" +version = "0.5.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "35a133f956daabe89a61a685c2649f13d82d5aa4bd5d12d1277e1072a21c0694" +dependencies = [ + "cfg_aliases 0.2.1", + "libc", + "once_cell", + "socket2", + "tracing", + "windows-sys 0.52.0", +] + +[[package]] name = "quote" version = "1.0.45" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -6992,6 +7129,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d2e8e8bcc7961af1fdac401278c6a831614941f6164ee3bf4ce61b7edb162207" dependencies = [ + "chacha20", "getrandom 0.4.2", "rand_core 0.10.1", ] @@ -7061,6 +7199,15 @@ ] [[package]] +name = "rand_pcg" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "caa0f4137e1c0a72f4c651489402276c8e8e1cf081f3b0ba156d2cbeef09e86a" +dependencies = [ + "rand_core 0.10.1", +] + +[[package]] name = "rand_xorshift" version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -7280,13 +7427,19 @@ "http-body", "http-body-util", "hyper", + "hyper-rustls", "hyper-util", "js-sys", "log", "percent-encoding", "pin-project-lite", + "quinn", + "rustls", + "rustls-pki-types", + "rustls-platform-verifier", "sync_wrapper 1.0.2", "tokio", + "tokio-rustls", "tower 0.5.3", "tower-http", "tower-service", @@ -7475,10 +7628,38 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "be040f8b0a225e40375822a563fa9524378b9d63112f53e19ffff34df5d33fdd" dependencies = [ + "web-time", "zeroize", ] [[package]] +name = "rustls-platform-verifier" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "26d1e2536ce4f35f4846aa13bff16bd0ff40157cdb14cc056c7b14ba41233ba0" +dependencies = [ + "core-foundation 0.10.1", + "core-foundation-sys", + "jni", + "log", + "once_cell", + "rustls", + "rustls-native-certs 0.8.3", + "rustls-platform-verifier-android", + "rustls-webpki", + "security-framework 3.7.0", + "security-framework-sys", + "webpki-root-certs", + "windows-sys 0.52.0", +] + +[[package]] +name = "rustls-platform-verifier-android" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f87165f0995f63a9fbeea62b64d10b4d9d8e78ec6d7d51fb2125fda7bb36788f" + +[[package]] name = "rustls-webpki" version = "0.103.12" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -7834,6 +8015,22 @@ checksum = "703d5c7ef118737c72f1af64ad2f6f8c5e1921f818cdcb97b8fe6fc69bf66214" [[package]] +name = "simd_cesu8" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94f90157bb87cddf702797c5dadfa0be7d266cdf49e22da2fcaa32eff75b2c33" +dependencies = [ + "rustc_version", + "simdutf8", +] + +[[package]] +name = "simdutf8" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e3a9fe34e3e7a50316060351f37187a3f546bce95496156754b601a5fa71b76e" + +[[package]] name = "siphasher" version = "0.3.11" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -8551,6 +8748,21 @@ ] [[package]] +name = "tinyvec" +version = "1.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3e61e67053d25a4e82c844e8424039d9745781b3fc4f32b8d55ed50f5f667ef3" +dependencies = [ + "tinyvec_macros", +] + +[[package]] +name = "tinyvec_macros" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" + +[[package]] name = "tokio" version = "1.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -9207,6 +9419,25 @@ ] [[package]] +name = "web-time" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a6580f308b1fad9207618087a65c04e7a10bc77e02c8e84e9b00dd4b12fa0bb" +dependencies = [ + "js-sys", + "wasm-bindgen", +] + +[[package]] +name = "webpki-root-certs" +version = "1.0.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0d46a5a140e6f7afeccd8eae97eff335163939eac8b929834875168b29b3d267" +dependencies = [ + "rustls-pki-types", +] + +[[package]] name = "webpki-roots" version = "1.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" ``` </details>
This adds support for more OpenTelemetry environment variables: - `$OTEL_EXPORTER_OTLP_*` - `$OTEL_SDK_DISABLED` - `$OTEL_SERVICE_NAME` - `$OTEL_RESOURCE_ATTRIBUTES` - `$TRACEPARENT` - `TRACESTATE`
This integrates the Mercury `opentelemetry-rust` fork with fixes from a couple PRs: See: - open-telemetry/opentelemetry-rust#3586 - open-telemetry/opentelemetry-rust#3587 Follow-up to Arian's review: #18 (review)
Right now we run CI on `pull_request` and `push`, which means it runs twice on PRs. Let's only run it once.
…22) There are many small `BatchReadBlobs` requests, and they're large enough that compression would help. When the server advertises zstd support in its capabilities, advertise zstd as an acceptable response compressor and decompress any zstd-compressed blobs the server returns. NB: It would be nice if we could only compress blobs above a certain size, but we don't know the size when we make the request. We continue to advertise IDENTITY an acceptable response compressor, so we may need to patch our cache server to choose an appropriate size threshold above which to compress data (Bazel chooses 100 bytes, AIUI). In a moderately sized build with ~81,000 BatchReadBlobs requests, this patch shows significant benefits: - Request latency dropped: - p100: 16,653ms to 3,468ms (4.8x speedup) - p90: 953ms to 593ms - p75: 529ms to 312ms - p50: 235ms to 161ms - p25: 110ms to 96ms - p0: 67ms to 68ms - Cumulative blob size over the network dropped from 5,807MiB to 1,626MiB (3.57x compression ratio) - Blob size distribution over the wire improved: - p100: 3.7M to 741.8K - p90: 188.4K to 58.0K - p75: 103.2K to 33.9K - p50: 12.9K to 3.7K - p25: 32B to 45B (regression) - p0: 15B to 2B (regression)
Compress batched blob uploads whose `data` exceeds a configurable threshold (default 100 bytes) with zstd, when the server advertises zstd in its `supported_batch_update_compressors` capability.
This allows consumers to allowlist environment variables without patching buck itself. Concretely, my motivation on this is propagating things like OpenTelemetry endpoints into test harnesses: at Mercury we have a fancy test harness which starts postgres and stuff around tests which we would like to have observability for. We would like to *not* make that Facebook's problem, thus the buckconfig addition.
|
Are you planning to upstream this? |
Author
|
|
Ah nice okay ty. In that case I think I'm inclined to wait a little bit to see what they say? If they aren't interested then I'd rather we just amend the hardcoded list, I think |
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
7 participants
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.
This allows consumers to allowlist environment variables without patching buck itself. Concretely, my motivation on this is propagating things like OpenTelemetry endpoints into test harnesses: at Mercury we have a fancy test harness which starts postgres and stuff around tests which we would like to have observability for.
We would like to not make that Facebook's problem, thus the buckconfig addition.
A bit :vibed:. The e2e tests are completely :vibed: since we can't run them outside fb so I haven't looked hard at them at all, so feel free to hack them up.