sagemath/sage

python3 spkg-configure.m4: Require ensurepip

Closed this issue · 20 comments

On Debian, package python3-venv needs to be installed to un-sabotage python3.

Needed in #33812.

CC: @dimpase

Component: packages: standard

Author: Matthias Koeppe

Branch/Commit: 52dca7d

Reviewer: Tobias Diez, Dima Pasechnik

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

New commits:

eef9e41build/pkgs/python3/spkg-configure.m4: Check for module ensurepip
1340d95build/pkgs/python3/distros/debian.txt: Add python3-venv

Commit: 1340d95

Author: Matthias Koeppe

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

3a5323e.github/workflows/build.yml: Install python3-venv

Changed commit from 1340d95 to 3a5323e

comment:5

Shouldn't python3-venv then be better included in the base image, then installing it manually in the build workflow?

comment:6

It will already be in all future base images by the change in build/pkgs/python3/distros/debian.txt

comment:8

Replying to @mkoeppe:

It will already be in all future base images by the change in build/pkgs/python3/distros/debian.txt

Okay, then why add it manually in the build workflow? For testing purposes this is fine, but I guess it should be removed before merging this ticket.

comment:9

It's a no-op in the future and we can remove it in a later ticket, but not now because I want to use it for the tests of all the tickets that depend on it.

comment:10

Makes sense. Then I would propose to squash the current commits and remove python3-venv from the build workflow in a new commit. Then you could cherry pick the squashed commit in other tickets that depend on this one here.

Or at least add a comment in the workflow that it should be removed in the future.

Otherwise this looks good to me.

Changed commit from 3a5323e to 52dca7d

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

52dca7d.github/workflows/build.yml: Add comment on python3-venv
comment:12

I've added the comment, good idea.

Reviewer: Tobias Diez

comment:14

Let's get this in please

Changed reviewer from Tobias Diez to Tobias Diez, Dima Pasechnik

comment:15

lgtm

comment:16

Thanks!