From 68e96eb29095443c078167effaf8f30e430cbbf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20M=C3=A1rquez=20P=C3=A9rez=20Mu=C3=B1=C3=ADz=20D?= =?UTF-8?q?=C3=ADaz=20Puras=20Thaureaux?= Date: Thu, 23 Mar 2023 20:34:18 +0000 Subject: cc_prebuilt_library_{shared,static}: macros->rules Convert cc_prebuilt_library_{shared,static} to use proper rules instead of macros, and in doing so also correctly support export_{,system_}includes. Also add a missing `alwayslink = False` test to *_static_test Test: cc_prebuilt_library_{shared,static}_test.bzl Test: mixed_droid.sh Bug: 229374533 Change-Id: I58ba298eee631e3d7ecd1fdf376d84f22004c945 --- rules/cc/cc_library_common.bzl | 19 ++++ rules/cc/cc_prebuilt_library_shared.bzl | 45 ++++++-- rules/cc/cc_prebuilt_library_shared_test.bzl | 83 +++++++++++++-- rules/cc/cc_prebuilt_library_static.bzl | 49 ++++++--- rules/cc/cc_prebuilt_library_static_test.bzl | 150 +++++++++++++++++++++++++-- 5 files changed, 307 insertions(+), 39 deletions(-) diff --git a/rules/cc/cc_library_common.bzl b/rules/cc/cc_library_common.bzl index 164d6629..9cdabeda 100644 --- a/rules/cc/cc_library_common.bzl +++ b/rules/cc/cc_library_common.bzl @@ -402,3 +402,22 @@ def create_cc_androidmk_provider(*, static_deps, whole_archive_deps, dynamic_dep local_whole_static_libs = local_whole_static_libs, local_shared_libs = local_shared_libs, ) + +def create_cc_prebuilt_library_info(ctx, lib_to_link): + "Create the CcInfo for a prebuilt_library_{shared,static}" + + compilation_context = cc_common.create_compilation_context( + includes = depset(get_includes_paths(ctx, ctx.attr.export_includes)), + system_includes = depset(get_includes_paths(ctx, ctx.attr.export_system_includes)), + ) + linker_input = cc_common.create_linker_input( + owner = ctx.label, + libraries = depset(direct = [lib_to_link] if lib_to_link != None else []), + ) + linking_context = cc_common.create_linking_context( + linker_inputs = depset(direct = [linker_input]), + ) + return CcInfo( + compilation_context = compilation_context, + linking_context = linking_context, + ) diff --git a/rules/cc/cc_prebuilt_library_shared.bzl b/rules/cc/cc_prebuilt_library_shared.bzl index d713a928..25144f86 100644 --- a/rules/cc/cc_prebuilt_library_shared.bzl +++ b/rules/cc/cc_prebuilt_library_shared.bzl @@ -12,16 +12,39 @@ # See the License for the specific language governing permissions and # limitations under the License. -def cc_prebuilt_library_shared( - name, - shared_library, - alwayslink = None, - **kwargs): - "Bazel macro to correspond with the *_cc_prebuilt_library_shared Soong module types" +load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain") +load(":cc_library_common.bzl", "create_cc_prebuilt_library_info") - native.cc_import( - name = name, - shared_library = shared_library, - alwayslink = alwayslink, - **kwargs +def _cc_prebuilt_library_shared_impl(ctx): + lib = ctx.file.shared_library + files = ctx.attr.shared_library.files if lib != None else None + cc_toolchain = find_cpp_toolchain(ctx) + feature_configuration = cc_common.configure_features( + ctx = ctx, + cc_toolchain = cc_toolchain, ) + cc_info = create_cc_prebuilt_library_info( + ctx, + cc_common.create_library_to_link( + actions = ctx.actions, + dynamic_library = lib, + feature_configuration = feature_configuration, + cc_toolchain = cc_toolchain, + ) if lib != None else None, + ) + return [DefaultInfo(files = files), cc_info] + +cc_prebuilt_library_shared = rule( + implementation = _cc_prebuilt_library_shared_impl, + attrs = dict( + shared_library = attr.label( + providers = [CcInfo], + allow_single_file = True, + ), + export_includes = attr.string_list(), + export_system_includes = attr.string_list(), + ), + toolchains = ["@bazel_tools//tools/cpp:toolchain_type"], + fragments = ["cpp"], + provides = [CcInfo], +) diff --git a/rules/cc/cc_prebuilt_library_shared_test.bzl b/rules/cc/cc_prebuilt_library_shared_test.bzl index 79084a0a..8fe63f85 100644 --- a/rules/cc/cc_prebuilt_library_shared_test.bzl +++ b/rules/cc/cc_prebuilt_library_shared_test.bzl @@ -16,30 +16,56 @@ limitations under the License. load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") load("//build/bazel/rules/cc:cc_prebuilt_library_shared.bzl", "cc_prebuilt_library_shared") +load("//build/bazel/rules/test_common:paths.bzl", "get_output_and_package_dir_based_path") + +_fake_expected_lib = "{[()]}" def _cc_prebuilt_library_shared_test_impl(ctx): env = analysistest.begin(ctx) target = analysistest.target_under_test(env) expected_lib = ctx.attr.expected_lib - cc_info = target[CcInfo] + compilation_context = cc_info.compilation_context linker_inputs = cc_info.linking_context.linker_inputs.to_list() libs_to_link = [] for lib in linker_inputs: libs_to_link += lib.libraries - asserts.true( - env, - expected_lib in [lib.dynamic_library.basename for lib in libs_to_link], - "\nExpected the target to include the shared library %s; but instead got:\n\t%s\n" % (expected_lib, libs_to_link), - ) + if expected_lib == _fake_expected_lib: + asserts.true( + env, + len(libs_to_link) == 0, + "\nExpected the shared library to be empty, but instead got:\n\t%s\n" % str(libs_to_link), + ) + else: + asserts.true( + env, + expected_lib in [lib.dynamic_library.basename for lib in libs_to_link], + "\nExpected the target to include the shared library %s; but instead got:\n\t%s\n" % (expected_lib, libs_to_link), + ) + + # Checking for the expected {,system_}includes + assert_template = "\nExpected the %s for " + expected_lib + " to be:\n\t%s\n, but instead got:\n\t%s\n" + expand_paths = lambda paths: [get_output_and_package_dir_based_path(env, p) for p in paths] + expected_includes = expand_paths(ctx.attr.expected_includes) + expected_system_includes = expand_paths(ctx.attr.expected_system_includes) + + includes = compilation_context.includes.to_list() + for include in expected_includes: + asserts.true(env, include in includes, assert_template % ("includes", expected_includes, includes)) + + system_includes = compilation_context.system_includes.to_list() + for include in expected_system_includes: + asserts.true(env, include in system_includes, assert_template % ("system_includes", expected_system_includes, system_includes)) return analysistest.end(env) _cc_prebuilt_library_shared_test = analysistest.make( _cc_prebuilt_library_shared_test_impl, attrs = dict( - expected_lib = attr.string(mandatory = True), + expected_lib = attr.string(default = _fake_expected_lib), + expected_includes = attr.string_list(), + expected_system_includes = attr.string_list(), ), ) @@ -53,11 +79,52 @@ def _cc_prebuilt_library_shared_simple(): shared_library = lib, tags = ["manual"], ) + _cc_prebuilt_library_shared_test( + name = test_name, + target_under_test = name, + expected_lib = lib, + ) + return test_name + +def _cc_prebuilt_library_shared_default_library_field(): + name = "_cc_prebuilt_library_shared_default_library_field" + test_name = name + "_test" + lib = None + + cc_prebuilt_library_shared( + name = name, + shared_library = lib, + tags = ["manual"], + ) + _cc_prebuilt_library_shared_test( + name = test_name, + target_under_test = name, + expected_lib = lib, # We expect the default of _fake_expected_lib + ) + + return test_name + +def _cc_prebuilt_library_shared_has_all_includes(): + name = "_cc_prebuilt_library_shared_has_all_includes" + test_name = name + "_test" + lib = "libfoo.so" + includes = ["includes"] + system_includes = ["system_includes"] + + cc_prebuilt_library_shared( + name = name, + shared_library = lib, + export_includes = includes, + export_system_includes = system_includes, + tags = ["manual"], + ) _cc_prebuilt_library_shared_test( name = test_name, target_under_test = name, expected_lib = lib, + expected_includes = includes, + expected_system_includes = system_includes, ) return test_name @@ -67,5 +134,7 @@ def cc_prebuilt_library_shared_test_suite(name): name = name, tests = [ _cc_prebuilt_library_shared_simple(), + _cc_prebuilt_library_shared_default_library_field(), + _cc_prebuilt_library_shared_has_all_includes(), ], ) diff --git a/rules/cc/cc_prebuilt_library_static.bzl b/rules/cc/cc_prebuilt_library_static.bzl index 1981fbf7..1d84adac 100644 --- a/rules/cc/cc_prebuilt_library_static.bzl +++ b/rules/cc/cc_prebuilt_library_static.bzl @@ -12,18 +12,41 @@ # See the License for the specific language governing permissions and # limitations under the License. -def cc_prebuilt_library_static( - name, - static_library, - export_includes = [], # @unused - export_system_includes = [], # @unused - **kwargs): - "Bazel macro to correspond with the *_cc_prebuilt_library_static Soong module types" +load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain") +load(":cc_library_common.bzl", "create_cc_prebuilt_library_info") - # TODO: Handle includes similarly to cc_library_static - # e.g. includes = ["clang-r416183b/prebuilt_include/llvm/lib/Fuzzer"], - native.cc_import( - name = name, - static_library = static_library, - **kwargs +def _cc_prebuilt_library_static_impl(ctx): + lib = ctx.file.static_library + files = ctx.attr.static_library.files if lib != None else None + cc_toolchain = find_cpp_toolchain(ctx) + feature_configuration = cc_common.configure_features( + ctx = ctx, + cc_toolchain = cc_toolchain, ) + cc_info = create_cc_prebuilt_library_info( + ctx, + cc_common.create_library_to_link( + actions = ctx.actions, + static_library = lib, + alwayslink = ctx.attr.alwayslink, + feature_configuration = feature_configuration, + cc_toolchain = cc_toolchain, + ) if lib != None else None, + ) + return [DefaultInfo(files = files), cc_info] + +cc_prebuilt_library_static = rule( + implementation = _cc_prebuilt_library_static_impl, + attrs = dict( + static_library = attr.label( + providers = [CcInfo], + allow_single_file = True, + ), + alwayslink = attr.bool(default = False), + export_includes = attr.string_list(), + export_system_includes = attr.string_list(), + ), + toolchains = ["@bazel_tools//tools/cpp:toolchain_type"], + fragments = ["cpp"], + provides = [CcInfo], +) diff --git a/rules/cc/cc_prebuilt_library_static_test.bzl b/rules/cc/cc_prebuilt_library_static_test.bzl index 5a386e4c..326e1953 100644 --- a/rules/cc/cc_prebuilt_library_static_test.bzl +++ b/rules/cc/cc_prebuilt_library_static_test.bzl @@ -1,5 +1,5 @@ """ -Copyright (C) 2022 The Android Open Source Project +Copyright (C) 2023 The Android Open Source Project Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -16,59 +16,193 @@ limitations under the License. load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") load("//build/bazel/rules/cc:cc_prebuilt_library_static.bzl", "cc_prebuilt_library_static") +load("//build/bazel/rules/test_common:paths.bzl", "get_output_and_package_dir_based_path") + +_fake_expected_lib = "{[()]}" def _cc_prebuilt_library_static_alwayslink_test_impl(ctx): env = analysistest.begin(ctx) target = analysistest.target_under_test(env) expected_lib = ctx.attr.expected_lib + alwayslink = ctx.attr.alwayslink cc_info = target[CcInfo] linker_inputs = cc_info.linking_context.linker_inputs.to_list() libs_to_link = [] for l in linker_inputs: libs_to_link += l.libraries + has_lib = False has_alwayslink = False libs = {} for lib_to_link in libs_to_link: lib = lib_to_link.static_library.basename libs[lib_to_link.static_library] = lib_to_link.alwayslink if lib == expected_lib: + has_lib = True has_alwayslink = lib_to_link.alwayslink if has_alwayslink: break - asserts.true(env, has_alwayslink, "\nExpected to find the static library `%s` unconditionally in the linker_input, with alwayslink set:\n\t%s" % (expected_lib, str(libs))) + asserts.true( + env, + has_lib, + "\nExpected to find the static library `%s` in the linker_input:\n\t%s" % (expected_lib, str(libs)), + ) + asserts.equals( + env, + has_alwayslink, + alwayslink, + "\nExpected to find the static library `%s` unconditionally in the linker_input, with alwayslink set to %s:\n\t%s" % (expected_lib, alwayslink, str(libs)), + ) return analysistest.end(env) _cc_prebuilt_library_static_alwayslink_test = analysistest.make( _cc_prebuilt_library_static_alwayslink_test_impl, - attrs = {"expected_lib": attr.string()}, + attrs = { + "expected_lib": attr.string(), + "alwayslink": attr.bool(), + }, ) -def _cc_prebuilt_library_static_given_alwayslink_lib(): - name = "_cc_prebuilt_library_static_given_alwayslink_lib" +def _cc_prebuilt_library_static_alwayslink_lib(alwayslink): + name = "_cc_prebuilt_library_static_alwayslink_lib_" + str(alwayslink) test_name = name + "_test" lib = "libfoo.a" cc_prebuilt_library_static( name = name, static_library = lib, - alwayslink = True, + alwayslink = alwayslink, tags = ["manual"], ) - _cc_prebuilt_library_static_alwayslink_test( name = test_name, target_under_test = name, expected_lib = lib, + alwayslink = alwayslink, ) return test_name +def _cc_prebuilt_library_static_test_impl(ctx): + env = analysistest.begin(ctx) + target = analysistest.target_under_test(env) + expected_lib = ctx.attr.expected_lib + cc_info = target[CcInfo] + compilation_context = cc_info.compilation_context + linker_inputs = cc_info.linking_context.linker_inputs.to_list() + libs_to_link = [] + for lib in linker_inputs: + libs_to_link += lib.libraries + + if expected_lib == _fake_expected_lib: + asserts.true( + env, + len(libs_to_link) == 0, + "\nExpected the static library to be empty, but instead got:\n\t%s\n" % str(libs_to_link), + ) + else: + asserts.true( + env, + expected_lib in [lib.static_library.basename for lib in libs_to_link], + "\nExpected the target to include the static library %s; but instead got:\n\t%s\n" % (expected_lib, libs_to_link), + ) + + # Checking for the expected {,system_}includes + assert_template = "\nExpected the %s for " + expected_lib + " to be:\n\t%s\n, but instead got:\n\t%s\n" + expand_paths = lambda paths: [get_output_and_package_dir_based_path(env, p) for p in paths] + expected_includes = expand_paths(ctx.attr.expected_includes) + expected_system_includes = expand_paths(ctx.attr.expected_system_includes) + + includes = compilation_context.includes.to_list() + for include in expected_includes: + asserts.true(env, include in includes, assert_template % ("includes", expected_includes, includes)) + + system_includes = compilation_context.system_includes.to_list() + for include in expected_system_includes: + asserts.true(env, include in system_includes, assert_template % ("system_includes", expected_system_includes, system_includes)) + + return analysistest.end(env) + +_cc_prebuilt_library_static_test = analysistest.make( + _cc_prebuilt_library_static_test_impl, + attrs = dict( + expected_lib = attr.string(default = _fake_expected_lib), + expected_includes = attr.string_list(), + expected_system_includes = attr.string_list(), + ), +) + +def _cc_prebuilt_library_static_simple(): + name = "_cc_prebuilt_library_static_simple" + test_name = name + "_test" + lib = "libfoo.a" + + cc_prebuilt_library_static( + name = name, + static_library = lib, + tags = ["manual"], + ) + _cc_prebuilt_library_static_test( + name = test_name, + target_under_test = name, + expected_lib = lib, + ) + + return test_name + +def _cc_prebuilt_library_static_None(): + name = "_cc_prebuilt_library_static_None" + test_name = name + "_test" + lib = None + + cc_prebuilt_library_static( + name = name, + static_library = lib, + tags = ["manual"], + ) + _cc_prebuilt_library_static_test( + name = test_name, + target_under_test = name, + expected_lib = lib, # We expect the default of _fake_expected_lib + ) + + return test_name + +def _cc_prebuilt_library_static_has_all_includes(): + name = "_cc_prebuilt_library_static_has_all_includes" + test_name = name + "_test" + lib = "libfoo.a" + includes = ["includes"] + system_includes = ["system_includes"] + + cc_prebuilt_library_static( + name = name, + static_library = lib, + export_includes = includes, + export_system_includes = system_includes, + tags = ["manual"], + ) + _cc_prebuilt_library_static_test( + name = test_name, + target_under_test = name, + expected_lib = lib, + expected_includes = includes, + expected_system_includes = system_includes, + ) + + return test_name + +# TODO: Test that is alwayslink = alse + def cc_prebuilt_library_static_test_suite(name): native.test_suite( name = name, tests = [ - _cc_prebuilt_library_static_given_alwayslink_lib(), + _cc_prebuilt_library_static_simple(), + _cc_prebuilt_library_static_None(), + _cc_prebuilt_library_static_alwayslink_lib(True), + _cc_prebuilt_library_static_alwayslink_lib(False), + _cc_prebuilt_library_static_has_all_includes(), ], ) -- cgit v1.2.3 From 74a9785737ef9401e09558c365239e75243aab0a Mon Sep 17 00:00:00 2001 From: Usta Shrestha Date: Thu, 13 Apr 2023 23:23:26 -0400 Subject: cosmetic: warmup runs made clearer Test: `incremental_build.sh -c clean -b prod soong` and verify rows for WARMUP in summary.csv and metrics.csv Bug: NA Change-Id: I3df521daa30ce7f91072570f4ffef5f0314a737f --- scripts/incremental_build/cuj_catalog.py | 191 +++++++++++++------------ scripts/incremental_build/incremental_build.py | 10 +- 2 files changed, 100 insertions(+), 101 deletions(-) diff --git a/scripts/incremental_build/cuj_catalog.py b/scripts/incremental_build/cuj_catalog.py index e2d4868e..6db3c8d8 100644 --- a/scripts/incremental_build/cuj_catalog.py +++ b/scripts/incremental_build/cuj_catalog.py @@ -25,6 +25,7 @@ import uuid from enum import Enum from pathlib import Path from typing import Callable, Optional +from typing import Final from typing import TypeAlias import util @@ -106,8 +107,12 @@ class CujGroup: if len(self.steps) < 2: return f'{self.steps[0].verb} {self.description}'.strip() return ' '.join( - [f'({chr(ord("a") + i)}) {step.verb} {self.description}'.strip() for - i, step in enumerate(self.steps)]) + [f'({chr(ord("a") + i)}) {step.verb} {self.description}'.strip() for + i, step in enumerate(self.steps)]) + + +Warmup: Final[CujGroup] = CujGroup('WARMUP', + [CujStep('no change', lambda: None)]) class InWorkspace(Enum): @@ -122,7 +127,7 @@ class InWorkspace(Enum): @staticmethod def ws_counterpart(src_path: Path) -> Path: return util.get_out_dir().joinpath('soong/workspace').joinpath( - de_src(src_path)) + de_src(src_path)) def verifier(self, src_path: Path) -> Verifier: @skip_when_soong_only @@ -146,7 +151,7 @@ class InWorkspace(Enum): if self != actual: raise AssertionError( - f'{ws_path} expected {self.name} but got {actual.name}') + f'{ws_path} expected {self.name} but got {actual.name}') logging.info(f'VERIFIED {de_src(ws_path)} {self.name}') return f @@ -181,8 +186,8 @@ def modify_revert(file: Path, text: str = '//BOGUS line\n') -> CujGroup: f.truncate() return CujGroup(de_src(file), [ - CujStep('modify', add_line), - CujStep('revert', revert) + CujStep('modify', add_line), + CujStep('revert', revert) ]) @@ -202,8 +207,8 @@ def create_delete(file: Path, ws: InWorkspace, def create(): if file.exists(): raise RuntimeError( - f'File {file} already exists. Interrupted an earlier run?\n' - 'TIP: `repo status` and revert changes!!!') + f'File {file} already exists. Interrupted an earlier run?\n' + 'TIP: `repo status` and revert changes!!!') file.parent.mkdir(parents=True, exist_ok=True) file.touch(exist_ok=False) with open(file, mode="w") as f: @@ -216,8 +221,8 @@ def create_delete(file: Path, ws: InWorkspace, file.unlink(missing_ok=False) return CujGroup(de_src(file), [ - CujStep('create', create, ws.verifier(file)), - CujStep('delete', delete, InWorkspace.OMISSION.verifier(file)), + CujStep('create', create, ws.verifier(file)), + CujStep('delete', delete, InWorkspace.OMISSION.verifier(file)), ]) @@ -227,8 +232,8 @@ def create_delete_bp(bp_file: Path) -> CujGroup: an Android.bp file. """ return create_delete( - bp_file, InWorkspace.SYMLINK, - 'filegroup { name: "test-bogus-filegroup", srcs: ["**/*.md"] }') + bp_file, InWorkspace.SYMLINK, + 'filegroup { name: "test-bogus-filegroup", srcs: ["**/*.md"] }') def delete_restore(original: Path, ws: InWorkspace) -> CujGroup: @@ -252,12 +257,12 @@ def delete_restore(original: Path, ws: InWorkspace) -> CujGroup: original.rename(copied) return CujGroup(de_src(original), [ - CujStep('delete', - move_to_tempdir_to_mimic_deletion, - InWorkspace.OMISSION.verifier(original)), - CujStep('restore', - lambda: copied.rename(original), - ws.verifier(original)) + CujStep('delete', + move_to_tempdir_to_mimic_deletion, + InWorkspace.OMISSION.verifier(original)), + CujStep('restore', + lambda: copied.rename(original), + ws.verifier(original)) ]) @@ -274,7 +279,7 @@ def replace_link_with_dir(p: Path): create_dir: CujStep delete_dir: CujStep create_dir, delete_dir, *tail = create_delete_bp( - p.joinpath('Android.bp')).steps + p.joinpath('Android.bp')).steps assert len(tail) == 0 def replace_it(): @@ -282,11 +287,11 @@ def replace_link_with_dir(p: Path): create_dir.apply_change() return CujGroup(cd.description, [ - create_file, - CujStep(f'{de_src(p)}/Android.bp instead of', - replace_it, - create_dir.verify), - delete_dir + create_file, + CujStep(f'{de_src(p)}/Android.bp instead of', + replace_it, + create_dir.verify), + delete_dir ]) @@ -311,14 +316,14 @@ def content_verfiers( def contains(): if not search(): raise AssertionError( - f'{de_src(ws_build_file)} expected to contain {content}') + f'{de_src(ws_build_file)} expected to contain {content}') logging.info(f'VERIFIED {de_src(ws_build_file)} contains {content}') @skip_when_soong_only def does_not_contain(): if search(): raise AssertionError( - f'{de_src(ws_build_file)} not expected to contain {content}') + f'{de_src(ws_build_file)} not expected to contain {content}') logging.info(f'VERIFIED {de_src(ws_build_file)} does not contain {content}') return contains, does_not_contain @@ -329,22 +334,22 @@ def modify_revert_kept_build_file(build_file: Path) -> CujGroup: step1, step2, *tail = modify_revert(build_file, content).steps assert len(tail) == 0 ws_build_file = InWorkspace.ws_counterpart(build_file).with_name( - 'BUILD.bazel') + 'BUILD.bazel') merge_prover, merge_disprover = content_verfiers(ws_build_file, content) return CujGroup(de_src(build_file), [ - CujStep(step1.verb, - step1.apply_change, - _sequence(step1.verify, merge_prover)), - CujStep(step2.verb, - step2.apply_change, - _sequence(step2.verify, merge_disprover)) + CujStep(step1.verb, + step1.apply_change, + _sequence(step1.verify, merge_prover)), + CujStep(step2.verb, + step2.apply_change, + _sequence(step2.verify, merge_disprover)) ]) def create_delete_kept_build_file(build_file: Path) -> CujGroup: content = f'//BOGUS {uuid.uuid4()}\n' ws_build_file = InWorkspace.ws_counterpart(build_file).with_name( - 'BUILD.bazel') + 'BUILD.bazel') if build_file.name == 'BUILD.bazel': ws = InWorkspace.NOT_UNDER_SYMLINK elif build_file.name == 'BUILD': @@ -359,32 +364,32 @@ def create_delete_kept_build_file(build_file: Path) -> CujGroup: step1, step2, *tail = create_delete(build_file, ws, content).steps assert len(tail) == 0 return CujGroup(de_src(build_file), [ - CujStep(step1.verb, - step1.apply_change, - _sequence(step1.verify, merge_prover)), - CujStep(step2.verb, - step2.apply_change, - _sequence(step2.verify, merge_disprover)) + CujStep(step1.verb, + step1.apply_change, + _sequence(step1.verify, merge_prover)), + CujStep(step2.verb, + step2.apply_change, + _sequence(step2.verify, merge_disprover)) ]) def create_delete_unkept_build_file(build_file) -> CujGroup: content = f'//BOGUS {uuid.uuid4()}\n' ws_build_file = InWorkspace.ws_counterpart(build_file).with_name( - 'BUILD.bazel') + 'BUILD.bazel') step1: CujStep step2: CujStep step1, step2, *tail = create_delete( - build_file, InWorkspace.SYMLINK, content).steps + build_file, InWorkspace.SYMLINK, content).steps assert len(tail) == 0 _, merge_disprover = content_verfiers(ws_build_file, content) return CujGroup(de_src(build_file), [ - CujStep(step1.verb, - step1.apply_change, - _sequence(step1.verify, merge_disprover)), - CujStep(step2.verb, - step2.apply_change, - _sequence(step2.verify, merge_disprover)) + CujStep(step1.verb, + step1.apply_change, + _sequence(step1.verify, merge_disprover)), + CujStep(step2.verb, + step2.apply_change, + _sequence(step2.verify, merge_disprover)) ]) @@ -407,9 +412,9 @@ def _kept_build_cujs() -> list[CujGroup]: pkg.joinpath('BUILD.bazel')] return [ - *[create_delete_kept_build_file(build_file) for build_file in examples], - create_delete(pkg.joinpath('BUILD/kept-dir'), InWorkspace.SYMLINK), - modify_revert_kept_build_file(util.any_file_under(kept, 'BUILD'))] + *[create_delete_kept_build_file(build_file) for build_file in examples], + create_delete(pkg.joinpath('BUILD/kept-dir'), InWorkspace.SYMLINK), + modify_revert_kept_build_file(util.any_file_under(kept, 'BUILD'))] def _unkept_build_cujs() -> list[CujGroup]: @@ -417,15 +422,15 @@ def _unkept_build_cujs() -> list[CujGroup]: unkept = src('bionic') pkg = util.any_dir_under(unkept, *PKG) return [ - *[create_delete_unkept_build_file(build_file) for build_file in [ - pkg.joinpath('BUILD'), - pkg.joinpath('BUILD.bazel'), - ]], - *[create_delete(build_file, InWorkspace.OMISSION) for build_file in [ - unkept.joinpath('bogus-unkept/BUILD'), - unkept.joinpath('bogus-unkept/BUILD.bazel'), - ]], - create_delete(pkg.joinpath('BUILD/unkept-dir'), InWorkspace.SYMLINK) + *[create_delete_unkept_build_file(build_file) for build_file in [ + pkg.joinpath('BUILD'), + pkg.joinpath('BUILD.bazel'), + ]], + *[create_delete(build_file, InWorkspace.OMISSION) for build_file in [ + unkept.joinpath('bogus-unkept/BUILD'), + unkept.joinpath('bogus-unkept/BUILD.bazel'), + ]], + create_delete(pkg.joinpath('BUILD/unkept-dir'), InWorkspace.SYMLINK) ] @@ -447,52 +452,48 @@ def get_cujgroups() -> list[CujGroup]: ''')) android_bp_cujs = [ - modify_revert(src('Android.bp')), - *[create_delete_bp(d.joinpath('Android.bp')) for d in - [ancestor, pkg_free, leaf_pkg_free]] + modify_revert(src('Android.bp')), + *[create_delete_bp(d.joinpath('Android.bp')) for d in + [ancestor, pkg_free, leaf_pkg_free]] ] mixed_build_launch_cujs = [ - modify_revert(src('bionic/libc/tzcode/asctime.c')), - modify_revert(src('bionic/libc/stdio/stdio.cpp')), - modify_revert(src('packages/modules/adb/daemon/main.cpp')), - modify_revert(src('frameworks/base/core/java/android/view/View.java')), + modify_revert(src('bionic/libc/tzcode/asctime.c')), + modify_revert(src('bionic/libc/stdio/stdio.cpp')), + modify_revert(src('packages/modules/adb/daemon/main.cpp')), + modify_revert(src('frameworks/base/core/java/android/view/View.java')), ] unreferenced_file_cujs = [ - *[create_delete(d.joinpath('unreferenced.txt'), InWorkspace.SYMLINK) for - d in [ancestor, pkg]], - *[create_delete(d.joinpath('unreferenced.txt'), InWorkspace.UNDER_SYMLINK) - for d - in [pkg_free, leaf_pkg_free]] + *[create_delete(d.joinpath('unreferenced.txt'), InWorkspace.SYMLINK) for + d in [ancestor, pkg]], + *[create_delete(d.joinpath('unreferenced.txt'), InWorkspace.UNDER_SYMLINK) + for d + in [pkg_free, leaf_pkg_free]] ] def clean(): if ui.get_user_input().log_dir.is_relative_to(util.get_top_dir()): raise AssertionError( - f'specify a different LOG_DIR: {ui.get_user_input().log_dir}') + f'specify a different LOG_DIR: {ui.get_user_input().log_dir}') if util.get_out_dir().exists(): shutil.rmtree(util.get_out_dir()) return [ - CujGroup('', [CujStep('clean', clean)]), - CujGroup('', [CujStep('no change', lambda: None)]), - - create_delete(src('bionic/libc/tzcode/globbed.c'), - InWorkspace.UNDER_SYMLINK), - - # TODO (usta): find targets that should be affected - *[delete_restore(f, InWorkspace.SYMLINK) for f in [ - util.any_file('version_script.txt'), - util.any_file('AndroidManifest.xml')]], - - *unreferenced_file_cujs, - *mixed_build_launch_cujs, - *android_bp_cujs, - *_unkept_build_cujs(), - *_kept_build_cujs(), - replace_link_with_dir(pkg.joinpath('bogus.txt')), - # TODO(usta): add a dangling symlink + CujGroup('', [CujStep('clean', clean)]), + Warmup, + + create_delete(src('bionic/libc/tzcode/globbed.c'), + InWorkspace.UNDER_SYMLINK), + + # TODO (usta): find targets that should be affected + *[delete_restore(f, InWorkspace.SYMLINK) for f in [ + util.any_file('version_script.txt'), + util.any_file('AndroidManifest.xml')]], + + *unreferenced_file_cujs, + *mixed_build_launch_cujs, + *android_bp_cujs, + *_unkept_build_cujs(), + *_kept_build_cujs(), + replace_link_with_dir(pkg.joinpath('bogus.txt')), + # TODO(usta): add a dangling symlink ] - - -def warmup_index() -> int: - return 1 diff --git a/scripts/incremental_build/incremental_build.py b/scripts/incremental_build/incremental_build.py index b95d3a84..b43e0fb1 100644 --- a/scripts/incremental_build/incremental_build.py +++ b/scripts/incremental_build/incremental_build.py @@ -32,9 +32,9 @@ from typing import Mapping import cuj_catalog import perf_metrics +import pretty import ui import util -import pretty MAX_RUN_COUNT: int = 5 @@ -213,13 +213,11 @@ def main(): run_dir_gen = util.next_path(user_input.log_dir.joinpath(util.RUN_DIR_PREFIX)) - def run_cuj_group(cuj_group: cuj_catalog.CujGroup, is_warmup: bool): + def run_cuj_group(cuj_group: cuj_catalog.CujGroup): for cujstep in cuj_group.steps: desc = cujstep.verb desc = f'{desc} {cuj_group.description}'.strip() desc = f'{desc} {user_input.description}'.strip() - if is_warmup: - desc = f'WARMUP {desc}' logging.info('START %s %s [%s]', build_type.name, ' '.join(user_input.targets), desc) cujstep.apply_change() @@ -234,9 +232,9 @@ def main(): for build_type in user_input.build_types: # warm-up run reduces variations attributable to OS caches - run_cuj_group(cuj_catalog.get_cujgroups()[cuj_catalog.warmup_index()], True) + run_cuj_group(cuj_catalog.Warmup) for i in user_input.chosen_cujgroups: - run_cuj_group(cuj_catalog.get_cujgroups()[i], False) + run_cuj_group(cuj_catalog.get_cujgroups()[i]) perf_metrics.tabulate_metrics_csv(user_input.log_dir) perf_metrics.display_tabulated_metrics(user_input.log_dir) -- cgit v1.2.3 From 2d1861a4ee792c03a0caf3bae28ff9719d7845c3 Mon Sep 17 00:00:00 2001 From: Romain Jobredeaux Date: Fri, 14 Apr 2023 11:49:32 -0400 Subject: Allow sdk transition wrappers in exports of aar_import. The Bazel Android rules hard-code the names of rule types that can be exported by an aar_import rule, but the AOSP-specific wrappers we've written don't match the names, even though they are equally valid for export. Test: Presubmits Change-Id: I14cf50ec553380d377a685b65e5eac4404c64f9a --- rules/android/aar_import.bzl | 2 +- rules/android/aar_import_aosp_internal/BUILD.bazel | 0 rules/android/aar_import_aosp_internal/attrs.bzl | 40 ++++++++++++++++++++++ rules/android/aar_import_aosp_internal/rule.bzl | 33 ++++++++++++++++++ 4 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 rules/android/aar_import_aosp_internal/BUILD.bazel create mode 100644 rules/android/aar_import_aosp_internal/attrs.bzl create mode 100644 rules/android/aar_import_aosp_internal/rule.bzl diff --git a/rules/android/aar_import.bzl b/rules/android/aar_import.bzl index e93a821f..76abef03 100644 --- a/rules/android/aar_import.bzl +++ b/rules/android/aar_import.bzl @@ -14,7 +14,7 @@ """Macro wrapping the aar_import for bp2build. """ -load("@rules_android//rules:rules.bzl", _aar_import = "aar_import") +load("//build/bazel/rules/android/aar_import_aosp_internal:rule.bzl", _aar_import = "aar_import") load("//build/bazel/rules/java:sdk_transition.bzl", "sdk_transition", "sdk_transition_attrs") # TODO(b/277801336): document these attributes. diff --git a/rules/android/aar_import_aosp_internal/BUILD.bazel b/rules/android/aar_import_aosp_internal/BUILD.bazel new file mode 100644 index 00000000..e69de29b diff --git a/rules/android/aar_import_aosp_internal/attrs.bzl b/rules/android/aar_import_aosp_internal/attrs.bzl new file mode 100644 index 00000000..1b685b3f --- /dev/null +++ b/rules/android/aar_import_aosp_internal/attrs.bzl @@ -0,0 +1,40 @@ +# Copyright (C) 2023 The Android Open Source Project +# +# 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. + +load( + "@rules_android//rules:attrs.bzl", + _attrs = "attrs", +) +load( + "@rules_android//rules/aar_import:attrs.bzl", + _BASE_ATTRS = "ATTRS", +) + +ATTRS = _attrs.replace( + _BASE_ATTRS, + exports = attr.label_list( + allow_files = False, + allow_rules = [ + "aar_import", + "java_import", + "kt_jvm_import", + "aar_import_sdk_transition", + "java_import_sdk_transition", + "kt_jvm_import_sdk_transition", + ], + doc = "The closure of all rules reached via `exports` attributes are considered " + + "direct dependencies of any rule that directly depends on the target with " + + "`exports`. The `exports` are not direct deps of the rule they belong to.", + ), +) diff --git a/rules/android/aar_import_aosp_internal/rule.bzl b/rules/android/aar_import_aosp_internal/rule.bzl new file mode 100644 index 00000000..79580f2a --- /dev/null +++ b/rules/android/aar_import_aosp_internal/rule.bzl @@ -0,0 +1,33 @@ +# Copyright (C) 2023 The Android Open Source Project +# +# 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. + +"""aar_import rule.""" + +load(":attrs.bzl", _ATTRS = "ATTRS") +load("@rules_android//rules/aar_import:impl.bzl", _impl = "impl") +load("@rules_android//rules/aar_import:rule.bzl", "RULE_DOC") + +aar_import = rule( + attrs = _ATTRS, + fragments = ["android"], + implementation = _impl, + doc = RULE_DOC, + provides = [ + AndroidIdeInfo, + AndroidLibraryResourceClassJarProvider, + AndroidNativeLibsInfo, + JavaInfo, + ], + toolchains = ["@rules_android//toolchains/android:toolchain_type"], +) -- cgit v1.2.3