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.

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

Use system python for generation of conway_polynomials and elliptic_curves #30934

Closed
tobiasdiez opened this issue Nov 18, 2020 · 18 comments
Closed

Comments

@tobiasdiez
Copy link
Contributor

Instead of sage's python, the installation of conway_polynomials and elliptic_curves now uses the system python. This enables one to call make conway_polynomials without a prior compilation of python (which is helpful for #30371). Since the installation of these packages uses python only to convert files into the right format, using the system python should be fine.

Depends on #30731

CC: @mkoeppe

Component: build

Reviewer: Tobias Diez

Issue created by migration from https://trac.sagemath.org/ticket/30934

@tobiasdiez tobiasdiez added this to the sage-9.3 milestone Nov 18, 2020
@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 18, 2020

comment:2

These packages declare $(PYTHON) as a dependency, so they are allowed to use Sage's python.

sage-system-python is not the correct choice here. It can be a wide range of Python versions (including 2.x), and certainly existence of the sqlite module in that python, as required in build/pkgs/elliptic_curves/spkg-install.py, is not guaranteed.
(Note #30627 renames this script to sage-bootstrap-python.)

@mkoeppe mkoeppe removed this from the sage-9.3 milestone Nov 18, 2020
@tobiasdiez
Copy link
Contributor Author

comment:3

Thanks for the feedback. So what is the correct choice if the user does have a suitable system python?

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 18, 2020

comment:4

When there is a suitable system python3, then Sage creates a venv over it, so python3 works. When there is no suitable system python3, then Sage builds one, so python3 also works.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 19, 2020

Changed commit from 218fe09 to 9f15544

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 19, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

9f15544Use python3 instead of system-python

@tobiasdiez
Copy link
Contributor Author

comment:6

Replying to @mkoeppe:

When there is a suitable system python3, then Sage creates a venv over it, so python3 works. When there is no suitable system python3, then Sage builds one, so python3 also works.

Thanks! I've now used python3 and it works indeed in the context of #30371.

@tobiasdiez tobiasdiez added this to the sage-9.3 milestone Nov 25, 2020
@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 25, 2020

comment:8

This change should not be done for one package but uniformly for all packages - in #30731.
You can push the branch from here to that ticket to get it started.

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 25, 2020

Dependencies: #30731

@mkoeppe mkoeppe removed this from the sage-9.3 milestone Nov 25, 2020
@tobiasdiez
Copy link
Contributor Author

comment:9

Ok, then close this ticket here, although I don't really see why #30731 could't build upon this ticket.

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 25, 2020

comment:10

Because this ticket on its own makes the codebase less uniform.

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 25, 2020

Reviewer: Tobias Diez

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 25, 2020

Changed author from Tobias Diez to none

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 18, 2020

Changed commit from 9f15544 to 7cb43a5

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 18, 2020

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

f7323eeMerge branch 'develop' of git://github.com/sagemath/sage into public/build/sharedSystemPython
7cb43a5Remove other instances of sage-python23

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 19, 2020

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

df2822cUpdate documentation regarding use of python3 vs. sage-bootstrap-python

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 19, 2020

Changed commit from 7cb43a5 to df2822c

@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 19, 2020

Changed commit from df2822c to none

@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 19, 2020

Changed branch from public/build/sharedSystemPython to none

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants