Skip to content

lzma implementation #5717

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

lzma implementation #5717

wants to merge 3 commits into from
+3,425 −333

Conversation

arihant2math
Copy link
Collaborator

Based on #5709 because of the zlib abstraction.

@arihant2math arihant2math mentioned this pull request Apr 20, 2025
Merged
@arihant2math arihant2math marked this pull request as ready for review April 20, 2025 05:48
@arihant2math
Copy link
Collaborator Author

This turned out surprisingly effective, about 2/3s of the tests pass, which is enough for me at the moment. The main missing parts involve a bunch of unsafe things that xz2 doesn't really support (to my knowledge atleast).

As a side note I also abstracted out Compressor like Decompressor was abstracted

@arihant2math arihant2math force-pushed the lzma branch 2 times, most recently from c09442c to 2bad5d2 Compare April 21, 2025 00:01
@arihant2math arihant2math marked this pull request as draft April 21, 2025 02:24
@arihant2math arihant2math marked this pull request as ready for review April 21, 2025 02:24
@arihant2math
Copy link
Collaborator Author

Lots of tests are failing because they were previously blocked due to no implementation at all.

@arihant2math arihant2math changed the title Lzma implementation lzma implementation Apr 21, 2025
@coolreader18 coolreader18 mentioned this pull request Apr 21, 2025
Open
@coolreader18
Copy link
Member

coolreader18 commented Apr 21, 2025 •
edited
Loading

Would you be able to squash these commits down? Ideally into one for copying the files from cpython, and one for all the rest?

@arihant2math
Copy link
Collaborator Author

Sure

@arihant2math arihant2math force-pushed the lzma branch 3 times, most recently from 19a43f5 to ca2dc44 Compare April 21, 2025 19:40
@arihant2math
Copy link
Collaborator Author

It's at 3 now, I committed things in a weird fashion so I can't get it below that.

@arihant2math arihant2math mentioned this pull request Apr 23, 2025
Open
@arihant2math arihant2math requested a review from youknowone April 27, 2025 06:47
youknowone
youknowone reviewed Apr 30, 2025
View reviewed changes
Lib/tarfile.py
@@ -392,7 +392,8 @@ def __init__(self, name, mode, comptype, fileobj, bufsize,

elif comptype == "xz":
try:
import lzma
# TODO: RUSTPYTHON remove underscore
Copy link
Member

Choose a reason for hiding this comment

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

Is this name changed to raise ImportError? What happens if it is lzma?

Copy link
Collaborator Author

@arihant2math arihant2math Apr 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, if left as is everything breaks because lzma is expected to be unimportable.

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

@youknowone youknowone youknowone left review comments

@coolreader18 coolreader18 Awaiting requested review from coolreader18

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
A-stdlib
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@arihant2math @coolreader18 @youknowone