Merge "Fix packaging outputs commands" into main am: 1f68c0091b am: ab5b1344cf

Original change: https://android-review.googlesource.com/c/platform/build/+/3266868

Change-Id: I090cccc1cae5385ec0ba44a3204fffbe02434314
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

ci/build_test_suites.py

@@ -22,6 +22,7 @@ import os
 import pathlib
 import subprocess
 import sys
+from typing import Callable
 from build_context import BuildContext
 import optimized_targets
 
@@ -68,7 +69,7 @@ class BuildPlanner:
       return BuildPlan(set(self.args.extra_targets), set())
 
     build_targets = set()
-    packaging_commands = []
+    packaging_commands_getters = []
     for target in self.args.extra_targets:
       if self._unused_target_exclusion_enabled(
           target
@@ -84,9 +85,11 @@ class BuildPlanner:
           target, self.build_context, self.args
       )
       build_targets.update(target_optimizer.get_build_targets())
-      packaging_commands.extend(target_optimizer.get_package_outputs_commands())
+      packaging_commands_getters.append(
+          target_optimizer.get_package_outputs_commands
+      )
 
-    return BuildPlan(build_targets, packaging_commands)
+    return BuildPlan(build_targets, packaging_commands_getters)
 
   def _unused_target_exclusion_enabled(self, target: str) -> bool:
     return (
@@ -98,7 +101,7 @@ class BuildPlanner:
 @dataclass(frozen=True)
 class BuildPlan:
   build_targets: set[str]
-  packaging_commands: list[list[str]]
+  packaging_commands_getters: list[Callable[[], list[list[str]]]]
 
 
 def build_test_suites(argv: list[str]) -> int:
@@ -180,9 +183,10 @@ def execute_build_plan(build_plan: BuildPlan):
   except subprocess.CalledProcessError as e:
     raise BuildFailureError(e.returncode) from e
 
-  for packaging_command in build_plan.packaging_commands:
+  for packaging_commands_getter in build_plan.packaging_commands_getters:
     try:
-      run_command(packaging_command)
+      for packaging_command in packaging_commands_getter():
+        run_command(packaging_command)
     except subprocess.CalledProcessError as e:
       raise BuildFailureError(e.returncode) from e
 

ci/build_test_suites_test.py

@@ -276,7 +276,8 @@ class BuildPlannerTest(unittest.TestCase):
 
     build_plan = build_planner.create_build_plan()
 
-    self.assertEqual(len(build_plan.packaging_commands), 0)
+    for packaging_command in self.run_packaging_commands(build_plan):
+      self.assertEqual(len(packaging_command), 0)
 
   def test_build_optimization_on_optimizes_target(self):
     build_targets = {'target_1', 'target_2'}
@@ -306,7 +307,7 @@ class BuildPlannerTest(unittest.TestCase):
 
     build_plan = build_planner.create_build_plan()
 
-    self.assertIn([f'packaging {optimized_target_name}'], build_plan.packaging_commands)
+    self.assertIn(packaging_commands, self.run_packaging_commands(build_plan))
 
   def test_individual_build_optimization_off_doesnt_optimize(self):
     build_targets = {'target_1', 'target_2'}
@@ -328,7 +329,8 @@ class BuildPlannerTest(unittest.TestCase):
 
     build_plan = build_planner.create_build_plan()
 
-    self.assertFalse(build_plan.packaging_commands)
+    for packaging_command in self.run_packaging_commands(build_plan):
+      self.assertEqual(len(packaging_command), 0)
 
   def test_target_output_used_target_built(self):
     build_target = 'test_target'
@@ -485,6 +487,12 @@ class BuildPlannerTest(unittest.TestCase):
         ],
     }
 
+  def run_packaging_commands(self, build_plan: build_test_suites.BuildPlan):
+    return [
+        packaging_command_getter()
+        for packaging_command_getter in build_plan.packaging_commands_getters
+    ]
+
 
 def wait_until(
     condition_function: Callable[[], bool],

ci/optimized_targets.py

@@ -121,13 +121,13 @@ class OptimizedBuildTarget(ABC):
     process_result = subprocess.run(
         args=[
             f'{src_top / self._SOONG_UI_BASH_PATH}',
-            '--dumpvar-mode',
-            '--abs',
-            soong_vars,
+            '--dumpvars-mode',
+            f'--abs-vars={" ".join(soong_vars)}',
         ],
         env=os.environ,
         check=False,
         capture_output=True,
+        text=True,
     )
     if not process_result.returncode == 0:
       logging.error('soong dumpvars command failed! stderr:')
@@ -142,7 +142,7 @@ class OptimizedBuildTarget(ABC):
     try:
       return {
           line.split('=')[0]: line.split('=')[1].strip("'")
-          for line in process_result.stdout.split('\n')
+          for line in process_result.stdout.strip().split('\n')
       }
     except IndexError as e:
       raise RuntimeError(
@@ -214,10 +214,13 @@ class GeneralTestsOptimizer(OptimizedBuildTarget):
   normally built.
   """
 
-  # List of modules that are always required to be in general-tests.zip.
-  _REQUIRED_MODULES = frozenset(
-      ['cts-tradefed', 'vts-tradefed', 'compatibility-host-util']
-  )
+  # List of modules that are built alongside general-tests as dependencies.
+  _REQUIRED_MODULES = frozenset([
+      'cts-tradefed',
+      'vts-tradefed',
+      'compatibility-host-util',
+      'general-tests-shared-libs',
+  ])
 
   def get_build_targets_impl(self) -> set[str]:
     change_info_file_path = os.environ.get('CHANGE_INFO')
@@ -286,6 +289,10 @@ class GeneralTestsOptimizer(OptimizedBuildTarget):
     host_config_files = []
     target_config_files = []
     for module in self.modules_to_build:
+      # The required modules are handled separately, no need to package.
+      if module in self._REQUIRED_MODULES:
+        continue
+
       host_path = host_out_testcases / module
       if os.path.exists(host_path):
         host_paths.append(host_path)
@@ -303,6 +310,7 @@ class GeneralTestsOptimizer(OptimizedBuildTarget):
 
     zip_commands.extend(
         self._get_zip_test_configs_zips_commands(
+            src_top,
             dist_dir,
             host_out,
             product_out,
@@ -311,27 +319,27 @@ class GeneralTestsOptimizer(OptimizedBuildTarget):
         )
     )
 
-    zip_command = self._base_zip_command(
-        host_out, dist_dir, 'general-tests.zip'
-    )
+    zip_command = self._base_zip_command(src_top, dist_dir, 'general-tests.zip')
 
     # Add host testcases.
-    zip_command.extend(
-        self._generate_zip_options_for_items(
-            prefix='host',
-            relative_root=f'{src_top / soong_host_out}',
-            directories=host_paths,
-        )
-    )
+    if host_paths:
+      zip_command.extend(
+          self._generate_zip_options_for_items(
+              prefix='host',
+              relative_root=f'{src_top / soong_host_out}',
+              directories=host_paths,
+          )
+      )
 
     # Add target testcases.
-    zip_command.extend(
-        self._generate_zip_options_for_items(
-            prefix='target',
-            relative_root=f'{src_top / product_out}',
-            directories=target_paths,
-        )
-    )
+    if target_paths:
+      zip_command.extend(
+          self._generate_zip_options_for_items(
+              prefix='target',
+              relative_root=f'{src_top / product_out}',
+              directories=target_paths,
+          )
+      )
 
     # TODO(lucafarsi): Push this logic into a general-tests-minimal build command
     # Add necessary tools. These are also hardcoded in general-tests.mk.
@@ -365,6 +373,7 @@ class GeneralTestsOptimizer(OptimizedBuildTarget):
 
   def _get_zip_test_configs_zips_commands(
       self,
+      src_top: pathlib.Path,
       dist_dir: pathlib.Path,
       host_out: pathlib.Path,
       product_out: pathlib.Path,
@@ -428,7 +437,7 @@ class GeneralTestsOptimizer(OptimizedBuildTarget):
     zip_commands = []
 
     tests_config_zip_command = self._base_zip_command(
-        host_out, dist_dir, 'general-tests_configs.zip'
+        src_top, dist_dir, 'general-tests_configs.zip'
     )
     tests_config_zip_command.extend(
         self._generate_zip_options_for_items(
@@ -442,16 +451,14 @@ class GeneralTestsOptimizer(OptimizedBuildTarget):
         self._generate_zip_options_for_items(
             prefix='target',
             relative_root=str(product_out),
-            list_files=[
-                f"{product_out / 'target_general-tests_list'}"
-            ],
+            list_files=[f"{product_out / 'target_general-tests_list'}"],
         ),
     )
 
     zip_commands.append(tests_config_zip_command)
 
     tests_list_zip_command = self._base_zip_command(
-        host_out, dist_dir, 'general-tests_list.zip'
+        src_top, dist_dir, 'general-tests_list.zip'
     )
     tests_list_zip_command.extend(
         self._generate_zip_options_for_items(

ci/optimized_targets_test.py

@@ -220,18 +220,6 @@ class GeneralTestsOptimizerTest(fake_filesystem_unittest.TestCase):
     ):
       package_commands = optimizer.get_package_outputs_commands()
 
-  @mock.patch('subprocess.run')
-  def test_no_build_outputs_packaging_fails(self, subprocess_run):
-    subprocess_run.return_value = self._get_soong_vars_output()
-    optimizer = self._create_general_tests_optimizer()
-
-    targets = optimizer.get_build_targets()
-
-    with self.assertRaisesRegex(
-        RuntimeError, 'No items specified to be added to zip'
-    ):
-      package_commands = optimizer.get_package_outputs_commands()
-
   def _create_general_tests_optimizer(self, build_context: BuildContext = None):
     if not build_context:
       build_context = self._create_build_context()
@@ -321,7 +309,7 @@ class GeneralTestsOptimizerTest(fake_filesystem_unittest.TestCase):
     """
     for command in commands:
       self.assertEqual(
-          '/tmp/top/host_out/prebuilts/build-tools/linux-x86/bin/soong_zip',
+          '/tmp/top/prebuilts/build-tools/linux-x86/bin/soong_zip',
           command[0],
       )
       self.assertEqual('-d', command[1])