-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Ruby, Rust: add zstd compression option (and fix compression in Rust) #19613
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes the Rust extractor’s trap file extension handling and adds Zstandard (zstd) as an optional compression method to both the Rust and Ruby extractors, along with integration tests for each.
- Introduce
Compression::Zstd
path in shared and Rust extractor code, fix builder methods, and update config. - Add zstd dependency to Cargo and Bazel manifests and update extractor YAML options.
- Add parameterized integration tests for zstd compression in both Rust and Ruby.
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
shared/tree-sitter-extractor/src/trap.rs | Add Zstd branch, use constants for compression levels |
shared/tree-sitter-extractor/Cargo.toml | Add zstd crate dependency |
rust/ql/integration-tests/hello-project/test_project.py | Parametrize rust tests over compression modes |
rust/extractor/src/trap.rs | Remove .into() call, pass compression enum directly, update extension |
rust/extractor/src/config.rs | Rename compression →trap_compression , implement Zstd mapping |
rust/codeql-extractor.yml | Include zstd in option description and allowed pattern |
ruby/ql/integration-tests/compression/test.py | Add Ruby integration test for zstd compression |
ruby/ql/integration-tests/compression/source.rb | Add sample Ruby source for compression test |
ruby/ql/integration-tests/compression/methods.ql | Add test query for compression roundtrip |
ruby/ql/integration-tests/compression/methods.expected | Add expected output for compression test |
ruby/codeql-extractor.yml | Update Ruby extractor options to allow zstd |
misc/bazel/3rdparty/tree_sitter_extractors_deps/defs.bzl | Add zstd label in dependencies |
misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.zstd-sys-2.0.15+zstd.1.5.7.bazel | Auto‐generated zstd-sys target |
misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.zstd-safe-7.2.4.bazel | Auto‐generated zstd-safe target |
misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.zstd-0.13.3.bazel | Auto‐generated zstd target |
misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.pkg-config-0.3.32.bazel | Auto‐generated pkg-config target |
misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.jobserver-0.1.32.bazel | Auto‐generated jobserver target |
misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.cc-1.2.7.bazel | Update cc crate features, add jobserver dep |
misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.bazel | Add zstd alias |
MODULE.bazel | Reference zstd in repository list |
Comments suppressed due to low confidence (1)
rust/ql/integration-tests/hello-project/test_project.py:14
- The test_do_not_print_env function no longer has the @pytest.mark.ql_test decorator, which may prevent it from being recognized as a QL test. Re-add @pytest.mark.ql_test(None) or adjust the marker accordingly.
@pytest.mark.parametrize("rust_edition", [2024])
expected_suffixes = [".trap"] + suffix | ||
|
||
def is_of_expected_format(file): | ||
return file.name == "metadata.trap.gz" or \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test only special-cases metadata.trap.gz; when using Zstandard compression, metadata.trap.zst will not match this condition and cause the test to fail. Consider matching the metadata file suffix dynamically or adding a case for ".zst".
return file.name == "metadata.trap.gz" or \ | |
expected_metadata_name = f"metadata.trap{''.join(suffix)}" | |
return file.name == expected_metadata_name or \ |
Copilot uses AI. Check for mistakes.
Why is |
This helps in the early stages of extractor development, and I also think we should only turn on compression if we need it (Swift never did for example). |
It turns out that the trap compression option in Rust was broken, as we were not setting the right extension to the written trap file. This fixes it, and adds
zstd
as a compression option to both the Rust and Ruby extractor.The default compression (gzip for ruby, none for rust) is unchanged.
Integration tests checking the compression option are added to both languages.