#29032 closed enhancement (duplicate)

spkg-configure.m4 for python3 with SAGE_LOCAL as a venv for system pythons

Reported by: embray Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: build: configure Keywords:
Cc: Merged in:
Authors: Erik Bray Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: u/mkoeppe/build/configure/python3 (Commits, GitHub, GitLab) Commit: 99b032ea660ca4f0ce4a0197aaf0810dd8460f3e
Dependencies: #29357 Stopgaps:

Status badges

Description (last modified by embray)

This is an experimental alternative proposal to #27824.

It isn't yet as "complete" as #27824 as it lacks some of the fixes (e.g. the patches to Pillow) which I didn't need for my tests, but which may still be needed on other platforms (TBD).


The spkg-configure.m4 itself is taken almost directly from #27824, as most of it was great, except for a couple minor changes:

  • No temporary "config_venv" to test the system Python's packages; instead just run the system Python (if detected) with the -S flag.
  • AC_SUBST([SAGE_SYSTEM_PYTHON]) instead of AC_SUBST([PYTHON_FOR_VENV])

The changes to build/make/deps are also inspired by #27824 but are simpler in my opinion: Instead of creating a virtualenv nested inside SAGE_LOCAL, SAGE_LOCAL itself is just treated as a virtualenv, and we install the necessary files for the Python interpreter to treat it as such, which keeps things simpler, as SAGE_LOCAL + source sage-env already fills the same purpose as a virtualenv.


The changes to src/sage/env.py are reworked just a little bit (in particular, the version in #27824 was buggy on Cygwin, as it would privilege system DLLs over DLLs in SAGE_LOCAL).

This was tested on Ubuntu 18.04, and make ptestlong passes from a clean build with the system Python detected. No guarantees about OSX or other platforms.

Change History (16)

comment:1 Changed 21 months ago by embray

  • Description modified (diff)

comment:2 Changed 21 months ago by embray

See also #29033 which expands on this to add support for a system Python 3.6.

comment:3 Changed 21 months ago by embray

  • Status changed from new to needs_review

comment:4 follow-up: Changed 21 months ago by mkoeppe

Thanks for working of this. I have started to merge a few things from this branch.

One concern that I have is that in this approach, there is a $SAGE_LOCAL/bin/activate which signals to users that it would "activate" $SAGE_LOCAL. But it doesn't. It only activates the venv.

comment:5 follow-up: Changed 21 months ago by mkoeppe

A comment regarding the variable SAGE_SYSTEM_PYTHON: I think this is not specific enough because build/bin/sage-system-python means something different from that.

comment:6 in reply to: ↑ 4 ; follow-up: Changed 21 months ago by embray

Replying to mkoeppe:

Thanks for working of this. I have started to merge a few things from this branch.

One concern that I have is that in this approach, there is a $SAGE_LOCAL/bin/activate which signals to users that it would "activate" $SAGE_LOCAL. But it doesn't. It only activates the venv.

Are you sure? I'll double check, but with this branch as written it should not install any bin/activate scripts.

comment:7 in reply to: ↑ 5 Changed 21 months ago by embray

Replying to mkoeppe:

A comment regarding the variable SAGE_SYSTEM_PYTHON: I think this is not specific enough because build/bin/sage-system-python means something different from that.

Right, that's a bit confusing. I wasn't totally sure how I felt about that variable name either, but I couldn't think of something better. I think it does make some sense in that just as build/bin/sage-system-python runs the system Python, SAGE_SYSTEM_PYTHON points to the path to the system Python if it is being used to run Sage. But I'm open to other ideas here.

comment:8 in reply to: ↑ 6 Changed 21 months ago by embray

Replying to embray:

Replying to mkoeppe:

Thanks for working of this. I have started to merge a few things from this branch.

One concern that I have is that in this approach, there is a $SAGE_LOCAL/bin/activate which signals to users that it would "activate" $SAGE_LOCAL. But it doesn't. It only activates the venv.

Are you sure? I'll double check, but with this branch as written it should not install any bin/activate scripts.

I confirmed that as expected it does not install an activate script. Is it possible you had that hanging around from something else?

comment:9 Changed 19 months ago by mkoeppe

  • Dependencies changed from #29002 to #29357

comment:10 Changed 19 months ago by mkoeppe

  • Branch changed from u/embray/build/configure/python3 to u/mkoeppe/build/configure/python3

comment:11 Changed 19 months ago by mkoeppe

  • Commit changed from b6b9c3abf499407bd18c9eddabe9f9557ab6dc67 to 8ce6517a17c2f4a8a6706e6fb2fbd0019c729959

Rebased on top of #29357 (and 9.1.beta7)


New commits:

80b3933Re-configure if src/bin/sage-env-config.in changes
f1b3424Also reconfigure if build/bin/sage-build-env-config.in or build/pkgs/sage_conf/src/*.in change
8ce6517Add spkg-configure.m4 for Python 3 to detect and use a system Python 3.

comment:12 Changed 19 months ago by git

  • Commit changed from 8ce6517a17c2f4a8a6706e6fb2fbd0019c729959 to 99b032ea660ca4f0ce4a0197aaf0810dd8460f3e

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

f5f493fRe-configure if src/bin/sage-env-config.in changes
fe6aa38Also reconfigure if build/bin/sage-build-env-config.in or build/pkgs/sage_conf/src/*.in change
99b032eAdd spkg-configure.m4 for Python 3 to detect and use a system Python 3.

comment:13 Changed 19 months ago by mkoeppe

Rebased again on top of #29357 (and 9.1.beta8)

comment:14 Changed 19 months ago by mkoeppe

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

@embray I have adopted the approach of using venv.EnvBuilder on SAGE_LOCAL in #27824. Thanks for your work on it.

Could you elaborate on what the environment variable VIRTUAL_ENV is needed for?

The present ticket can be closed as a duplicate.

comment:15 Changed 19 months ago by dimpase

  • Reviewers set to Dima Pasechnik
  • Status changed from needs_review to positive_review

comment:16 Changed 16 months ago by chapoton

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