From 64ea4e87667ea78e5d5fe2a637d1e52af36ca875 Mon Sep 17 00:00:00 2001 From: Victor Liu <67168434+notvictorl@users.noreply.github.com> Date: Thu, 11 Jun 2026 14:26:11 -0400 Subject: [PATCH 1/5] Skip RPATH patching on Schema v1 builds --- .../build_management/build_archive.py | 11 +- .../build_management/build_manager.py | 76 ++++++- .../build_management/build_manager_test.py | 208 ++++++++++++++++++ 3 files changed, 292 insertions(+), 3 deletions(-) diff --git a/src/clusterfuzz/_internal/build_management/build_archive.py b/src/clusterfuzz/_internal/build_management/build_archive.py index c25fa8fae0..374bdff582 100644 --- a/src/clusterfuzz/_internal/build_management/build_archive.py +++ b/src/clusterfuzz/_internal/build_management/build_archive.py @@ -66,6 +66,11 @@ def __init__(self, reader: archive.ArchiveReader): self._reader = reader self._fuzz_targets = None + @abc.abstractmethod + def archive_schema_version(self) -> int: + """Returns the schema version number for this archive.""" + raise NotImplementedError + def list_members(self) -> List[archive.ArchiveMemberInfo]: return self._reader.list_members() @@ -162,6 +167,10 @@ class DefaultBuildArchive(BuildArchive): """Default class for handling builds. This should work with everything. """ + @override + def archive_schema_version(self) -> int: + return 0 + def get_path_for_target(self, fuzz_target: str) -> str: """Returns the path in the archive of the fuzz_target if found. This is needed because target name normalization means we're losing @@ -344,8 +353,8 @@ def root_dir(self) -> str: self._root_dir = super().root_dir() # pylint: disable=attribute-defined-outside-init return self._root_dir + @override def archive_schema_version(self) -> int: - """Returns the schema version number for this archive.""" return self._archive_schema_version @override diff --git a/src/clusterfuzz/_internal/build_management/build_manager.py b/src/clusterfuzz/_internal/build_management/build_manager.py index 02b84e4d18..6afe4b6d8b 100644 --- a/src/clusterfuzz/_internal/build_management/build_manager.py +++ b/src/clusterfuzz/_internal/build_management/build_manager.py @@ -51,6 +51,11 @@ # File name for storing current build revision. REVISION_FILE_NAME = 'REVISION' +# File name for storing the archive schema version retrieved during unpacking. +# The version is derived from clusterfuzz_manifest.json if it exists in the +# archive, otherwise it defaults to 0. +SCHEMA_VERSION_FILE_NAME = '.schema_version' + # Various build type mapping strings. BUILD_TYPE_SUBSTRINGS = [ '-beta', '-stable', '-debug', '-release', '-symbolized', '-extended_stable' @@ -350,6 +355,7 @@ def __init__(self, # Every fetched build is a release one, except when SymbolizedBuild # explicitly downloads a debug build self._build_type = 'release' + self._schema_version = None def _reset_cwd(self): """Reset current working directory. Needed to clean up build @@ -378,6 +384,48 @@ def _pre_setup(self): environment.set_value(self.env_prefix + 'APP_PATH', '') environment.set_value(self.env_prefix + 'APP_PATH_DEBUG', '') + def _schema_version_path(self): + """Returns the path to the schema version metadata file in `build_dir`.""" + return os.path.join(self.build_dir, SCHEMA_VERSION_FILE_NAME) + + def _read_schema_version_from_disk(self) -> int | None: + """Reads the schema version from `SCHEMA_VERSION_FILE_NAME` in `build_dir`. + + Returns None if the file is missing or cannot be read, so callers can + distinguish "no version recorded yet" from a recorded version of 0. + """ + schema_version_path = self._schema_version_path() + if not os.path.exists(schema_version_path): + return None + try: + return int(utils.read_data_from_file(schema_version_path) or 0) + except Exception as e: + logs.warning( + f'Failed to read schema version from {schema_version_path}: {e}') + return None + + def _write_schema_version(self, schema_version): + """Writes schema version to `SCHEMA_VERSION_FILE_NAME` in `build_dir`.""" + schema_version_path = self._schema_version_path() + try: + utils.write_data_to_file(str(schema_version), schema_version_path) + except Exception as e: + logs.warning( + f'Failed to write schema version to {schema_version_path}: {e}') + + def _get_schema_version(self): + """Gets the schema version of the build, preferring the in-memory value and + falling back to `SCHEMA_VERSION_FILE_NAME` in `build_dir`. Defaults to 0 + when no version has been recorded. + + Note: For `SymbolizedBuild`, this naively looks for `.schema_version` at + `build_dir` and assumes both release and debug builds share the same schema + version. If they are different, the release build's schema version is used. + """ + if self._schema_version is None: + self._schema_version = self._read_schema_version_from_disk() + return self._schema_version or 0 + def _patch_rpath(self, binary_path, instrumented_library_paths): """Patch rpaths of a binary to point to instrumented libraries""" rpaths = get_rpaths(binary_path) @@ -419,8 +467,13 @@ def _post_setup_success(self, update_revision=True): # Update rpaths if necessary (for e.g. instrumented libraries). instrumented_library_paths = environment.get_instrumented_libraries_paths() - if instrumented_library_paths: - self._patch_rpaths(instrumented_library_paths) + if not instrumented_library_paths: + return + if self._get_schema_version() > 0: + logs.info('Skipping RPATH patching for schema v1+ build.') + return + + self._patch_rpaths(instrumented_library_paths) @contextlib.contextmanager def _download_and_open_build_archive(self, base_build_dir: str, @@ -563,6 +616,25 @@ def _unpack_build(self, fuzz_target=fuzz_target_to_unpack, trusted=trusted) + schema_version = build.archive_schema_version() + + # Sync in-memory schema version with disk if we haven't read it yet. + # This helps detect mismatches in shared build dirs (e.g., + # SymbolizedBuild). + if self._schema_version is None: + self._schema_version = self._read_schema_version_from_disk() + + if self._schema_version is None: + # First time unpacking: record the schema version. + self._write_schema_version(schema_version) + self._schema_version = schema_version + elif self._schema_version != schema_version: + # Following unpack: flag if the new archive version doesn't match. + logs.error( + f'Schema version mismatch for build in {self.build_dir}. ' + f'Existing version is {self._schema_version}, but new archive ' + f'{build_url} has version {schema_version}.') + _emit_job_build_retrieval_metric(unpack_start_time, 'unpack', self._build_type) diff --git a/src/clusterfuzz/_internal/tests/core/build_management/build_manager_test.py b/src/clusterfuzz/_internal/tests/core/build_management/build_manager_test.py index d1c3cc868e..ad91c07eae 100644 --- a/src/clusterfuzz/_internal/tests/core/build_management/build_manager_test.py +++ b/src/clusterfuzz/_internal/tests/core/build_management/build_manager_test.py @@ -1580,6 +1580,33 @@ def test_patch_rpaths_existing_msan(self): rpaths = build_manager.get_rpaths(os.environ['APP_PATH']) self.assertListEqual(['/msan/lib', '/msan/usr/lib'], rpaths) + def test_patch_rpaths_schema_v1(self): + """Tests that RPATH is not patched when schema version is > 0.""" + + def mock_unpack_build_schema_v1(actual_self, + base_build_dir, + build_dir, + url, + http_build_url=None): + self.mock_unpack_build('rpath_new', actual_self, base_build_dir, + build_dir, url, http_build_url) + schema_version_path = os.path.join(build_dir, + build_manager.SCHEMA_VERSION_FILE_NAME) + utils.write_data_to_file('1', schema_version_path) + return True + + self.mock._unpack_build.side_effect = mock_unpack_build_schema_v1 + + build = build_manager.RegularBuild(self.base_build_dir, 1337, + 'chained-origins') + self.assertTrue(build.setup()) + self.assertEqual( + os.path.join(self.base_build_dir, 'revisions', 'app'), + os.environ['APP_PATH']) + + rpaths = build_manager.get_rpaths(os.environ['APP_PATH']) + self.assertListEqual([], rpaths) + class SortBuildUrlsByRevisionTest(unittest.TestCase): """Test _sort_build_urls_by_revision.""" @@ -1969,3 +1996,184 @@ def test_is_discovery_non_fuzzing(self): self.mock.is_engine_fuzzer_job.return_value = False build = build_manager.Build('/base', 1, build_prefix='', fuzz_target=None) self.assertFalse(build.is_discovery) + + +class GetSchemaVersionTest(fake_filesystem_unittest.TestCase): + """Tests _get_schema_version resolution.""" + + def setUp(self): + fake_filesystem_unittest.TestCase.setUp(self) + self.setUpPyfakefs() + self.base_build_dir = '/base_build_dir' + self.fs.create_dir(self.base_build_dir) + + def test_regular_build(self): + """Tests reading schema version for regular build.""" + build = build_manager.RegularBuild(self.base_build_dir, 1337, + 'gs://dummy/url.zip') + self.fs.create_file( + os.path.join(build.build_dir, '.schema_version'), contents='1') + self.assertEqual(build._get_schema_version(), 1) + + def test_symbolized_build(self): + """Tests reading schema version for symbolized build.""" + build = build_manager.SymbolizedBuild(self.base_build_dir, 1337, + 'gs://dummy/release.zip', + 'gs://dummy/debug.zip') + self.fs.create_file( + os.path.join(build.build_dir, '.schema_version'), contents='2') + self.assertEqual(build._get_schema_version(), 2) + + def test_missing_file(self): + """Tests that 0 is returned if file is missing.""" + build = build_manager.RegularBuild(self.base_build_dir, 1337, + 'gs://dummy/url.zip') + self.assertEqual(build._get_schema_version(), 0) + + def test_get_schema_version_caching(self): + """Tests that _get_schema_version caches results.""" + build = build_manager.RegularBuild(self.base_build_dir, 1337, + 'gs://dummy/url.zip') + self.fs.create_file( + os.path.join(build.build_dir, '.schema_version'), contents='1') + + with mock.patch( + 'clusterfuzz._internal.base.utils.read_data_from_file', + return_value='1') as mock_read_call: + # First call should trigger read. + v1 = build._get_schema_version() + + self.assertEqual(v1, 1) + + # Second call should use cache. + v2 = build._get_schema_version() + + self.assertEqual(v2, 1) + self.assertEqual(mock_read_call.call_count, 1) + + # Clear call history + mock_read_call.reset_mock() + + # Call again, should still be cached. + v3 = build._get_schema_version() + + self.assertEqual(v3, 1) + self.assertEqual(mock_read_call.call_count, 0) + + +class UnpackBuildSchemaVersionTest(fake_filesystem_unittest.TestCase): + """Tests schema version handling during _unpack_build.""" + + def setUp(self): + test_utils.set_up_pyfakefs(self) + test_helpers.patch(self, [ + 'clusterfuzz._internal.build_management.build_manager._make_space', + 'clusterfuzz._internal.system.shell.clear_temp_directory', + 'clusterfuzz._internal.build_management.build_manager.Build._open_build_archive', + ]) + self.mock._make_space.return_value = True + + test_helpers.patch_environ(self) + os.environ['BUILDS_DIR'] = '/builds' + os.environ['FAIL_RETRIES'] = '1' + os.environ['APP_NAME'] = FAKE_APP_NAME + os.environ['JOB_NAME'] = 'job' + + self.base_build_dir = '/base_build_dir' + self.fs.create_dir(self.base_build_dir) + + self.build = build_manager.RegularBuild(self.base_build_dir, 1337, + 'gs://dummy/url.zip') + # build_dir for RegularBuild is base_build_dir/revisions + self.fs.create_dir(self.build.build_dir) + + def mock_archive(self, schema_version): + archive_mock = mock.MagicMock() + archive_mock.unpacked_size.return_value = 100 + archive_mock.archive_schema_version.return_value = schema_version + archive_mock.list_fuzz_targets.return_value = [] + return archive_mock + + def test_first_unpack_writes_version(self): + """Tests that the first unpack writes the schema version.""" + archive = self.mock_archive(1) + self.mock._open_build_archive.return_value.__enter__.return_value = archive + schema_file = os.path.join(self.build.build_dir, '.schema_version') + + result = self.build._unpack_build(self.base_build_dir, self.build.build_dir, + 'gs://dummy/url.zip') + + self.assertTrue(result) + self.assertTrue(os.path.exists(schema_file)) + self.assertEqual(utils.read_data_from_file(schema_file), 1) + self.assertEqual(self.build._schema_version, 1) + + def test_second_unpack_same_version(self): + """Tests that subsequent unpack with same version does not overwrite and doesn't log error.""" + schema_file = os.path.join(self.build.build_dir, '.schema_version') + self.fs.create_file(schema_file, contents='1') + self.build._schema_version = 1 + + archive = self.mock_archive(1) + self.mock._open_build_archive.return_value.__enter__.return_value = archive + + with mock.patch.object(self.build, '_write_schema_version') as mock_write: + with mock.patch( + 'clusterfuzz._internal.metrics.logs.error') as mock_log_error: + result = self.build._unpack_build( + self.base_build_dir, self.build.build_dir, 'gs://dummy/url.zip') + + self.assertTrue(result) + mock_write.assert_not_called() + mock_log_error.assert_not_called() + + def test_second_unpack_different_version_logs_error(self): + """Tests that subsequent unpack with different version logs error and does not overwrite.""" + schema_file = os.path.join(self.build.build_dir, '.schema_version') + self.fs.create_file(schema_file, contents='1') + self.build._schema_version = 1 + + archive = self.mock_archive(2) # Second archive has version 2 + self.mock._open_build_archive.return_value.__enter__.return_value = archive + + with mock.patch.object(self.build, '_write_schema_version') as mock_write: + with mock.patch( + 'clusterfuzz._internal.metrics.logs.error') as mock_log_error: + result = self.build._unpack_build( + self.base_build_dir, self.build.build_dir, 'gs://dummy/url.zip') + + self.assertTrue(result) + mock_write.assert_not_called() + mock_log_error.assert_called_once() + error_msg = mock_log_error.call_args[0][0] + self.assertIn('Schema version mismatch', error_msg) + self.assertIn('1', error_msg) # Existing + self.assertIn('2', error_msg) # New + + def test_unpack_reads_from_disk_if_not_in_memory(self): + """Tests that unpack reads from disk if _schema_version is not in memory.""" + build = build_manager.SymbolizedBuild(self.base_build_dir, 1337, + 'gs://dummy/release.zip', + 'gs://dummy/debug.zip') + self.fs.create_dir(build.build_dir) + self.fs.create_dir(build.release_build_dir) + self.fs.create_dir(build.debug_build_dir) + + schema_file = os.path.join(build.build_dir, '.schema_version') + self.fs.create_file(schema_file, contents='1') + build._schema_version = None # Not in memory + + archive = self.mock_archive(1) + self.mock._open_build_archive.return_value.__enter__.return_value = archive + + with mock.patch.object(build, '_write_schema_version') as mock_write: + with mock.patch( + 'clusterfuzz._internal.metrics.logs.error') as mock_log_error: + # Unpack debug build (simulating it running after release build, but with clean memory) + result = build._unpack_build(self.base_build_dir, build.debug_build_dir, + 'gs://dummy/debug.zip') + + self.assertTrue(result) + mock_write.assert_not_called() + self.assertEqual(build._schema_version, 1) # Should be synced to memory + mock_log_error.assert_not_called() From 4bb44a154d30f291fd08f396646feae522a63cb3 Mon Sep 17 00:00:00 2001 From: Victor Liu <67168434+notvictorl@users.noreply.github.com> Date: Fri, 26 Jun 2026 15:18:37 -0400 Subject: [PATCH 2/5] Remove .schema_version and always extract clusterfuzz_manifest.json --- .../build_management/build_archive.py | 2 + .../build_management/build_manager.py | 137 ++++++------- .../build_management/build_manager_test.py | 183 +++--------------- 3 files changed, 79 insertions(+), 243 deletions(-) diff --git a/src/clusterfuzz/_internal/build_management/build_archive.py b/src/clusterfuzz/_internal/build_management/build_archive.py index 374bdff582..3c7b59c7ab 100644 --- a/src/clusterfuzz/_internal/build_management/build_archive.py +++ b/src/clusterfuzz/_internal/build_management/build_archive.py @@ -40,6 +40,7 @@ 'afl-showmap', 'afl-tmin', 'centipede', + 'clusterfuzz_manifest.json', 'honggfuzz', 'jazzer_agent_deploy.jar', 'jazzer_driver', @@ -430,6 +431,7 @@ def _get_common_files(self) -> List[str]: return [ 'args.gn', 'llvm-symbolizer', + 'clusterfuzz_manifest.json', ] def get_target_dependencies( diff --git a/src/clusterfuzz/_internal/build_management/build_manager.py b/src/clusterfuzz/_internal/build_management/build_manager.py index 6afe4b6d8b..075c16b1f1 100644 --- a/src/clusterfuzz/_internal/build_management/build_manager.py +++ b/src/clusterfuzz/_internal/build_management/build_manager.py @@ -16,6 +16,7 @@ from collections import namedtuple import contextlib import datetime +import json import os import re import shutil @@ -51,11 +52,6 @@ # File name for storing current build revision. REVISION_FILE_NAME = 'REVISION' -# File name for storing the archive schema version retrieved during unpacking. -# The version is derived from clusterfuzz_manifest.json if it exists in the -# archive, otherwise it defaults to 0. -SCHEMA_VERSION_FILE_NAME = '.schema_version' - # Various build type mapping strings. BUILD_TYPE_SUBSTRINGS = [ '-beta', '-stable', '-debug', '-release', '-symbolized', '-extended_stable' @@ -384,47 +380,18 @@ def _pre_setup(self): environment.set_value(self.env_prefix + 'APP_PATH', '') environment.set_value(self.env_prefix + 'APP_PATH_DEBUG', '') - def _schema_version_path(self): - """Returns the path to the schema version metadata file in `build_dir`.""" - return os.path.join(self.build_dir, SCHEMA_VERSION_FILE_NAME) - - def _read_schema_version_from_disk(self) -> int | None: - """Reads the schema version from `SCHEMA_VERSION_FILE_NAME` in `build_dir`. - - Returns None if the file is missing or cannot be read, so callers can - distinguish "no version recorded yet" from a recorded version of 0. - """ - schema_version_path = self._schema_version_path() - if not os.path.exists(schema_version_path): - return None - try: - return int(utils.read_data_from_file(schema_version_path) or 0) - except Exception as e: - logs.warning( - f'Failed to read schema version from {schema_version_path}: {e}') - return None - - def _write_schema_version(self, schema_version): - """Writes schema version to `SCHEMA_VERSION_FILE_NAME` in `build_dir`.""" - schema_version_path = self._schema_version_path() + def _read_schema_version_from_manifest(self, build_dir: str) -> int: + """Reads archive_schema_version from clusterfuzz_manifest.json.""" + manifest_path = os.path.join(build_dir, 'clusterfuzz_manifest.json') + if not os.path.exists(manifest_path): + return 0 try: - utils.write_data_to_file(str(schema_version), schema_version_path) + with open(manifest_path) as f: + manifest = json.load(f) + return int(manifest.get('archive_schema_version') or 0) except Exception as e: - logs.warning( - f'Failed to write schema version to {schema_version_path}: {e}') - - def _get_schema_version(self): - """Gets the schema version of the build, preferring the in-memory value and - falling back to `SCHEMA_VERSION_FILE_NAME` in `build_dir`. Defaults to 0 - when no version has been recorded. - - Note: For `SymbolizedBuild`, this naively looks for `.schema_version` at - `build_dir` and assumes both release and debug builds share the same schema - version. If they are different, the release build's schema version is used. - """ - if self._schema_version is None: - self._schema_version = self._read_schema_version_from_disk() - return self._schema_version or 0 + logs.warning(f'Failed to read schema version from {manifest_path}: {e}') + return 0 def _patch_rpath(self, binary_path, instrumented_library_paths): """Patch rpaths of a binary to point to instrumented libraries""" @@ -438,22 +405,38 @@ def _patch_rpath(self, binary_path, instrumented_library_paths): set_rpaths(binary_path, rpaths) - def _patch_rpaths(self, instrumented_library_paths): + def _patch_rpaths(self, + schema_version: int, + build_dir: Optional[str] = None, + app_path_env: Optional[str] = None): """Patch rpaths of builds to point to instrumented libraries.""" + instrumented_library_paths = environment.get_instrumented_libraries_paths() + if not instrumented_library_paths: + return + if schema_version > 0: + logs.info('Skipping RPATH patching for schema v1+ build.') + return + + if not build_dir: + build_dir = self.build_dir + if environment.is_engine_fuzzer_job(): # Import here as this path is not available in App Engine context. from clusterfuzz._internal.bot.fuzzers import utils as fuzzer_utils - for target_path in fuzzer_utils.get_fuzz_targets(self.build_dir): + for target_path in fuzzer_utils.get_fuzz_targets(build_dir): self._patch_rpath(target_path, instrumented_library_paths) else: - app_path = environment.get_value('APP_PATH') - if app_path: - self._patch_rpath(app_path, instrumented_library_paths) - - app_path_debug = environment.get_value('APP_PATH_DEBUG') - if app_path_debug: - self._patch_rpath(app_path_debug, instrumented_library_paths) + if app_path_env: + app_paths = [environment.get_value(app_path_env)] + else: + app_paths = [ + environment.get_value('APP_PATH'), + environment.get_value('APP_PATH_DEBUG') + ] + for app_path in app_paths: + if app_path: + self._patch_rpath(app_path, instrumented_library_paths) def _post_setup_success(self, update_revision=True): """Common post-setup.""" @@ -465,16 +448,6 @@ def _post_setup_success(self, update_revision=True): timestamp_file_path = os.path.join(self.base_build_dir, TIMESTAMP_FILE) utils.write_data_to_file(time.time(), timestamp_file_path) - # Update rpaths if necessary (for e.g. instrumented libraries). - instrumented_library_paths = environment.get_instrumented_libraries_paths() - if not instrumented_library_paths: - return - if self._get_schema_version() > 0: - logs.info('Skipping RPATH patching for schema v1+ build.') - return - - self._patch_rpaths(instrumented_library_paths) - @contextlib.contextmanager def _download_and_open_build_archive(self, base_build_dir: str, build_dir: str, build_url: str): @@ -616,25 +589,6 @@ def _unpack_build(self, fuzz_target=fuzz_target_to_unpack, trusted=trusted) - schema_version = build.archive_schema_version() - - # Sync in-memory schema version with disk if we haven't read it yet. - # This helps detect mismatches in shared build dirs (e.g., - # SymbolizedBuild). - if self._schema_version is None: - self._schema_version = self._read_schema_version_from_disk() - - if self._schema_version is None: - # First time unpacking: record the schema version. - self._write_schema_version(schema_version) - self._schema_version = schema_version - elif self._schema_version != schema_version: - # Following unpack: flag if the new archive version doesn't match. - logs.error( - f'Schema version mismatch for build in {self.build_dir}. ' - f'Existing version is {self._schema_version}, but new archive ' - f'{build_url} has version {schema_version}.') - _emit_job_build_retrieval_metric(unpack_start_time, 'unpack', self._build_type) @@ -841,6 +795,8 @@ def setup(self): self._setup_application_path(build_update=build_update) self._post_setup_success(update_revision=build_update) + schema_version = self._read_schema_version_from_manifest(self.build_dir) + self._patch_rpaths(schema_version) return True @@ -947,16 +903,31 @@ def setup(self): else: logs.info('Build already exists.') + release_schema_version = ( + self._read_schema_version_from_manifest(self.release_build_dir) + if self.release_build_url else 0) + debug_schema_version = ( + self._read_schema_version_from_manifest(self.debug_build_dir) + if self.debug_build_url else 0) + if self.release_build_url: self._setup_application_path( self.release_build_dir, build_update=build_update) environment.set_value('BUILD_URL', self.release_build_url) + self._patch_rpaths( + release_schema_version, + build_dir=self.release_build_dir, + app_path_env='APP_PATH') if self.debug_build_url: # Note: this will override LLVM_SYMBOLIZER_PATH, APP_DIR etc from the # previous release setup, which may not be desirable behaviour. self._setup_application_path( self.debug_build_dir, 'APP_PATH_DEBUG', build_update=build_update) + self._patch_rpaths( + debug_schema_version, + build_dir=self.debug_build_dir, + app_path_env='APP_PATH_DEBUG') self._post_setup_success(update_revision=build_update) return True @@ -1061,6 +1032,8 @@ def setup(self): 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) + schema_version = self._read_schema_version_from_manifest(self.build_dir) + self._patch_rpaths(schema_version) return True diff --git a/src/clusterfuzz/_internal/tests/core/build_management/build_manager_test.py b/src/clusterfuzz/_internal/tests/core/build_management/build_manager_test.py index ad91c07eae..7482461077 100644 --- a/src/clusterfuzz/_internal/tests/core/build_management/build_manager_test.py +++ b/src/clusterfuzz/_internal/tests/core/build_management/build_manager_test.py @@ -1590,9 +1590,8 @@ def mock_unpack_build_schema_v1(actual_self, http_build_url=None): self.mock_unpack_build('rpath_new', actual_self, base_build_dir, build_dir, url, http_build_url) - schema_version_path = os.path.join(build_dir, - build_manager.SCHEMA_VERSION_FILE_NAME) - utils.write_data_to_file('1', schema_version_path) + manifest_path = os.path.join(build_dir, 'clusterfuzz_manifest.json') + utils.write_data_to_file('{"archive_schema_version": 1}', manifest_path) return True self.mock._unpack_build.side_effect = mock_unpack_build_schema_v1 @@ -1998,8 +1997,8 @@ def test_is_discovery_non_fuzzing(self): self.assertFalse(build.is_discovery) -class GetSchemaVersionTest(fake_filesystem_unittest.TestCase): - """Tests _get_schema_version resolution.""" +class ReadSchemaVersionFromManifestTest(fake_filesystem_unittest.TestCase): + """Tests _read_schema_version_from_manifest resolution.""" def setUp(self): fake_filesystem_unittest.TestCase.setUp(self) @@ -2008,172 +2007,34 @@ def setUp(self): self.fs.create_dir(self.base_build_dir) def test_regular_build(self): - """Tests reading schema version for regular build.""" + """Tests reading schema version from manifest for regular build.""" build = build_manager.RegularBuild(self.base_build_dir, 1337, 'gs://dummy/url.zip') self.fs.create_file( - os.path.join(build.build_dir, '.schema_version'), contents='1') - self.assertEqual(build._get_schema_version(), 1) + os.path.join(build.build_dir, 'clusterfuzz_manifest.json'), + contents='{"archive_schema_version": 1}') + self.assertEqual( + build._read_schema_version_from_manifest(build.build_dir), 1) def test_symbolized_build(self): - """Tests reading schema version for symbolized build.""" + """Tests reading schema version from manifest for release and debug dirs.""" build = build_manager.SymbolizedBuild(self.base_build_dir, 1337, 'gs://dummy/release.zip', 'gs://dummy/debug.zip') self.fs.create_file( - os.path.join(build.build_dir, '.schema_version'), contents='2') - self.assertEqual(build._get_schema_version(), 2) + os.path.join(build.release_build_dir, 'clusterfuzz_manifest.json'), + contents='{"archive_schema_version": 1}') + self.fs.create_file( + os.path.join(build.debug_build_dir, 'clusterfuzz_manifest.json'), + contents='{"archive_schema_version": 2}') + self.assertEqual( + build._read_schema_version_from_manifest(build.release_build_dir), 1) + self.assertEqual( + build._read_schema_version_from_manifest(build.debug_build_dir), 2) def test_missing_file(self): - """Tests that 0 is returned if file is missing.""" - build = build_manager.RegularBuild(self.base_build_dir, 1337, - 'gs://dummy/url.zip') - self.assertEqual(build._get_schema_version(), 0) - - def test_get_schema_version_caching(self): - """Tests that _get_schema_version caches results.""" + """Tests that 0 is returned if manifest file is missing.""" build = build_manager.RegularBuild(self.base_build_dir, 1337, 'gs://dummy/url.zip') - self.fs.create_file( - os.path.join(build.build_dir, '.schema_version'), contents='1') - - with mock.patch( - 'clusterfuzz._internal.base.utils.read_data_from_file', - return_value='1') as mock_read_call: - # First call should trigger read. - v1 = build._get_schema_version() - - self.assertEqual(v1, 1) - - # Second call should use cache. - v2 = build._get_schema_version() - - self.assertEqual(v2, 1) - self.assertEqual(mock_read_call.call_count, 1) - - # Clear call history - mock_read_call.reset_mock() - - # Call again, should still be cached. - v3 = build._get_schema_version() - - self.assertEqual(v3, 1) - self.assertEqual(mock_read_call.call_count, 0) - - -class UnpackBuildSchemaVersionTest(fake_filesystem_unittest.TestCase): - """Tests schema version handling during _unpack_build.""" - - def setUp(self): - test_utils.set_up_pyfakefs(self) - test_helpers.patch(self, [ - 'clusterfuzz._internal.build_management.build_manager._make_space', - 'clusterfuzz._internal.system.shell.clear_temp_directory', - 'clusterfuzz._internal.build_management.build_manager.Build._open_build_archive', - ]) - self.mock._make_space.return_value = True - - test_helpers.patch_environ(self) - os.environ['BUILDS_DIR'] = '/builds' - os.environ['FAIL_RETRIES'] = '1' - os.environ['APP_NAME'] = FAKE_APP_NAME - os.environ['JOB_NAME'] = 'job' - - self.base_build_dir = '/base_build_dir' - self.fs.create_dir(self.base_build_dir) - - self.build = build_manager.RegularBuild(self.base_build_dir, 1337, - 'gs://dummy/url.zip') - # build_dir for RegularBuild is base_build_dir/revisions - self.fs.create_dir(self.build.build_dir) - - def mock_archive(self, schema_version): - archive_mock = mock.MagicMock() - archive_mock.unpacked_size.return_value = 100 - archive_mock.archive_schema_version.return_value = schema_version - archive_mock.list_fuzz_targets.return_value = [] - return archive_mock - - def test_first_unpack_writes_version(self): - """Tests that the first unpack writes the schema version.""" - archive = self.mock_archive(1) - self.mock._open_build_archive.return_value.__enter__.return_value = archive - schema_file = os.path.join(self.build.build_dir, '.schema_version') - - result = self.build._unpack_build(self.base_build_dir, self.build.build_dir, - 'gs://dummy/url.zip') - - self.assertTrue(result) - self.assertTrue(os.path.exists(schema_file)) - self.assertEqual(utils.read_data_from_file(schema_file), 1) - self.assertEqual(self.build._schema_version, 1) - - def test_second_unpack_same_version(self): - """Tests that subsequent unpack with same version does not overwrite and doesn't log error.""" - schema_file = os.path.join(self.build.build_dir, '.schema_version') - self.fs.create_file(schema_file, contents='1') - self.build._schema_version = 1 - - archive = self.mock_archive(1) - self.mock._open_build_archive.return_value.__enter__.return_value = archive - - with mock.patch.object(self.build, '_write_schema_version') as mock_write: - with mock.patch( - 'clusterfuzz._internal.metrics.logs.error') as mock_log_error: - result = self.build._unpack_build( - self.base_build_dir, self.build.build_dir, 'gs://dummy/url.zip') - - self.assertTrue(result) - mock_write.assert_not_called() - mock_log_error.assert_not_called() - - def test_second_unpack_different_version_logs_error(self): - """Tests that subsequent unpack with different version logs error and does not overwrite.""" - schema_file = os.path.join(self.build.build_dir, '.schema_version') - self.fs.create_file(schema_file, contents='1') - self.build._schema_version = 1 - - archive = self.mock_archive(2) # Second archive has version 2 - self.mock._open_build_archive.return_value.__enter__.return_value = archive - - with mock.patch.object(self.build, '_write_schema_version') as mock_write: - with mock.patch( - 'clusterfuzz._internal.metrics.logs.error') as mock_log_error: - result = self.build._unpack_build( - self.base_build_dir, self.build.build_dir, 'gs://dummy/url.zip') - - self.assertTrue(result) - mock_write.assert_not_called() - mock_log_error.assert_called_once() - error_msg = mock_log_error.call_args[0][0] - self.assertIn('Schema version mismatch', error_msg) - self.assertIn('1', error_msg) # Existing - self.assertIn('2', error_msg) # New - - def test_unpack_reads_from_disk_if_not_in_memory(self): - """Tests that unpack reads from disk if _schema_version is not in memory.""" - build = build_manager.SymbolizedBuild(self.base_build_dir, 1337, - 'gs://dummy/release.zip', - 'gs://dummy/debug.zip') - self.fs.create_dir(build.build_dir) - self.fs.create_dir(build.release_build_dir) - self.fs.create_dir(build.debug_build_dir) - - schema_file = os.path.join(build.build_dir, '.schema_version') - self.fs.create_file(schema_file, contents='1') - build._schema_version = None # Not in memory - - archive = self.mock_archive(1) - self.mock._open_build_archive.return_value.__enter__.return_value = archive - - with mock.patch.object(build, '_write_schema_version') as mock_write: - with mock.patch( - 'clusterfuzz._internal.metrics.logs.error') as mock_log_error: - # Unpack debug build (simulating it running after release build, but with clean memory) - result = build._unpack_build(self.base_build_dir, build.debug_build_dir, - 'gs://dummy/debug.zip') - - self.assertTrue(result) - mock_write.assert_not_called() - self.assertEqual(build._schema_version, 1) # Should be synced to memory - mock_log_error.assert_not_called() + self.assertEqual( + build._read_schema_version_from_manifest(build.build_dir), 0) From cef212ab2feda45f2f4c622bb4f97983d6b909d8 Mon Sep 17 00:00:00 2001 From: Victor Liu <67168434+notvictorl@users.noreply.github.com> Date: Fri, 26 Jun 2026 16:39:36 -0400 Subject: [PATCH 3/5] Remove unused self._schema_version --- src/clusterfuzz/_internal/build_management/build_manager.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/clusterfuzz/_internal/build_management/build_manager.py b/src/clusterfuzz/_internal/build_management/build_manager.py index 075c16b1f1..c590aae941 100644 --- a/src/clusterfuzz/_internal/build_management/build_manager.py +++ b/src/clusterfuzz/_internal/build_management/build_manager.py @@ -351,7 +351,6 @@ def __init__(self, # Every fetched build is a release one, except when SymbolizedBuild # explicitly downloads a debug build self._build_type = 'release' - self._schema_version = None def _reset_cwd(self): """Reset current working directory. Needed to clean up build From 0691dd8d1a03a6c1b875fd21bcdff315cc36ac7b Mon Sep 17 00:00:00 2001 From: Victor Liu <67168434+notvictorl@users.noreply.github.com> Date: Mon, 29 Jun 2026 14:33:36 -0400 Subject: [PATCH 4/5] Remove unused archive_schema_version() --- .../_internal/build_management/build_archive.py | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/clusterfuzz/_internal/build_management/build_archive.py b/src/clusterfuzz/_internal/build_management/build_archive.py index 3c7b59c7ab..4c94fda8dc 100644 --- a/src/clusterfuzz/_internal/build_management/build_archive.py +++ b/src/clusterfuzz/_internal/build_management/build_archive.py @@ -67,11 +67,6 @@ def __init__(self, reader: archive.ArchiveReader): self._reader = reader self._fuzz_targets = None - @abc.abstractmethod - def archive_schema_version(self) -> int: - """Returns the schema version number for this archive.""" - raise NotImplementedError - def list_members(self) -> List[archive.ArchiveMemberInfo]: return self._reader.list_members() @@ -168,10 +163,6 @@ class DefaultBuildArchive(BuildArchive): """Default class for handling builds. This should work with everything. """ - @override - def archive_schema_version(self) -> int: - return 0 - def get_path_for_target(self, fuzz_target: str) -> str: """Returns the path in the archive of the fuzz_target if found. This is needed because target name normalization means we're losing @@ -354,8 +345,8 @@ def root_dir(self) -> str: self._root_dir = super().root_dir() # pylint: disable=attribute-defined-outside-init return self._root_dir - @override def archive_schema_version(self) -> int: + """Returns the schema version number for this archive.""" return self._archive_schema_version @override From e680bc4f18d3e0efd78f9680f753caf12e6546fc Mon Sep 17 00:00:00 2001 From: Victor Liu <67168434+notvictorl@users.noreply.github.com> Date: Mon, 29 Jun 2026 15:47:51 -0400 Subject: [PATCH 5/5] improve hotpath + cleanup --- .../build_management/build_manager.py | 34 ++++++------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/src/clusterfuzz/_internal/build_management/build_manager.py b/src/clusterfuzz/_internal/build_management/build_manager.py index c590aae941..8b72711ec6 100644 --- a/src/clusterfuzz/_internal/build_management/build_manager.py +++ b/src/clusterfuzz/_internal/build_management/build_manager.py @@ -405,20 +405,21 @@ def _patch_rpath(self, binary_path, instrumented_library_paths): set_rpaths(binary_path, rpaths) def _patch_rpaths(self, - schema_version: int, - build_dir: Optional[str] = None, - app_path_env: Optional[str] = None): + build_dir: str | None = None, + app_path_env: str | None = None): """Patch rpaths of builds to point to instrumented libraries.""" instrumented_library_paths = environment.get_instrumented_libraries_paths() if not instrumented_library_paths: return - if schema_version > 0: - logs.info('Skipping RPATH patching for schema v1+ build.') - return if not build_dir: build_dir = self.build_dir + schema_version = self._read_schema_version_from_manifest(build_dir) + if schema_version > 0: + logs.info('Skipping RPATH patching for schema v1+ build.') + return + if environment.is_engine_fuzzer_job(): # Import here as this path is not available in App Engine context. from clusterfuzz._internal.bot.fuzzers import utils as fuzzer_utils @@ -794,8 +795,7 @@ def setup(self): self._setup_application_path(build_update=build_update) self._post_setup_success(update_revision=build_update) - schema_version = self._read_schema_version_from_manifest(self.build_dir) - self._patch_rpaths(schema_version) + self._patch_rpaths() return True @@ -902,21 +902,12 @@ def setup(self): else: logs.info('Build already exists.') - release_schema_version = ( - self._read_schema_version_from_manifest(self.release_build_dir) - if self.release_build_url else 0) - debug_schema_version = ( - self._read_schema_version_from_manifest(self.debug_build_dir) - if self.debug_build_url else 0) - if self.release_build_url: self._setup_application_path( self.release_build_dir, build_update=build_update) environment.set_value('BUILD_URL', self.release_build_url) self._patch_rpaths( - release_schema_version, - build_dir=self.release_build_dir, - app_path_env='APP_PATH') + build_dir=self.release_build_dir, app_path_env='APP_PATH') if self.debug_build_url: # Note: this will override LLVM_SYMBOLIZER_PATH, APP_DIR etc from the @@ -924,9 +915,7 @@ def setup(self): self._setup_application_path( self.debug_build_dir, 'APP_PATH_DEBUG', build_update=build_update) self._patch_rpaths( - debug_schema_version, - build_dir=self.debug_build_dir, - app_path_env='APP_PATH_DEBUG') + build_dir=self.debug_build_dir, app_path_env='APP_PATH_DEBUG') self._post_setup_success(update_revision=build_update) return True @@ -1031,8 +1020,7 @@ def setup(self): 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) - schema_version = self._read_schema_version_from_manifest(self.build_dir) - self._patch_rpaths(schema_version) + self._patch_rpaths() return True