Skip to content

fix: gate shorebird_updater_supported on having a known rust target#130

Merged
eseidel merged 1 commit intoshorebird/devfrom
shorebird/gate-updater-rust-target
Apr 8, 2026
Merged

fix: gate shorebird_updater_supported on having a known rust target#130
eseidel merged 1 commit intoshorebird/devfrom
shorebird/gate-updater-rust-target

Conversation

@eseidel
Copy link
Copy Markdown

@eseidel eseidel commented Apr 8, 2026

Summary

build_rust_updater.gni previously set shorebird_updater_supported from target_os alone (is_android || is_ios || ...). But GN evaluates BUILD files under every toolchain in use, including host and sub toolchains that can hit (target_os, target_cpu) combos we don't explicitly handle. When such a toolchain entered the if (shorebird_updater_supported) block but none of the per-cpu cases set shorebird_updater_rust_target, GN blew up:

ERROR at build_rust_updater.gni:72: Undefined identifier in string expansion.
  shorebird_updater_output_lib = "$...$shorebird_updater_rust_target..."
"shorebird_updater_rust_target" is not currently in scope.

Reproduced on Windows host builds of android engine variants: win_build.sh's gn --android --android-cpu=arm64 invocation, evaluated under the Windows host toolchain, landed on an is_win/target_cpu combo our is_win branch doesn't cover.

Fix

Flip the logic. Compute shorebird_updater_rust_target first (default empty), then derive shorebird_updater_supported from whether it got assigned. Unhandled combos become unsupported, so consuming BUILD.gn files' existing if (shorebird_updater_supported) gate cleanly skips the updater for that toolchain — which is correct, since the updater only needs to be built for toolchains whose final binary is the engine.

No new os/cpu combos added. No behavior change for already-handled combos. Just safer unhandled-combo behavior.

Tracking: shorebirdtech/_shorebird#2014
Follow-up to: #124, #127, #128

build_rust_updater.gni previously set shorebird_updater_supported based
only on target_os (is_android || is_ios || ...). But GN evaluates BUILD
files under every toolchain in use, including host and sub toolchains
that can have unusual (target_os, target_cpu) combinations we don't
handle. When a toolchain entered the `if (shorebird_updater_supported)`
block but hit none of the os/cpu cases that set
`shorebird_updater_rust_target`, GN blew up:

  ERROR at build_rust_updater.gni:72: Undefined identifier in string
    expansion.
    shorebird_updater_output_lib = "$...$shorebird_updater_rust_target..."
  "shorebird_updater_rust_target" is not currently in scope.

This happens on Windows host builds of android engine variants when
the host toolchain(s) land on a combo like (is_win, target_cpu != x64)
or similar, which our is_win branch doesn't cover.

Flip the logic: compute shorebird_updater_rust_target first, default
to empty, and derive `shorebird_updater_supported` from whether it
got a value. Unhandled combos become unsupported, so the consuming
BUILD.gn files' `if (shorebird_updater_supported)` gate cleanly skips
the updater for that toolchain — which is correct, since the updater
only needs to be built for toolchains whose final binary is the engine.

Tracking: shorebirdtech/_shorebird#2014
Follow-up to: #124, #127, #128
@eseidel eseidel requested a review from bdero April 8, 2026 07:21
Copy link
Copy Markdown
Member

@bdero bdero left a comment

Choose a reason for hiding this comment

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

Clearly, my original change to add support for building the Rust updater in GN was not complete. I'd consider just reverting the whole thing to unblock things if continues to give you trouble.

@eseidel
Copy link
Copy Markdown
Author

eseidel commented Apr 8, 2026

No no, this is very good. This is taking us in the right direction. I'm glad for us to finally do this.

@eseidel
Copy link
Copy Markdown
Author

eseidel commented Apr 8, 2026

The Clankers are extremely good at this.

@eseidel eseidel merged commit 2b210e0 into shorebird/dev Apr 8, 2026
6 of 8 checks passed
@eseidel eseidel deleted the shorebird/gate-updater-rust-target branch April 8, 2026 14:07
@eseidel
Copy link
Copy Markdown
Author

eseidel commented Apr 8, 2026

I asked Claude if we should move to the full "just call the Rust compiler directly" solution. It gave me this long history of how Fuchsia and Chromium had started with Cargo like we are, and then slowly migrated to calling the Rust compiler directly. It recommended that we stay where we were, just calling Cargo for now, even though it added all this extra noise, because the full solution was a lot more work.

eseidel added a commit that referenced this pull request Apr 8, 2026
Picks up the updater roll (#122) plus the GN/build fixes needed to
build the engine against the new ureq-based updater stack on all three
platforms (#123, #124, #125, #127, #128, #130). This is the first
engine revision where a full Linux + macOS + Windows build completes
against the rolled updater.

Build: https://github.com/shorebirdtech/_build_engine/actions/runs/24139880449
eseidel added a commit that referenced this pull request Apr 8, 2026
Picks up the updater roll (#122) plus the GN/build fixes needed to
build the engine against the new ureq-based updater stack on all three
platforms (#123, #124, #125, #127, #128, #130). This is the first
engine revision where a full Linux + macOS + Windows build completes
against the rolled updater.

Build: https://github.com/shorebirdtech/_build_engine/actions/runs/24139880449
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