Support Platform-specify wheels and add rpds_py as a dependency #41143

cxzhong · 2025-11-07T10:37:31Z

This PR is to allow the platform-specify wheels and preserve the backward compatibility. Then add a package rpds_py as a dependency of jsonschema. and I set when running make dist to make a distribution. it will download all wheels.

📝 Checklist

The title is concise and informative.
The description explains in detail what this PR is about.
I have linked a relevant issue or discussion.
I have created tests covering the changes.
I have updated the documentation and checked the documentation preview.

⌛ Dependencies

- Introduced a new `tarballs_info` property in the Package class to manage multiple tarballs for platform-specific wheels. - Implemented `find_tarball_for_platform` method to select the appropriate tarball based on the current platform and Python version. - Enhanced the `Tarball` class to handle platform-specific wheels, including methods for downloading wheels using pip and checking for cached wheels. - Updated checksum verification to support multiple tarballs, ensuring integrity checks for downloaded files. - Added logic to fall back to traditional download methods if pip fails or if no cached wheels are found. - Created new dependencies and package version files for the `rpds_py` package.

… with 'rpds_py'

Remove unused import 'unicode_to_str' from formatter.py

github-actions · 2025-11-07T12:11:13Z

Documentation preview for this PR (built with commit 7d67167; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

…h Python version and free-threading status from Sage's Python, and remove pip download logic for platform-specific wheels.

…mproved compatibility checks and streamline the process of finding the best matching tarball for the current platform and Python version.

…r improved functionality

…aging.utils, improving compatibility with multi-platform wheels.

Updated the download fallback method in tarball.py.

Removed 'text=True' argument from subprocess calls.

Removed steps for making distribution and downloading packages with '--disable-download-from-upstream-url'.

Copilot

Pull Request Overview

This PR adds support for platform-specific wheels in the Sage build system and introduces rpds_py as a new dependency. The implementation allows packages to specify multiple platform-specific wheel files in their checksums.ini, with automatic selection based on the target platform's compatibility tags.

Key Changes:

Extended tarball and package handling to support multiple platform-specific wheels per package
Added platform detection using packaging.tags and packaging.utils modules via subprocess calls
Added rpds_py package with 52 platform-specific wheel entries for Python 3.11-3.14 across multiple platforms

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
build/sage_bootstrap/tarball.py	Extended `Tarball` class to handle multi-tarball packages with new methods `_download_multiple_wheels()` and `_download_single_wheel()` for platform-specific wheel downloading
build/sage_bootstrap/package.py	Added `tarballs_info` property and `find_tarball_for_platform()` method for platform detection; modified `_init_checksum()` to parse multiple tarball entries
build/pkgs/rpds_py/type	New package type file marking rpds_py as a "standard" package
build/pkgs/rpds_py/package-version.txt	Version specification for rpds_py 0.28.0
build/pkgs/rpds_py/dependencies	Dependencies declaration for rpds_py requiring pip and packaging
build/pkgs/rpds_py/checksums.ini	Comprehensive checksums for 52 platform-specific wheels covering Python 3.11-3.14 across Linux, macOS, and Windows
build/pkgs/rpds_py/SPKG.rst	Documentation for the new rpds_py package describing its purpose and platform support
build/pkgs/jsonschema/dependencies	Updated to add rpds_py as a dependency for jsonschema
build/make/Makefile.in	Added `SAGE_DOWNLOAD_ALL_WHEELS` environment variable for downloading all wheels during dist creation
.github/workflows/release.yml	Removed duplicate steps and simplified workflow (though this introduced conditional logic issues)

Comments suppressed due to low confidence (2)

build/sage_bootstrap/tarball.py:19

Import of 'sys' is not used.

import sys

build/sage_bootstrap/tarball.py:20

Import of 'subprocess' is not used.

import subprocess

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

build/sage_bootstrap/tarball.py

build/sage_bootstrap/package.py

.github/workflows/release.yml

build/sage_bootstrap/tarball.py

Copilot · 2025-11-13T08:00:05Z

build/sage_bootstrap/package.py

+        for i, tarball in enumerate(tarballs):
+            info = {
+                'tarball': tarball,
+                'sha256': sha256s[i] if i < len(sha256s) else None,
+                'sha1': sha1s[i] if i < len(sha1s) else None,
+                'upstream_url': upstream_urls[i] if i < len(upstream_urls) else None,
+            }
+            self.__tarballs_info.append(info)


Potential bug: The code assumes that checksums.ini entries are ordered such that the i-th sha256 corresponds to the i-th tarball. However, if a line in checksums.ini has only a tarball without a corresponding sha256 (or vice versa), the indices will be misaligned. For example:

tarball=file1.whl tarball=file2.whl sha256=abc123

Would incorrectly assign sha256=abc123 to tarball=file1.whl (index 0) instead of file2.whl. Consider grouping entries by blank lines or using a state machine to ensure correct pairing of tarball/sha256/sha1/upstream_url entries.

.github/workflows/release.yml

Copilot · 2025-11-13T08:00:06Z

build/sage_bootstrap/package.py

+                python_code = 'import packaging.utils, json, sys; _, _, _, tags = packaging.utils.parse_wheel_filename(sys.argv[1]); print(json.dumps([str(t) for t in tags]))'
+                result = subprocess.run(
+                    [sage_script, '-python', '-c', python_code, tarball],


[nitpick] Potential security consideration: The code executes subprocess.run() with user-controlled input (tarball variable from checksums.ini). While the tarball filename comes from a trusted source (the package's checksums.ini file), it's passed directly to a subprocess call. Consider validating the tarball filename to ensure it doesn't contain shell metacharacters or path traversal sequences, even though it's from a trusted source.

build/sage_bootstrap/package.py

build/sage_bootstrap/tarball.py

Copilot · 2025-11-13T08:00:07Z

build/sage_bootstrap/tarball.py

+            except Exception as e:
+                log.warning(f'Error checking cached file: {e}')


The broad except Exception handler at line 306 catches all exceptions, including potentially serious ones like KeyboardInterrupt (though technically it's BaseException). It would be better to catch specific exceptions like (IOError, ValueError, ChecksumError) to avoid masking unexpected errors. The same issue exists at line 358.

Co-authored-by: Copilot <[email protected]>

…rove parsing efficiency

Co-authored-by: Copilot <[email protected]>

…l_for_platform method

…rity

cxzhong · 2025-11-14T07:34:45Z

TODO: I will delete python 3.11 packages and 3.13t packages

orlitzky · 2025-11-14T14:00:40Z

I think this is the right approach, but I happen to be on a platform (riscv/musl) where binary wheels don't exist.

One thing I noticed though is that the new package can be disabled when --disable-notebook is passed to ./configure. There is a long list of packages in configure.ac that get disabled when the notebook is disabled. That should allow people on niche platforms to continue to build sage so long as they disable the notebook.

cxzhong · 2025-11-14T16:55:20Z

I think this is the right approach, but I happen to be on a platform (riscv/musl) where binary wheels don't exist.

One thing I noticed though is that the new package can be disabled when --disable-notebook is passed to ./configure. There is a long list of packages in configure.ac that get disabled when the notebook is disabled. That should allow people on niche platforms to continue to build sage so long as they disable the notebook.

yes, I will fix this. I think I can report to upstream to make wheel for some specify platform. Otherwise you have to have a rust toolchain.

cxzhong · 2025-11-16T12:17:38Z

I think this is the right approach, but I happen to be on a platform (riscv/musl) where binary wheels don't exist.

One thing I noticed though is that the new package can be disabled when --disable-notebook is passed to ./configure. There is a long list of packages in configure.ac that get disabled when the notebook is disabled. That should allow people on niche platforms to continue to build sage so long as they disable the notebook.

Do you think this way is better or we just remove the notebook dependencies from sage-distro is better?( But some notebook dependencies are also dependencies for building documents, like juypter-sphinx)

cxzhong · 2025-11-16T12:17:57Z

I think this is the right approach, but I happen to be on a platform (riscv/musl) where binary wheels don't exist.

One thing I noticed though is that the new package can be disabled when --disable-notebook is passed to ./configure. There is a long list of packages in configure.ac that get disabled when the notebook is disabled. That should allow people on niche platforms to continue to build sage so long as they disable the notebook.

I have done it.

cxzhong added 2 commits November 7, 2025 18:35

Fix dependency specification for jsonschema by replacing 'pyrsistent'…

f6d35f6

… with 'rpds_py'

cxzhong closed this Nov 7, 2025

Refactor tarball.py: remove unused import and improve logging messages

1ac314f

cxzhong reopened this Nov 7, 2025

Fix dependency specification for jsonschema by restoring 'pyrsistent'

838c4ca

Remove unused import 'unicode_to_str' from formatter.py

cxzhong added 9 commits November 7, 2025 23:01

Enhance package.py and tarball.py: implement subprocess calls to fetc…

72a0c1c

…h Python version and free-threading status from Sage's Python, and remove pip download logic for platform-specific wheels.

Enhance tarball selection in package.py: utilize packaging.tags for i…

da8cbf3

…mproved compatibility checks and streamline the process of finding the best matching tarball for the current platform and Python version.

Update dependency specification for rpds_py to include 'packaging' fo…

aba2315

…r improved functionality

Remove platform-specific wheel checks from Package and Tarball classes

1479c77

Refactor tarball parsing in package.py to utilize subprocess for pack…

e9a145c

…aging.utils, improving compatibility with multi-platform wheels.

Use compatible write for old python

5e3fd07

write the comment

dc2ed41

Updated the download fallback method in tarball.py.

Remove 'text=True' from subprocess calls

b51fe59

Removed 'text=True' argument from subprocess calls.

Merge branch 'develop' into Add_rpds_py

475db59

cxzhong changed the title ~~TEST: Add rpds_py as a dependency~~ Support Paltform-specify wheels and add rpds_py as a dependency Nov 12, 2025

cxzhong marked this pull request as ready for review November 12, 2025 06:38

cxzhong changed the title ~~Support Paltform-specify wheels and add rpds_py as a dependency~~ Support Plattform-specify wheels and add rpds_py as a dependency Nov 12, 2025

cxzhong changed the title ~~Support Plattform-specify wheels and add rpds_py as a dependency~~ Support Platform-specify wheels and add rpds_py as a dependency Nov 12, 2025

cxzhong added the s: needs review label Nov 12, 2025

cxzhong added 3 commits November 13, 2025 13:52

Add support for downloading all wheels for distribution in Tarball class

f102156

Update release.yml

3e7f0ff

Removed steps for making distribution and downloading packages with '--disable-download-from-upstream-url'.

Update release.yml

0378ce7

cxzhong requested a review from Copilot November 13, 2025 07:48

Copilot started reviewing on behalf of cxzhong November 13, 2025 07:48 View session

Copilot finished reviewing on behalf of cxzhong November 13, 2025 07:53

Copilot AI reviewed Nov 13, 2025

View reviewed changes

Apply suggestions from code review

af01d2c

Co-authored-by: Copilot <[email protected]>

cxzhong marked this pull request as draft November 13, 2025 08:27

github-actions bot removed the s: needs review label Nov 13, 2025

cxzhong and others added 5 commits November 13, 2025 20:28

Refactor tarball selection logic to prioritize wheel tarballs and imp…

d7ff54e

…rove parsing efficiency

Apply suggestions from code review

5a6baed

Co-authored-by: Copilot <[email protected]>

Merge branch 'Add_rpds_py' of github.com:cxzhong/sage into Add_rpds_py

d52bc6b

Remove redundant json import and add it at the top of the find_tarbal…

448f5ee

…l_for_platform method

Reorder import statements in find_tarball_for_platform method for cla…

7d67167

…rity

cxzhong marked this pull request as ready for review November 14, 2025 05:13

github-actions bot added the s: needs review label Nov 14, 2025

cxzhong requested a review from orlitzky November 14, 2025 07:20

cxzhong added c: build v: large labels Nov 14, 2025

pass package rpds_py in the disable-notebook

a032ac8

		except Exception as e:
		log.warning(f'Error checking cached file: {e}')

Uh oh!

Support Platform-specify wheels and add rpds_py as a dependency #41143

Are you sure you want to change the base?

Support Platform-specify wheels and add rpds_py as a dependency #41143

Conversation

cxzhong commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Checklist

⌛ Dependencies

Uh oh!

github-actions bot commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

cxzhong commented Nov 14, 2025

Uh oh!

orlitzky commented Nov 14, 2025

Uh oh!

cxzhong commented Nov 14, 2025

Uh oh!

cxzhong commented Nov 16, 2025

Uh oh!

cxzhong commented Nov 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cxzhong commented Nov 7, 2025 •

edited

Loading

github-actions bot commented Nov 7, 2025 •

edited

Loading