Commit 29fb38f0 authored by Andrew Grieve's avatar Andrew Grieve Committed by Commit Bot

SuperSize: Fix missing metadata for Trichrome.

Reworks a lot of how arg parsing works so that there is less room for
failure (hopefully). Including:
 * Rename: --no-source-paths -> --no-output-directory
 * Make --ssargs-file top_args only.

Also allows passing --no-native via command-line and have that
work when .ssargs file is used.

Bug: 1132015
Change-Id: Ia4e46e056d9878e51d497041a231cb5dcde846fe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2429449
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: default avatarSamuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810457}
parent 4421aa86
This diff is collapsed.
...@@ -331,7 +331,7 @@ class _Session(object): ...@@ -331,7 +331,7 @@ class _Session(object):
any_path_within_output_directory=elf_path) any_path_within_output_directory=elf_path)
if output_directory_finder.Tentative(): if output_directory_finder.Tentative():
tool_prefix = path_util.ToolPrefixFinder( tool_prefix = path_util.ToolPrefixFinder(
output_directory_finder=output_directory_finder, output_directory=output_directory_finder.Finalized(),
linker_name='ld').Finalized() linker_name='ld').Finalized()
args = [ args = [
...@@ -503,7 +503,7 @@ def Run(args, on_config_error): ...@@ -503,7 +503,7 @@ def Run(args, on_config_error):
linker_name = size_infos[-1].build_config.get(models.BUILD_CONFIG_LINKER_NAME) linker_name = size_infos[-1].build_config.get(models.BUILD_CONFIG_LINKER_NAME)
tool_prefix_finder = path_util.ToolPrefixFinder( tool_prefix_finder = path_util.ToolPrefixFinder(
value=args.tool_prefix, value=args.tool_prefix,
output_directory_finder=output_directory_finder, output_directory=output_directory_finder.Tentative(),
linker_name=linker_name) linker_name=linker_name)
session = _Session(size_infos, output_directory_finder, tool_prefix_finder) session = _Session(size_infos, output_directory_finder, tool_prefix_finder)
......
...@@ -160,16 +160,10 @@ class IntegrationTest(unittest.TestCase): ...@@ -160,16 +160,10 @@ class IntegrationTest(unittest.TestCase):
]) ])
def _CreateTestArgs(self): def _CreateTestArgs(self):
return argparse.Namespace( parser = argparse.ArgumentParser()
**{ archive.AddArguments(parser)
'is_bundle': False, ret = parser.parse_args(['foo'])
'java_only': False, return ret
'native_only': False,
'no_java': False,
'no_native': False,
'relocations': False,
'source_directory': _TEST_SOURCE_DIR,
})
def _CloneSizeInfo(self, def _CloneSizeInfo(self,
use_output_directory=True, use_output_directory=True,
...@@ -183,54 +177,58 @@ class IntegrationTest(unittest.TestCase): ...@@ -183,54 +177,58 @@ class IntegrationTest(unittest.TestCase):
cache_key = (use_output_directory, use_elf, use_apk, use_minimal_apks, cache_key = (use_output_directory, use_elf, use_apk, use_minimal_apks,
use_pak, use_aux_elf) use_pak, use_aux_elf)
if cache_key not in IntegrationTest.cached_size_info: if cache_key not in IntegrationTest.cached_size_info:
elf_path = _TEST_ELF_PATH if use_elf or use_aux_elf else None
output_directory = _TEST_OUTPUT_DIR if use_output_directory else None
knobs = archive.SectionSizeKnobs() knobs = archive.SectionSizeKnobs()
opts = archive.ContainerArchiveOptions(self._CreateTestArgs())
# Override for testing. Lower the bar for compacting symbols, to allow # Override for testing. Lower the bar for compacting symbols, to allow
# smaller test cases to be created. # smaller test cases to be created.
knobs.max_same_name_alias_count = 3 knobs.max_same_name_alias_count = 3
apk_path = None
minimal_apks_path = None args = self._CreateTestArgs()
args.elf_file = _TEST_ELF_PATH if use_elf or use_aux_elf else None
args.map_file = _TEST_MAP_PATH
args.output_directory = _TEST_OUTPUT_DIR if use_output_directory else None
args.source_directory = _TEST_SOURCE_DIR
args.tool_prefix = _TEST_TOOL_PREFIX
apk_so_path = None apk_so_path = None
size_info_prefix = None size_info_prefix = None
extracted_minimal_apk_path = None extracted_minimal_apk_path = None
if use_apk: if use_apk:
apk_path = _TEST_APK_PATH args.apk_file = _TEST_APK_PATH
elif use_minimal_apks: elif use_minimal_apks:
minimal_apks_path = _TEST_MINIMAL_APKS_PATH args.minimal_apks_file = _TEST_MINIMAL_APKS_PATH
extracted_minimal_apk_path = _TEST_APK_PATH extracted_minimal_apk_path = _TEST_APK_PATH
if use_apk or use_minimal_apks: if use_apk or use_minimal_apks:
apk_so_path = _TEST_APK_SO_PATH apk_so_path = _TEST_APK_SO_PATH
if output_directory: if args.output_directory:
if use_apk: if use_apk:
orig_path = _TEST_APK_PATH orig_path = _TEST_APK_PATH
else: else:
orig_path = _TEST_MINIMAL_APKS_PATH.replace('.minimal.apks', '.aab') orig_path = _TEST_MINIMAL_APKS_PATH.replace('.minimal.apks', '.aab')
size_info_prefix = os.path.join( size_info_prefix = os.path.join(args.output_directory, 'size-info',
output_directory, 'size-info', os.path.basename(orig_path)) os.path.basename(orig_path))
pak_files = None pak_files = None
pak_info_file = None pak_info_file = None
if use_pak: if use_pak:
pak_files = [_TEST_APK_LOCALE_PAK_PATH, _TEST_APK_PAK_PATH] pak_files = [_TEST_APK_LOCALE_PAK_PATH, _TEST_APK_PAK_PATH]
pak_info_file = _TEST_PAK_INFO_PATH pak_info_file = _TEST_PAK_INFO_PATH
linker_name = 'gold' linker_name = 'gold'
# For simplicity, using |args| for both params. This is okay since
# |args.ssargs_file| is unassigned.
opts = archive.ContainerArchiveOptions(args, args)
with _AddMocksToPath(): with _AddMocksToPath():
build_config = {} build_config = {}
metadata = archive.CreateMetadata(_TEST_MAP_PATH, elf_path, apk_path, metadata = archive.CreateMetadata(args, linker_name, build_config)
minimal_apks_path, _TEST_TOOL_PREFIX,
output_directory, linker_name,
build_config)
container, raw_symbols = archive.CreateContainerAndSymbols( container, raw_symbols = archive.CreateContainerAndSymbols(
knobs=knobs, knobs=knobs,
opts=opts, opts=opts,
container_name='', container_name='',
metadata=metadata, metadata=metadata,
map_path=_TEST_MAP_PATH, map_path=args.map_file,
tool_prefix=_TEST_TOOL_PREFIX, tool_prefix=args.tool_prefix,
output_directory=output_directory, output_directory=args.output_directory,
elf_path=elf_path, source_directory=args.source_directory,
apk_path=apk_path or extracted_minimal_apk_path, elf_path=args.elf_file,
apk_path=args.apk_file or extracted_minimal_apk_path,
apk_so_path=apk_so_path, apk_so_path=apk_so_path,
pak_files=pak_files, pak_files=pak_files,
pak_info_file=pak_info_file, pak_info_file=pak_info_file,
...@@ -260,7 +258,7 @@ class IntegrationTest(unittest.TestCase): ...@@ -260,7 +258,7 @@ class IntegrationTest(unittest.TestCase):
if not use_elf: if not use_elf:
args += ['--output-directory', _TEST_OUTPUT_DIR] args += ['--output-directory', _TEST_OUTPUT_DIR]
else: else:
args += ['--no-source-paths'] args += ['--no-output-directory']
if use_apk: if use_apk:
args += ['-f', _TEST_APK_PATH] args += ['-f', _TEST_APK_PATH]
elif use_minimal_apks: elif use_minimal_apks:
......
...@@ -78,23 +78,24 @@ class OutputDirectoryFinder(_PathFinder): ...@@ -78,23 +78,24 @@ class OutputDirectoryFinder(_PathFinder):
def Verify(self): def Verify(self):
if not self._value or not os.path.isdir(self._value): if not self._value or not os.path.isdir(self._value):
raise Exception('Bad --%s. Path not found: %s' % raise Exception(
(self._name, self._value)) 'Invalid --output-directory. Path not found: {}\n'
'Use --no-output-directory to disable features that rely on it.'.
format(self._value))
class ToolPrefixFinder(_PathFinder): class ToolPrefixFinder(_PathFinder):
def __init__(self, value=None, output_directory_finder=None, def __init__(self, value=None, output_directory=None, linker_name=None):
linker_name=None):
super(ToolPrefixFinder, self).__init__( super(ToolPrefixFinder, self).__init__(
name='tool-prefix', value=value) name='tool-prefix', value=value)
self._output_directory_finder = output_directory_finder self._output_directory = output_directory
self._linker_name = linker_name; self._linker_name = linker_name;
def IsLld(self): def IsLld(self):
return self._linker_name.startswith('lld') if self._linker_name else True return self._linker_name.startswith('lld') if self._linker_name else True
def Detect(self): def Detect(self):
output_directory = self._output_directory_finder.Tentative() output_directory = self._output_directory
if output_directory: if output_directory:
ret = None ret = None
if self.IsLld(): if self.IsLld():
......
git_revision=abc123
linker_name=gold linker_name=gold
map_file_name=test.map map_file_name=test.map
tool_prefix=tools/binary_size/libsupersize/testdata/mock_toolchain/ tool_prefix=tools/binary_size/libsupersize/testdata/mock_toolchain/
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment