analyze_bcpf: Indent multi-line text to improve formatting

Addresses a comment on the review of the initial changes to add
analyze_bcpf script.

Bug: 202154151
Test: m analyze_bcpf && analyze_bcpf --bcpf art-bootclasspath-fragment
      m analyze_bcpf && analyze_bcpf --bcpf art-bootclasspath-fragment --fix
      atest --host analyze_bcpf_test
Change-Id: I8c030dcc1a29f106ca57dca8c97a5cb4425e7674
This commit is contained in:
Paul Duffin
2022-04-04 16:59:36 +01:00
parent dbbb8374a2
commit ea836c2baf

View File

@@ -394,13 +394,17 @@ class BcpfAnalyzer:
def reformat_report_test(text): def reformat_report_test(text):
return re.sub(r"(.)\n([^\s])", r"\1 \2", text) return re.sub(r"(.)\n([^\s])", r"\1 \2", text)
def report(self, text, **kwargs): def report(self, text="", **kwargs):
# Concatenate lines that are not separated by a blank line together to # Concatenate lines that are not separated by a blank line together to
# eliminate formatting applied to the supplied text to adhere to python # eliminate formatting applied to the supplied text to adhere to python
# line length limitations. # line length limitations.
text = self.reformat_report_test(text) text = self.reformat_report_test(text)
logging.info("%s", text, **kwargs) logging.info("%s", text, **kwargs)
def report_dedent(self, text, **kwargs):
text = textwrap.dedent(text)
self.report(text, **kwargs)
def run_command(self, cmd, *args, **kwargs): def run_command(self, cmd, *args, **kwargs):
cmd_line = " ".join(cmd) cmd_line = " ".join(cmd)
logging.debug("Running %s", cmd_line) logging.debug("Running %s", cmd_line)
@@ -442,9 +446,7 @@ class BcpfAnalyzer:
def load_module_info(self): def load_module_info(self):
module_info_file = os.path.join(self.product_out_dir, module_info_file = os.path.join(self.product_out_dir,
"module-info.json") "module-info.json")
self.report(f""" self.report(f"\nMaking sure that {module_info_file} is up to date.\n")
Making sure that {module_info_file} is up to date.
""")
output = self.build_file_read_output(module_info_file) output = self.build_file_read_output(module_info_file)
lines = output.lines() lines = output.lines()
for line in lines: for line in lines:
@@ -496,61 +498,62 @@ Making sure that {module_info_file} is up to date.
optimizations that can be applied. optimizations that can be applied.
""" """
self.report(f"Analyzing bootclasspath_fragment module {self.bcpf}") self.report(f"Analyzing bootclasspath_fragment module {self.bcpf}")
self.report(f""" self.report_dedent(f"""
Run this tool to help initialize a bootclasspath_fragment module. Before you Run this tool to help initialize a bootclasspath_fragment module.
start make sure that: Before you start make sure that:
1. The current checkout is up to date. 1. The current checkout is up to date.
2. The environment has been initialized using lunch, e.g. 2. The environment has been initialized using lunch, e.g.
lunch aosp_arm64-userdebug lunch aosp_arm64-userdebug
3. You have added a bootclasspath_fragment module to the appropriate Android.bp 3. You have added a bootclasspath_fragment module to the appropriate
file. Something like this: Android.bp file. Something like this:
bootclasspath_fragment {{ bootclasspath_fragment {{
name: "{self.bcpf}", name: "{self.bcpf}",
contents: [ contents: [
"...", "...",
], ],
// The bootclasspath_fragments that provide APIs on which this
// depends.
fragments: [
{{
apex: "com.android.art",
module: "art-bootclasspath-fragment",
}},
],
}}
4. You have added it to the platform_bootclasspath module in
frameworks/base/boot/Android.bp. Something like this:
// The bootclasspath_fragments that provide APIs on which this depends. platform_bootclasspath {{
fragments: [ name: "platform-bootclasspath",
{{ fragments: [
apex: "com.android.art", ...
module: "art-bootclasspath-fragment", {{
}}, apex: "{self.apex}",
], module: "{self.bcpf}",
}} }},
],
}}
4. You have added it to the platform_bootclasspath module in 5. You have added an sdk module. Something like this:
frameworks/base/boot/Android.bp. Something like this:
platform_bootclasspath {{ sdk {{
name: "platform-bootclasspath", name: "{self.sdk}",
fragments: [ bootclasspath_fragments: ["{self.bcpf}"],
... }}
{{ """)
apex: "{self.apex}",
module: "{self.bcpf}",
}},
],
}}
5. You have added an sdk module. Something like this:
sdk {{
name: "{self.sdk}",
bootclasspath_fragments: ["{self.bcpf}"],
}}
""")
# Make sure that the module-info.json file is up to date. # Make sure that the module-info.json file is up to date.
self.load_module_info() self.load_module_info()
self.report(""" self.report_dedent("""
Cleaning potentially stale files. Cleaning potentially stale files.
""") """)
# Remove the out/soong/hiddenapi files. # Remove the out/soong/hiddenapi files.
shutil.rmtree(f"{self.out_dir}/soong/hiddenapi", ignore_errors=True) shutil.rmtree(f"{self.out_dir}/soong/hiddenapi", ignore_errors=True)
@@ -605,26 +608,26 @@ merge these properties into it.
if result.file_changes: if result.file_changes:
if self.fix: if self.fix:
file_change_message = """ file_change_message = textwrap.dedent("""
The following files were modified by this script:""" The following files were modified by this script:
""")
else: else:
file_change_message = """ file_change_message = textwrap.dedent("""
The following modifications need to be made:""" The following modifications need to be made:
""")
self.report(f""" self.report(file_change_message)
{file_change_message}""")
result.file_changes.sort() result.file_changes.sort()
for file_change in result.file_changes: for file_change in result.file_changes:
self.report(f""" self.report(f" {file_change.path}")
{file_change.path} self.report(f" {file_change.description}")
{file_change.description} self.report()
""".lstrip("\n"))
if not self.fix: if not self.fix:
self.report(""" self.report_dedent("""
Run the command again with the --fix option to automatically make the above Run the command again with the --fix option to automatically
changes. make the above changes.
""".lstrip()) """.lstrip("\n"))
def new_file_change(self, file, description): def new_file_change(self, file, description):
return FileChange( return FileChange(
@@ -635,11 +638,10 @@ changes.
if not (module_line.startswith("< ") and if not (module_line.startswith("< ") and
monolithic_line.startswith("> ") and not separator_line): monolithic_line.startswith("> ") and not separator_line):
# Something went wrong. # Something went wrong.
self.report(f"""Invalid build output detected: self.report("Invalid build output detected:")
module_line: "{module_line}" self.report(f" module_line: '{module_line}'")
monolithic_line: "{monolithic_line}" self.report(f" monolithic_line: '{monolithic_line}'")
separator_line: "{separator_line}" self.report(f" separator_line: '{separator_line}'")
""")
sys.exit(1) sys.exit(1)
if significant: if significant:
@@ -698,10 +700,9 @@ changes.
if module_signature != monolithic_signature: if module_signature != monolithic_signature:
# Something went wrong. # Something went wrong.
self.report(f"""Inconsistent signatures detected: self.report("Inconsistent signatures detected:")
module_signature: "{module_signature}" self.report(f" module_signature: '{module_signature}'")
monolithic_signature: "{monolithic_signature}" self.report(f" monolithic_signature: '{monolithic_signature}'")
""")
sys.exit(1) sys.exit(1)
diffs[module_signature] = (module_flags, monolithic_flags) diffs[module_signature] = (module_flags, monolithic_flags)
@@ -749,85 +750,91 @@ changes.
return diffs return diffs
def build_monolithic_stubs_flags(self): def build_monolithic_stubs_flags(self):
self.report(f""" self.report_dedent(f"""
Attempting to build {_STUB_FLAGS_FILE} to verify that the Attempting to build {_STUB_FLAGS_FILE} to verify that the
bootclasspath_fragment has the correct API stubs available... bootclasspath_fragment has the correct API stubs available...
""") """)
# Build the hiddenapi-stubs-flags.txt file. # Build the hiddenapi-stubs-flags.txt file.
diffs = self.build_hiddenapi_flags(_STUB_FLAGS_FILE) diffs = self.build_hiddenapi_flags(_STUB_FLAGS_FILE)
if diffs: if diffs:
self.report(f""" self.report_dedent(f"""
There is a discrepancy between the stub API derived flags created by the There is a discrepancy between the stub API derived flags
bootclasspath_fragment and the platform_bootclasspath. See preceding error created by the bootclasspath_fragment and the
messages to see which flags are inconsistent. The inconsistencies can occur for platform_bootclasspath. See preceding error messages to see
a couple of reasons: which flags are inconsistent. The inconsistencies can occur for
a couple of reasons:
If you are building against prebuilts of the Android SDK, e.g. by using If you are building against prebuilts of the Android SDK, e.g.
TARGET_BUILD_APPS then the prebuilt versions of the APIs this by using TARGET_BUILD_APPS then the prebuilt versions of the
bootclasspath_fragment depends upon are out of date and need updating. See APIs this bootclasspath_fragment depends upon are out of date
go/update-prebuilts for help. and need updating. See go/update-prebuilts for help.
Otherwise, this is happening because there are some stub APIs that are either Otherwise, this is happening because there are some stub APIs
provided by or used by the contents of the bootclasspath_fragment but which are that are either provided by or used by the contents of the
not available to it. There are 4 ways to handle this: bootclasspath_fragment but which are not available to it. There
are 4 ways to handle this:
1. A java_sdk_library in the contents property will automatically make its stub 1. A java_sdk_library in the contents property will
APIs available to the bootclasspath_fragment so nothing needs to be done. automatically make its stub APIs available to the
bootclasspath_fragment so nothing needs to be done.
2. If the API provided by the bootclasspath_fragment is created by an api_only 2. If the API provided by the bootclasspath_fragment is created
java_sdk_library (or a java_library that compiles files generated by a by an api_only java_sdk_library (or a java_library that compiles
separate droidstubs module then it cannot be added to the contents and files generated by a separate droidstubs module then it cannot
instead must be added to the api.stubs property, e.g. be added to the contents and instead must be added to the
api.stubs property, e.g.
bootclasspath_fragment {{ bootclasspath_fragment {{
name: "{self.bcpf}", name: "{self.bcpf}",
... ...
api: {{ api: {{
stubs: ["$MODULE-api-only"]," stubs: ["$MODULE-api-only"],"
}}, }},
}} }}
3. If the contents use APIs provided by another bootclasspath_fragment then 3. If the contents use APIs provided by another
it needs to be added to the fragments property, e.g. bootclasspath_fragment then it needs to be added to the
fragments property, e.g.
bootclasspath_fragment {{
name: "{self.bcpf}",
...
// The bootclasspath_fragments that provide APIs on which this depends.
fragments: [
...
{{
apex: "com.android.other",
module: "com.android.other-bootclasspath-fragment",
}},
],
}}
4. If the contents use APIs from a module that is not part of
another bootclasspath_fragment then it must be added to the
additional_stubs property, e.g.
bootclasspath_fragment {{ bootclasspath_fragment {{
name: "{self.bcpf}", name: "{self.bcpf}",
... ...
// The bootclasspath_fragments that provide APIs on which this depends. additional_stubs: ["android-non-updatable"],
fragments: [ }}
...
{{
apex: "com.android.other",
module: "com.android.other-bootclasspath-fragment",
}},
],
}}
4. If the contents use APIs from a module that is not part of another Like the api.stubs property these are typically
bootclasspath_fragment then it must be added to the additional_stubs java_sdk_library modules but can be java_library too.
property, e.g.
bootclasspath_fragment {{ Note: The "android-non-updatable" is treated as if it was a
name: "{self.bcpf}", java_sdk_library which it is not at the moment but will be in
... future.
additional_stubs: ["android-non-updatable"], """)
}}
Like the api.stubs property these are typically java_sdk_library modules but
can be java_library too.
Note: The "android-non-updatable" is treated as if it was a java_sdk_library
which it is not at the moment but will be in future.
""")
return diffs return diffs
def build_monolithic_flags(self, result): def build_monolithic_flags(self, result):
self.report(f""" self.report_dedent(f"""
Attempting to build {_FLAGS_FILE} to verify that the Attempting to build {_FLAGS_FILE} to verify that the
bootclasspath_fragment has the correct hidden API flags... bootclasspath_fragment has the correct hidden API flags...
""") """)
# Build the hiddenapi-flags.csv file and extract any differences in # Build the hiddenapi-flags.csv file and extract any differences in
# the flags between this bootclasspath_fragment and the monolithic # the flags between this bootclasspath_fragment and the monolithic
@@ -838,32 +845,34 @@ bootclasspath_fragment has the correct hidden API flags...
self.load_all_flags() self.load_all_flags()
if result.diffs: if result.diffs:
self.report(f""" self.report_dedent(f"""
There is a discrepancy between the hidden API flags created by the There is a discrepancy between the hidden API flags created by
bootclasspath_fragment and the platform_bootclasspath. See preceding error the bootclasspath_fragment and the platform_bootclasspath. See
messages to see which flags are inconsistent. The inconsistencies can occur for preceding error messages to see which flags are inconsistent.
a couple of reasons: The inconsistencies can occur for a couple of reasons:
If you are building against prebuilts of this bootclasspath_fragment then the If you are building against prebuilts of this
prebuilt version of the sdk snapshot (specifically the hidden API flag files) bootclasspath_fragment then the prebuilt version of the sdk
are inconsistent with the prebuilt version of the apex {self.apex}. Please snapshot (specifically the hidden API flag files) are
ensure that they are both updated from the same build. inconsistent with the prebuilt version of the apex {self.apex}.
Please ensure that they are both updated from the same build.
1. There are custom hidden API flags specified in the one of the files in 1. There are custom hidden API flags specified in the one of the
frameworks/base/boot/hiddenapi which apply to the bootclasspath_fragment but files in frameworks/base/boot/hiddenapi which apply to the
which are not supplied to the bootclasspath_fragment module. bootclasspath_fragment but which are not supplied to the
bootclasspath_fragment module.
2. The bootclasspath_fragment specifies invalid "package_prefixes" or 2. The bootclasspath_fragment specifies invalid
"split_packages" properties that match packages and classes that it does not "split_packages", "single_packages" and/of "package_prefixes"
provide. properties that match packages and classes that it does not
provide.
""") """)
# Check to see if there are any hiddenapi related properties that # Check to see if there are any hiddenapi related properties that
# need to be added to the # need to be added to the
self.report(""" self.report_dedent("""
Checking custom hidden API flags.... Checking custom hidden API flags....
""") """)
self.check_frameworks_base_boot_hidden_api_files(result) self.check_frameworks_base_boot_hidden_api_files(result)
def report_hidden_api_flag_file_changes(self, result, property_name, def report_hidden_api_flag_file_changes(self, result, property_name,
@@ -1060,25 +1069,26 @@ Checking custom hidden API flags....
)) ))
if split_packages: if split_packages:
self.report(f""" self.report_dedent(f"""
bootclasspath_fragment {self.bcpf} contains classes in packages that also bootclasspath_fragment {self.bcpf} contains classes in packages
contain classes provided by other sources, those packages are called split that also contain classes provided by other bootclasspath
packages. Split packages should be avoided where possible but are often modules. Those packages are called split packages. Split
unavoidable when modularizing existing code. 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 The hidden api processing needs to know which packages are split
which are not) so that it can optimize the hidden API flags to remove (and conversely which are not) so that it can optimize the
unnecessary implementation details. hidden API flags to remove unnecessary implementation details.
""")
self.report(""" By default (for backwards compatibility) the
By default (for backwards compatibility) the bootclasspath_fragment assumes that bootclasspath_fragment assumes that all packages are split
all packages are split unless one of the package_prefixes or split_packages unless one of the package_prefixes or split_packages properties
properties are specified. While that is safe it is not optimal and can lead to are specified. While that is safe it is not optimal and can lead
unnecessary implementation details leaking into the hidden API flags. Adding an to unnecessary implementation details leaking into the hidden
empty split_packages property allows the flags to be optimized and remove any API flags. Adding an empty split_packages property allows the
unnecessary implementation details. flags to be optimized and remove any unnecessary implementation
""") details.
""")
if single_packages: if single_packages:
result.property_changes.append( result.property_changes.append(
@@ -1091,7 +1101,7 @@ unnecessary implementation details.
contain classes from other bootclasspath modules. Packages contain classes from other bootclasspath modules. Packages
should only be listed here when necessary for legacy should only be listed here when necessary for legacy
purposes, new packages should match a package prefix. purposes, new packages should match a package prefix.
"""), """),
action=PropertyChangeAction.REPLACE, action=PropertyChangeAction.REPLACE,
)) ))
@@ -1111,36 +1121,38 @@ unnecessary implementation details.
signature_patterns_files = signature_patterns_files.removeprefix( signature_patterns_files = signature_patterns_files.removeprefix(
self.top_dir) self.top_dir)
self.report(f""" self.report_dedent(f"""
The purpose of the hiddenapi split_packages and package_prefixes properties is The purpose of the hiddenapi split_packages and package_prefixes
to allow the removal of implementation details from the hidden API flags to properties is to allow the removal of implementation details
reduce the coupling between sdk snapshots and the APEX runtime. It cannot from the hidden API flags to reduce the coupling between sdk
eliminate that coupling completely though. Doing so may require changes to the snapshots and the APEX runtime. It cannot eliminate that
code. coupling completely though. Doing so may require changes to the
code.
This tool provides support for managing those properties but it cannot decide This tool provides support for managing those properties but it
whether the set of package prefixes suggested is appropriate that needs the cannot decide whether the set of package prefixes suggested is
input of the developer. appropriate that needs the input of the developer.
Please run the following command: Please run the following command:
m {signature_patterns_files} m {signature_patterns_files}
And then check the '{signature_patterns_files}' for any mention of And then check the '{signature_patterns_files}' for any mention
implementation classes and packages (i.e. those classes/packages that do not of implementation classes and packages (i.e. those
contain any part of an API surface, including the hidden API). If they are classes/packages that do not contain any part of an API surface,
found then the code should ideally be moved to a package unique to this module including the hidden API). If they are found then the code
that is contained within a package that is part of an API surface. 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: The format of the file is a list of patterns:
* Patterns for split packages will list every class in that package. * Patterns for split packages will list every class in that package.
* Patterns for package prefixes will end with .../**. * Patterns for package prefixes will end with .../**.
* Patterns for packages which are not split but cannot use a package prefix * Patterns for packages which are not split but cannot use a
because there are sub-packages which are provided by another module will end package prefix because there are sub-packages which are provided
with .../*. by another module will end with .../*.
""") """)
def compute_hiddenapi_package_properties(self): def compute_hiddenapi_package_properties(self):
trie = signature_trie() trie = signature_trie()