Fix packaging outputs commands

There were a few issues with the output packaging process that were
found during testing of the general-tests optimization.

First and foremost is that the packaging commands were trying to be
created before the build ran, when the outputs don't exist. I've changed
the logic to just collect the methods themselves which will then be run
during build plan execution after the build has completed.

A few other smaller issues include fixing the path to the soong_zip
binary, incorrect execution of the soong dumpvars command, and not
building the shared libs zip.

Test: atest build_test_suites_test; atest optimized_targets_test
Bug: 358215235
Change-Id: I8a3f54738f8bb5d871aadf7423844076c38b54a6

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,8 +183,9 @@ 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:
+      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',
+            '--dumpvars-mode',
-            '--abs',
+            f'--abs-vars={" ".join(soong_vars)}',
-            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.
+  # List of modules that are built alongside general-tests as dependencies.
-  _REQUIRED_MODULES = frozenset(
+  _REQUIRED_MODULES = frozenset([
-      ['cts-tradefed', 'vts-tradefed', 'compatibility-host-util']
+      '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,11 +319,10 @@ class GeneralTestsOptimizer(OptimizedBuildTarget):
         )
     )
 
-    zip_command = self._base_zip_command(
+    zip_command = self._base_zip_command(src_top, dist_dir, 'general-tests.zip')
-        host_out, dist_dir, 'general-tests.zip'
-    )
 
     # Add host testcases.
+    if host_paths:
       zip_command.extend(
           self._generate_zip_options_for_items(
               prefix='host',
@@ -325,6 +332,7 @@ class GeneralTestsOptimizer(OptimizedBuildTarget):
       )
 
     # Add target testcases.
+    if target_paths:
       zip_command.extend(
           self._generate_zip_options_for_items(
               prefix='target',
@@ -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=[
+            list_files=[f"{product_out / 'target_general-tests_list'}"],
-                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])