Skip to content

build: disable unused zstd legacy feature - #3495

Merged
blackmwk merged 3 commits into
apache:mainfrom
kevinjqliu:kevinjqliu-drop-brotli-zstd-legacy
Oct 21, 2026
Merged

blackmwk merged 3 commits into
apache:mainfrom
kevinjqliu:kevinjqliu-drop-brotli-zstd-legacy

Conversation

@kevinjqliu

@kevinjqliu kevinjqliu commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

  • Related to #3239 and the wheel license work in #3493.

What changes are included in this PR?

zstd's legacy feature (decoders for pre-1.0 zstd frames) is on by default but nothing here needs it, since Parquet, Puffin and Avro all use standard zstd frames. Turning it off means that code isn't compiled in, so the pyiceberg-core wheel LICENSE doesn't need the BSD-2 attribution for it.

Same setting upstream: arrow-rs parquet, arrow-rs arrow-ipc, DataFusion workspace and avro-rs all use zstd with default-features = false.

Are these changes tested?

Dependency-only change, covered by existing tests:

  • cargo check --all-targets: passed
  • cargo test -p iceberg --lib: passed (1991 passed, 0 failed)

AI Disclosure

Drafted with Copilot. Worth a look: that no other crate relies on zstd's legacy feature.

kevinjqliu and others added 2 commits October 20, 2026 13:31
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Brotli is still a supported Parquet codec for reads and writes.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@kevinjqliu kevinjqliu changed the title build: drop unused brotli and zstd legacy features build: disable unused zstd legacy feature Oct 20, 2026
@kevinjqliu
kevinjqliu marked this pull request as ready for review October 20, 2026 20:43
Copilot AI balanced review requested due to automatic review settings October 20, 2026 20:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

1 open issue
What changed in this PR

Disables zstd’s default features to avoid compiling the legacy decoder code path, simplifying wheel license attributions and aligning with upstream dependency configurations.

Changes:

  • Switch zstd dependency to default-features = false in the workspace Cargo.toml.
File Description
Cargo.toml Disables zstd default features (including legacy frame support) to reduce compiled code and license attribution needs.

Review effort: Lite

Comment thread Cargo.toml

@dannycjones dannycjones left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great find on this, that removes the dependency on BSD-2-Claude inside C files too.

@blackmwk
blackmwk added this pull request to the merge queue Oct 21, 2026
Merged via the queue into apache:main with commit 000e791 Oct 21, 2026
24 checks passed
@kevinjqliu
kevinjqliu deleted the kevinjqliu-drop-brotli-zstd-legacy branch October 21, 2026 15:55
dannycjones added a commit to dannycjones/iceberg-rust that referenced this pull request Oct 26, 2026
Backport of apache#3495 to 0.11.x.

Dropping zstd's default features turns off zstd-sys/legacy, so the
vendored legacy decoders under zstd/lib/legacy are no longer compiled
into the pyiceberg-core wheels.

Co-authored-by: Kevin Liu <kevinjqliu@users.noreply.github.com>
Co-authored-by: Danny C Jones <djonesoa@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants