analyze_bcpf: Add --fix option (26f19919) · Commits · e / os / android_build_soong

android/bazel.go

+3 −0

Original line number	Diff line number	Diff line
		@@ -509,6 +509,9 @@ var (

		"brotli-fuzzer-corpus", // b/202015218: outputs are in location incompatible with bazel genrule handling.

		// python modules
		"analyze_bcpf", // depends on bpmodify a blueprint_go_binary.

		// b/203369847: multiple genrules in the same package creating the same file
		// //development/sdk/...
		"platform_tools_properties",

scripts/hiddenapi/Android.bp

+4 −0

Original line number	Diff line number	Diff line
		@@ -22,6 +22,8 @@ python_binary_host {
		name: "analyze_bcpf",
		main: "analyze_bcpf.py",
		srcs: ["analyze_bcpf.py"],
		// Make sure that the bpmodify tool is built.
		data: [":bpmodify"],
		libs: [
		"signature_trie",
		],
		@@ -43,6 +45,8 @@ python_test_host {
		"analyze_bcpf.py",
		"analyze_bcpf_test.py",
		],
		// Make sure that the bpmodify tool is built.
		data: [":bpmodify"],
		libs: [
		"signature_trie",
		],

scripts/hiddenapi/analyze_bcpf.py

+253 −17

Original line number	Diff line number	Diff line
		@@ -98,6 +98,33 @@ class ModuleInfo:
		return paths[0]


		def extract_indent(line):
		return re.match(r"([ \t]*)", line).group(1)


		_SPECIAL_PLACEHOLDER: str = "SPECIAL_PLACEHOLDER"


		@dataclasses.dataclass
		class BpModifyRunner:

		bpmodify_path: str

		def add_values_to_property(self, property_name, values, module_name,
		bp_file):
		cmd = [
		self.bpmodify_path, "-a", values, "-property", property_name, "-m",
		module_name, "-w", bp_file, bp_file
		]

		logging.debug(" ".join(cmd))
		subprocess.run(
		cmd,
		stderr=subprocess.STDOUT,
		stdout=log_stream_for_subprocess(),
		check=True)


		@dataclasses.dataclass
		class FileChange:
		path: str
		@@ -129,6 +156,95 @@ class HiddenApiPropertyChange:
		snippet += "],\n"
		return snippet

		def fix_bp_file(self, bcpf_bp_file, bcpf, bpmodify_runner: BpModifyRunner):
		# Add an additional placeholder value to identify the modification that
		# bpmodify makes.
		bpmodify_values = [_SPECIAL_PLACEHOLDER]
		bpmodify_values.extend(self.values)

		packages = ",".join(bpmodify_values)
		bpmodify_runner.add_values_to_property(
		f"hidden_api.{self.property_name}", packages, bcpf, bcpf_bp_file)

		with open(bcpf_bp_file, "r", encoding="utf8") as tio:
		lines = tio.readlines()
		lines = [line.rstrip("\n") for line in lines]

		if self.fixup_bpmodify_changes(bcpf_bp_file, lines):
		with open(bcpf_bp_file, "w", encoding="utf8") as tio:
		for line in lines:
		print(line, file=tio)

		def fixup_bpmodify_changes(self, bcpf_bp_file, lines):
		# Find the line containing the placeholder that has been inserted.
		place_holder_index = -1
		for i, line in enumerate(lines):
		if _SPECIAL_PLACEHOLDER in line:
		place_holder_index = i
		break
		if place_holder_index == -1:
		logging.debug("Could not find %s in %s", _SPECIAL_PLACEHOLDER,
		bcpf_bp_file)
		return False

		# Remove the place holder. Do this before inserting the comment as that
		# would change the location of the place holder in the list.
		place_holder_line = lines[place_holder_index]
		if place_holder_line.endswith("],"):
		place_holder_line = place_holder_line.replace(
		f'"{_SPECIAL_PLACEHOLDER}"', "")
		lines[place_holder_index] = place_holder_line
		else:
		del lines[place_holder_index]

		# Scan forward to the end of the property block to remove a blank line
		# that bpmodify inserts.
		end_property_array_index = -1
		for i in range(place_holder_index, len(lines)):
		line = lines[i]
		if line.endswith("],"):
		end_property_array_index = i
		break
		if end_property_array_index == -1:
		logging.debug("Could not find end of property array in %s",
		bcpf_bp_file)
		return False

		# If bdmodify inserted a blank line afterwards then remove it.
		if (not lines[end_property_array_index + 1] and
		lines[end_property_array_index + 2].endswith("},")):
		del lines[end_property_array_index + 1]

		# Scan back to find the preceding property line.
		property_line_index = -1
		for i in range(place_holder_index, 0, -1):
		line = lines[i]
		if line.lstrip().startswith(f"{self.property_name}: ["):
		property_line_index = i
		break
		if property_line_index == -1:
		logging.debug("Could not find property line in %s", bcpf_bp_file)
		return False

		# 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
		not re.match("([ \t]+)// ", line_preceding_property)):
		# Extract the indent from the property line and use it to format the
		# comment.
		indent = extract_indent(lines[property_line_index])
		comment_lines = format_comment_as_lines(self.property_comment,
		indent)

		# If the line before the comment is not blank then insert an extra
		# blank line at the beginning of the comment.
		if line_preceding_property:
		comment_lines.insert(0, "")

		# Insert the comment before the property.
		lines[property_line_index:property_line_index] = comment_lines
		return True


		@dataclasses.dataclass()
		class Result:
		@@ -148,6 +264,9 @@ class Result:

		@dataclasses.dataclass()
		class BcpfAnalyzer:
		# Path to this tool.
		tool_path: str

		# Directory pointed to by ANDROID_BUILD_OUT
		top_dir: str

		@@ -168,6 +287,10 @@ class BcpfAnalyzer:
		# informational purposes.
		sdk: str

		# If true then this will attempt to automatically fix any issues that are
		# found.
		fix: bool = False

		# All the signatures, loaded from all-flags.csv, initialized by
		# load_all_flags().
		_signatures: typing.Set[str] = dataclasses.field(default_factory=set)
		@@ -354,10 +477,24 @@ Cleaning potentially stale files.
		self.build_monolithic_flags(result)

		# If there were any changes that need to be made to the Android.bp
		# file then report them.
		# file then either apply or report them.
		if result.property_changes:
		bcpf_dir = self.module_info.module_path(self.bcpf)
		bcpf_bp_file = os.path.join(self.top_dir, bcpf_dir, "Android.bp")
		if self.fix:
		tool_dir = os.path.dirname(self.tool_path)
		bpmodify_path = os.path.join(tool_dir, "bpmodify")
		bpmodify_runner = BpModifyRunner(bpmodify_path)
		for property_change in result.property_changes:
		property_change.fix_bp_file(bcpf_bp_file, self.bcpf,
		bpmodify_runner)

		result.file_changes.append(
		self.new_file_change(
		bcpf_bp_file,
		f"Updated hidden_api properties of '{self.bcpf}'"))

		else:
		hiddenapi_snippet = ""
		for property_change in result.property_changes:
		hiddenapi_snippet += property_change.snippet(" ")
		@@ -378,8 +515,15 @@ merge these properties into it.
		"""))

		if result.file_changes:
		self.report("""
		The following modifications need to be made:""")
		if self.fix:
		file_change_message = """
		The following files were modified by this script:"""
		else:
		file_change_message = """
		The following modifications need to be made:"""

		self.report(f"""
		{file_change_message}""")
		result.file_changes.sort()
		for file_change in result.file_changes:
		self.report(f"""
		@@ -387,6 +531,12 @@ The following modifications need to be made:""")
		{file_change.description}
		""".lstrip("\n"))

		if not self.fix:
		self.report("""
		Run the command again with the --fix option to automatically make the above
		changes.
		""".lstrip())

		def new_file_change(self, file, description):
		return FileChange(
		path=os.path.relpath(file, self.top_dir), description=description)
		@@ -665,6 +815,81 @@ Checking custom hidden API flags....
		values=[rel_bcpf_flags_file],
		))

		def fix_hidden_api_flag_files(self, result, property_name, flags_file,
		rel_bcpf_flags_file, bcpf_flags_file):
		# Read the file in frameworks/base/boot/hiddenapi/<file> copy any
		# flags that relate to the bootclasspath_fragment into a local
		# file in the hiddenapi subdirectory.
		tmp_flags_file = flags_file + ".tmp"

		# Make sure the directory containing the bootclasspath_fragment specific
		# hidden api flags exists.
		os.makedirs(os.path.dirname(bcpf_flags_file), exist_ok=True)

		bcpf_flags_file_exists = os.path.exists(bcpf_flags_file)

		matched_signatures = set()
		# Open the flags file to read the flags from.
		with open(flags_file, "r", encoding="utf8") as f:
		# Open a temporary file to write the flags (minus any removed
		# flags).
		with open(tmp_flags_file, "w", encoding="utf8") as t:
		# Open the bootclasspath_fragment file for append just in
		# case it already exists.
		with open(bcpf_flags_file, "a", encoding="utf8") as b:
		for line in iter(f.readline, ""):
		signature = line.rstrip()
		if signature in self.signatures:
		# The signature is provided by the
		# bootclasspath_fragment so write it to the new
		# bootclasspath_fragment specific file.
		print(line, file=b, end="")
		matched_signatures.add(signature)
		else:
		# The signature is NOT provided by the
		# bootclasspath_fragment. Copy it to the new
		# monolithic file.
		print(line, file=t, end="")

		# If the bootclasspath_fragment specific flags file is not empty
		# then it contains flags. That could either be new flags just moved
		# from frameworks/base or previous contents of the file. In either
		# case the file must not be removed.
		if matched_signatures:
		# There are custom flags related to the bootclasspath_fragment
		# so replace the frameworks/base/boot/hiddenapi file with the
		# file that does not contain those flags.
		shutil.move(tmp_flags_file, flags_file)

		result.file_changes.append(
		self.new_file_change(flags_file,
		f"Removed '{self.bcpf}' specific entries"))

		result.property_changes.append(
		HiddenApiPropertyChange(
		property_name=property_name,
		values=[rel_bcpf_flags_file],
		))

		# Make sure that the files are sorted.
		self.run_command([
		"tools/platform-compat/hiddenapi/sort_api.sh",
		bcpf_flags_file,
		])

		if bcpf_flags_file_exists:
		desc = f"Added '{self.bcpf}' specific entries"
		else:
		desc = f"Created with '{self.bcpf}' specific entries"
		result.file_changes.append(
		self.new_file_change(bcpf_flags_file, desc))
		else:
		# There are no custom flags related to the
		# bootclasspath_fragment so clean up the working files.
		os.remove(tmp_flags_file)
		if not bcpf_flags_file_exists:
		os.remove(bcpf_flags_file)

		def check_frameworks_base_boot_hidden_api_files(self, result):
		hiddenapi_dir = os.path.join(self.top_dir,
		"frameworks/base/boot/hiddenapi")
		@@ -695,9 +920,13 @@ Checking custom hidden API flags....
		bcpf_flags_file = os.path.join(self.top_dir, bcpf_dir,
		rel_bcpf_flags_file)

		self.report_hidden_api_flag_file_changes(result, property_name,
		flags_file,
		rel_bcpf_flags_file,
		if self.fix:
		self.fix_hidden_api_flag_files(result, property_name,
		flags_file, rel_bcpf_flags_file,
		bcpf_flags_file)
		else:
		self.report_hidden_api_flag_file_changes(
		result, property_name, flags_file, rel_bcpf_flags_file,
		bcpf_flags_file)


		@@ -750,6 +979,11 @@ def main(argv):
		"allow this script to give more useful messages and it may be"
		"required in future.",
		default="SPECIFY-SDK-OPTION")
		args_parser.add_argument(
		"--fix",
		help="Attempt to fix any issues found automatically.",
		action="store_true",
		default=False)
		args = args_parser.parse_args(argv[1:])
		top_dir = os.environ["ANDROID_BUILD_TOP"] + "/"
		out_dir = os.environ.get("OUT_DIR", os.path.join(top_dir, "out"))
		@@ -778,12 +1012,14 @@ def main(argv):
		print(f"Writing log to {abs_log_file}")
		try:
		analyzer = BcpfAnalyzer(
		tool_path=argv[0],
		top_dir=top_dir,
		out_dir=out_dir,
		product_out_dir=product_out_dir,
		bcpf=args.bcpf,
		apex=args.apex,
		sdk=args.sdk,
		fix=args.fix,
		)
		analyzer.analyze()
		finally:

scripts/hiddenapi/analyze_bcpf_test.py

+172 −13

Original line number	Diff line number	Diff line
		@@ -20,6 +20,8 @@ import tempfile
		import unittest
		import unittest.mock

		import sys

		import analyze_bcpf as ab

		_FRAMEWORK_HIDDENAPI = "frameworks/base/boot/hiddenapi"
		@@ -73,7 +75,8 @@ class TestAnalyzeBcpf(unittest.TestCase):
		fs=None,
		bcpf="bcpf",
		apex="apex",
		sdk="sdk"):
		sdk="sdk",
		fix=False):
		if fs:
		self.populate_fs(fs)

		@@ -86,12 +89,14 @@ class TestAnalyzeBcpf(unittest.TestCase):
		module_info = ab.ModuleInfo(modules)

		analyzer = ab.BcpfAnalyzer(
		tool_path=os.path.join(out_dir, "bin"),
		top_dir=top_dir,
		out_dir=out_dir,
		product_out_dir=product_out_dir,
		bcpf=bcpf,
		apex=apex,
		sdk=sdk,
		fix=fix,
		module_info=module_info,
		)
		analyzer.load_all_flags()
		@@ -114,7 +119,7 @@ that traverses multiple lines.
		that should not be reformatted.
		""", reformatted)

		def test_build_flags(self):
		def do_test_build_flags(self, fix):
		lines = """
		ERROR: Hidden API flags are inconsistent:
		< out/soong/.intermediates/bcpf-dir/bcpf-dir/filtered-flags.csv
		@@ -169,7 +174,7 @@ Lacme/test/Other;->getThing()Z,blocked
		""",
		}

		analyzer = self.create_analyzer_for_test(fs)
		analyzer = self.create_analyzer_for_test(fs, fix=fix)

		# Override the build_file_read_output() method to just return a fake
		# build operation.
		@@ -214,6 +219,11 @@ Lacme/test/Other;->getThing()Z,blocked
		result.property_changes,
		msg="property changes")

		return result

		def test_build_flags_report(self):
		result = self.do_test_build_flags(fix=False)

		expected_file_changes = [
		ab.FileChange(
		path="bcpf-dir/hiddenapi/"
		@@ -268,6 +278,86 @@ Lacme/test/Other;->getThing()Z,blocked
		self.assertEqual(
		expected_file_changes, result.file_changes, msg="file_changes")

		def test_build_flags_fix(self):
		result = self.do_test_build_flags(fix=True)

		expected_file_changes = [
		ab.FileChange(
		path="bcpf-dir/hiddenapi/"
		"hiddenapi-max-target-o-low-priority.txt",
		description="Created with 'bcpf' specific entries"),
		ab.FileChange(
		path="bcpf-dir/hiddenapi/hiddenapi-max-target-p.txt",
		description="Added 'bcpf' specific entries"),
		ab.FileChange(
		path="bcpf-dir/hiddenapi/hiddenapi-max-target-q.txt",
		description="Created with 'bcpf' specific entries"),
		ab.FileChange(
		path="bcpf-dir/hiddenapi/"
		"hiddenapi-max-target-r-low-priority.txt",
		description="Created with 'bcpf' specific entries"),
		ab.FileChange(
		path=_MAX_TARGET_O,
		description="Removed 'bcpf' specific entries"),
		ab.FileChange(
		path=_MAX_TARGET_P,
		description="Removed 'bcpf' specific entries"),
		ab.FileChange(
		path=_MAX_TARGET_Q,
		description="Removed 'bcpf' specific entries"),
		ab.FileChange(
		path=_MAX_TARGET_R,
		description="Removed 'bcpf' specific entries")
		]

		result.file_changes.sort()
		self.assertEqual(
		expected_file_changes, result.file_changes, msg="file_changes")

		expected_file_contents = {
		"bcpf-dir/hiddenapi/hiddenapi-max-target-o-low-priority.txt":
		"""
		Lacme/test/Class;-><init>()V
		""",
		"bcpf-dir/hiddenapi/hiddenapi-max-target-p.txt":
		"""
		Lacme/old/Class;->getWidget()Lacme/test/Widget;
		Lacme/test/Other;->getThing()Z
		""",
		"bcpf-dir/hiddenapi/hiddenapi-max-target-q.txt":
		"""
		Lacme/test/Widget;-><init()V
		""",
		"bcpf-dir/hiddenapi/hiddenapi-max-target-r-low-priority.txt":
		"""
		Lacme/test/Gadget;->NAME:Ljava/lang/String;
		""",
		_MAX_TARGET_O:
		"""
		Lacme/items/Magnet;->size:I
		""",
		_MAX_TARGET_P:
		"""
		Lacme/items/Rocket;->size:I
		""",
		_MAX_TARGET_Q:
		"""
		Lacme/items/Rock;->size:I
		""",
		_MAX_TARGET_R:
		"""
		Lacme/items/Lever;->size:I
		""",
		}
		for file_change in result.file_changes:
		path = file_change.path
		expected_contents = expected_file_contents[path].lstrip()
		abs_path = os.path.join(self.test_dir, path)
		with open(abs_path, "r", encoding="utf8") as tio:
		contents = tio.read()
		self.assertEqual(
		expected_contents, contents, msg=f"{path} contents")


		class TestHiddenApiPropertyChange(unittest.TestCase):

		@@ -279,6 +369,20 @@ class TestHiddenApiPropertyChange(unittest.TestCase):
		# Remove the directory after the test
		shutil.rmtree(self.test_dir)

		def check_change_fix(self, change, bpmodify_output, expected):
		file = os.path.join(self.test_dir, "Android.bp")

		with open(file, "w", encoding="utf8") as tio:
		tio.write(bpmodify_output.strip("\n"))

		bpmodify_runner = ab.BpModifyRunner(
		os.path.join(os.path.dirname(sys.argv[0]), "bpmodify"))
		change.fix_bp_file(file, "bcpf", bpmodify_runner)

		with open(file, "r", encoding="utf8") as tio:
		contents = tio.read()
		self.assertEqual(expected.lstrip("\n"), contents)

		def check_change_snippet(self, change, expected):
		snippet = change.snippet(" ")
		self.assertEqual(expected, snippet)
		@@ -296,6 +400,33 @@ class TestHiddenApiPropertyChange(unittest.TestCase):
		],
		""")

		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: [
		"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: [
		"android.provider",
		],
		},
		}
		""")

		def test_change_property_with_value_and_comment(self):
		change = ab.HiddenApiPropertyChange(
		property_name="split_packages",
		@@ -313,17 +444,45 @@ class TestHiddenApiPropertyChange(unittest.TestCase):
		],
		""")

		def test_change_without_value_or_empty_comment(self):
		change = ab.HiddenApiPropertyChange(
		property_name="split_packages",
		values=[],
		property_comment="Single line comment.",
		)

		self.check_change_snippet(
		self.check_change_fix(
		change, """
		// Single line comment.
		split_packages: [],
		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: [
		"android.provider",
		],

		single_packages: [
		"android.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: [
		"android.provider",
		],

		single_packages: [
		"android.system",
		],

		},
		}
		""")