Opened 5 months ago

Closed 4 months ago

#30934 closed enhancement (duplicate)

Use system python for generation of conway_polynomials and elliptic_curves

Reported by: gh-tobiasdiez Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: build Keywords:
Cc: mkoeppe Merged in:
Authors: Reviewers: Tobias Diez
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #30731 Stopgaps:

Status badges

Description

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.

Change History (16)

comment:1 Changed 5 months ago by gh-tobiasdiez

  • Status changed from new to needs_review

comment:2 Changed 5 months ago by mkoeppe

  • Milestone changed from sage-9.3 to sage-duplicate/invalid/wontfix

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

comment:3 Changed 5 months ago by gh-tobiasdiez

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

comment:4 follow-up: Changed 5 months ago by 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.

comment:5 Changed 5 months ago by git

  • Commit changed from 218fe09a5873f61fe4f0efc004502dea85f56de3 to 9f15544b8ddb9ca2770c6eec88ab80180672ce7e

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

9f15544Use python3 instead of system-python

comment:6 in reply to: ↑ 4 Changed 5 months ago by gh-tobiasdiez

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.

comment:7 Changed 5 months ago by gh-tobiasdiez

  • Milestone changed from sage-duplicate/invalid/wontfix to sage-9.3

comment:8 Changed 5 months ago by mkoeppe

  • Dependencies set to #30731
  • Milestone changed from sage-9.3 to sage-duplicate/invalid/wontfix

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.

comment:9 Changed 5 months ago by gh-tobiasdiez

  • Status changed from needs_review to positive_review

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

comment:10 Changed 5 months ago by mkoeppe

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

comment:11 Changed 5 months ago by mkoeppe

  • Authors Tobias Diez deleted
  • Reviewers set to Tobias Diez

comment:12 Changed 4 months ago by git

  • Commit changed from 9f15544b8ddb9ca2770c6eec88ab80180672ce7e to 7cb43a574c3869e27824b9b8202310910831ad44
  • Status changed from positive_review to needs_review

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

comment:13 Changed 4 months ago by gh-tobiasdiez

  • Status changed from needs_review to positive_review

comment:14 Changed 4 months ago by git

  • Commit changed from 7cb43a574c3869e27824b9b8202310910831ad44 to df2822c5c130811bcba1d7e65743b100444083dd
  • Status changed from positive_review to needs_review

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

comment:15 Changed 4 months ago by mkoeppe

  • Branch public/build/sharedSystemPython deleted
  • Commit df2822c5c130811bcba1d7e65743b100444083dd deleted
  • Status changed from needs_review to positive_review

comment:16 Changed 4 months ago by chapoton

  • Resolution set to duplicate
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.