clusterfuzzlite: Upload builds after bad build check. (#6531)

Fixes: #6525
Depends on: #6530

6 files changed, 44 insertions, 43 deletions

diff --git a/infra/base-images/base-runner/test_all.py b/infra/base-images/base-runner/test_all.py
index e7865ea4e..16dfcbfa9 100755
--- a/infra/base-images/base-runner/test_all.py
+++ b/infra/base-images/base-runner/test_all.py
@@ -20,12 +20,12 @@ import contextlib
 import multiprocessing
 import os
 import re
-import shutil
 import subprocess
 import stat
 import sys
+import tempfile
 
-TMP_FUZZER_DIR = '/tmp/not-out'
+BASE_TMP_FUZZER_DIR = '/tmp/not-out'
 
 EXECUTABLE = stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH
 
@@ -37,14 +37,6 @@ IGNORED_TARGETS = [
 IGNORED_TARGETS_RE = re.compile('^' + r'$|^'.join(IGNORED_TARGETS) + '$')
 
 
-def recreate_directory(directory):
-  """Creates |directory|. If it already exists than deletes it first before
-  creating."""
-  if os.path.exists(directory):
-    shutil.rmtree(directory)
-  os.mkdir(directory)
-
-
 def move_directory_contents(src_directory, dst_directory):
   """Moves contents of |src_directory| to |dst_directory|."""
   # Use mv because mv preserves file permissions. If we don't preserve file
@@ -140,28 +132,28 @@ def has_ignored_targets(out_dir):
 
 @contextlib.contextmanager
 def use_different_out_dir():
-  """Context manager that moves OUT to TMP_FUZZER_DIR. This is useful for
-  catching hardcoding. Note that this sets the environment variable OUT and
-  therefore must be run before multiprocessing.Pool is created. Resets OUT at
-  the end."""
+  """Context manager that moves OUT to subdirectory of BASE_TMP_FUZZER_DIR. This
+  is useful for catching hardcoding. Note that this sets the environment
+  variable OUT and therefore must be run before multiprocessing.Pool is created.
+  Resets OUT at the end."""
   # Use a fake OUT directory to catch path hardcoding that breaks on
   # ClusterFuzz.
-  out = os.getenv('OUT')
-  initial_out = out
-  recreate_directory(TMP_FUZZER_DIR)
-  out = TMP_FUZZER_DIR
-  # Set this so that run_fuzzer which is called by bad_build_check works
-  # properly.
-  os.environ['OUT'] = out
-  # We move the contents of the directory because we can't move the
-  # directory itself because it is a mount.
-  move_directory_contents(initial_out, out)
-  try:
-    yield out
-  finally:
-    move_directory_contents(out, initial_out)
-    shutil.rmtree(out)
-    os.environ['OUT'] = initial_out
+  initial_out = os.getenv('OUT')
+  os.makedirs(BASE_TMP_FUZZER_DIR, exist_ok=True)
+  # Use a random subdirectory of BASE_TMP_FUZZER_DIR to allow running multiple
+  # instances of test_all in parallel (useful for integration testing).
+  with tempfile.TemporaryDirectory(dir=BASE_TMP_FUZZER_DIR) as out:
+    # Set this so that run_fuzzer which is called by bad_build_check works
+    # properly.
+    os.environ['OUT'] = out
+    # We move the contents of the directory because we can't move the
+    # directory itself because it is a mount.
+    move_directory_contents(initial_out, out)
+    try:
+      yield out
+    finally:
+      move_directory_contents(out, initial_out)
+      os.environ['OUT'] = initial_out
 
 
 def test_all_outside_out(allowed_broken_targets_percentage):
diff --git a/infra/cifuzz/build_fuzzers.py b/infra/cifuzz/build_fuzzers.py
index 3d6c2eb13..6b6d78658 100644
--- a/infra/cifuzz/build_fuzzers.py
+++ b/infra/cifuzz/build_fuzzers.py
@@ -112,12 +112,23 @@ class Builder:  # pylint: disable=too-many-instance-attributes
 
     return True
 
+  def check_fuzzer_build(self):
+    """Checks the fuzzer build. Returns True on success or if config specifies
+    to skip check."""
+    if not self.config.bad_build_check:
+      return True
+
+    return check_fuzzer_build(self.config)
+
   def build(self):
     """Builds the image, checkouts the source (if needed), builds the fuzzers
     and then removes the unaffectted fuzzers. Returns True on success."""
     methods = [
-        self.build_image_and_checkout_src, self.build_fuzzers,
-        self.upload_build, self.remove_unaffected_fuzz_targets
+        self.build_image_and_checkout_src,
+        self.build_fuzzers,
+        self.remove_unaffected_fuzz_targets,
+        self.check_fuzzer_build,
+        self.upload_build,
     ]
     for method in methods:
       if not method():
diff --git a/infra/cifuzz/build_fuzzers_entrypoint.py b/infra/cifuzz/build_fuzzers_entrypoint.py
index c128ae937..e8e368f1b 100644
--- a/infra/cifuzz/build_fuzzers_entrypoint.py
+++ b/infra/cifuzz/build_fuzzers_entrypoint.py
@@ -42,14 +42,7 @@ def build_fuzzers_entrypoint():
                   config.commit_sha, config.pr_ref)
     return returncode
 
-  if not config.bad_build_check:
-    # If we've gotten to this point and we don't need to do bad_build_check,
-    # then the build has succeeded.
-    returncode = 0
-  elif build_fuzzers.check_fuzzer_build(config):
-    returncode = 0
-
-  return returncode
+  return 0
 
 
 def main():
diff --git a/infra/cifuzz/build_fuzzers_test.py b/infra/cifuzz/build_fuzzers_test.py
index 4e0004a7f..5c068ac4d 100644
--- a/infra/cifuzz/build_fuzzers_test.py
+++ b/infra/cifuzz/build_fuzzers_test.py
@@ -164,6 +164,9 @@ class BuildFuzzersIntegrationTest(unittest.TestCase):
     self.out_dir = os.path.join(self.workspace, 'build-out')
     test_helpers.patch_environ(self)
 
+    base_runner_path = os.path.join(INFRA_DIR, 'base-images', 'base-runner')
+    os.environ['PATH'] = os.environ['PATH'] + os.pathsep + base_runner_path
+
   def tearDown(self):
     self.temp_dir_obj.cleanup()
 
diff --git a/infra/cifuzz/fuzz_target.py b/infra/cifuzz/fuzz_target.py
index 2a6936b0e..ae92b14c9 100644
--- a/infra/cifuzz/fuzz_target.py
+++ b/infra/cifuzz/fuzz_target.py
@@ -18,11 +18,11 @@ import os
 import shutil
 import stat
 
-import config_utils
-
 import clusterfuzz.environment
 import clusterfuzz.fuzz
 
+import config_utils
+
 logging.basicConfig(
     format='%(asctime)s - %(name)s - %(levelname)s - %(message)s',
     level=logging.DEBUG)
diff --git a/infra/cifuzz/run_fuzzers_test.py b/infra/cifuzz/run_fuzzers_test.py
index 42bf0ea6b..9f5f9e025 100644
--- a/infra/cifuzz/run_fuzzers_test.py
+++ b/infra/cifuzz/run_fuzzers_test.py
@@ -370,7 +370,9 @@ class CoverageReportIntegrationTest(unittest.TestCase):
           commit_sha='0b95fe1039ed7c38fea1f97078316bfc1030c523',
           base_commit='da0746452433dc18bae699e355a9821285d863c8',
           sanitizer=self.SANITIZER,
-          is_github=True)
+          is_github=True,
+          # Needed for test not to fail because of permissions issues.
+          bad_build_check=False)
       self.assertTrue(build_fuzzers.build_fuzzers(build_config))
 
       # TODO(metzman): Get rid of this here and make 'compile' do this.