StrixOS/build_soong
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "analyze_bcpf: Compute hidden_api package properties" am: 91dac89447 am: c17a61f6f6 am: ee99468ff2

Browse Source 
Original change: https://android-review.googlesource.com/c/platform/build/soong/+/2034243

Change-Id: Ib1608a94d4a185e2b86c157f27970c74c0a2a0a7
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
This commit is contained in:
Treehugger Robot
2022-03-30 20:44:33 +00:00
committed by Automerger Merge Worker
parent 95d500cd04 ee99468ff2
commit dfab76d1ac
2 changed files with 469 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

312
scripts/hiddenapi/analyze_bcpf.py
View File

@@ -16,6 +16,7 @@
"""Analyze bootclasspath_fragment usage."""
import argparse
import dataclasses
import enum
import json
import logging
import os
@@ -25,8 +26,12 @@ import subprocess
import tempfile
import textwrap
import typing
from enum import Enum
import sys
from signature_trie import signature_trie
_STUB_FLAGS_FILE = "out/soong/hiddenapi/hiddenapi-stub-flags.txt"
_FLAGS_FILE = "out/soong/hiddenapi/hiddenapi-flags.csv"
@@ -135,6 +140,16 @@ class FileChange:
return self.path < other.path
class PropertyChangeAction(Enum):
"""Allowable actions that are supported by HiddenApiPropertyChange."""
# New values are appended to any existing values.
APPEND = 1
# New values replace any existing values.
REPLACE = 2
@dataclasses.dataclass
class HiddenApiPropertyChange:
@@ -144,6 +159,9 @@ class HiddenApiPropertyChange:
property_comment: str = ""
# The action that indicates how this change is applied.
action: PropertyChangeAction = PropertyChangeAction.APPEND
def snippet(self, indent):
snippet = "\n"
snippet += format_comment_as_text(self.property_comment, indent)
@@ -160,7 +178,29 @@ class HiddenApiPropertyChange:
# Add an additional placeholder value to identify the modification that
# bpmodify makes.
bpmodify_values = [_SPECIAL_PLACEHOLDER]
bpmodify_values.extend(self.values)
if self.action == PropertyChangeAction.APPEND:
# If adding the values to the existing values then pass the new
# values to bpmodify.
bpmodify_values.extend(self.values)
elif self.action == PropertyChangeAction.REPLACE:
# If replacing the existing values then it is not possible to use
# bpmodify for that directly. It could be used twice to remove the
# existing property and then add a new one but that does not remove
# any related comments and loses the position of the existing
# property as the new property is always added to the end of the
# containing block.
#
# So, instead of passing the new values to bpmodify this this just
# adds an extra placeholder to force bpmodify to format the list
# across multiple lines to ensure a consistent structure for the
# code that removes all the existing values and adds the new ones.
#
# This placeholder has to be different to the other placeholder as
# bpmodify dedups values.
bpmodify_values.append(_SPECIAL_PLACEHOLDER + "_REPLACE")
else:
raise ValueError(f"unknown action {self.action}")
packages = ",".join(bpmodify_values)
bpmodify_runner.add_values_to_property(
@@ -176,6 +216,22 @@ class HiddenApiPropertyChange:
print(line, file=tio)
def fixup_bpmodify_changes(self, bcpf_bp_file, lines):
"""Fixup the output of bpmodify.
The bpmodify tool does not support all the capabilities that this needs
so it is used to do what it can, including marking the place in the
Android.bp file where it makes its changes and then this gets passed a
list of lines from that file which it then modifies to complete the
change.
This analyzes the list of lines to find the indices of the significant
lines and then applies some changes. As those changes can insert and
delete lines (changing the indices of following lines) the changes are
generally done in reverse order starting from the end and working
towards the beginning. That ensures that the changes do not invalidate
the indices of following lines.
"""
# Find the line containing the placeholder that has been inserted.
place_holder_index = -1
for i, line in enumerate(lines):
@@ -226,6 +282,22 @@ class HiddenApiPropertyChange:
logging.debug("Could not find property line in %s", bcpf_bp_file)
return False
# If this change is replacing the existing values then they need to be
# removed and replaced with the new values. That will change the lines
# after the property but it is necessary to do here as the following
# code operates on earlier lines.
if self.action == PropertyChangeAction.REPLACE:
# This removes the existing values and replaces them with the new
# values.
indent = extract_indent(lines[property_line_index + 1])
insert = [f'{indent}"{x}",' for x in self.values]
lines[property_line_index + 1:end_property_array_index] = insert
if not self.values:
# If the property has no values then merge the ], onto the
# same line as the property name.
del lines[property_line_index + 1]
lines[property_line_index] = lines[property_line_index] + "],"
# Only insert a comment if the property does not already have a comment.
line_preceding_property = lines[(property_line_index - 1)]
if (self.property_comment and
@@ -262,6 +334,21 @@ class Result:
default_factory=list)
class ClassProvider(enum.Enum):
"""The source of a class found during the hidden API processing"""
BCPF = "bcpf"
OTHER = "other"
# A fake member to use when using the signature trie to compute the package
# properties from hidden API flags. This is needed because while that
# computation only cares about classes the trie expects a class to be an
# interior node but without a member it makes the class a leaf node. That causes
# problems when analyzing inner classes as the outer class is a leaf node for
# its own entry but is used as an interior node for inner classes.
_FAKE_MEMBER = ";->fake()V"
@dataclasses.dataclass()
class BcpfAnalyzer:
# Path to this tool.
@@ -388,7 +475,7 @@ Making sure that {module_info_file} is up to date.
return os.path.join(self.out_dir, "soong/.intermediates", module_path,
module_name)
def find_bootclasspath_fragment_output_file(self, basename):
def find_bootclasspath_fragment_output_file(self, basename, required=True):
# Find the output file of the bootclasspath_fragment with the specified
# base name.
found_file = ""
@@ -398,7 +485,7 @@ Making sure that {module_info_file} is up to date.
if f == basename:
found_file = os.path.join(dirpath, f)
break
if not found_file:
if not found_file and required:
raise Exception(f"Could not find {basename} in {bcpf_out_dir}")
return found_file
@@ -475,6 +562,8 @@ Cleaning potentially stale files.
result = Result()
self.build_monolithic_flags(result)
self.analyze_hiddenapi_package_properties(result)
self.explain_how_to_check_signature_patterns()
# If there were any changes that need to be made to the Android.bp
# file then either apply or report them.
@@ -929,6 +1018,223 @@ Checking custom hidden API flags....
result, property_name, flags_file, rel_bcpf_flags_file,
bcpf_flags_file)
@staticmethod
def split_package_comment(split_packages):
if split_packages:
return textwrap.dedent("""
The following packages contain classes from other modules on the
bootclasspath. That means that the hidden API flags for this
module has to explicitly list every single class this module
provides in that package to differentiate them from the classes
provided by other modules. That can include private classes that
are not part of the API.
""").strip("\n")
return "This module does not contain any split packages."
@staticmethod
def package_prefixes_comment():
return textwrap.dedent("""
The following packages and all their subpackages currently only
contain classes from this bootclasspath_fragment. Listing a package
here won't prevent other bootclasspath modules from adding classes
in any of those packages but it will prevent them from adding those
classes into an API surface, e.g. public, system, etc.. Doing so
will result in a build failure due to inconsistent flags.
""").strip("\n")
def analyze_hiddenapi_package_properties(self, result):
split_packages, single_packages, package_prefixes = \
self.compute_hiddenapi_package_properties()
# TODO(b/202154151): Find those classes in split packages that are not
# part of an API, i.e. are an internal implementation class, and so
# can, and should, be safely moved out of the split packages.
result.property_changes.append(
HiddenApiPropertyChange(
property_name="split_packages",
values=split_packages,
property_comment=self.split_package_comment(split_packages),
action=PropertyChangeAction.REPLACE,
))
if split_packages:
self.report(f"""
bootclasspath_fragment {self.bcpf} contains classes in packages that also
contain classes provided by other sources, those packages are called split
packages. Split packages should be avoided where possible but are often
unavoidable when modularizing existing code.
The hidden api processing needs to know which packages are split (and conversely
which are not) so that it can optimize the hidden API flags to remove
unnecessary implementation details.
""")
self.report("""
By default (for backwards compatibility) the bootclasspath_fragment assumes that
all packages are split unless one of the package_prefixes or split_packages
properties are specified. While that is safe it is not optimal and can lead to
unnecessary implementation details leaking into the hidden API flags. Adding an
empty split_packages property allows the flags to be optimized and remove any
unnecessary implementation details.
""")
if single_packages:
result.property_changes.append(
HiddenApiPropertyChange(
property_name="single_packages",
values=single_packages,
property_comment=textwrap.dedent("""
The following packages currently only contain classes from
this bootclasspath_fragment but some of their sub-packages
contain classes from other bootclasspath modules. Packages
should only be listed here when necessary for legacy
purposes, new packages should match a package prefix.
"""),
action=PropertyChangeAction.REPLACE,
))
if package_prefixes:
result.property_changes.append(
HiddenApiPropertyChange(
property_name="package_prefixes",
values=package_prefixes,
property_comment=self.package_prefixes_comment(),
action=PropertyChangeAction.REPLACE,
))
def explain_how_to_check_signature_patterns(self):
signature_patterns_files = self.find_bootclasspath_fragment_output_file(
"signature-patterns.csv", required=False)
if signature_patterns_files:
signature_patterns_files = signature_patterns_files.removeprefix(
self.top_dir)
self.report(f"""
The purpose of the hiddenapi split_packages and package_prefixes properties is
to allow the removal of implementation details from the hidden API flags to
reduce the coupling between sdk snapshots and the APEX runtime. It cannot
eliminate that coupling completely though. Doing so may require changes to the
code.
This tool provides support for managing those properties but it cannot decide
whether the set of package prefixes suggested is appropriate that needs the
input of the developer.
Please run the following command:
m {signature_patterns_files}
And then check the '{signature_patterns_files}' for any mention of
implementation classes and packages (i.e. those classes/packages that do not
contain any part of an API surface, including the hidden API). If they are
found then the code should ideally be moved to a package unique to this module
that is contained within a package that is part of an API surface.
The format of the file is a list of patterns:
* Patterns for split packages will list every class in that package.
* Patterns for package prefixes will end with .../**.
* Patterns for packages which are not split but cannot use a package prefix
because there are sub-packages which are provided by another module will end
with .../*.
""")
def compute_hiddenapi_package_properties(self):
trie = signature_trie()
# Populate the trie with the classes that are provided by the
# bootclasspath_fragment tagging them to make it clear where they
# are from.
sorted_classes = sorted(self.classes)
for class_name in sorted_classes:
trie.add(class_name + _FAKE_MEMBER, ClassProvider.BCPF)
monolithic_classes = set()
abs_flags_file = os.path.join(self.top_dir, _FLAGS_FILE)
with open(abs_flags_file, "r", encoding="utf8") as f:
for line in iter(f.readline, ""):
signature = self.line_to_signature(line)
class_name = self.signature_to_class(signature)
if (class_name not in monolithic_classes and
class_name not in self.classes):
trie.add(
class_name + _FAKE_MEMBER,
ClassProvider.OTHER,
only_if_matches=True)
monolithic_classes.add(class_name)
split_packages = []
single_packages = []
package_prefixes = []
self.recurse_hiddenapi_packages_trie(trie, split_packages,
single_packages, package_prefixes)
return split_packages, single_packages, package_prefixes
def recurse_hiddenapi_packages_trie(self, node, split_packages,
single_packages, package_prefixes):
nodes = node.child_nodes()
if nodes:
for child in nodes:
# Ignore any non-package nodes.
if child.type != "package":
continue
package = child.selector.replace("/", ".")
providers = set(child.get_matching_rows("**"))
if not providers:
# The package and all its sub packages contain no
# classes. This should never happen.
pass
elif providers == {ClassProvider.BCPF}:
# The package and all its sub packages only contain
# classes provided by the bootclasspath_fragment.
logging.debug("Package '%s.**' is not split", package)
package_prefixes.append(package)
# There is no point traversing into the sub packages.
continue
elif providers == {ClassProvider.OTHER}:
# The package and all its sub packages contain no
# classes provided by the bootclasspath_fragment.
# There is no point traversing into the sub packages.
logging.debug("Package '%s.**' contains no classes from %s",
package, self.bcpf)
continue
elif ClassProvider.BCPF in providers:
# The package and all its sub packages contain classes
# provided by the bootclasspath_fragment and other
# sources.
logging.debug(
"Package '%s.**' contains classes from "
"%s and other sources", package, self.bcpf)
providers = set(child.get_matching_rows("*"))
if not providers:
# The package contains no classes.
logging.debug("Package: %s contains no classes", package)
elif providers == {ClassProvider.BCPF}:
# The package only contains classes provided by the
# bootclasspath_fragment.
logging.debug("Package '%s.*' is not split", package)
single_packages.append(package)
elif providers == {ClassProvider.OTHER}:
# The package contains no classes provided by the
# bootclasspath_fragment. Child nodes make contain such
# classes.
logging.debug("Package '%s.*' contains no classes from %s",
package, self.bcpf)
elif ClassProvider.BCPF in providers:
# The package contains classes provided by both the
# bootclasspath_fragment and some other source.
logging.debug("Package '%s.*' is split", package)
split_packages.append(package)
self.recurse_hiddenapi_packages_trie(child, split_packages,
single_packages,
package_prefixes)
def newline_stripping_iter(iterator):
"""Return an iterator over the iterator that strips trailing white space."""

160
scripts/hiddenapi/analyze_bcpf_test.py
View File

@@ -358,6 +358,39 @@ Lacme/items/Lever;->size:I
self.assertEqual(
expected_contents, contents, msg=f"{path} contents")
def test_compute_hiddenapi_package_properties(self):
fs = {
"out/soong/.intermediates/bcpf-dir/bcpf/all-flags.csv":
"""
La/b/C;->m()V
La/b/c/D;->m()V
La/b/c/E;->m()V
Lb/c/D;->m()V
Lb/c/E;->m()V
Lb/c/d/E;->m()V
""",
"out/soong/hiddenapi/hiddenapi-flags.csv":
"""
La/b/C;->m()V
La/b/D;->m()V
La/b/E;->m()V
La/b/c/D;->m()V
La/b/c/E;->m()V
La/b/c/d/E;->m()V
Lb/c/D;->m()V
Lb/c/E;->m()V
Lb/c/d/E;->m()V
"""
}
analyzer = self.create_analyzer_for_test(fs)
analyzer.load_all_flags()
split_packages, single_packages, package_prefixes = \
analyzer.compute_hiddenapi_package_properties()
self.assertEqual(["a.b"], split_packages)
self.assertEqual(["a.b.c"], single_packages)
self.assertEqual(["b"], package_prefixes)
class TestHiddenApiPropertyChange(unittest.TestCase):
@@ -485,6 +518,133 @@ bootclasspath_fragment {
}
""")
def test_set_property_with_value_and_comment(self):
change = ab.HiddenApiPropertyChange(
property_name="split_packages",
values=["another.provider", "other.system"],
property_comment=_MULTI_LINE_COMMENT,
action=ab.PropertyChangeAction.REPLACE,
)
self.check_change_snippet(
change, """
// Lorem ipsum dolor sit amet, consectetur adipiscing elit. Ut arcu
// justo, bibendum eu malesuada vel, fringilla in odio. Etiam gravida
// ultricies sem tincidunt luctus.
split_packages: [
"another.provider",
"other.system",
],
""")
self.check_change_fix(
change, """
bootclasspath_fragment {
name: "bcpf",
// modified by the Soong or platform compat team.
hidden_api: {
max_target_o_low_priority: ["hiddenapi/hiddenapi-max-target-o-low-priority.txt"],
split_packages: [
"another.provider",
"other.system",
],
},
}
""", """
bootclasspath_fragment {
name: "bcpf",
// modified by the Soong or platform compat team.
hidden_api: {
max_target_o_low_priority: ["hiddenapi/hiddenapi-max-target-o-low-priority.txt"],
// Lorem ipsum dolor sit amet, consectetur adipiscing elit. Ut arcu
// justo, bibendum eu malesuada vel, fringilla in odio. Etiam gravida
// ultricies sem tincidunt luctus.
split_packages: [
"another.provider",
"other.system",
],
},
}
""")
def test_set_property_with_no_value_or_comment(self):
change = ab.HiddenApiPropertyChange(
property_name="split_packages",
values=[],
action=ab.PropertyChangeAction.REPLACE,
)
self.check_change_snippet(change, """
split_packages: [],
""")
self.check_change_fix(
change, """
bootclasspath_fragment {
name: "bcpf",
// modified by the Soong or platform compat team.
hidden_api: {
max_target_o_low_priority: ["hiddenapi/hiddenapi-max-target-o-low-priority.txt"],
split_packages: [
"another.provider",
"other.system",
],
package_prefixes: ["android.provider"],
},
}
""", """
bootclasspath_fragment {
name: "bcpf",
// modified by the Soong or platform compat team.
hidden_api: {
max_target_o_low_priority: ["hiddenapi/hiddenapi-max-target-o-low-priority.txt"],
split_packages: [],
package_prefixes: ["android.provider"],
},
}
""")
def test_set_empty_property_with_no_value_or_comment(self):
change = ab.HiddenApiPropertyChange(
property_name="split_packages",
values=[],
action=ab.PropertyChangeAction.REPLACE,
)
self.check_change_snippet(change, """
split_packages: [],
""")
self.check_change_fix(
change, """
bootclasspath_fragment {
name: "bcpf",
// modified by the Soong or platform compat team.
hidden_api: {
max_target_o_low_priority: ["hiddenapi/hiddenapi-max-target-o-low-priority.txt"],
split_packages: [],
package_prefixes: ["android.provider"],
},
}
""", """
bootclasspath_fragment {
name: "bcpf",
// modified by the Soong or platform compat team.
hidden_api: {
max_target_o_low_priority: ["hiddenapi/hiddenapi-max-target-o-low-priority.txt"],
split_packages: [],
package_prefixes: ["android.provider"],
},
}
""")
if __name__ == "__main__":
unittest.main(verbosity=3)