diff --git a/.github/workflows/tag.yaml b/.github/workflows/tag.yaml index 2648637..5469c16 100644 --- a/.github/workflows/tag.yaml +++ b/.github/workflows/tag.yaml @@ -17,6 +17,15 @@ on: # schedule: # - cron: "0 15 1,15 * *" + # WHILE THIS MODULE IS PRE-1.0, DO NOT DISPATCH THIS FOR A BREAKING CHANGE. + # ccv has no 0.x special case: a `feat!:`/`fix!:` subject or a + # `BREAKING CHANGE:` footer maps straight to IncMajor(), so a run over such + # a commit tags v1.0.0 -- and the guard on the release job below then skips + # the release, leaving a major tag with no artifacts behind it. Pre-1.0 + # breaking changes are released by pushing the intended minor tag by hand + # (`git tag v0.3.0 && git push origin v0.3.0`), which release.yml picks up + # on its own. + jobs: tag: permissions: diff --git a/CHANGELOG.md b/CHANGELOG.md index d7588aa..b47957d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,45 @@ it reaches 1.0. ## [Unreleased] +## [0.3.0] - 2026-08-23 + +### Changed + +- **Breaking:** `//flutter:remote_cache_trees` is a string flag taking `none` + (default), `workspaces` or `all` instead of a boolean. The assembled pub + cache and the prepared/overlay workspaces have very different economics — a + worker must materialize the pub cache locally either way, while remote-caching + the workspaces replaces a full `intl_utils` plus `build_runner` run with a + ~180MB download — so they are no longer one decision. Migration: + `--//flutter:remote_cache_trees` becomes + `--//flutter:remote_cache_trees=all`. + + The setting classifies _actions_, not individual outputs, so an action that + declares both kinds of tree counts as a pub cache and stays local under + `workspaces`. That is the dependency-preparation action of a library with + `assemble_dep_caches = False`, and of a pub package that assembles; the + default library path assembles in a separate action and is unaffected. + +- `docs/hermeticity.md` records what `remote_cache_trees=workspaces` can and + cannot achieve: the prepared trees are not reproducible (`.dart_tool` embeds + the producing sandbox's execroot in text _and_ inside the host native-asset + binaries JIT codegen links, where load commands cannot be rewritten), and the + SDK filegroup contributes 51 mtime-fingerprinted source directories. Sharing + therefore works between workers that share an output base or a fetched SDK, + and not between independently provisioned ones. + +### Fixed + +- `PrepareFlutterAppWorkspace` carries the same execution posture as every + other tree-producing action. It had none, so a ~100MB rsync of the library + workspace was remote-execution eligible (which would upload its + `no-remote-cache` input tree) and its own ~100MB output was uploaded on every + build under `--remote_upload_local_results` — the drain + `//flutter:remote_cache_trees` exists to prevent. +- The dependency-preparation log (`_pub_prepare.log`) no longer records a + wall-clock timestamp. It is a declared output, so the clock reading made the + action gratuitously unreproducible. + ## [0.2.1] - 2026-07-14 ### Fixed @@ -117,6 +156,7 @@ it reaches 1.0. - Deprecated, ignored `dart_proto_library` `options`/`grpc` attributes. -[Unreleased]: https://github.com/SpencerC/rules_flutter/compare/v0.2.1...HEAD +[Unreleased]: https://github.com/SpencerC/rules_flutter/compare/v0.3.0...HEAD +[0.3.0]: https://github.com/SpencerC/rules_flutter/compare/v0.2.1...v0.3.0 [0.2.1]: https://github.com/SpencerC/rules_flutter/compare/v0.2.0...v0.2.1 [0.2.0]: https://github.com/SpencerC/rules_flutter/commits/v0.2.0 diff --git a/docs/hermeticity.md b/docs/hermeticity.md index f49b8c6..87c4a7e 100644 --- a/docs/hermeticity.md +++ b/docs/hermeticity.md @@ -259,14 +259,41 @@ split by output shape: `no-remote-cache`. Uploading them via `--remote_upload_local_results` on every change has been observed draining a CI invocation for minutes after the last test finished, while rebuilding them locally takes seconds; they - remain eligible for the local disk cache (`--disk_cache`). Opt the trees - back into the remote cache — e.g. when a warm main-branch runner populates - a cache that ephemeral PR runners read — with: + remain eligible for the local disk cache (`--disk_cache`). Opt a kind of + tree back into the remote cache — e.g. when a warm main-branch runner + populates a cache that ephemeral PR runners read — with: ``` - build --//flutter:remote_cache_trees + build --//flutter:remote_cache_trees=workspaces # or =all; default =none ``` + The two kinds are separated because their economics differ. A worker has to + materialize the pub cache locally either way: it feeds the local-only + flutter build, and its own `@pub_*` repository inputs must be fetched to + compute any action key at all — so `=all` buys a multi-GB download in place + of a hardlink assembly. `=workspaces` buys ~180MB of download in place of a + full `intl_utils` plus `build_runner` codegen run, which is the trade that + usually pays. + + Classification is per *action*, not per output, so an action that declares + both kinds counts as a pub cache. That is the dependency-preparation action + of a library with `assemble_dep_caches = False` (and of a pub package that + assembles): it merges its cache in the same action that prepares the + workspace, so `=workspaces` leaves it local. Libraries on the default path + assemble in a separate `FlutterAssemblePubCache` action and are unaffected. + + Be aware of what `=workspaces` can and cannot do today. The prepared trees + are **not reproducible**: `.dart_tool` records the producing sandbox's + absolute execroot in about sixteen text files, and the host native-asset + binaries that JIT codegen links embed it in Mach-O/ELF load commands, which + cannot be rewritten. The Flutter SDK filegroup also contributes 51 source + *directories*, which Bazel fingerprints by mtime rather than content (hence + its "dependency checking of directories is unsound" warning), so a worker + that fetched the SDK itself never agrees on a key with one that fetched it + separately. `=workspaces` therefore helps a cache shared by workers that + share an output base or a fetched SDK, and does nothing for two independently + provisioned workers. + Android and iOS builds are additionally `no-sandbox`/`requires-network` (see the per-target table above) and are not affected by this section. diff --git a/flutter/BUILD.bazel b/flutter/BUILD.bazel index 4cd2e9a..7810ff9 100644 --- a/flutter/BUILD.bazel +++ b/flutter/BUILD.bazel @@ -21,20 +21,39 @@ bool_flag( visibility = ["//visibility:public"], ) -# The prepare/assemble/workspace actions produce large tree artifacts (an -# assembled pub cache is multi-GB; prepared and per-target overlay workspaces -# run ~100MB each) that change with every source edit. Uploading them via +# The prepare/assemble/workspace actions produce large tree artifacts that +# change with every source edit. Uploading them via # --remote_upload_local_results has been observed draining a CI gate for 485s -# after the last test finished, so by default they carry no-remote-cache: -# they are rebuilt locally when needed (the hardlink assembly runs in ~seconds -# per GB) and stay eligible for the local disk cache. Small, expensive, -# high-value results (staged pub packages, golden renders, flutter build -# outputs) remain remotely cached. Flip this flag to remote-cache the trees -# too — e.g. on an RBE fleet where executors share the cache: -# build --//flutter:remote_cache_trees -bool_flag( +# after the last test finished, so by default they carry no-remote-cache: they +# are rebuilt locally when needed (the hardlink assembly runs in ~seconds per +# GB) and stay eligible for the local disk cache. Small, expensive, high-value +# results (staged pub packages, golden renders, flutter build outputs) remain +# remotely cached. +# +# The two kinds of tree have very different economics, so this is not one +# decision: +# +# none (default) neither is remotely cached. +# workspaces The ~100MB prepared and per-target overlay workspaces are, the +# multi-GB assembled pub cache is not. This is the setting for a +# fleet whose workers start cold: a cold worker has to materialize +# the pub cache locally either way (it is an input to the +# local-only flutter build, and its own inputs -- the @pub_* +# repositories -- must be fetched to compute any action key at +# all), whereas downloading ~180MB of prepared workspace replaces +# a full intl_utils + build_runner codegen run. +# all Both. For an RBE fleet where executors share the cache and the +# pub cache upload is affordable. +# +# build --//flutter:remote_cache_trees=workspaces +string_flag( name = "remote_cache_trees", - build_setting_default = False, + build_setting_default = "none", + values = [ + "none", + "workspaces", + "all", + ], visibility = ["//visibility:public"], ) diff --git a/flutter/defs.bzl b/flutter/defs.bzl index e11bb3c..8724501 100644 --- a/flutter/defs.bzl +++ b/flutter/defs.bzl @@ -1475,6 +1475,17 @@ fi command = copy_script, mnemonic = "PrepareFlutterAppWorkspace", progress_message = "Preparing Flutter workspace for %s" % ctx.label.name, + # Same posture as every other tree-producing action, which this one was + # missing: a ~100MB rsync of the library workspace feeding a local-only + # flutter build. Without it the action was remote-execution eligible + # (which would have to upload its no-remote-cache input tree) and its + # own 100MB output was uploaded on every run under + # --remote_upload_local_results -- exactly the drain the flag exists to + # avoid. + execution_requirements = tree_output_execution_requirements( + _allow_remote_exec(ctx), + _remote_cache_trees(ctx), + ), ) android = _android_environment(ctx) diff --git a/flutter/private/flutter_actions.bzl b/flutter/private/flutter_actions.bzl index 8addd25..fff2b6f 100644 --- a/flutter/private/flutter_actions.bzl +++ b/flutter/private/flutter_actions.bzl @@ -320,7 +320,7 @@ with open(graph_path, "w", encoding="utf-8") as fh: json.dump(graph, fh, indent=2) fh.write("\\n")""" -def create_flutter_working_dir(ctx, pubspec_file, dart_files, other_files, data_files, extra_entries = [], allow_remote_exec = False, remote_cache_trees = False): +def create_flutter_working_dir(ctx, pubspec_file, dart_files, other_files, data_files, extra_entries = [], allow_remote_exec = False, remote_cache_trees = "none"): """Create a working directory structure for Flutter commands. Args: @@ -333,8 +333,9 @@ def create_flutter_working_dir(ctx, pubspec_file, dart_files, other_files, data_ workspace-relative paths (e.g. generated proto sources). These take precedence over the derived layout for the same file. allow_remote_exec: Whether //flutter:allow_remote_execution is set. - remote_cache_trees: Whether //flutter:remote_cache_trees is set; when - False the ~100MB seed tree carries no-remote-cache. + remote_cache_trees: The //flutter:remote_cache_trees value; unless + it names this action's tree kind, the ~100MB seed tree carries + no-remote-cache. Returns: Tuple of (working_dir, input_files) @@ -486,7 +487,7 @@ def flutter_assemble_pub_cache_action( ctx, dependency_pub_caches = [], allow_remote_exec = False, - remote_cache_trees = False): + remote_cache_trees = "none"): """Merge transitive dependency pub caches into a single assembled cache tree. This is the offline pub cache the library exposes. It is a pure function of @@ -501,9 +502,10 @@ def flutter_assemble_pub_cache_action( dependencies. allow_remote_exec: Whether //flutter:allow_remote_execution is set; when False the action carries no-remote-exec. - remote_cache_trees: Whether //flutter:remote_cache_trees is set; when - False the multi-GB assembled tree carries no-remote-cache (local - disk cache stays eligible). + remote_cache_trees: The //flutter:remote_cache_trees value; unless + it names this action's tree kind (only "all" does), the multi-GB + assembled tree carries no-remote-cache (local disk cache stays + eligible). Returns: The assembled pub cache tree artifact. @@ -578,7 +580,11 @@ echo "=== Pub cache assembly complete ===" command = script_content, mnemonic = "FlutterAssemblePubCache", progress_message = "Assembling pub cache for %s" % ctx.label.name, - execution_requirements = tree_output_execution_requirements(allow_remote_exec, remote_cache_trees), + execution_requirements = tree_output_execution_requirements( + allow_remote_exec, + remote_cache_trees, + kind = TREE_PUB_CACHE, + ), resource_set = heavy_action_resource_set, ) @@ -704,7 +710,7 @@ def flutter_pub_get_action( run_build_runner_build = False, is_pub_package = False, allow_remote_exec = False, - remote_cache_trees = False, + remote_cache_trees = "none", preassembled_cache = None, build_runner_cache = ""): """Prepare Flutter/Dart dependencies from declared pub_deps.json metadata. @@ -727,9 +733,11 @@ def flutter_pub_get_action( is_pub_package: Whether the target represents a hosted pub.dev package. allow_remote_exec: Whether //flutter:allow_remote_execution is set; when False the action carries no-remote-exec. - remote_cache_trees: Whether //flutter:remote_cache_trees is set; when - False the prepared workspace / dart_tool trees carry - no-remote-cache (local disk cache stays eligible). + remote_cache_trees: The //flutter:remote_cache_trees value; unless + it names this action's tree kind, the trees this action declares + carry no-remote-cache (local disk cache stays eligible). The kind + is TREE_PUB_CACHE whenever a pub cache is among those trees — see + prepare_deps_tree_kind. preassembled_cache: An assembled pub cache tree (from flutter_assemble_pub_cache_action) to use read-only instead of merging dependency_pub_caches here. When set, this action produces @@ -1360,7 +1368,9 @@ LOG_FILE="{pub_get_output}" echo "=== Flutter Dependency Preparation ===" > "$LOG_FILE" echo "Flutter binary: {flutter_bin}" >> "$LOG_FILE" echo "Workspace output: {workspace_dir}" >> "$LOG_FILE" -echo "Prepared at: $(date)" >> "$LOG_FILE" +# Deliberately no timestamp: this is a declared output, and a clock reading in +# it makes the action unreproducible for no benefit. The invocation's own start +# time is in the build log. echo "" >> "$LOG_FILE" if [ -f "$WORKSPACE_DIR_ABS/pub_deps.json" ]; then @@ -1371,6 +1381,23 @@ else exit 1 fi +# NOTE: these two declared trees are NOT reproducible, and that is why the +# flutter build downstream of them can never be a remote cache hit across +# machines. .dart_tool carries the sandbox's absolute execroot in ~16 text files +# (package_config.json alone holds one file:// URI per package) AND, decisively, +# inside the host native-asset binaries the JIT codegen builds: +# .dart_tool/hooks_runner/shared/objective_c/build//objective_c.dylib +# contains 17 copies of it in Mach-O load commands, which are length-prefixed +# and so cannot be rewritten. +# +# Canonicalizing the text was implemented and reverted: it fixes 16 of 17 +# sources of variance and leaves the build broken-or-unreproducible on the last +# one. Making this tree reproducible means not shipping the host native-asset +# build directory at all (per-action scratch, which downstream host-JIT +# consumers would then rebuild), and it only pays off alongside content-hashing +# the 51 source *directories* the Flutter SDK filegroup contributes -- Bazel +# fingerprints those by mtime, so a refetch on another worker changes the key +# regardless. Both are ruleset projects; see //flutter:remote_cache_trees. rm -rf "{dart_tool_dir}" mkdir -p "{dart_tool_dir}" if [ -d "$WORKSPACE_DIR_ABS/.dart_tool" ]; then @@ -1397,7 +1424,11 @@ echo "Status: Prepared dependencies from declared metadata" >> "$LOG_FILE" ), mnemonic = "FlutterPrepareDeps", progress_message = "Preparing Flutter dependencies for %s" % ctx.label.name, - execution_requirements = tree_output_execution_requirements(allow_remote_exec, remote_cache_trees), + execution_requirements = tree_output_execution_requirements( + allow_remote_exec, + remote_cache_trees, + kind = prepare_deps_tree_kind(pub_cache_dir in prepare_outputs), + ), resource_set = heavy_action_resource_set, # The cache opt-in needs the persistent directory reachable from the # action. It is an out-of-sandbox path (the consumer also passes @@ -1424,7 +1455,37 @@ def heavy_action_execution_requirements(allow_remote_exec): return None return {"no-remote-exec": "1"} -def tree_output_execution_requirements(allow_remote_exec, remote_cache_trees): +# The kinds of tree artifact //flutter:remote_cache_trees distinguishes. Pass +# one as tree_output_execution_requirements' `kind`. +TREE_WORKSPACE = "workspace" +TREE_PUB_CACHE = "pub_cache" + +_TREE_CACHE_KINDS = { + "none": [], + "workspaces": [TREE_WORKSPACE], + "all": [TREE_WORKSPACE, TREE_PUB_CACHE], +} + +def prepare_deps_tree_kind(has_own_pub_cache): + """The tree kind FlutterPrepareDeps must be classified under. + + That action always declares the prepared workspace and .dart_tool trees, + and — unless it consumes a preassembled cache read-only — its own pub + cache tree as well, which for an assembling target is the full multi-GB + merge. An action carries one posture for all of its outputs, so it takes + the more restrictive kind whenever a pub cache is among them: `workspaces` + promises not to put a pub cache on the wire. + + Args: + has_own_pub_cache: Whether a pub cache tree is among the action's + declared outputs. + + Returns: + TREE_PUB_CACHE or TREE_WORKSPACE. + """ + return TREE_PUB_CACHE if has_own_pub_cache else TREE_WORKSPACE + +def tree_output_execution_requirements(allow_remote_exec, remote_cache_trees, kind = TREE_WORKSPACE): """Execution requirements for actions producing large tree artifacts. The assembled pub cache (multi-GB) and the prepared/overlay workspace @@ -1433,26 +1494,45 @@ def tree_output_execution_requirements(allow_remote_exec, remote_cache_trees): for hundreds of seconds after the last real action finished. The default posture therefore adds no-remote-cache — Bazel still caches these actions in the local disk cache, and rebuilding them locally is cheap — alongside - the usual no-remote-exec. //flutter:remote_cache_trees opts the trees back - into the remote cache (e.g. on an RBE fleet where executors share it), and - //flutter:allow_remote_execution lifts the execution restriction. + the usual no-remote-exec. //flutter:remote_cache_trees opts a chosen kind + of tree back into the remote cache, and //flutter:allow_remote_execution + lifts the execution restriction. + + The two kinds are separable because their economics differ. A cold worker + has to materialize the pub cache locally whichever way the flag is set (it + feeds the local-only flutter build, and its own @pub_* repository inputs + must be fetched to compute any action key), so remote-caching it buys a + multi-GB download in place of a hardlink assembly. Remote-caching the + prepared workspaces buys ~180MB of download in place of a full intl_utils + plus build_runner codegen run. Args: allow_remote_exec: Whether //flutter:allow_remote_execution is set; when False the action carries no-remote-exec. - remote_cache_trees: Whether //flutter:remote_cache_trees is set; when - False (and execution is local) the action carries no-remote-cache. - Ignored under allow_remote_exec: remotely executed actions must - store their outputs in the remote CAS, so suppressing the cache - there would only force constant re-execution. + remote_cache_trees: The //flutter:remote_cache_trees value — "none", + "workspaces" or "all". A kind it does not name (and local + execution) means the action carries no-remote-cache. Ignored under + allow_remote_exec: remotely executed actions must store their + outputs in the remote CAS, so suppressing the cache there would + only force constant re-execution. + kind: TREE_WORKSPACE (default) or TREE_PUB_CACHE — which kind of tree + this action produces. Returns: An execution_requirements dict, or None when nothing is restricted. """ + if remote_cache_trees not in _TREE_CACHE_KINDS: + fail( + "//flutter:remote_cache_trees must be one of {}, got '{}'".format( + sorted(_TREE_CACHE_KINDS.keys()), + remote_cache_trees, + ), + ) + reqs = {} if not allow_remote_exec: reqs["no-remote-exec"] = "1" - if not remote_cache_trees: + if kind not in _TREE_CACHE_KINDS[remote_cache_trees]: reqs["no-remote-cache"] = "1" return reqs or None diff --git a/flutter/tests/exec_posture_test.bzl b/flutter/tests/exec_posture_test.bzl index d811799..fd2b7f7 100644 --- a/flutter/tests/exec_posture_test.bzl +++ b/flutter/tests/exec_posture_test.bzl @@ -3,11 +3,19 @@ load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest") load( "//flutter/private:flutter_actions.bzl", + "TREE_PUB_CACHE", + "TREE_WORKSPACE", "heavy_action_execution_requirements", "heavy_action_resource_set", + "prepare_deps_tree_kind", "tree_output_execution_requirements", ) +# The two postures tree_output_execution_requirements can return under local +# execution. +_LOCAL_ONLY = {"no-remote-exec": "1", "no-remote-cache": "1"} +_REMOTE_CACHED = {"no-remote-exec": "1"} + def _default_posture_test_impl(ctx): env = unittest.begin(ctx) @@ -33,31 +41,110 @@ def _resource_set_test_impl(ctx): def _tree_output_posture_test_impl(ctx): env = unittest.begin(ctx) - # Default posture: local execution AND no remote-cache upload of the fat - # tree outputs (the local disk cache stays eligible for both). + # "none" (the default): local execution AND no remote-cache upload of + # either kind of tree (the local disk cache stays eligible for both). + for kind in [TREE_WORKSPACE, TREE_PUB_CACHE]: + asserts.equals( + env, + _LOCAL_ONLY, + tree_output_execution_requirements(False, "none", kind = kind), + "none must keep %s out of the remote cache" % kind, + ) + + # The workspace kind is the default, so a caller that passes no kind gets + # the same answer as one that names it. asserts.equals( env, - {"no-remote-exec": "1", "no-remote-cache": "1"}, - tree_output_execution_requirements(False, False), + _LOCAL_ONLY, + tree_output_execution_requirements(False, "none"), ) - # //flutter:remote_cache_trees restores remote caching only. + # "workspaces" remote-caches the ~100MB workspace trees and leaves the + # multi-GB pub cache local -- the split the setting exists for. asserts.equals( env, - {"no-remote-exec": "1"}, - tree_output_execution_requirements(False, True), + _REMOTE_CACHED, + tree_output_execution_requirements(False, "workspaces", kind = TREE_WORKSPACE), ) + asserts.equals( + env, + _LOCAL_ONLY, + tree_output_execution_requirements(False, "workspaces", kind = TREE_PUB_CACHE), + ) + + # "all" remote-caches both. + for kind in [TREE_WORKSPACE, TREE_PUB_CACHE]: + asserts.equals( + env, + _REMOTE_CACHED, + tree_output_execution_requirements(False, "all", kind = kind), + "all must remote-cache %s" % kind, + ) # Under //flutter:allow_remote_execution nothing is restricted: # remotely executed actions must store outputs in the remote CAS, so # remote_cache_trees is ignored there. - asserts.equals(env, None, tree_output_execution_requirements(True, False)) - asserts.equals(env, None, tree_output_execution_requirements(True, True)) + for setting in ["none", "workspaces", "all"]: + for kind in [TREE_WORKSPACE, TREE_PUB_CACHE]: + asserts.equals( + env, + None, + tree_output_execution_requirements(True, setting, kind = kind), + ) + return unittest.end(env) + +def _prepare_deps_kind_test_impl(ctx): + env = unittest.begin(ctx) + + # FlutterPrepareDeps declares its own pub cache tree unless it consumes a + # preassembled one, and an action carries a single posture for all of its + # outputs. Classifying it as a workspace in that case would let + # `workspaces` upload a merged pub cache -- the exact split the setting + # exists to make. + asserts.equals( + env, + TREE_PUB_CACHE, + prepare_deps_tree_kind(True), + "a prepare action that emits a pub cache must be classified as one", + ) + asserts.equals( + env, + _LOCAL_ONLY, + tree_output_execution_requirements( + False, + "workspaces", + kind = prepare_deps_tree_kind(True), + ), + "workspaces must not remote-cache a prepare action carrying a pub cache", + ) + + # With a preassembled cache the action produces workspace trees only. + asserts.equals( + env, + TREE_WORKSPACE, + prepare_deps_tree_kind(False), + ) + asserts.equals( + env, + _REMOTE_CACHED, + tree_output_execution_requirements( + False, + "workspaces", + kind = prepare_deps_tree_kind(False), + ), + ) return unittest.end(env) _default_posture_test = unittest.make(_default_posture_test_impl) _resource_set_test = unittest.make(_resource_set_test_impl) _tree_output_posture_test = unittest.make(_tree_output_posture_test_impl) +_prepare_deps_kind_test = unittest.make(_prepare_deps_kind_test_impl) def exec_posture_test_suite(name): - unittest.suite(name, _default_posture_test, _resource_set_test, _tree_output_posture_test) + unittest.suite( + name, + _default_posture_test, + _prepare_deps_kind_test, + _resource_set_test, + _tree_output_posture_test, + )