Skip to content
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

sage --package create --pypi: Create a wheel package by default if possible #36794

Merged
merged 4 commits into from
Dec 14, 2023
Merged

sage --package create --pypi: Create a wheel package by default if possible #36794

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d25284e
sage -package create --pypi: If --source is not given, use --source=w…
mkoeppe Dec 1, 2023
af414b6
build/pkgs/six: Change to a wheel package
mkoeppe Dec 1, 2023
ec28623
build/make/Makefile.in: Update documentation of PYTHON_TOOLCHAIN
mkoeppe Dec 6, 2023
818468a
build/sage_bootstrap/{app,cmdline}.py: Suggested edits
mkoeppe Dec 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions build/make/Makefile.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,9 +322,8 @@ toolchain-deps:
all-toolchain: base-toolchain
+$(MAKE_REC) toolchain-deps

# All packages needed as a prerequisite to install other Python packages with
# pip or which are otherwise used by the Python build tools; these should be
# given as a prerequisite to any pip-installed packages
# Shorthand for a list of packages sufficient for building and installing
# typical Python packages from source. Wheel packages only need pip.
PYTHON_TOOLCHAIN = setuptools pip setuptools_scm wheel setuptools_wheel

# Trac #32056: Avoid installed setuptools leaking into the build of python3 by uninstalling it.
Expand Down
10 changes: 5 additions & 5 deletions build/pkgs/six/checksums.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
tarball=six-VERSION.tar.gz
sha1=06fa0bb50f2a4e2917fd14c21e9d2d5508ce0163
md5=a7c927740e4964dd29b72cebfc1429bb
cksum=1693137720
upstream_url=https://pypi.io/packages/source/s/six/six-VERSION.tar.gz
tarball=six-VERSION-py2.py3-none-any.whl
sha1=79e6f2e4f9e24898f1896df379871b9c9922f147
md5=529d7fd7e14612ccde86417b4402d6f3
cksum=2975792266
upstream_url=https://pypi.io/packages/py2.py3/s/six/six-VERSION-py2.py3-none-any.whl
2 changes: 1 addition & 1 deletion build/pkgs/six/dependencies
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
| $(PYTHON_TOOLCHAIN) $(PYTHON)
| pip $(PYTHON)

----------
Copy link
Collaborator

@kwankyu kwankyu Dec 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

PYTHON_TOOLCHAIN represents "a prerequisite to any pip-installed packages". It does not differentiate source and wheel python packages. Here changing it with just "pip" signals that PYTHON_TOOLCHAIN is only for source python packages. Is it?

On the other hand, packages in PYTHON_TOOLCHAIN are installed anyway because of other packages. So it seems that the change has no real benefit...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changing it with just "pip" signals that PYTHON_TOOLCHAIN is only for source python packages. Is it?

Indeed. I have updated the documentation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks. OK.

All lines of this file are ignored except the first.
3 changes: 0 additions & 3 deletions build/pkgs/six/spkg-install.in
View file
Open in desktop

This file was deleted.

14 changes: 13 additions & 1 deletion build/sage_bootstrap/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ def fix_checksum(self, package_name):
update.fix_checksum()

def create(self, package_name, version=None, tarball=None, pkg_type=None, upstream_url=None,
description=None, license=None, upstream_contact=None, pypi=False, source='normal'):
description=None, license=None, upstream_contact=None, pypi=False, source=None):
"""
Create a package

Expand All @@ -288,6 +288,14 @@ def create(self, package_name, version=None, tarball=None, pkg_type=None, upstre
if '-' in package_name:
raise ValueError('package names must not contain dashes, use underscore instead')
if pypi:
if source is None:
try:
if PyPiVersion(package_name, source='wheel').tarball.endswith('-none-any.whl'):
source = 'wheel'
else:
source = 'normal'
except PyPiError:
source = 'normal'
pypi_version = PyPiVersion(package_name, source=source)
Copy link
Collaborator

Choose a reason for hiding this comment

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

             if source is None:
                 try:
-                    source = 'wheel'
-                    if not PyPiVersion(package_name, source='wheel').tarball.endswith('-none-any.whl'):
+                    if PyPiVersion(package_name, source='wheel').tarball.endswith('-none-any.whl'):
+                        source = 'wheel'
+                    else
                         source = 'normal'
                 except PyPiError:

a bit simpler code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, done in 818468a

if source == 'normal':
if not tarball:
Expand All @@ -312,6 +320,10 @@ def create(self, package_name, version=None, tarball=None, pkg_type=None, upstre
license = pypi_version.license
if not upstream_contact:
upstream_contact = pypi_version.package_url
if upstream_url and not tarball:
tarball = upstream_url.rpartition('/')[2]
if tarball and source is None:
source = 'normal'
if tarball and not pkg_type:
# If we set a tarball, also make sure to create a "type" file,
# so that subsequent operations (downloading of tarballs) work.
Expand Down
2 changes: 1 addition & 1 deletion build/sage_bootstrap/cmdline.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ def make_parser():
'package_name', default=None, type=str,
help='Package name.')
parser_create.add_argument(
'--source', type=str, default='normal', help='Package source (one of normal, wheel, script, pip)')
'--source', type=str, default=None, help='Package source (one of normal, wheel, script, pip); default depends on provided arguments')
parser_create.add_argument(
Copy link
Collaborator

Choose a reason for hiding this comment

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

     parser_create.add_argument(
-        '--source', type=str, default=None, help='Package source (one of normal, wheel, script, pip; default depends on provided arguments)')
+        '--source', type=str, default=None, help='Package source (one of normal, wheel, script, pip); default depends on provided arguments')
     parser_create.add_argument(
         '--version', type=str, default=None, help='Package version')

following the style of other arguments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 818468a

'--version', type=str, default=None, help='Package version')
parser_create.add_argument(
Expand Down
Loading