Skip to content

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

Open
ivq wants to merge 27 commits into
python:mainfrom
ivq:xz_bcj_filters
Open

gh-115988: Add missing ARM64 and RISCV filter in lzma module#115989
ivq wants to merge 27 commits into
python:mainfrom
ivq:xz_bcj_filters

Conversation

@ivq
Copy link
Copy Markdown

@ivq ivq commented Feb 27, 2024

#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
Copy link
Copy Markdown

ghost commented Feb 27, 2024

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app
Copy link
Copy Markdown

bedevere-app Bot commented Feb 27, 2024

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.

Copy link
Copy Markdown
Member

@terryjreedy terryjreedy left a comment

Choose a reason for hiding this comment

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

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
Copy link
Copy Markdown

bedevere-app Bot commented Feb 27, 2024

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
Copy link
Copy Markdown

bedevere-app Bot commented Feb 28, 2024

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
Copy link
Copy Markdown
Author

ivq commented Feb 28, 2024

I have made the requested changes; please review again.

@terryjreedy
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.

Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

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
Copy link
Copy Markdown

bedevere-app Bot commented Mar 17, 2024

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
Copy link
Copy Markdown
Author

ivq commented Mar 17, 2024

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 ivq requested a review from serhiy-storchaka March 18, 2024 12:57
Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

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

Comment thread Doc/library/lzma.rst Outdated
Copy link
Copy Markdown
Member

@StanFromIreland StanFromIreland left a comment

Choose a reason for hiding this comment

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

This has conflicts, and the version has to be changed to next as this is not making it to 3.13 :-(

ivq added 2 commits July 13, 2025 16:41
Signed-off-by: Chien Wong <m@xv97.com>
@ivq ivq force-pushed the xz_bcj_filters branch from d609e87 to f381ca5 Compare July 13, 2025 09:31
@ivq
Copy link
Copy Markdown
Author

ivq commented Jul 13, 2025

Merged latest main branch and adjusted available version number. Please review.

@ivq ivq requested review from StanFromIreland and gpshead July 13, 2025 09:33
ivq added 2 commits July 13, 2025 17:35
Signed-off-by: Chien Wong <m@xv97.com>
Signed-off-by: Chien Wong <m@xv97.com>
@furkanonder
Copy link
Copy Markdown
Contributor

!buildbot onder-riscv64

@bedevere-bot
Copy link
Copy Markdown

The regex 'onder-riscv64' did not match any buildbot builder. Is the requested builder in the list of stable builders?

@furkanonder
Copy link
Copy Markdown
Contributor

!buildbot riscv64

@bedevere-bot
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
Copy link
Copy Markdown

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

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 15, 2026
@serhiy-storchaka
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 github-actions Bot removed the stale Stale PR or inactive for long period of time. label May 21, 2026
@hugovk
Copy link
Copy Markdown
Member

hugovk commented May 22, 2026

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

@serhiy-storchaka
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 ivq requested a review from AA-Turner as a code owner May 27, 2026 14:05
Signed-off-by: Chien Wong <m@xv97.com>
@ivq ivq force-pushed the xz_bcj_filters branch from da8fed3 to 9cc998f Compare May 27, 2026 14:08
@read-the-docs-community
Copy link
Copy Markdown

read-the-docs-community Bot commented May 27, 2026

Documentation build overview

📚 cpython-previews | 🛠️ Build #32871667 | 📁 Comparing 5ed9891 against main (55f2518)

  🔍 Preview build  

3 files changed
± library/lzma.html
± whatsnew/3.16.html
± whatsnew/changelog.html

@ivq
Copy link
Copy Markdown
Author

ivq commented May 27, 2026

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

@gpshead gpshead enabled auto-merge (squash) May 27, 2026 15:17
Comment thread Doc/whatsnew/3.16.rst
instead of failing later, when encounter non-ASCII data.
(Contributed by Serhiy Storchaka in :gh:`62259`.)

lzma
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.

Module names should be sorted alphabetically. Move this entry above os.

@StanFromIreland StanFromIreland disabled auto-merge May 27, 2026 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting merge type-feature A feature request or enhancement

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

9 participants