Skip to content

Replace astral-tokio-tar with tar-codec - #19979

Open
woodruffw wants to merge 2 commits into
mainfrom
ww/tar-codec
Open

Replace astral-tokio-tar with tar-codec#19979
woodruffw wants to merge 2 commits into
mainfrom
ww/tar-codec

Conversation

@woodruffw

@woodruffw woodruffw commented Jun 24, 2026

Copy link
Copy Markdown
Member

Summary

WIP, I've done the read path but not the (full) write path yet.

This adds a preview feature that replaces astral-tokio-tar with tar-codec on both our read and write paths.

Notes:

Test Plan

Ensure existing tests don't break, plus new tests.

@woodruffw woodruffw self-assigned this Jun 24, 2026
@woodruffw woodruffw added the internal A refactor or improvement that is not user-facing label Jun 24, 2026
@codspeed-hq

codspeed-hq Bot commented Jun 24, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by ×2.1

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 2 improved benchmarks
✅ 21 untouched benchmarks
⏩ 6 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
WallTime unpack_sdist_many_files 731 ms 326.7 ms ×2.2
Simulation unpack_sdist_many_files 333.4 ms 176.6 ms +88.8%

Tip

Curious why this is faster? Use the CodSpeed MCP and ask your agent.


Comparing ww/tar-codec (2dcf539) with main (3a9573c)

Open in CodSpeed

Footnotes

  1. 6 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@zanieb

zanieb commented Jun 24, 2026

Copy link
Copy Markdown
Member

Is the intent to do this behind preview first, or?

@woodruffw

Copy link
Copy Markdown
Member Author

Is the intent to do this behind preview first, or?

I didn't have a strong intent either way 🙂. I guess we probably should?

@charliermarsh

Copy link
Copy Markdown
Member

I would personally be ok to do this outside of preview, as long as we have sufficient testing, since it's not clear that we want to change any specific behavior in response to user feedback, etc.

@zanieb

zanieb commented Jun 24, 2026

Copy link
Copy Markdown
Member

I think the main issue is that it will be much more restrictive. I guarantee we will break people.

@charliermarsh

Copy link
Copy Markdown
Member

That makes sense, I forgot that this would actually be stricter. I’m a little worried that we won’t get enough feedback to know when we can stabilize it.

@zanieb

zanieb commented Jun 24, 2026

Copy link
Copy Markdown
Member

I think another benefit of having a preview code path is that when we stabilize it we can trivially retain the old code path to allow opt-in to legacy when we change the default, which would give us the signal we need for long-term removal.

Alas that lengthens the period of time in which we have two implementations for differentials :)

@woodruffw

Copy link
Copy Markdown
Member Author

Yeah, I think I lean towards a preview feature here. I'll get this into a working shape and then massage the old tokio-tar paths back into place.

@astral-sh-bot

astral-sh-bot Bot commented Jun 26, 2026

Copy link
Copy Markdown

uv test inventory changes

This PR changes the tests when compared with the main base revision.

  • Added tests: 18
  • Removed tests: 0
  • Changed suites: 3
uv-build-backend: +1 / -0

Added:

  • uv-build-backend::tests::built_by_uv_building_tar_codec

Removed: none

uv-extract: +14 / -0

Added:

  • uv-extract::stream::tests::strict_decoder_rejects_excluded_tar_framing
  • uv-extract::stream::tests::strict_decoder_rejects_non_utf8_member_names
  • uv-extract::stream::tests::tar_http_errors_are_detected_through_the_source_chain
  • uv-extract::stream::tests::untar_preserves_archive_owned_and_missing_symlink_targets
  • uv-extract::stream::tests::untar_reads_gnu_archives
  • uv-extract::stream::tests::untar_records_files_and_preserves_executable_intent
  • uv-extract::stream::tests::untar_rejects_ambient_symlink_targets
  • uv-extract::stream::tests::untar_rejects_external_symlink_targets
  • uv-extract::stream::tests::untar_rejects_hardlinks
  • uv-extract::stream::tests::untar_rejects_malformed_archives
  • uv-extract::stream::tests::untar_rejects_member_path_traversal
  • uv-extract::stream::tests::untar_selects_backend_from_preview_feature
  • uv-extract::stream::tests::untar_skips_symlinks_on_windows
  • uv-extract::stream::tests::untar_uses_default_name_validation

Removed: none

uv-publish: +3 / -0

Added:

  • uv-publish::tests::source_dist_pkg_info_reads_single_top_level_file
  • uv-publish::tests::source_dist_pkg_info_rejects_duplicates
  • uv-publish::tests::source_dist_pkg_info_requires_a_file

Removed: none

zanieb added a commit that referenced this pull request Jul 16, 2026
A `.whl.tar.zst` member is recorded before traversal containment is
enforced, so a skipped `../` member can survive `RECORD` healing and
cause uninstall to remove an unrelated environment executable. Collect
only members that were successfully unpacked.

This overlaps the tar-wheel extraction work in
[#19979](#19979).
@woodruffw
woodruffw changed the base branch from main to release/0.12.0 July 27, 2026 14:16
@woodruffw
woodruffw changed the base branch from release/0.12.0 to main July 27, 2026 14:40
@woodruffw
woodruffw marked this pull request as ready for review July 27, 2026 19:30
@zanieb
zanieb self-requested a review July 30, 2026 18:51
Signed-off-by: William Woodruff <william@yossarian.net>

Tighten extraction policy

Signed-off-by: William Woodruff <william@yossarian.net>

Bump tar-codec

Signed-off-by: William Woodruff <william@yossarian.net>

Cheeky clippy fix

Signed-off-by: William Woodruff <william@yossarian.net>

Tweak our decoding policy

Signed-off-by: William Woodruff <william@yossarian.net>

Fix some snapshots

Signed-off-by: William Woodruff <william@yossarian.net>

Add an offset filter

This offset ranges by platform.

Signed-off-by: William Woodruff <william@yossarian.net>

Bump tar-codec

Signed-off-by: William Woodruff <william@yossarian.net>

Migrate the write path

Signed-off-by: William Woodruff <william@yossarian.net>

Remove a stale test

Signed-off-by: William Woodruff <william@yossarian.net>

Re-add both tar backends

Signed-off-by: William Woodruff <william@yossarian.net>

Bump snapshot

Signed-off-by: William Woodruff <william@yossarian.net>

Update some snapshots

Signed-off-by: William Woodruff <william@yossarian.net>

Fix snapshots

Signed-off-by: William Woodruff <william@yossarian.net>

Fix benchmarking

Signed-off-by: William Woodruff <william@yossarian.net>

Undo benchmark naches

Signed-off-by: William Woodruff <william@yossarian.net>

Bump tar-codec, use the new finalization API

Signed-off-by: William Woodruff <william@yossarian.net>

Briefer description

Signed-off-by: William Woodruff <william@yossarian.net>

Fix snapshot

Signed-off-by: William Woodruff <william@yossarian.net>

Bump snapshots

Signed-off-by: William Woodruff <william@yossarian.net>

Fix snapshot

Signed-off-by: William Woodruff <william@yossarian.net>

Clippy

Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
// as raw binary.
let decode_policy = DecodePolicy::default().pax_policy(
PaxDecodePolicy::default()
.allow_unknown_pax_vendor_records(true)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal A refactor or improvement that is not user-facing performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants