FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-115988: Add missing ARM64 and RISCV filter in lzma module by ivq · Pull Request #115989 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

gh-115988: Add missing ARM64 and RISCV filter in lzma module - #115989

Merged
gpshead merged 28 commits into
python:mainfrom
ivq:xz_bcj_filters
May 28, 2026
ConversationCommits (28)ChecksFiles changed
Merged

gh-115988: Add missing ARM64 and RISCV filter in lzma module#115989
gpshead merged 28 commits into
python:mainfrom
ivq:xz_bcj_filters

Conversation

ivq commented Feb 27, 2024 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Contributor

#115988

Example:

import lzma

filter = [
    { "id": lzma.FILTER_RISCV },
    { "id": lzma.FILTER_LZMA2 },
]

in_fn = "in.bin"
out_fn = "out.bin"

with open(in_fn, mode="rb") as f:
    in_data = f.read()

with lzma.open(out_fn, mode="wb", filters=filter) as f:
    f.write(in_data)

📚 Documentation preview 📚: https://cpython-previews--115989.org.readthedocs.build/

ghost commented Feb 27, 2024 •
edited by ghost
Loading

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

bedevere-app Bot mentioned this pull request Feb 27, 2024
Closed

bedevere-app Bot commented Feb 27, 2024

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

terryjreedy requested changes Feb 27, 2024
View reviewed changes

terryjreedy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Does test_lzma test the existing constants in any way? Such as for validity of the spelling? If so, the new ones should be added.

Someone more familiar with lzma should do a commit review.

Comment thread Doc/library/lzma.rst Outdated
Comment thread Modules/_lzmamodule.c
Comment thread Modules/_lzmamodule.c

bedevere-app Bot commented Feb 27, 2024

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

bedevere-app Bot commented Feb 28, 2024

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

ivq commented Feb 28, 2024

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again.

pitrou mentioned this pull request Feb 29, 2024
Closed
ivq requested a review from terryjreedy March 12, 2024 13:44

terryjreedy commented Mar 12, 2024

Copy link
Copy Markdown
Member

I cannot do a commit review of this as it is out of my expertise. I requested reviews from two coredevs who have edited the file most recently.

serhiy-storchaka reviewed Mar 13, 2024
View reviewed changes

serhiy-storchaka left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

They are pretty new features (https://xz.tukaani.org/format/xz-file-format-1.2.0.txt):

        1.2.0     2024-01-19    Added RISC-V filter and updated URLs in
                                Sections 0.2 and 7. The URL of this
                                specification was changed.

        1.1.0     2022-12-11    Added ARM64 filter and clarified 32-bit
                                ARM endianness in Section 5.3.2,
                                language improvements in Section 5.4

Adding support for them is a new feature in Python, so you need to add versionadded or versionchanged directives in the module documentation and add the corresponding entry in the What's New file.

You should also specify that the new filters (work? are supported? are available?) only with the specified XZ (library? file format?) version. Please add tests for new filters. They should produce expected result or raise an expected exception depending on the xz version.

I am not sure whether these constants should be defined if the underlying library does not support them. The new tests should show what is better.

bedevere-app Bot commented Mar 17, 2024

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

ivq commented Mar 17, 2024

Copy link
Copy Markdown
Contributor Author

They are pretty new features (xz.tukaani.org/format/xz-file-format-1.2.0.txt):

        1.2.0     2024-01-19    Added RISC-V filter and updated URLs in
                                Sections 0.2 and 7. The URL of this
                                specification was changed.

        1.1.0     2022-12-11    Added ARM64 filter and clarified 32-bit
                                ARM endianness in Section 5.3.2,
                                language improvements in Section 5.4

Adding support for them is a new feature in Python, so you need to add versionadded or versionchanged directives in the module documentation and add the corresponding entry in the What's New file.

You should also specify that the new filters (work? are supported? are available?) only with the specified XZ (library? file format?) version. Please add tests for new filters. They should produce expected result or raise an expected exception depending on the xz version.

I am not sure whether these constants should be defined if the underlying library does not support them. The new tests should show what is better.

OK. Done. Correct my if anything is missing.

ivq requested a review from serhiy-storchaka March 18, 2024 12:57
serhiy-storchaka approved these changes Mar 18, 2024
View reviewed changes

serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM. You only need to add an entry in the What's New document.

Comment thread Doc/library/lzma.rst Outdated

bedevere-bot commented Jul 13, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @furkanonder for commit 1a17fd5 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F115989%2Fmerge

The command will test the builders whose names match following regular expression: riscv64

The builders matched are:

  • riscv64 Ubuntu23 PR

github-actions Bot commented Apr 15, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 15, 2026

serhiy-storchaka commented May 21, 2026

Copy link
Copy Markdown
Member

Let's remove the version attributes, they are still discussed.

@hugovk, any chance to include this in 3.15 (after excluding the discussed part)?

github-actions Bot removed the stale Stale PR or inactive for long period of time. label May 21, 2026

hugovk commented May 22, 2026

Copy link
Copy Markdown
Member

This has been "ready" for two years, what would be the justification to include in 3.15 after the freeze?

serhiy-storchaka commented May 22, 2026

Copy link
Copy Markdown
Member

The main reason is my feeling of guilt. We could have merged it long ago, but once again we missed the deadline.

This is not critical feature, so if no, then no. It will be in 3.16 then.

ivq requested a review from AA-Turner as a code owner May 27, 2026 14:05
Remove lzma version constants
9cc998f 
Signed-off-by: Chien Wong <m@xv97.com>
ivq force-pushed the xz_bcj_filters branch from da8fed3 to 9cc998f Compare May 27, 2026 14:08

read-the-docs-community Bot commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown

Documentation build overview

📚 cpython-previews | 🛠️ Build #32887315 | 📁 Comparing b52d942 against main (55f2518)

  🔍 Preview build  

5 files changed · ± 5 modified

± Modified

ivq commented May 27, 2026

Copy link
Copy Markdown
Contributor Author

The version attributes are removed. Hope it lands in 3.16.

gpshead enabled auto-merge (squash) May 27, 2026 15:17
serhiy-storchaka reviewed May 27, 2026
View reviewed changes
Comment thread Doc/whatsnew/3.16.rst Outdated
StanFromIreland disabled auto-merge May 27, 2026 15:58
Put whatsnew entry alphabetically
b52d942 
Signed-off-by: Chien Wong <m@xv97.com>
gpshead merged commit cf2cd0b into python:main May 28, 2026
55 checks passed
serhiy-storchaka removed their assignment Jul 15, 2026
This was referenced Jul 22, 2026
Closed
Closed
Closed
Merged
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

gpshead gpshead approved these changes
serhiy-storchaka serhiy-storchaka approved these changes
terryjreedy Awaiting requested review from terryjreedy
methane Awaiting requested review from methane
StanFromIreland Awaiting requested review from StanFromIreland
AA-Turner Awaiting requested review from AA-Turner AA-Turner is a code owner

Assignees

gpshead

Labels

type-feature A feature request or enhancement

Projects

Compression issues 🗜
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

Back | FazBrowse Home | New Git URL