Merge branch 'master' into chore/build-retrieval-improvements

Author: vitorguidi

src/clusterfuzz/_internal/bot/untrusted_runner/build_setup_host.py

@@ -31,25 +31,6 @@ def _clear_env():
   environment.remove_key('FUZZ_TARGET')
 
 
-def _handle_response(build, response):
-  """Handle build setup response."""
-  if not response.result:
-    _clear_env()
-    return False
-
-  _update_env_from_response(response)
-
-  if not environment.get_value('APP_PATH'):
-    fuzzer_directory = environment.get_value('FUZZER_DIR')
-    if fuzzer_directory:
-      build_manager.set_environment_vars([fuzzer_directory])
-
-  environment.set_value('APP_REVISION', build.revision)
-  build.fuzz_targets = list(response.fuzz_targets)
-
-  return True
-
-
 def _update_env_from_response(response):
   """Update environment variables from response."""
   environment.set_value('APP_PATH', response.app_path)
@@ -64,11 +45,37 @@ def _update_env_from_response(response):
 class RemoteRegularBuild(build_manager.RegularBuild):
   """Remote regular build."""
 
+  def __init__(self, *args, **kwargs):
+    super().__init__(*args, **kwargs)
+    self._fuzz_targets = []
+
+  def _handle_response(self, response):
+    """Handle build setup response."""
+    if not response.result:
+      _clear_env()
+      return False
+
+    _update_env_from_response(response)
+
+    if not environment.get_value('APP_PATH'):
+      fuzzer_directory = environment.get_value('FUZZER_DIR')
+      if fuzzer_directory:
+        build_manager.set_environment_vars([fuzzer_directory])
+
+    environment.set_value('APP_REVISION', self.revision)
+    self._fuzz_targets = list(response.fuzz_targets)
+
+    return True
+
   def setup(self):
     request = untrusted_runner_pb2.SetupRegularBuildRequest(  # pylint: disable=no-member
         base_build_dir=self.base_build_dir,
         revision=self.revision,
         build_url=self.build_url,
         fuzz_target=self.fuzz_target,
         build_prefix=self.build_prefix)
-    return _handle_response(self, host.stub().SetupRegularBuild(request))
+    return self._handle_response(host.stub().SetupRegularBuild(request))
+
+  @property
+  def fuzz_targets(self):
+    return self._fuzz_targets

src/clusterfuzz/_internal/build_management/build_manager.py

@@ -341,7 +341,9 @@ def __init__(self,
     self.build_prefix = build_prefix
     self.env_prefix = build_prefix + '_' if build_prefix else ''
     # This is used by users of the class to learn the fuzz targets in the build.
-    self.fuzz_targets = None
+    self._fuzz_targets = None
+    self._unpack_everything = environment.get_value(
+        'UNPACK_ALL_FUZZ_TARGETS_AND_FILES', default_value=False)
     # This is used by users of the class to instruct the class which fuzz
     # target to unpack.
     self.fuzz_target = fuzz_target
@@ -461,15 +463,12 @@ def _download_and_open_build_archive(self, base_build_dir: str,
       shell.remove_file(build_local_archive)
 
   def _open_build_archive(self, base_build_dir: str, build_dir: str,
-                          build_url: str, http_build_url: Optional[str],
-                          unpack_everything: Optional[bool]):
+                          build_url: str, http_build_url: Optional[str]):
     """Gets a handle on a build archive for the current build. Depending on the
     provided parameters, this function might download the build archive into
     the build directory or directly use remote HTTP archive.
 
     Args:
-        unpack_everything: wether we should unpack the whole archive or try
-        selective unpacking.
         base_build_dir: the base build directory.
         build_dir: the current build directory.
         build_url: the build URL.
@@ -487,7 +486,8 @@ def _open_build_archive(self, base_build_dir: str, build_dir: str,
     allow_unpack_over_http = environment.get_value(
         'ALLOW_UNPACK_OVER_HTTP', default_value=False)
     can_unzip_over_http = (
-        allow_unpack_over_http and not unpack_everything and http_build_url and
+        allow_unpack_over_http and not self._unpack_everything and
+        http_build_url and
         build_archive.unzip_over_http_compatible(http_build_url))
 
     if not can_unzip_over_http:
@@ -507,9 +507,6 @@ def _unpack_build(self,
     # Track time taken to unpack builds so that it doesn't silently regress.
     start_time = time.time()
 
-    unpack_everything = environment.get_value(
-        'UNPACK_ALL_FUZZ_TARGETS_AND_FILES')
-
     logs.info(f'Unpacking build from {build_url} into {build_dir}.')
 
     # Free up memory.
@@ -524,17 +521,13 @@ def _unpack_build(self,
 
     try:
       with self._open_build_archive(base_build_dir, build_dir, build_url,
-                                    http_build_url, unpack_everything) as build:
+                                    http_build_url) as build:
         unpack_start_time = time.time()
-        unpack_everything = environment.get_value(
-            'UNPACK_ALL_FUZZ_TARGETS_AND_FILES')
-
-        if not unpack_everything:
+        if not self._unpack_everything:
           # We will never unpack the full build so we need to get the targets
           # from the build archive.
           list_fuzz_target_start_time = time.time()
-          self.fuzz_targets = list(build.list_fuzz_targets())
-          # In minutes, as per metric definition.
+          self._fuzz_targets = list(build.list_fuzz_targets())
           _emit_job_build_retrieval_metric(list_fuzz_target_start_time,
                                            'list_fuzz_targets',
                                            self._build_type)
@@ -569,12 +562,7 @@ def _unpack_build(self,
       logs.error(f'Unable to unpack build archive {build_url}: {e}')
       return False
 
-    if unpack_everything:
-      list_fuzz_target_start_time = time.time()
-      self.fuzz_targets = list(self._get_fuzz_targets_from_dir(build_dir))
-      _emit_job_build_retrieval_metric(list_fuzz_target_start_time,
-                                       'list_fuzz_targets', self._build_type)
-    else:
+    if not self._unpack_everything:
       # If this is partial build due to selected build files, then mark it as
       # such so that it is not re-used.
       partial_build_file_path = os.path.join(build_dir, PARTIAL_BUILD_FILE)
@@ -607,6 +595,14 @@ def build_dir(self):
     """The build directory. Usually a subdirectory of base_build_dir."""
     raise NotImplementedError
 
+  @property
+  def fuzz_targets(self):
+    if not self._fuzz_targets and self._unpack_everything:
+      # we can lazily compute that when unpacking the whole archive, since we
+      # know all the fuzzers will be in the build directory.
+      self._fuzz_targets = list(self._get_fuzz_targets_from_dir(self.build_dir))
+    return self._fuzz_targets
+
   def exists(self):
     """Check if build already exists."""
     revision_file = os.path.join(self.build_dir, REVISION_FILE_NAME)
@@ -747,8 +743,8 @@ def setup(self):
       # This list will be incomplete because the directory on disk does not have
       # all fuzz targets. This is fine. The way fuzz_targets are added to db, it
       # does not clobber complete lists.
-      assert self.fuzz_targets is None
-      self.fuzz_targets = list(self._get_fuzz_targets_from_dir(self.build_dir))
+      assert self._fuzz_targets is None
+      self._fuzz_targets = list(self._get_fuzz_targets_from_dir(self.build_dir))
 
     self._setup_application_path(build_update=build_update)
     self._post_setup_success(update_revision=build_update)
@@ -760,7 +756,7 @@ class SplitTargetBuild(RegularBuild):
 
   def setup(self, *args, **kwargs):
     result = super().setup(*args, **kwargs)
-    self.fuzz_targets = list(_split_target_build_list_targets())
+    self._fuzz_targets = list(_split_target_build_list_targets())
     return result
 
 
@@ -799,7 +795,7 @@ def setup(self):
 
     # Select a fuzzer, now that a list is available
     fuzz_targets = fuchsia.undercoat.list_fuzzers(handle)
-    self.fuzz_targets = list(fuzz_targets)
+    self._fuzz_targets = list(fuzz_targets)
     return True
 
 
@@ -968,7 +964,7 @@ def setup(self):
     else:
       logs.info('Build already exists.')
 
-    self.fuzz_targets = list(self._get_fuzz_targets_from_dir(self.build_dir))
+    self._fuzz_targets = list(self._get_fuzz_targets_from_dir(self.build_dir))
     self._setup_application_path(build_update=build_update)
     self._post_setup_success(update_revision=build_update)
     return True