From 44fac23cbfe97409b28f758661a57641f0160e1c Mon Sep 17 00:00:00 2001 From: Greg Estren Date: Thu, 11 Mar 2021 22:50:41 -0500 Subject: [PATCH 1/2] Get tests passing. - cquery now returns short config hashes (expect shorter lengths) - Starlark flags must now use "--//flag=value" syntax --- tools/ctexplain/bazel_api_test.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tools/ctexplain/bazel_api_test.py b/tools/ctexplain/bazel_api_test.py index e3d404c4f329eb..d3eb17817ac77c 100644 --- a/tools/ctexplain/bazel_api_test.py +++ b/tools/ctexplain/bazel_api_test.py @@ -45,7 +45,7 @@ def testBasicCquery(self): self.assertEqual(len(cts), 1) self.assertEqual(cts[0].label, '//testapp:fg') self.assertIsNone(cts[0].config) - self.assertGreater(len(cts[0].config_hash), 10) + self.assertGreater(len(cts[0].config_hash), 6) self.assertIn('PlatformConfiguration', cts[0].transitive_fragments) def testFailedCquery(self): @@ -134,7 +134,9 @@ def testConfigWithDefines(self): def testConfigWithStarlarkFlags(self): self.ScratchFile('testapp/defs.bzl', [ - 'def _flag_impl(settings, attr):', ' pass', 'string_flag = rule(', + 'def _flag_impl(settings, attr):', + ' pass', + 'string_flag = rule(', ' implementation = _flag_impl,', ' build_setting = config.string(flag = True)' ')' @@ -144,7 +146,7 @@ def testConfigWithStarlarkFlags(self): 'string_flag(name = "my_flag", build_setting_default = "nada")', 'filegroup(name = "fg", srcs = ["a.file"])', ]) - cquery_args = ['//testapp:fg', '--//testapp:my_flag', 'algo'] + cquery_args = ['//testapp:fg', '--//testapp:my_flag=algo'] cts = self._bazel_api.cquery(cquery_args)[2] config = self._bazel_api.get_config(cts[0].config_hash) user_defined_options = config.options['user-defined'] From 2a4cc251f95b47f0480581f0b24ab2cfea53b51f Mon Sep 17 00:00:00 2001 From: Greg Estren Date: Thu, 11 Mar 2021 23:38:54 -0500 Subject: [PATCH 2/2] Report build trimmability. --- tools/ctexplain/BUILD | 13 ++ tools/ctexplain/analyses/summary.py | 33 ++++- tools/ctexplain/analyses/summary_test.py | 28 ++++- tools/ctexplain/bazel_api.py | 10 +- tools/ctexplain/ctexplain.py | 26 ++-- tools/ctexplain/integration_test.py | 125 +++++++++++++++++++ tools/ctexplain/lib.py | 152 ++++++++++++++++++++++- tools/ctexplain/lib_test.py | 122 +++++++++++++++++- tools/ctexplain/types.py | 4 +- 9 files changed, 483 insertions(+), 30 deletions(-) create mode 100644 tools/ctexplain/integration_test.py diff --git a/tools/ctexplain/BUILD b/tools/ctexplain/BUILD index 2ee9b757bb0cd8..b45e4d4fdcf277 100644 --- a/tools/ctexplain/BUILD +++ b/tools/ctexplain/BUILD @@ -106,3 +106,16 @@ py_test( "//third_party/py/frozendict", ], ) + +py_test( + name = "integration_test", + size = "small", + srcs = ["integration_test.py"], + python_version = "PY3", + deps = [ + ":analyses", + ":bazel_api", + ":lib", + "//src/test/py/bazel:test_base", + ], +) diff --git a/tools/ctexplain/analyses/summary.py b/tools/ctexplain/analyses/summary.py index cf9a1bbde169fc..c9ad9ff87c2c25 100644 --- a/tools/ctexplain/analyses/summary.py +++ b/tools/ctexplain/analyses/summary.py @@ -13,13 +13,14 @@ # See the License for the specific language governing permissions and # limitations under the License. """Analysis that summarizes basic graph info.""" +from typing import Mapping from typing import Tuple # Do not edit this line. Copybara replaces it with PY2 migration helper. from dataclasses import dataclass from tools.ctexplain.types import ConfiguredTarget -# Do not edit this line. Copybara replaces it with PY2 migration helper..third_party.bazel.tools.ctexplain.util as util +import tools.ctexplain.util as util @dataclass(frozen=True) @@ -29,17 +30,31 @@ class _Summary(): configurations: int # Number of unique target labels. targets: int - # Number of configured targets. + # Number of configured targets in the actual build (without trimming). configured_targets: int # Number of targets that produce multiple configured targets. This is more # subtle than computing configured_targets - targets. For example, if # targets=2 and configured_targets=4, that could mean both targets are # configured twice. Or it could mean the first target is configured 3 times. repeated_targets: int + # Number of configured targets if the build were optimally trimmed. + trimmed_configured_targets: int -def analyze(cts: Tuple[ConfiguredTarget, ...]) -> _Summary: - """Runs the analysis on a build's configured targets.""" +def analyze( + cts: Tuple[ConfiguredTarget, ...], + trimmed_cts: Mapping[ConfiguredTarget, Tuple[ConfiguredTarget, ...]] + ) -> _Summary: + """Runs the analysis on a build's configured targets. + + Args: + cts: A build's untrimmed configured targets. + trimmed_cts: The equivalent trimmed cts, where each map entry maps a trimmed + ct to the untrimmed cts that reduce to it. + + Returns: + Analysis result as a _Summary. + """ configurations = set() targets = set() label_count = {} @@ -50,8 +65,8 @@ def analyze(cts: Tuple[ConfiguredTarget, ...]) -> _Summary: configured_targets = len(cts) repeated_targets = sum([1 for count in label_count.values() if count > 1]) - return _Summary( - len(configurations), len(targets), configured_targets, repeated_targets) + return _Summary(len(configurations), len(targets), configured_targets, + repeated_targets, len(trimmed_cts)) def report(result: _Summary) -> None: @@ -64,9 +79,15 @@ def report(result: _Summary) -> None: result: the analysis result """ ct_surplus = util.percent_diff(result.targets, result.configured_targets) + trimmed_ct_surplus = util.percent_diff(result.targets, + result.trimmed_configured_targets) + trimming_reduction = util.percent_diff(result.configured_targets, + result.trimmed_configured_targets) print(f""" Configurations: {result.configurations} Targets: {result.targets} Configured targets: {result.configured_targets} ({ct_surplus} vs. targets) Targets with multiple configs: {result.repeated_targets} +Configured targets with optimal trimming: {result.trimmed_configured_targets} ({trimmed_ct_surplus} vs. targets) +Trimming impact on configured target graph size: {trimming_reduction} """) diff --git a/tools/ctexplain/analyses/summary_test.py b/tools/ctexplain/analyses/summary_test.py index 0636d272e2ad89..c85fcef434fd63 100644 --- a/tools/ctexplain/analyses/summary_test.py +++ b/tools/ctexplain/analyses/summary_test.py @@ -18,7 +18,7 @@ # Do not edit this line. Copybara replaces it with PY2 migration helper. from frozendict import frozendict -# Do not edit this line. Copybara replaces it with PY2 migration helper..third_party.bazel.tools.ctexplain.analyses.summary as summary +import tools.ctexplain.analyses.summary as summary from tools.ctexplain.types import Configuration from tools.ctexplain.types import ConfiguredTarget from tools.ctexplain.types import NullConfiguration @@ -26,7 +26,7 @@ class SummaryTest(unittest.TestCase): - def testAnalysis(self): + def testAnalysisNoTrimming(self): config1 = Configuration(None, frozendict({'a': frozendict({'b': 'c'})})) config2 = Configuration(None, frozendict({'d': frozendict({'e': 'f'})})) @@ -35,12 +35,34 @@ def testAnalysis(self): ct3 = ConfiguredTarget('//foo', NullConfiguration(), 'null', None) ct4 = ConfiguredTarget('//bar', config1, 'hash1', None) - res = summary.analyze((ct1, ct2, ct3, ct4)) + untrimmed_ct_map = {ct1: (ct1,), ct2: (ct2,), ct3: (ct3,), ct4: (ct4,)} + + res = summary.analyze((ct1, ct2, ct3, ct4), untrimmed_ct_map) self.assertEqual(3, res.configurations) self.assertEqual(2, res.targets) self.assertEqual(4, res.configured_targets) self.assertEqual(1, res.repeated_targets) + def testAnalysWithTrimming(self): + config1 = Configuration(None, frozendict({'a': frozendict({'b': 'c'})})) + config2 = Configuration(None, frozendict({'d': frozendict({'e': 'f'})})) + + ct1 = ConfiguredTarget('//foo', config1, 'hash1', None) + ct2 = ConfiguredTarget('//foo', config2, 'hash2', None) + ct3 = ConfiguredTarget('//bar', config1, 'hash1', None) + ct4 = ConfiguredTarget('//bar', config2, 'hash2', None) + + trimmed_ct1 = ConfiguredTarget('//foo', config1, 'trimmed_hash1', None) + trimmed_ct2 = ConfiguredTarget('//bar', config1, 'trimmed_hash1', None) + trimmed_cts = {trimmed_ct1: (ct1, ct2), trimmed_ct2: (ct3, ct4)} + + res = summary.analyze((ct1, ct2, ct3, ct4), trimmed_cts) + self.assertEqual(2, res.configurations) + self.assertEqual(2, res.targets) + self.assertEqual(4, res.configured_targets) + self.assertEqual(2, res.trimmed_configured_targets) + self.assertEqual(2, res.repeated_targets) + if __name__ == '__main__': unittest.main() diff --git a/tools/ctexplain/bazel_api.py b/tools/ctexplain/bazel_api.py index e19203169f3ae6..c723a9a6f88b1f 100644 --- a/tools/ctexplain/bazel_api.py +++ b/tools/ctexplain/bazel_api.py @@ -43,7 +43,7 @@ def run_bazel_in_client(args: List[str]) -> Tuple[int, List[str], List[str]]: Tuple of (return code, stdout, stderr) """ result = subprocess.run( - ["blaze"] + args, + ["bazel"] + args, cwd=os.getcwd(), stdout=subprocess.PIPE, stderr=subprocess.PIPE, @@ -84,13 +84,17 @@ def cquery(self, return (False, stderr, ()) cts = [] + # TODO(gregce): cquery should return sets of targets and mostly does. But + # certain targets might repeat. This line was added in response to a build + # that showed @local_config_cc//:toolchain appearing twice. + seen_cts = set() for line in stdout: if not line.strip(): continue ctinfo = _parse_cquery_result_line(line) - if ctinfo is not None: + if ctinfo is not None and ctinfo not in seen_cts: cts.append(ctinfo) - + seen_cts.add(ctinfo) return (True, stderr, tuple(cts)) def get_config(self, config_hash: str) -> Configuration: diff --git a/tools/ctexplain/ctexplain.py b/tools/ctexplain/ctexplain.py index cb300a45cd9516..e049c1d232f7ce 100644 --- a/tools/ctexplain/ctexplain.py +++ b/tools/ctexplain/ctexplain.py @@ -43,11 +43,14 @@ from absl import flags from dataclasses import dataclass -# Do not edit this line. Copybara replaces it with PY2 migration helper..third_party.bazel.tools.ctexplain.analyses.summary as summary +# Do not edit this line. Copybara replaces it with PY2 migration helper.. +import tools.ctexplain.analyses.summary as summary from tools.ctexplain.bazel_api import BazelApi -# Do not edit this line. Copybara replaces it with PY2 migration helper..third_party.bazel.tools.ctexplain.lib as lib +# Do not edit this line. Copybara replaces it with PY2 migration helper.. +import tools.ctexplain.lib as lib from tools.ctexplain.types import ConfiguredTarget -# Do not edit this line. Copybara replaces it with PY2 migration helper..third_party.bazel.tools.ctexplain.util as util +# Do not edit this line. Copybara replaces it with PY2 migration helper.. +import tools.ctexplain.util as util FLAGS = flags.FLAGS @@ -57,26 +60,28 @@ class Analysis(): """Supported analysis type.""" # The value in --analysis= that triggers this analysis. key: str - # The function that invokes this analysis. - exec: Callable[[Tuple[ConfiguredTarget, ...]], None] + # The function that invokes this analysis. First parameter is the build's + # untrimmed configured targets. Second parameter is what the build's + # configured targets would be if it were perfectly trimmed. + exec: Callable[[Tuple[ConfiguredTarget, ...], Tuple[ConfiguredTarget, ...]], None] # User-friendly analysis description. description: str available_analyses = [ Analysis( "summary", - lambda x: summary.report(summary.analyze(x)), + lambda x, y: summary.report(summary.analyze(x, y)), "summarizes build graph size and how trimming could help" ), Analysis( "culprits", - lambda x: print("this analysis not yet implemented"), + lambda x, y: print("this analysis not yet implemented"), "shows which flags unnecessarily fork configured targets. These\n" + "are conceptually mergeable." ), Analysis( "forked_targets", - lambda x: print("this analysis not yet implemented"), + lambda x, y: print("this analysis not yet implemented"), "ranks targets by how many configured targets they\n" + "create. These may be legitimate forks (because they behave " + "differently with\n different flags) or identical clones that are " @@ -84,7 +89,7 @@ class Analysis(): ), Analysis( "cloned_targets", - lambda x: print("this analysis not yet implemented"), + lambda x, y: print("this analysis not yet implemented"), "ranks targets by how many behavior-identical configured\n targets " + "they produce. These are conceptually mergeable." ) @@ -151,8 +156,9 @@ def main(argv): build_desc = ",".join(labels) with util.ProgressStep(f"Collecting configured targets for {build_desc}"): cts = lib.analyze_build(BazelApi(), labels, build_flags) + trimmed_cts = lib.trim_configured_targets(cts) for analysis in FLAGS.analysis: - analyses[analysis].exec(cts) + analyses[analysis].exec(cts, trimmed_cts) if __name__ == "__main__": diff --git a/tools/ctexplain/integration_test.py b/tools/ctexplain/integration_test.py new file mode 100644 index 00000000000000..8da1579c9ba069 --- /dev/null +++ b/tools/ctexplain/integration_test.py @@ -0,0 +1,125 @@ +# Lint as: python3 +# Copyright 2021 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Tests expected results over actual builds.""" +from typing import Mapping +from typing import Tuple +import unittest + +from src.test.py.bazel import test_base +import tools.ctexplain.analyses.summary as summary +import tools.ctexplain.bazel_api as bazel_api +import tools.ctexplain.lib as lib +from tools.ctexplain.types import ConfiguredTarget + +Cts = Tuple[ConfiguredTarget, ...] +TrimmedCts = Mapping[ConfiguredTarget, Cts] + + +class IntegrationTest(test_base.TestBase): + + _bazel_api: bazel_api.BazelApi = None + + def setUp(self): + test_base.TestBase.setUp(self) + self._bazel_api = bazel_api.BazelApi(self.RunBazel) + self.ScratchFile('WORKSPACE') + self.CreateWorkspaceWithDefaultRepos('repo/WORKSPACE') + self.ScratchFile('tools/allowlists/function_transition_allowlist/BUILD', [ + 'package_group(', + ' name = "function_transition_allowlist",', + ' packages = ["//..."])', + ]) + + def tearDown(self): + test_base.TestBase.tearDown(self) + + def _get_cts(self, labels: Tuple[str, ...], + build_flags: Tuple[str, ...]) -> Tuple[Cts, TrimmedCts]: + """Returns a build's configured targets. + + Args: + labels: The targets to build. + build_flags: The build flags to use. + + Returns: + Tuple (cts, trimmed_cts) where cts is the set of untrimmed configured + targets for the build and trimmed_cts maps trimmed configured targets to + their untrimmed variants. + """ + cts = lib.analyze_build(self._bazel_api, labels, build_flags) + trimmed_cts = lib.trim_configured_targets(cts) + return (cts, trimmed_cts) + + # Simple example of a build where trimming makes a big diffrence. + # + # Building ":split" builds ":dep1" and its subgraph in two distinct + # configurations: one with --python_version="PY3" (the default) and + # one with --python_version="PY2". + # + # None of these rules need the Python configuration. So they should + # all be reducible to the same trimmed equivalent. + def testHighlyTrimmableBuild(self): + self.ScratchFile('testapp/defs.bzl', [''' +def _rule_impl(ctx): + pass +simple_rule = rule( + implementation = _rule_impl, + attrs = { "deps": attr.label_list(), }, +) + +def _my_transition_impl(settings, attr): + return {"//command_line_option:python_version": "PY2"} +_my_transition = transition( + implementation = _my_transition_impl, + inputs = [], + outputs = ["//command_line_option:python_version"], +) + +splitter_rule = rule( + implementation = _rule_impl, + attrs = { + "_allowlist_function_transition": attr.label( + default = "//tools/allowlists/function_transition_allowlist", + ), + "cfg1_deps": attr.label_list(), + "cfg2_deps": attr.label_list(cfg = _my_transition), + }, +)''']) + self.ScratchFile('testapp/BUILD', [''' +load(":defs.bzl", "simple_rule", "splitter_rule") +simple_rule( + name = "dep1", + deps = [":dep2"], +) +simple_rule(name = "dep2") +splitter_rule( + name = "buildme", + cfg1_deps = [":dep1"], + cfg2_deps = [":dep1"], +) +''']) + cts, trimmed_cts = self._get_cts(('//testapp:buildme',), ()) + stats = summary.analyze(cts, trimmed_cts) + self.assertEqual(stats.configurations, 3) + # It's hard to guess the exact number of configured targets since the + # dependency graph has a bunch of (often changing) implicit deps. But we + # expect it to be substantailly greater than the number of targets. + self.assertLess(stats.targets, stats.configured_targets / 1.5) + self.assertEqual(stats.targets, stats.trimmed_configured_targets) + self.assertGreater(stats.repeated_targets, 6) + + +if __name__ == '__main__': + unittest.main() diff --git a/tools/ctexplain/lib.py b/tools/ctexplain/lib.py index 1ddb8c9257521b..e6b70a1e139707 100644 --- a/tools/ctexplain/lib.py +++ b/tools/ctexplain/lib.py @@ -13,13 +13,18 @@ # See the License for the specific language governing permissions and # limitations under the License. """General-purpose business logic.""" +from typing import List +from typing import Mapping +from typing import Set from typing import Tuple -# Do not edit this line. Copybara replaces it with PY2 migration helper..third_party.bazel.tools.ctexplain.bazel_api as bazel_api +from frozendict import frozendict +from tools.ctexplain.bazel_api import BazelApi +from tools.ctexplain.types import Configuration from tools.ctexplain.types import ConfiguredTarget -def analyze_build(bazel: bazel_api.BazelApi, labels: Tuple[str, ...], +def analyze_build(bazel: BazelApi, labels: Tuple[str, ...], build_flags: Tuple[str, ...]) -> Tuple[ConfiguredTarget, ...]: """Gets a build invocation's configured targets. @@ -51,7 +56,146 @@ def analyze_build(bazel: bazel_api.BazelApi, labels: Tuple[str, ...], hashes_to_configs[ct.config_hash] = bazel.get_config(ct.config_hash) config = hashes_to_configs[ct.config_hash] cts_with_configs.append( - ConfiguredTarget(ct.label, config, ct.config_hash, - ct.transitive_fragments)) + ConfiguredTarget( + ct.label, + config, + ct.config_hash, + ct.transitive_fragments)) return tuple(cts_with_configs) + + +def trim_configured_targets( + cts: Tuple[ConfiguredTarget, ...] + ) -> Mapping[ConfiguredTarget, Tuple[ConfiguredTarget, ...]]: + """Trims a set of configured targets to only include needed fragments. + + An "untrimmed" configured target contains all fragments and user-defined + options (like Starlark flags or --define) in its configuration, regardless of + whether the target actually needs them. For example, configurations include + both Java-related and C++-related fragments that encapsulate Java-related + flags and C++ flags, respectively. A C++ binary doesn't "need" the Java + fragment since C++ compilation doesn't use Java flags. + + A "trimmed" configured target only includes the fragments and user-defined + options actually needed. So a trimmed cc_binary includes the C++ fragment but + not the Java fragment. It includes --define foo=bar if it has a select() on + --foo (or the "$(foo)" Make variable in one of its attributes), otherwise it + drops it. And so on. + + Args: + cts: Set of untrimmed configured targets. + + Returns: + A mapping from each trimmed configured target to the untrimmed ones that + that reduce to it. For example, if a cc_binary appears in two configurations + that only differ in Java options, the trimmed, the map has one key (the + config minus its java options) with two values (each original config). + """ + ans = {} + for ct in cts: + trimmed_ct = _trim_configured_target(ct, _get_required_fragments(ct)) + ans.setdefault(trimmed_ct, []).append(ct) + return ans + + +# All configurations have the same config fragment -> options fragment map. +# Compute it once for efficiency. +_options_to_fragments: Mapping[str, List[str]] = {} + + +def _get_required_fragments(ct: ConfiguredTarget) -> Set[str]: + """Normalizes and returns the fragments required by a configured target. + + Normalization means: + - If a rule class (like cc_binary) requires a fragment, include it. + - If a select() requires a flag, figure out which options fragment (which + is not the same as a configuration fragment) that means. For example, + "--copt" belongs to options fragment CppOptions. Then choose one of the + configuration fragments that requires CppOptions: CppConfiguration or + ObjcConfiguration. Either is sufficient to guarantee the presence of + CppOptions. Heuristically, we choose the fragment requiring the smallest + number of option fragments to try to be as granular as possible. + - If a target consumes --define foo= anywhere, include "--define foo". + - If a target consumes a Starlark flag, include that flag's label. + + Args: + ct: Untrimmed configured target. + + Returns: + Set of required configuration pieces. + """ + if not ct.config.fragments: # Null configurations are already empty. + return set() + if not _options_to_fragments: + for fragment, options_fragments in ct.config.fragments.items(): + for o in options_fragments: + _options_to_fragments.setdefault(o, set()).add(fragment) + + ans = [] + for req in ct.transitive_fragments: + if (req in ct.config.fragments or req.startswith("--") or + req == "CoreOptions"): + # The requirement is a fragment, Starlark flag, or core (universally + # required) option. + ans.append(req) + elif req in _options_to_fragments: + # The requirement is a native option (like "--copt"). Find a matching + # fragment. + fragments = _options_to_fragments[req] + matches = [(len(ct.config.fragments[frag]), frag) for frag in fragments] + # TODO(gregce): scan for existing fulfilling fragment, prefer that. + ans.append(sorted(matches)[0][1]) # Sort each entry by count, then name. + else: + raise ValueError(f"{ct.label}: don't understand requirement {req}") + return set(ans) + + +# CoreOptions entries that should always be trimmed because they change with +# configuration changes but don't actually cause those changes. +_trimmable_core_options = ( + "affected by starlark transition", + "transition directory name fragment", +) + + +def _trim_configured_target(ct: ConfiguredTarget, + required_fragments: Set[str]) -> ConfiguredTarget: + """Trims a configured target to only the config pieces it needs. + + Args: + ct: Untrimmed configured tareget. + required_fragments: Configuration pieces the configured target requires, + as reported by _get_required_fragments. + + Returns: + Trimmed copy of the configured target. Both config.fragments and + config.options are suitably trimmed. + """ + trimmed_options = {} + for (options_class, options) in ct.config.options.items(): + # CoreOptions are universally included with no owning fragments. + if options_class == "CoreOptions": + trimmed_options[options_class] = frozendict({ + k: v for k, v in options.items() if k not in _trimmable_core_options + }) + elif options_class == "user-defined": + # Include each user-defined option on a case-by-case basis, since + # user-defined requirements are declared directly on each option. + trimmed_options["user-defined"] = frozendict({ + name: val + for name, val in ct.config.options["user-defined"].items() + if name in required_fragments + }) + else: + associated_fragments = set(_options_to_fragments[options_class]) + if associated_fragments & required_fragments: + trimmed_options[options_class] = options + + trimmed_fragments = frozendict({ + fragment: options for fragment, options in ct.config.fragments.items() + if fragment in required_fragments + }) + trimmed_config = Configuration(trimmed_fragments, frozendict(trimmed_options)) + return ConfiguredTarget( + ct.label, trimmed_config, "trimmed hash", ct.transitive_fragments) diff --git a/tools/ctexplain/lib_test.py b/tools/ctexplain/lib_test.py index 6616e6d72c047d..bbd53a45447bfd 100644 --- a/tools/ctexplain/lib_test.py +++ b/tools/ctexplain/lib_test.py @@ -14,10 +14,13 @@ # limitations under the License. """Tests for lib.py.""" import unittest +from frozendict import frozendict + from src.test.py.bazel import test_base -# Do not edit this line. Copybara replaces it with PY2 migration helper..third_party.bazel.tools.ctexplain.bazel_api as bazel_api -# Do not edit this line. Copybara replaces it with PY2 migration helper..third_party.bazel.tools.ctexplain.lib as lib +import tools.ctexplain.bazel_api as bazel_api +import tools.ctexplain.lib as lib from tools.ctexplain.types import Configuration +from tools.ctexplain.types import ConfiguredTarget from tools.ctexplain.types import HostConfiguration from tools.ctexplain.types import NullConfiguration @@ -85,6 +88,121 @@ def testAnalyzeBuildNoRepeats(self): [ct.label for ct in cts], ['//testapp:a', '//testapp:h', '//testapp:other', '//testapp:h.src']) + def testBasicTrimming(self): + fragments = frozendict({ + 'FooFragment': ('FooOptions',), + 'BarFragment': ('BarOptions',), + }) + options1 = frozendict({ + 'FooOptions': frozendict({'foo_opt': 'foo_val1'}), + 'BarOptions': frozendict({'bar_opt': 'bar_val1'}), + }) + options2 = frozendict({ + 'FooOptions': frozendict({'foo_opt': 'foo_val2'}), + 'BarOptions': frozendict({'bar_opt': 'bar_val1'}), + }) + options3 = frozendict({ + 'FooOptions': frozendict({'foo_opt': 'foo_val1'}), + 'BarOptions': frozendict({'bar_opt': 'bar_val2'}), + }) + + config1 = Configuration(fragments, options1) + config2 = Configuration(fragments, options2) + config3 = Configuration(fragments, options3) + + ct1 = ConfiguredTarget('//foo', config1, 'hash1', ('FooFragment',)) + ct2 = ConfiguredTarget('//foo', config2, 'hash2', ('FooFragment',)) + ct3 = ConfiguredTarget('//foo', config3, 'hash3', ('FooFragment',)) + + get_foo_opt = lambda x: x.config.options['FooOptions']['foo_opt'] + trimmed_cts = sorted(lib.trim_configured_targets((ct1, ct2, ct3)).items(), + key=lambda x: get_foo_opt(x[0])) + + self.assertEqual(len(trimmed_cts), 2) + self.assertEqual(get_foo_opt(trimmed_cts[0][0]), 'foo_val1') + self.assertListEqual(trimmed_cts[0][1], [ct1, ct3]) + self.assertEqual(get_foo_opt(trimmed_cts[1][0]), 'foo_val2') + self.assertListEqual(trimmed_cts[1][1], [ct2]) + + # Requirements are FragmentOptions, not Fragments. + def testTrimRequiredOptions(self): + config = Configuration( + fragments=frozendict({ + 'FooFragment': ('FooOptions',), + 'BarFragment': ('BarOptions',), + 'GreedyFragment': ('FooOptions', 'BarOptions'), + }), + options=frozendict({ + 'FooOptions': frozendict({'foo_opt': 'foo_val'}), + 'BarOptions': frozendict({'bar_opt': 'bar_val'}), + }) + ) + + ct = ConfiguredTarget('//foo', config, 'hash', ('FooOptions',)) + trimmed_cts = lib.trim_configured_targets((ct,)) + trimmed_ct = list(trimmed_cts.keys())[0] + + self.assertEqual(len(trimmed_cts), 1) + # Currently expect to keep the requiring fragment (of all that require + # 'FooOptions') with the smallest number of total requirements. + self.assertTupleEqual(tuple(trimmed_ct.config.fragments.keys()), + ('FooFragment',)) + self.assertEqual(trimmed_ct.config.options, + frozendict( + {'FooOptions': frozendict({'foo_opt': 'foo_val'})} + )) + + def testTrimUserDefinedFlags(self): + config = Configuration( + fragments=frozendict({'FooFragment': ('FooOptions',)}), + options=frozendict({ + 'FooOptions': frozendict({}), + 'user-defined': frozendict({ + '--define:foo': 'foo_val', + '--define:bar': 'bar_val', + '--//starlark_foo_flag': 'starlark_foo', + '--//starlark_bar_flag': 'starlark_bar', + }), + }) + ) + + required = ('FooFragment', '--define:foo', '--//starlark_bar_flag') + ct = ConfiguredTarget('//foo', config, 'hash', required) + trimmed_cts = lib.trim_configured_targets((ct,)) + trimmed_ct = list(trimmed_cts.keys())[0] + + self.assertEqual(len(trimmed_cts), 1) + self.assertTupleEqual(tuple(trimmed_ct.config.fragments.keys()), + ('FooFragment',)) + self.assertEqual(trimmed_ct.config.options, + frozendict({ + 'FooOptions': frozendict({}), + 'user-defined': frozendict({ + '--define:foo': 'foo_val', + '--//starlark_bar_flag': 'starlark_bar', + }), + })) + + def testTrimUnnecessaryCoreOptions(self): + config = Configuration( + fragments=frozendict({}), + options=frozendict({ + 'CoreOptions': frozendict({ + 'affected by starlark transition': 'drop this', + 'keep this': 'keep val', + 'transition directory name fragment': 'drop this too', + }) + })) + + ct = ConfiguredTarget('//foo', config, 'hash', ()) + trimmed_cts = lib.trim_configured_targets((ct,)) + trimmed_ct = list(trimmed_cts.keys())[0] + + self.assertEqual(len(trimmed_cts), 1) + self.assertEqual(trimmed_ct.config.options, + frozendict({ + 'CoreOptions': frozendict({'keep this': 'keep val'}), + })) if __name__ == '__main__': unittest.main() diff --git a/tools/ctexplain/types.py b/tools/ctexplain/types.py index c47ab9fb4b995c..f4e758cb423c1e 100644 --- a/tools/ctexplain/types.py +++ b/tools/ctexplain/types.py @@ -71,7 +71,7 @@ class HostConfiguration(Configuration): aren't "special" compared to normal configurations. """ # We don't currently read the host config's fragments or option values. - fragments: Tuple[str, ...] = () + fragments: Mapping[str, Tuple[str, ...]] = field(default_factory=lambda: frozendict({})) options: Mapping[str, Mapping[str, str]] = field(default_factory=lambda: frozendict({})) @@ -83,7 +83,7 @@ class NullConfiguration(Configuration): By definition this has no fragments or options. """ - fragments: Tuple[str, ...] = () + fragments: Mapping[str, Tuple[str, ...]] = field(default_factory=lambda: frozendict({})) options: Mapping[str, Mapping[str, str]] = field(default_factory=lambda: frozendict({}))