Add handling for reformatted archives conditional on archive format version #5145
Conversation
letitz
left a comment
letitz
left a comment
There was a problem hiding this comment.
First round. I'm excited to see this land!
letitz
left a comment
letitz
left a comment
There was a problem hiding this comment.
Smaller comments this time around, except that we still need tests :)
src/clusterfuzz/_internal/tests/core/build_management/build_archive_test.py
Show resolved
Hide resolved
| to_extract = [f.name for f in to_extract] | ||
| self.assertCountEqual(to_extract, needed_files) | ||
|
|
||
| @parameterized.parameterized.expand(['/b/build/', 'build/', '']) |
There was a problem hiding this comment.
Why delete this test?
There was a problem hiding this comment.
Ah sorry, this is actually something I meant to ask about but forgot over the weekend... this test seems exactly the same as the test that proceeds it other than in name. Do you happen to know if the test case is still relevant?
src/clusterfuzz/_internal/tests/core/build_management/build_archive_test.py
Show resolved
Hide resolved
| return self._archive_schema_version | ||
| def __init__(self, | ||
| reader: archive.ArchiveReader, | ||
| archive_schema_version: int = 0): |
There was a problem hiding this comment.
Given that this is only used if the archive is missing, let's call it default_archive_schema_version.
Also, let's document this method:
def __init__(self, ...):
"""Initializes a `ChromiumBuildArchive` with the given reader.
Arguments:
reader: See `DefaultBuildArchive`.
default_archive_schema_version: Specifies which version of a build archive to
expect if `clusterfuzz_manifest.json` is missing or badly formatted.
"""|
|
||
| Expects a manifest file named `clusterfuzz_manifest.json` in the root of the | ||
| archive to decide which schema version to use when interpreting its contents. | ||
| The legacy schema is applied to archives with no manifest. |
There was a problem hiding this comment.
We should document what the legacy schema is, and what version 1 looks like.
There was a problem hiding this comment.
Done. Does this look good to you?
src/clusterfuzz/_internal/tests/core/build_management/build_archive_test.py
Outdated
Show resolved
Hide resolved
| 'swiftshader/libGLESv2.so', | ||
| 'instrumented_libraries/msan/lib/libgcrypt.so.11.8.2', |
There was a problem hiding this comment.
In real .runtime_deps files, I would expect the reference to e.g. libgcrypt.so to be prefixed with ../../. Can you look at such a file in a ClusterFuzz build to see what it looks like?
There was a problem hiding this comment.
Here's an example:
./zucchini_patch_fuzzer
zucchini_patch_fuzzer.owners
../../testing/test_env.py
../../testing/xvfb.py
../../testing/scripts/common.py
../../.vpython3
../../build/util/lib/init.py
../../build/util/lib/proto/
../../.vpython3
../../build/util/lib/init.py
../../build/util/lib/results/
./libbase.so
../../tools/valgrind/asan/
../../third_party/llvm-build/Release+Asserts/bin/llvm-symbolizer
./libbase_allocator_partition_allocator_src_partition_alloc_raw_ptr.so
./libbase_allocator_partition_allocator_src_partition_alloc_allocator_base.so
../../tools/valgrind/asan/
../../third_party/llvm-build/Release+Asserts/bin/llvm-symbolizer
../../tools/memory/sanitizer/escalate_sanitizer_warnings.py
../../third_party/instrumented_libs/binaries/msan-chained-origins-noble-lib/lib
third_party/instrumented_libs/binaries/msan-chained-origins-noble-lib/lib/ld-linux-x86-64.so.2
./libc++.so
./libatomic.so
./libbase_allocator_partition_allocator_src_partition_alloc_allocator_core.so
./libbase_allocator_partition_allocator_src_partition_alloc_allocator_shim.so
./libthird_party_abseil-cpp_absl.so
./libthird_party_boringssl.so
./libthird_party_perfetto_libperfetto.so
./libchrome_zlib.so
./libthird_party_icu_icui18n.so
./libicuuc.so
icudtl.dat
bin/run_zucchini_patch_fuzzer
../../.vpython3
../../testing/location_tags.json
The answer seems to be that some are prefixed with relative separators and some are not.
|
Can you please explain me simply briefly in my reply. What's this about?
And what should I do?
And is there any else to fixed by my side , because there are lots of
coding samples you give me,, kindly just tell me what should I do
…On Fri, 30 Jan 2026, 23:50 Martin Verde, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In
src/clusterfuzz/_internal/tests/core/build_management/build_archive_test.py
<#5145 (comment)>:
> @@ -245,11 +290,50 @@ def test_other_fuzzer_not_extracted(self, dir_prefix):
to_extract = [f.name for f in to_extract]
self.assertCountEqual(to_extract, needed_files)
+ @parameterized.parameterized.expand(['/b/build/', 'build/', ''])
+ def test_possible_dependencies(self, dir_prefix):
+ """Tests that all the necessary dependencies are correctly extracted from
+ the runtime_deps file.
+
+ Under the current archive schema, dependency paths in `runtime_deps` files
Done.
—
Reply to this email directly, view it on GitHub
<#5145 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/B332O6OI26BTIHVLDP7TJP34JOKU7AVCNFSM6AAAAACTARK4KKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTOMRZGYZTKMZZGE>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
3 participants
Opening on behalf of @mverde to start reviewing early.