Skip to content

DUX-5515 GHA: build-and-test: Optimize!#25

Draft
9999years wants to merge 38 commits into
mercury-headfrom
wiggles/dux-5515/wxoq
Draft

DUX-5515 GHA: build-and-test: Optimize!#25
9999years wants to merge 38 commits into
mercury-headfrom
wiggles/dux-5515/wxoq

Conversation

@9999years

Copy link
Copy Markdown
Member
  • Run lib tests with cargo-nextest

    cargo test runs one test binary at a time, which serializes badly across the ~100 crates in this workspace; nextest schedules tests from all binaries in parallel. Doctests still go through cargo test --doc since nextest does not support them.

    test.py falls back to cargo test --lib when nextest is not installed.

  • Split lint and rustdoc into parallel jobs

    clippy (~3 min) and rustdoc (~4 min) ran serially ahead of the ~20 min test phase in every build-and-test job. Move them to dedicated jobs so they overlap with the tests instead: per-platform lint jobs run test.py --lint-rust-only, and a single Linux job runs test.py --rustdoc-only (rustdoc output is platform-independent enough that running it three times bought nothing).

    Adds a --test-only flag to test.py so the build-and-test jobs skip the lint/rustdoc phases they used to run inline.

  • Cancel superseded PR runs

    A force-push previously left the old run burning three runners for the better part of an hour. Key the concurrency group on the PR number so new pushes cancel in-flight runs; pushes to mercury-head are keyed on the ref and never cancelled.

  • Use macos-latest-xlarge for macOS jobs

    macOS was the slowest platform (~50 min wall clock) and the work is compile-bound, so it scales with cores. macos-latest-xlarge is a GitHub-hosted M-series runner with double the cores of macos-latest; no org runner setup is required, but it is billed per minute.

lf- and others added 30 commits March 11, 2026 01:39
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).
Avoids needing to rebase the entire patch of facebook#1100, since we don't care about supporting cell segmentation at Mercury anyway.
Note that this has been merged upstream and can be removed the next time we update buck2.

See: facebook#1247
Note that this has been merged upstream and can be removed the next time we update buck2.

See: facebook@8c2fdd9
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
…t-crash

dont have soft errors promote to hard errors
I was surprised to learn that my code which the target exprs were used as a list no longer worked after a buck2 upgrade.
These are documented as being a list, yet they were not actually parsed
as such! https://buck2.build/docs/api/bxl/cli_args/#target_expr

```
def _collect_nix_deps(ctx):
    print(ctx.cli_args)
    pass

main = bxl.main(
    impl = _collect_nix_deps,
    cli_args = {
        "target": bxl.cli_args.target_expr("Target(s) to build and import"),
    },
    doc = """
        Extracts the direct/buck2-transitive Nix dependency list of a buck2 target into a messy json file.
    """,
)
```

Before:

```
$ cargo r -p buck2 -- bxl test.bxl:main -- --target //:foo --target //:bar
   Compiling buck2_bxl v0.1.0 (/Users/jade/co/buck2/app/buck2_bxl)
   Compiling buck2 v0.1.0 (/Users/jade/co/buck2/app/buck2)
    Finished `dev` profile [optimized + debuginfo] target(s) in 5.84s
     Running `target/debug/buck2 bxl 'test.bxl:main' -- --target '//:foo' --target '//:bar'`
buck2 daemon constraint mismatch: Version mismatch; killing daemon...
Starting new buck2 daemon...
Connected to new buck2 daemon.
error: the argument '--target <target>' cannot be used multiple times
```

After:

```
   Compiling buck2_bxl v0.1.0 (/Users/jade/co/buck2/app/buck2_bxl)
   Compiling buck2 v0.1.0 (/Users/jade/co/buck2/app/buck2)
    Finished `dev` profile [optimized + debuginfo] target(s) in 9.08s
     Running `target/debug/buck2 bxl 'test.bxl:main' -- --target '//:buck2' --target '//:buck2_bundle'`
buck2 daemon constraint mismatch: Version mismatch; killing daemon...
Starting new buck2 daemon...
Connected to new buck2 daemon.
struct(target=[gh_facebook_buck2//:buck2, gh_facebook_buck2//:buck2_bundle])
```

I have no way to run the tests
(facebook#1027), so you will have to
forgive my attempt to add a test case here; try getting Claude to fix
the bits I missed.

The OSS build is down due to April Fools, so while I've tested this
locally with some build fix hacks, I don't think the external CI will
say much useful.
fix: bxl target patterns didn't accept multiple args
use BlockingExecuter = BuckBlockingExecutor::default_concurrency(fs.dupe()) for macos
The signature of `compile` changed from Tonic 0.12 to 0.14. The new
signature requires the type of `protos` and `includes` to be the same.
This prepares for the tonic 0.14 upgrade by passing both `protos` and
`includes` as `&[&str]` instead of one being `&[String]`.

Upstream: facebook/buck2@1a14d2b
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
Summary:
`DefaultConfigParserFileOps::read_dir` uses `std::fs::read_dir` which returns entries in platform-dependent, unspecified order. The DICE-based `ReadDirOutput` is already documented as returning sorted entries, but `DefaultConfigParserFileOps` (used during initial daemon startup) did not match this contract.

This means when buckconfig directories contain multiple files defining the same config key, which value wins could vary across runs depending on filesystem traversal order.

Sort entries by name in `DefaultConfigParserFileOps::read_dir` to match the DICE path's contract and ensure deterministic config resolution.

Reviewed By: AishwaryaSivaraman

Differential Revision: D101209619

fbshipit-source-id: e47dedb2530640e4cd0e96112b757e28969e591c

Co-authored-by: Neil Mitchell <ndmitchell@meta.com>
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!
…wyzskk

Allow source labels to omit eponymous target names
* 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>
9999years and others added 8 commits July 2, 2026 12:35
This adds a new `OtelEventSink` which sends `InvocationRecord` wide
events to an OpenTelemetry collector, if configured.
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.
- Run lib tests with cargo-nextest

  `cargo test` runs one test binary at a time, which serializes badly
  across the ~100 crates in this workspace; nextest schedules tests from
  all binaries in parallel. Doctests still go through `cargo test --doc`
  since nextest does not support them.

  test.py falls back to `cargo test --lib` when nextest is not
  installed.

- Split lint and rustdoc into parallel jobs

  clippy (~3 min) and rustdoc (~4 min) ran serially ahead of the ~20 min
  test phase in every build-and-test job. Move them to dedicated jobs so
  they overlap with the tests instead: per-platform lint jobs run
  `test.py --lint-rust-only`, and a single Linux job runs `test.py
  --rustdoc-only` (rustdoc output is platform-independent enough that
  running it three times bought nothing).

  Adds a `--test-only` flag to test.py so the build-and-test jobs skip
  the lint/rustdoc phases they used to run inline.

- Cancel superseded PR runs

  A force-push previously left the old run burning three runners for the
  better part of an hour. Key the concurrency group on the PR number so
  new pushes cancel in-flight runs; pushes to mercury-head are keyed on
  the ref and never cancelled.

- Use macos-latest-xlarge for macOS jobs

  macOS was the slowest platform (~50 min wall clock) and the work is
  compile-bound, so it scales with cores. macos-latest-xlarge is a
  GitHub-hosted M-series runner with double the cores of macos-latest;
  no org runner setup is required, but it is billed per minute.
@linear-code

linear-code Bot commented Jul 9, 2026

Copy link
Copy Markdown

DUX-5515

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants