fix: remove legacy pre-GN cargo preamble from CI build scripts#127
Merged
eseidel merged 1 commit intoshorebird/devfrom Apr 8, 2026
Merged
fix: remove legacy pre-GN cargo preamble from CI build scripts#127eseidel merged 1 commit intoshorebird/devfrom
eseidel merged 1 commit intoshorebird/devfrom
Conversation
mac_build.sh, linux_build.sh, and win_build.sh each had a leftover 'cd $UPDATER_SRC/library && cargo build --target ...' preamble from before bdero's #120 GN/Ninja integration of the rust updater build. mac_build.sh even had a FIXME at the top documenting the intent to delete it. These preambles: 1. Bypass the GN action entirely, so none of the env-var plumbing in build_rust_updater.py applies (no IPHONEOS_DEPLOYMENT_TARGET from #123, no CFLAGS_*_windows_msvc=/MT from #125, no Android NDK env). 2. Write to the source-tree default cargo target dir at third_party/updater/target/, ignoring #124's redirect to $root_out_dir/cargo_target/, so the engine 'rm -rf out' clobber never touches their output. 3. On the persistent Sandpiper runner this means stale rlibs from pre-fix builds keep getting reused, masking #123 entirely and producing the same iOS arm64 zstd_sys deployment-target failure after every fix attempt. Delete the library cargo invocations from all three scripts. The GN build of any engine target that depends transitively on //flutter/shell/common/shorebird:updater (which is most of them) will now invoke build_rust_updater.py via Ninja and produce libupdater.a / updater.lib in the right place with the right env vars. The patch tool's separate cargo build in mac_build.sh / linux_build.sh is a standalone CLI not linked into the engine, so the GN build does not cover it. Left in place with a TODO to migrate it later. Tracking: shorebirdtech/_shorebird#2014 Follow-up to: #120, #123, #124, #125
eseidel
force-pushed
the
shorebird/remove-legacy-cargo-preamble
branch
from
9108153 to
467c5e5
Compare
bdero
approved these changes
Apr 8, 2026
eseidel
added a commit
that referenced
this pull request
Apr 8, 2026
The action only declared its stamp file as an output, which worked when shorebird_updater_output_lib lived under //flutter/third_party/... because GN treated source-tree paths as existing files needing no rule. After #124 moved the cargo target dir into $root_out_dir/cargo_target, the lib path became a build-dir path and Ninja correctly demands a rule producing it. Without the lib in outputs: ninja: error: 'cargo_target/aarch64-linux-android/release/libupdater.a', needed by 'libflutter.so', missing and no known rule to make it Add shorebird_updater_output_lib to the action's outputs alongside the stamp file. The cargo invocation already produces the file at this path; this just tells Ninja which rule is responsible. Tracking: shorebirdtech/_shorebird#2014 Follow-up to: #124, #127
eseidel
added a commit
that referenced
this pull request
Apr 8, 2026
…128) The action only declared its stamp file as an output, which worked when shorebird_updater_output_lib lived under //flutter/third_party/... because GN treated source-tree paths as existing files needing no rule. After #124 moved the cargo target dir into $root_out_dir/cargo_target, the lib path became a build-dir path and Ninja correctly demands a rule producing it. Without the lib in outputs: ninja: error: 'cargo_target/aarch64-linux-android/release/libupdater.a', needed by 'libflutter.so', missing and no known rule to make it Add shorebird_updater_output_lib to the action's outputs alongside the stamp file. The cargo invocation already produces the file at this path; this just tells Ninja which rule is responsible. Tracking: shorebirdtech/_shorebird#2014 Follow-up to: #124, #127
eseidel
added a commit
that referenced
this pull request
Apr 8, 2026
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
added a commit
that referenced
this pull request
Apr 8, 2026
…130) 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
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
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
2 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.
Summary
mac_build.sh,linux_build.sh, andwin_build.sheach had a leftovercd $UPDATER_SRC/library && cargo build --target ...preamble from before #120's GN/Ninja integration of the rust updater build.mac_build.sheven had aFIXMEat the top documenting the intent to delete it. These preambles:build_rust_updater.pyapplies — noIPHONEOS_DEPLOYMENT_TARGETfrom fix: thread Apple deployment target into Rust updater build #123, noCFLAGS_*_windows_msvc=/MTfrom fix: force static CRT for cc-compiled C deps on Windows #125, no Android NDK env from Integrate Rust updater build into GN/Ninja #120's own_configure_android_env.third_party/updater/target/, ignoring fix: put cargo target dir inside the GN build output dir #124's redirect to$root_out_dir/cargo_target/. The enginerm -rf outclobber never touches their output.libzstd_sys-*.rlibthat was compiled before the env-var fix existed.Fix
Delete the library cargo invocations from all three scripts. Any engine target that depends transitively on
//flutter/shell/common/shorebird:updater(which is most of them) will now invokebuild_rust_updater.pyvia Ninja and producelibupdater.a/updater.libin the right place with the right env vars.The
patchtool's separate cargo build inmac_build.sh/linux_build.shis a standalone CLI, not linked into the engine, so the GN build does not cover it. Left in place with aTODOto migrate it later.Note for ops
Once this lands, the orphaned
~/.engine_checkout/flutter/engine/src/flutter/third_party/updater/target/directory on the Sandpiper host should be deleted by hand. It's the stale rlibs that keep masking the fixes. Nothing will write to that path anymore after this PR.Known followup not in this PR
build_rust_updateraction'soutputs = [ _stamp ]doesn't includeshorebird_updater_output_lib, which causes a separate ninja error'cargo_target/.../libupdater.a' missing and no known rule to make itonce the legacy preamble is gone. That's a one-line fix in BUILD.gn — separate PR.Tracking: shorebirdtech/_shorebird#2014
Follow-up to: #120, #123, #124, #125