Skip to content

chore(rust): selective builds#59563

Open
nickbest-ph wants to merge 10 commits into
masterfrom
nick/rust-builds
Open

chore(rust): selective builds#59563
nickbest-ph wants to merge 10 commits into
masterfrom
nick/rust-builds

Conversation

@nickbest-ph
Copy link
Copy Markdown
Contributor

@nickbest-ph nickbest-ph commented May 22, 2026

Problem

Every rust change triggers builds, tests, and Docker image pushes for all 19 rust services.

Most changes are isolated to single services, where we can cut out 90% of the overall build/test/CI runner action.

Forcing services to deploy when none of their underlying application code has changed is unideal.

Changes

Add selective builds to the Rust CI and Docker build pipelines. Only crates that are transitively affected by changed files are built, tested and deployed.

A new Rust CLI tool is the source of truth for selectivity. It:

  1. gets changed files via git diff ...HEAD (or something similar)
  2. Maps files to crates and builds a directory-to-crate map using cargo metadata. Always picks the longest matching prefix to avoid attributing nested crate's file changes to a parent crate.
  3. Builds a reverse dependency graph from cargo metadata and does a BFS from the changed crates from step 2 to find everything that depends on them
  4. Maps crates -> images by reading .github/rust-images.yml to find which crates' binary targets correspond to which image names
  5. outputs a json blob that can be consumed by CI jobs to what tests to run/what images to build/what services to deploy

example output: { "rebuild_all": false, "images": ["capture", "capture-logs"], "crates": ["capture", "capture-logs", "common-kafka"], "directly_changed": ["common-kafka"]}

How do selective test runs work?

In ci-rust.yml file added an affected job that:

  • builds affected-services tool
  • uses the affected crates list from the affected-services tool, passes it into the test shard computation and filters the packages dict down to only the affected crates

How do selective docker image builds work?

  • runs affected-services in rust-docker-build.yaml and outputs the affected image names in a image-filter list
  • build-images job passes the image-filter to the _rust-build-images.yaml
  • the load-matrix step filters the images that enter the build matrix by the image-filter values

How do selective deploys work?

In the `rust-docker-build.yml the deploy matrix is static. Every service is listed, but each matrix entry has a guard step that checks whether the build-images job produced a digest for the service. If no digest exists because the image wasn't built, all subsequent steps are skipped.

How did you test this code?

Validated the affected-services Rust binary locally against the live workspace (cargo metadata on the 52-crate workspace):

# Leaf service — only capture + capture-logs
$ cargo run -p affected-services -- --files rust/capture/src/main.rs
→ images: ["capture", "capture-logs"], crates: ["capture", "capture-logs"]

# Shared library fan-out
$ cargo run -p affected-services -- --files rust/common/kafka/src/lib.rs
→ images: 8 of 19, crates: 9

# Proto external mapping
$ cargo run -p affected-services -- --files proto/personhog.proto
→ images: 7 of 19 (all personhog + kafka-assigner + property-defs-rs)

# Multi-binary crate
$ cargo run -p affected-services -- --files rust/feature-flags/src/lib.rs
→ images: ["feature-flags", "flags-cache-warmer"]

# Rebuild-all trigger
$ cargo run -p affected-services -- --files rust/Cargo.lock
→ rebuild_all: true, all 19 images

# Non-crate image (migration files)
$ cargo run -p affected-services -- --files rust/persons_migrations/new.sql
→ images: ["sqlx-migrate"], crates: []

# Full graph dump for inspection
$ cargo run -p affected-services -- --dump-graph

Copy link
Copy Markdown
Contributor

@eli-r-ph eli-r-ph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! This looks like the right idea to me 👍

@nickbest-ph nickbest-ph marked this pull request as ready for review May 29, 2026 22:45
@assign-reviewers-posthog assign-reviewers-posthog Bot requested a review from a team May 29, 2026 22:45
Copy link
Copy Markdown
Contributor

@mendral-app mendral-app Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Supply Chain Security Review

⚠️ Review recommended — 2 findings across 2 files

Tag @mendral-app with feedback or questions. View session

Comment thread .github/workflows/ci-rust.yml Outdated
Comment thread .github/workflows/rust-docker-build.yml Outdated
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 29, 2026

Comments Outside Diff (1)

  1. .github/workflows/rust-docker-build.yml, line 253-262 (link)

    P1 No fallback when PUSH_BEFORE_SHA is empty

    For a push event, github.event.before is the all-zeros SHA (0000000000000000000000000000000000000000) when the branch is brand-new or after a force-push. With an empty or null SHA, affected-services --base-ref "" will run git diff ""...HEAD, which fails — and with the missing exit-status check in the tool, silently returns no affected images and skips all Docker builds.

    The ci-rust.yml handles this correctly with the || 'origin/master' fallback. The same safety net is missing here.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: .github/workflows/rust-docker-build.yml
    Line: 253-262
    
    Comment:
    **No fallback when `PUSH_BEFORE_SHA` is empty**
    
    For a `push` event, `github.event.before` is the all-zeros SHA (`0000000000000000000000000000000000000000`) when the branch is brand-new or after a force-push. With an empty or null SHA, `affected-services --base-ref ""` will run `git diff ""...HEAD`, which fails — and with the missing exit-status check in the tool, silently returns no affected images and skips all Docker builds.
    
    The `ci-rust.yml` handles this correctly with the `|| 'origin/master'` fallback. The same safety net is missing here.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
rust/affected-services/src/main.rs:97-102
`get_changed_files` doesn't check `out.status.success()`. When `git diff` exits non-zero — for example when `base_ref` is an all-zeros SHA (first push to a branch, force push), points to a commit not present in a shallow clone, or is otherwise invalid — stdout is empty and the function silently returns an empty file list. The downstream effect is zero affected crates/images detected, so CI builds and Docker pushes are quietly skipped rather than failing loudly.

```suggestion
fn get_changed_files(base_ref: &str) -> Result<Vec<String>> {
    let out = Command::new("git")
        .args(["diff", "--name-only", &format!("{base_ref}...HEAD")])
        .output()
        .context("failed to run git diff")?;
    anyhow::ensure!(
        out.status.success(),
        "git diff exited with {}: {}",
        out.status,
        String::from_utf8_lossy(&out.stderr).trim()
    );
    Ok(String::from_utf8(out.stdout)?
```

### Issue 2 of 2
.github/workflows/rust-docker-build.yml:253-262
**No fallback when `PUSH_BEFORE_SHA` is empty**

For a `push` event, `github.event.before` is the all-zeros SHA (`0000000000000000000000000000000000000000`) when the branch is brand-new or after a force-push. With an empty or null SHA, `affected-services --base-ref ""` will run `git diff ""...HEAD`, which fails — and with the missing exit-status check in the tool, silently returns no affected images and skips all Docker builds.

The `ci-rust.yml` handles this correctly with the `|| 'origin/master'` fallback. The same safety net is missing here.

Reviews (1): Last reviewed commit: "lint and pin" | Re-trigger Greptile

Comment thread rust/affected-services/src/main.rs
Comment thread .github/workflows/rust-docker-build.yml
Comment thread .github/workflows/ci-rust.yml Outdated
@eli-r-ph eli-r-ph self-requested a review May 30, 2026 00:22
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.

2 participants