Skip to content

Commit 004eb24

Browse files
committed
Reduce duplicate Android configurations
Macro-internal targets carry `tags = ["manual"]`, so a wildcard pattern stops requesting them beside the transitioned edges that already reach them: Bazel's build unit is `(label, configuration)`, and the private `<name>_apk` was analysed and executed once in the command line's `fastbuild` and again at `opt` under the `_flutter_apk` wrapper. One `_internal_tags`/`_internal_kwargs` pair owns the tagging, including the debug-only manifest library, which takes no kwargs of its own. The bundle guard now runs in the configuration the APK ships: the same release-compilation-mode transition as the wrapper, Java/native contributions through rules_android's `android_split_transition`, and the asset contribution left unsplit as `android_binary.assets` is. It checks every split rather than the first, emitting one `<name>.<platform>.checked` marker per configured platform, so a build naming two `--android_platforms` has both of its shipped configurations verified. `tests/consumer`'s lockfile records the extension digest the example lockfiles already carried; that module's CI step does not pass `--lockfile_mode=error`, which is why it stayed stale.
1 parent b0df0b4 commit 004eb24

2 files changed

Lines changed: 141 additions & 38 deletions

File tree

tests/consumer/MODULE.bazel.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tools/flutter/android.bzl

Lines changed: 140 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,17 @@ load(
2525
"ArtProfileInfo",
2626
"DataBindingV2Info",
2727
)
28+
29+
# Not re-exported by rules_android's `rules/rules.bzl`, but its own file is
30+
# `visibility(PROJECT_VISIBILITY)` with `PROJECT_VISIBILITY = "public"`, so the
31+
# load is legal on the pinned 0.7.3. Its sibling `android_transition` -- the 1:1
32+
# one, documented for exactly this purpose -- is unusable here: it returns
33+
# `None` when `--android_platforms` is unset, which is this repository's default,
34+
# so it would leave the check in the plain configuration the APK does not use.
35+
# The split keys are `platform.name` (`rules/android_split_transition.bzl`,
36+
# `_handle_android_platforms`); an upgrade that renames the file breaks this
37+
# `load` loudly, which is the intended failure mode.
38+
load("@rules_android//rules:android_split_transition.bzl", "android_split_transition")
2839
load(
2940
"@rules_android//rules:rules.bzl",
3041
"ApkInfo",
@@ -428,15 +439,14 @@ def _contributions_for(mode):
428439
def _libraries_required(mode):
429440
return ["engine"] if mode == "debug" else ["aot_library", "engine"]
430441

431-
def _flutter_bundle_check_impl(ctx):
432-
marker = ctx.actions.declare_file(ctx.label.name + ".checked")
433-
434-
check_abis(ctx.attr.abis, str(ctx.label))
435-
mode = ctx.attr._mode[BuildSettingInfo].value
436-
expected = _contributions_for(mode)
442+
def _check_split(ctx, mode, expected, deps, marker):
443+
"""Checks one configured slice of the bundle, and writes its marker.
437444
445+
`deps` is one split of the Java/native contributions plus the assets
446+
contribution, which `android_binary` keeps in its own target configuration.
447+
"""
438448
by_kind = {}
439-
for dep in ctx.attr.contributions:
449+
for dep in deps:
440450
info = dep[FlutterBundleContributionInfo]
441451
if info.kind in by_kind:
442452
fail("Two contributions declare kind '{}'.".format(info.kind))
@@ -533,10 +543,57 @@ def _flutter_bundle_check_impl(ctx):
533543
progress_message = "Checking bundle contributions %{label}",
534544
)
535545

536-
return [DefaultInfo(files = depset([marker]))]
546+
def _flutter_bundle_check_impl(ctx):
547+
check_abis(ctx.attr.abis, str(ctx.label))
548+
mode = ctx.attr._mode[BuildSettingInfo].value
549+
expected = _contributions_for(mode)
550+
551+
# One marker per split, because `android_binary` packages one per split:
552+
# `--android_platforms` naming two platforms ships both, so both are
553+
# checked. Unset -- this repository's default -- is a single split keyed by
554+
# the first `--platforms` value.
555+
splits = ctx.split_attr.contributions
556+
if not splits:
557+
# rules_android 0.7.3 always yields at least one branch. If a later
558+
# version yields none, an empty DefaultInfo would satisfy the
559+
# `build_test` that names this target while checking nothing, so this
560+
# guard fails closed instead.
561+
fail("flutter_bundle_check: the Android dependency split produced no configuration.")
562+
563+
markers = []
564+
for platform in sorted(splits.keys()):
565+
marker = ctx.actions.declare_file("{}.{}.checked".format(ctx.label.name, platform))
566+
_check_split(
567+
ctx,
568+
mode,
569+
expected,
570+
splits[platform] + [ctx.attr.assets_contribution],
571+
marker,
572+
)
573+
markers.append(marker)
574+
575+
return [DefaultInfo(files = depset(markers))]
576+
577+
# Pin release builds to `opt`; preserve explicitly non-fastbuild modes.
578+
def _pin_release_compilation_mode_impl(settings, _attr):
579+
mode = settings["//tools/flutter:mode"]
580+
compilation_mode = str(settings["//command_line_option:compilation_mode"])
581+
if mode == "release" and compilation_mode == "fastbuild":
582+
compilation_mode = "opt"
583+
return {"//command_line_option:compilation_mode": compilation_mode}
584+
585+
_pin_release_compilation_mode = transition(
586+
implementation = _pin_release_compilation_mode_impl,
587+
inputs = [
588+
"//tools/flutter:mode",
589+
"//command_line_option:compilation_mode",
590+
],
591+
outputs = ["//command_line_option:compilation_mode"],
592+
)
537593

538594
flutter_bundle_check = rule(
539595
implementation = _flutter_bundle_check_impl,
596+
cfg = _pin_release_compilation_mode,
540597
doc = """Fails the build if the bundle is missing a piece for any ABI it declares.
541598
542599
Instantiated by flutter_android_libs, not written by hand. Three checks, in
@@ -547,15 +604,35 @@ code assets the manifest names for that ABI.
547604
548605
The last two are one guard split across phases, because analysis sees only that
549606
a label was supplied. Handing the arm64 jar to the x86_64 slot resolves, builds,
550-
and ships an empty ABI.""",
607+
and ships an empty ABI.
608+
609+
All three run once per `android_binary` dependency split, emitting one
610+
`<name>.<platform>.checked` marker each: with `--android_platforms` naming two
611+
platforms the APK packages two configurations, so both are checked.""",
551612
attrs = {
552613
"abis": attr.string_list(
553614
mandatory = True,
554615
doc = "The ABIs the bundle claims to support. Every native contribution must cover each.",
555616
),
556617
"contributions": attr.label_list(
618+
cfg = android_split_transition,
619+
providers = [FlutterBundleContributionInfo],
620+
mandatory = True,
621+
doc = """The Java/native contributions, checked once per split.
622+
623+
`android_binary` reaches its own `deps` through rules_android's
624+
`android_split_transition`, so this edge takes it too: the check then inspects
625+
the configured inputs the APK ships instead of a third target-configuration
626+
copy of them.""",
627+
),
628+
"assets_contribution": attr.label(
557629
providers = [FlutterBundleContributionInfo],
558630
mandatory = True,
631+
doc = """The asset contribution, in this target's own configuration.
632+
633+
Deliberately unsplit: `android_binary.assets` is `cfg = "target"`
634+
(rules_android `rules/attrs.bzl`), so splitting here would check a bundle the
635+
APK does not package.""",
559636
),
560637
"_checker": attr.label(
561638
default = "//tools/flutter:check_native_assets.py",
@@ -649,6 +726,27 @@ def _per_abi(supplied, abis, kind):
649726
return _plugin_libs(kind, abis)
650727
return {abi: supplied[abi] for abi in abis if abi in supplied}
651728

729+
# Macro-internal targets are not API. They carry `manual`, so a wildcard
730+
# pattern stops requesting them beside the transitioned edges that already
731+
# reach them -- Bazel's build unit is `(label, configuration)`, so an untagged
732+
# internal target is analysed and executed once per configuration. See
733+
# `.pi-flow/docs/adr/0010-tag-macro-internal-targets-manual.md`. The caller's
734+
# own tags are forwarded rather than replaced: a caller asking for
735+
# `no-remote-cache` on the APK means the graph that produces it too.
736+
#
737+
# Every internal declaration in this file goes through one of these two. A new
738+
# one that does not is a wildcard root, which is the omission this centralises.
739+
def _internal_tags(kwargs):
740+
tags = list(kwargs.get("tags", []))
741+
if "manual" not in tags:
742+
tags.append("manual")
743+
return tags
744+
745+
def _internal_kwargs(kwargs):
746+
internal = dict(kwargs)
747+
internal["tags"] = _internal_tags(kwargs)
748+
return internal
749+
652750
def flutter_android_libs(
653751
name,
654752
abis,
@@ -715,7 +813,8 @@ def flutter_android_libs(
715813
Under `mode=debug` the `aot_library` contribution and its `_libapp`
716814
export are dropped: debug ships no AOT snapshot, so nothing supplies
717815
one and android_binary must not expect it.
718-
**kwargs: visibility, tags.
816+
**kwargs: visibility and tags for the caller-facing join and check.
817+
Macro-internal helper and contribution targets append `manual`.
719818
"""
720819
no_plugin_graph = plugins == None
721820
if native_libs == None:
@@ -728,6 +827,7 @@ def flutter_android_libs(
728827
embedding_deps = flutter_embedding_deps(maven_repo)
729828

730829
check_abis(abis, "flutter_android_libs " + name)
830+
internal_kwargs = _internal_kwargs(kwargs)
731831

732832
# A non-empty dict has to cover the declared ABI set exactly. `{}` is the
733833
# deliberate declaration that this app has no recipe or plugin-native
@@ -800,7 +900,7 @@ def flutter_android_libs(
800900
name = libapp_jars[abi],
801901
src = "{}_{}".format(aot, abi),
802902
abi = abi,
803-
**kwargs
903+
**internal_kwargs
804904
)
805905

806906
engine_stripped[abi] = "{}_engine_{}_stripped".format(name, abi)
@@ -811,7 +911,7 @@ def flutter_android_libs(
811911
_MODE_RELEASE: engine_jar_label(abi, "release"),
812912
})),
813913
abi = abi,
814-
**kwargs
914+
**internal_kwargs
815915
)
816916

817917
# One java_import per contribution, not per ABI: android_binary collects
@@ -820,12 +920,12 @@ def flutter_android_libs(
820920
java_import(
821921
name = name + "_libapp",
822922
jars = [libapp_jars[abi] for abi in abis],
823-
**kwargs
923+
**internal_kwargs
824924
)
825925
java_import(
826926
name = name + "_engine",
827927
jars = [engine_stripped[abi] for abi in abis],
828-
**kwargs
928+
**internal_kwargs
829929
)
830930

831931
# Which contributions vary by ABI is the location's property, not a second
@@ -857,6 +957,7 @@ def flutter_android_libs(
857957
"registrant",
858958
]
859959
contributions = []
960+
asset_contribution = None
860961
aot_contribution = None
861962
for kind, location in _ANDROID_CONTRIBUTIONS:
862963
contribution = "{}_{}_contribution".format(name, kind)
@@ -872,17 +973,21 @@ def flutter_android_libs(
872973
# emptiness is still explicit at this boundary; the two structural
873974
# runtime contributions and the asset/AOT/engine never are.
874975
empty = kind in empty_kinds and not srcs and not libraries,
875-
**kwargs
976+
**internal_kwargs
876977
)
877978
if kind == "aot_library":
878979
# Declare AOT for all modes; debug excludes it before inputs are needed.
879980
aot_contribution = contribution
981+
elif kind == "assets":
982+
# android_binary's assets edge does not take its deps split.
983+
asset_contribution = contribution
880984
else:
881985
contributions.append(contribution)
882986

883987
flutter_bundle_check(
884988
name = name + "_check",
885989
abis = abis,
990+
assets_contribution = asset_contribution,
886991
contributions = contributions + select({
887992
_MODE_DEBUG: [],
888993
_MODE_RELEASE: [aot_contribution],
@@ -958,23 +1063,6 @@ _MANIFEST_VALUES = {
9581063
"targetSdkVersion": "36",
9591064
}
9601065

961-
# Pin release builds to `opt`; preserve explicitly non-fastbuild modes.
962-
def _pin_release_compilation_mode_impl(settings, _attr):
963-
mode = settings["//tools/flutter:mode"]
964-
compilation_mode = str(settings["//command_line_option:compilation_mode"])
965-
if mode == "release" and compilation_mode == "fastbuild":
966-
compilation_mode = "opt"
967-
return {"//command_line_option:compilation_mode": compilation_mode}
968-
969-
_pin_release_compilation_mode = transition(
970-
implementation = _pin_release_compilation_mode_impl,
971-
inputs = [
972-
"//tools/flutter:mode",
973-
"//command_line_option:compilation_mode",
974-
],
975-
outputs = ["//command_line_option:compilation_mode"],
976-
)
977-
9781066
def _flutter_apk_impl(ctx):
9791067
apk = ctx.attr.apk
9801068
info = apk[ApkInfo]
@@ -1194,13 +1282,17 @@ def flutter_android_binary(
11941282
maven_repo: the repository the embedding's Maven deps resolve in; must
11951283
match what `flutter_embedding_library` was given.
11961284
engine_jars: ABI -> an engine jar overriding the ABI table's.
1197-
**kwargs: passed to every android_binary this declares -- visibility,
1198-
tags, and anything else android_binary accepts, except
1199-
`proguard_specs`, which the release-compilation-mode wrapper refuses
1200-
(see docs_internal/compilation-mode-pinning.md). The emitted
1201-
`<name>_check_test` takes only visibility and tags.
1285+
**kwargs: android_binary attributes for the private packaging target --
1286+
anything android_binary accepts, except `proguard_specs`, which the
1287+
release-compilation-mode wrapper refuses (see
1288+
docs_internal/compilation-mode-pinning.md). `visibility` lands on the
1289+
public APK wrapper rather than the private target, and `tags` are
1290+
forwarded to both, with `manual` appended for the macro-internal half
1291+
(`.pi-flow/docs/adr/0010-tag-macro-internal-targets-manual.md`). The
1292+
emitted `<name>_check_test` takes only visibility and tags.
12021293
"""
12031294
check_abis(abis, "flutter_android_binary " + name)
1295+
internal_tags = _internal_tags(kwargs)
12041296

12051297
if kwargs.get("proguard_specs"):
12061298
fail(
@@ -1270,6 +1362,7 @@ def flutter_android_binary(
12701362
name = versioned_manifest,
12711363
manifest = manifest,
12721364
pubspec = pubspec,
1365+
tags = internal_tags,
12731366
)
12741367

12751368
if resource_files == None:
@@ -1288,6 +1381,13 @@ def flutter_android_binary(
12881381
# Android manifest merging requires a package.
12891382
custom_package = values["applicationId"],
12901383
resource_files = [],
1384+
# Reached only through the private APK's `deps`, and only under
1385+
# `mode=debug`. Without `manual`, wildcard expansion builds this
1386+
# debug-only library in the release configuration, where nothing
1387+
# packages it, and requests it a second time in debug beside the
1388+
# APK's split edge. Measured cost is in
1389+
# `.pi-flow/configuration-forks/issues/02-collapse-android-split-duplication.md`.
1390+
tags = internal_tags,
12911391
)
12921392
debug_manifest_deps = select({
12931393
_MODE_DEBUG: [":" + debug_manifest_lib],
@@ -1359,10 +1459,12 @@ def flutter_android_binary(
13591459
plugins = plugins,
13601460
registrant = registrant,
13611461
maven_repo = maven_repo,
1462+
tags = internal_tags,
13621463
)
13631464

13641465
packaging_kwargs = dict(kwargs)
13651466
wrapper_visibility = packaging_kwargs.pop("visibility", None)
1467+
packaging_kwargs.pop("tags", None)
13661468

13671469
packaged = target + "_apk"
13681470
android_binary(
@@ -1374,6 +1476,7 @@ def flutter_android_binary(
13741476
resource_files = resource_files,
13751477
deps = deps + [":" + join] + debug_manifest_deps,
13761478
visibility = ["//visibility:private"],
1479+
tags = internal_tags,
13771480
**packaging_kwargs
13781481
)
13791482

0 commit comments

Comments
 (0)