Opened 3 years ago
Closed 3 years ago
#28041 closed enhancement (fixed)
py3: Docker images for python3-based Sage
Reported by: | galois | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.0 |
Component: | distribution | Keywords: | docker, py3, gitlab-ci, days101 |
Cc: | saraedum, zerline, roed | Merged in: | |
Authors: | Samuel Lelièvre, Julian Rüth | Reviewers: | David Roe |
Report Upstream: | N/A | Work issues: | |
Branch: | ca24332 (Commits, GitHub, GitLab) | Commit: | ca243324e4d9706a0c189193beebec09eed33a2b |
Dependencies: | Stopgaps: |
Description (last modified by )
Docker images for python3-based Sage are useful in general, and in particular for continuous integration or other testing workflows for packages that depend on Sage.
The infrastructure introduced by #24655 automatically produces Docker images for python2-based Sage. In order to extend it to also produce python3-based ones,
- we update the
micro_release
target formake
inMakefile
to not remove theconfig
directory
- we adapt
.gitlab-ci.yml
anddocker/Dockerfile
, duplicating all the docker targets for py3, creating aWITH_PYTHON
and aDOCKER_TAG_SUFFIX
environment variables, and using them in the shell scriptsdocker/*.sh
.
To play with the generated Python 2/3 docker images, you can go to https://gitlab-hooks-flau3jeaza-ew.a.run.app/status/trac/branch/u%2Fsaraedum%2F28041
Change History (57)
comment:1 Changed 3 years ago by
- Cc saraedum added
- Component changed from PLEASE CHANGE to distribution
- Description modified (diff)
- Keywords docker py3 gitlab-ci added
- Summary changed from MR22: Docker python3 to py3: Docker images for python3-based Sage
- Type changed from PLEASE CHANGE to enhancement
comment:2 Changed 3 years ago by
comment:3 Changed 3 years ago by
- Keywords days101 added
- Status changed from needs_review to needs_work
comment:4 Changed 3 years ago by
I'm trying to get this working, but the build is stalling out at:
Step 61/80 : RUN sudo $SAGE_ROOT/sage --nodotsage -c "install_scripts('/usr/bin')" ---> Running in d4ce02ca02d6 Forcing sage-location, probably because a new package was installed. Cleaning up, do not interrupt this. Done cleaning. Traceback (most recent call last): File "/home/sage/sage/src/bin/sage-eval", line 4, in <module> from sage.all import * ImportError: No module named sage.all The command '/bin/sh -c sudo $SAGE_ROOT/sage --nodotsage -c "install_scripts('/usr/bin')"' returned a non-zero code: 1
The problem is that SAGE_PYTHON_VERSION
is not being set properly when running $SAGE_ROOT/sage
.
This is because there are two copies of sage-env-config
, where SAGE_PYTHON_VERSION
is defined: One under $SAGE_ROOT/src/bin
and one other $SAGE_ROOT/local/bin
. This is normal when doing Sage development, and I believe, if it exists, it's the one under src/bin
that will be used by default when developing/building.
The problem in this case is that although the one in local/bin
correctly has SAGE_PYTHON_VERSION=3
, the one in src/bin
still has SAGE_PYTHON_VERSION=2
for some reason.
I wonder if this is a remnant from an earlier build stage that was cached, but not properly updated for some reason (I thought that different --build-arg
flags would avoid use incompatible cached images built with different --build-arg
s, but it's possible that that arg isn't being included in the build early enough.
comment:5 Changed 3 years ago by
The problem seems to start here:
Step 56/80 : RUN make micro_release ---> Using cache ---> 2fc1a353e2ac
When running make micro_release
, ./configure
gets re-run, but without any flags we passed it previously. So it doesn't preserve our --with-python=3
from earlier.
comment:6 Changed 3 years ago by
- Cc zerline added
(CC Odile who was interested in having a working Python 3 Docker image)
comment:7 Changed 3 years ago by
Sorry for this. We had already figured the broken ./configure
when micro_release
is run. I thought that Samuel had pushed a fix for this here already.
comment:8 Changed 3 years ago by
Isn't the preservation of config
good enough for this? (See the changeset here.)
comment:9 Changed 3 years ago by
I was just about to say: I think the preservation of config.status
is good enough in fact.
comment:10 Changed 3 years ago by
Specifically, one of the prereqs of micro_release
is the bdist-clean
target, which in turn calls $(MAKE) misc-clean
which deletes config.status
, forcing configure
to be re-run.
comment:11 follow-up: ↓ 13 Changed 3 years ago by
What the bdist-clean
? Is this a automake target? Otherwise is it reasonable that the build system's config is visible after running it?
comment:12 Changed 3 years ago by
Not sure, but I think just swapping sagelib-clean
and bdist-clean
in the prereqs for micro_release
should suffice. It's fragile, but I think it will work. Testing now.
comment:13 in reply to: ↑ 11 Changed 3 years ago by
Replying to saraedum:
What the
bdist-clean
? Is this a automake target? Otherwise is it reasonable that the build system's config is visible after running it?
No, this is just a pseudo-target in Sage's top-level Makefile. I have no idea why specifically it's named that. It's also used by the fast-rebuild-clean
target. It just calls clean
(which is in build/make/Makefile
for some reason), and then misc-clean
.
All clean
does apparently is:
clean: @echo "Deleting package build directories..." rm -rf "$(SAGE_LOCAL)/var/tmp/sage/build"
which is certainly worth doing. But maybe this needs to be detangled a bit...
comment:14 Changed 3 years ago by
The following files in docker/
probably still need changes:
build-docker.sh
pull-gitlab.sh
push-dockerhub.sh
push-gitlab.sh
setup-make-parallelity.sh
test-dev.sh
update-env.sh
comment:15 Changed 3 years ago by
Yes, most likely. One I just get the docker build working locally I will try adding all the other relevant updates.
It would be nice for starters to get a Python 3 image on Docker Hub, even if I have to push it manually, then get future ones building automatically.
comment:16 Changed 3 years ago by
I think that once your Dockerfile goes through, you just need to add your branch to build-from-clean
, pull your image from the GitLab registry and push it to Docker Hub.
comment:17 Changed 3 years ago by
For Odile's use I went ahead and pushed a prototype of this up to Dockerhub: sagemath/sagemath-dev-py3:alpha
Unfortunately it still has a bug. The sage-entrypoint
still tries to run make build
(understandably) which results in re-configuring (needlessly) and breaking things.
A workaround for now is to set the PREREQ_OPTIONS variable which is used by Sage's top-level Makefile when it runs configure. So you can set this when running the container like:
docker run -ti --rm -e PREREQ_OPTIONS='--with-python=3' sagemath/sagemath-dev-py3:alpha
Next up, I think we should modify the Docker build to at least keep config.status so that this doesn't keep happening.
comment:18 Changed 3 years ago by
Apparently I was not able to successfully upload the Docker image to hub.docker.com because (AFAICT) of issue.
But I was able to upload it to our image registry on gitlab.com. So try:
docker run -ti --rm -e PREREQ_OPTIONS='--with-python=3' registry.gitlab.com/sagemath/dev/sage/sagemath-dev-py3:alpha
You can also use this image as a base for another Docker image by doing something like:
FROM registry.gitlab.com/sagemath/dev/sage/sagemath-dev-py3:alpha ENV PREREQ_OPTIONS='--with-python=3' ...
That is, you would ensure PREREQ_OPTIONS
is set as an environment variable in the new image, by default.
This is not ideal and should be fixed, but it's a start anyways...
comment:19 Changed 3 years ago by
comment:20 Changed 3 years ago by
- Branch changed from u/galois/mrs/22/docker-python3 to u/saraedum/mrs/22/docker-python3
comment:21 Changed 3 years ago by
- Commit changed from 7c23b4287435e9df1b7ac03c113d861f2ca3a7bd to 2f108acfd0c41e736d327772a6e144a10337a028
Branch pushed to git repo; I updated commit sha1. New commits:
2f108ac | Build py3 from clean
|
comment:22 Changed 3 years ago by
- Commit changed from 2f108acfd0c41e736d327772a6e144a10337a028 to f5ba6d3c54fbb4e68e7a9ae891e3bd3d8eb581ed
Branch pushed to git repo; I updated commit sha1. New commits:
f5ba6d3 | Fix VARIABLES of Python3 builds
|
comment:23 Changed 3 years ago by
- Commit changed from f5ba6d3c54fbb4e68e7a9ae891e3bd3d8eb581ed to a21467a28fe44b32f66ed9b669ddebd90fb8fff0
Branch pushed to git repo; I updated commit sha1. New commits:
a21467a | Limit docker build output
|
comment:24 Changed 3 years ago by
- Commit changed from a21467a28fe44b32f66ed9b669ddebd90fb8fff0 to 6619e3171933f97c4cda24823d055def2ff4c86f
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
6619e31 | Limit docker build output
|
comment:25 Changed 3 years ago by
- Commit changed from 6619e3171933f97c4cda24823d055def2ff4c86f to 497077d29f3ad5d1cea4d6edc13fce442d1858c6
Branch pushed to git repo; I updated commit sha1. New commits:
497077d | Merge latest beta into docker-py3
|
comment:26 Changed 3 years ago by
- Commit changed from 497077d29f3ad5d1cea4d6edc13fce442d1858c6 to 155e3ac26de16a7808287d618c2b8ca23891e617
Branch pushed to git repo; I updated commit sha1. New commits:
155e3ac | Build docker in parallel
|
comment:27 Changed 3 years ago by
- Commit changed from 155e3ac26de16a7808287d618c2b8ca23891e617 to 7c9ea5510b96a9ca0887c80a389e59101f159fb7
Branch pushed to git repo; I updated commit sha1. New commits:
7c9ea55 | Build docker in parallel
|
comment:28 Changed 3 years ago by
- Commit changed from 7c9ea5510b96a9ca0887c80a389e59101f159fb7 to d36ffe11333b5fc3181de80a547c331da4b14b81
Branch pushed to git repo; I updated commit sha1. New commits:
d36ffe1 | Push Python 3 docker images to *-py3
|
comment:29 Changed 3 years ago by
- Commit changed from d36ffe11333b5fc3181de80a547c331da4b14b81 to a229ffcedaf9d27fb7389d6612a65e58ead938e3
Branch pushed to git repo; I updated commit sha1. New commits:
a229ffc | Undo docker changes
|
comment:30 Changed 3 years ago by
- Status changed from needs_work to needs_review
comment:31 Changed 3 years ago by
See #28367 for the related README update.
comment:32 Changed 3 years ago by
We pushed initial docker -py3
images to the Docker Hub.
comment:33 Changed 3 years ago by
- Commit changed from a229ffcedaf9d27fb7389d6612a65e58ead938e3 to 22da0878d4de129b13b4847c4090808601d9b27a
Branch pushed to git repo; I updated commit sha1. New commits:
22da087 | do not remove config.status from dev image
|
comment:34 Changed 3 years ago by
- Status changed from needs_review to needs_work
- Work issues set to check that the sagemath-dev image actually works
New commits:
22da087 | do not remove config.status from dev image
|
comment:35 Changed 3 years ago by
- Status changed from needs_work to needs_review
- Work issues check that the sagemath-dev image actually works deleted
comment:36 follow-up: ↓ 38 Changed 3 years ago by
When this is for sure ready can we squash some of these commits?
comment:37 follow-up: ↓ 39 Changed 3 years ago by
Just out of curiosity, is there a specific meaning to having a section in .gitlab-ci.yml
that begins with a .
, as in .py3
? Does that just emphasize that it's not a real build stage (rather a "mix-in" for multiple build stages)? Or is that a syntax specifically recognized by the file format?
If I understand correctly, this is now keeping config.status
in the image. But what if someone runs sage -i <some_optional_package>
in one of these py3 images? In other words, if a user (or dockerfile author) does something that still results in ./configure
being re-run, will this keep the --with-python=3
setting?
I fear not, since we still haven't fully resolved #27373 (which related to system package settings specifically, but more broadly to retaining the values of flags passed to ./configure
). The larger problem of course being that we have "runtime" features like sage -i
that depend on running ./configure
at all...
comment:38 in reply to: ↑ 36 Changed 3 years ago by
Replying to embray:
When this is for sure ready can we squash some of these commits?
Feel free to squash.
comment:39 in reply to: ↑ 37 Changed 3 years ago by
Replying to embray:
Just out of curiosity, is there a specific meaning to having a section in
.gitlab-ci.yml
that begins with a.
, as in.py3
? Does that just emphasize that it's not a real build stage (rather a "mix-in" for multiple build stages)? Or is that a syntax specifically recognized by the file format?
Without out the .
it would be a build job. As a default as port of the test
stage I believe.
If I understand correctly, this is now keeping
config.status
in the image. But what if someone runssage -i <some_optional_package>
in one of these py3 images? In other words, if a user (or dockerfile author) does something that still results in./configure
being re-run, will this keep the--with-python=3
setting?
Does sage -i
run configure
when config.status
is present?
I fear not, since we still haven't fully resolved #27373 (which related to system package settings specifically, but more broadly to retaining the values of flags passed to
./configure
). The larger problem of course being that we have "runtime" features likesage -i
that depend on running./configure
at all...
I don't think making --with-python=3
persistent is within the scope of this ticket. But it's certainly a problem that we should be aware of.
comment:40 Changed 3 years ago by
- Commit changed from 22da0878d4de129b13b4847c4090808601d9b27a to c3d9c146ae520ed9198835bc3c100428f9a9b26a
Branch pushed to git repo; I updated commit sha1. New commits:
c3d9c14 | Keep misc-clean around longer in Makefile
|
comment:41 Changed 3 years ago by
- Commit changed from c3d9c146ae520ed9198835bc3c100428f9a9b26a to 54319874059f41512b6e812f020aed93eb7d0c0e
Branch pushed to git repo; I updated commit sha1. New commits:
5431987 | Merge remote-tracking branch 'trac/develop' into 28041
|
comment:42 Changed 3 years ago by
- Commit changed from 54319874059f41512b6e812f020aed93eb7d0c0e to cb61f30d371a69c7b0385b5b300c447774b8caa8
comment:43 Changed 3 years ago by
- Commit changed from cb61f30d371a69c7b0385b5b300c447774b8caa8 to 9a9dd58910e1b04f1e21ae23673022592513b3b8
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
9a9dd58 | Merge remote-tracking branch 'trac/develop' into 28041
|
comment:44 Changed 3 years ago by
- Branch u/saraedum/mrs/22/docker-python3 deleted
- Commit 9a9dd58910e1b04f1e21ae23673022592513b3b8 deleted
- Description modified (diff)
comment:45 Changed 3 years ago by
- Branch set to u/saraedum/28041
comment:46 Changed 3 years ago by
- Commit set to 89c78f8b903f225a5f2b18cc41adc245c9326964
Branch pushed to git repo; I updated commit sha1. New commits:
89c78f8 | Cleanup py-3 script
|
comment:47 Changed 3 years ago by
- Description modified (diff)
the docker images should be available on binder shortly: https://gitlab.com/sagemath/dev/trac/pipelines/81996545
comment:48 Changed 3 years ago by
- Cc roed added
comment:49 Changed 3 years ago by
- Commit changed from 89c78f8b903f225a5f2b18cc41adc245c9326964 to 1d02785aa8dca73ba9d19f9fc486d5bed8c6200a
Branch pushed to git repo; I updated commit sha1. New commits:
1d02785 | Add a test-hook for build-from-clean
|
comment:50 Changed 3 years ago by
I started a full build-from-clean
here: https://gitlab.com/sagemath/dev/trac/pipelines/82005383
comment:51 Changed 3 years ago by
- Commit changed from 1d02785aa8dca73ba9d19f9fc486d5bed8c6200a to ca243324e4d9706a0c189193beebec09eed33a2b
Branch pushed to git repo; I updated commit sha1. New commits:
ca24332 | Do not run build-from-latest for explicit builds
|
comment:52 Changed 3 years ago by
The Python 2 and Python 3 images seem to be functional. (At least they work in Binder.)
comment:53 Changed 3 years ago by
- Reviewers set to David Roe
- Status changed from needs_review to positive_review
Looks good to me. Both Python 2 and Python 3 built fine.
comment:54 Changed 3 years ago by
- Status changed from positive_review to needs_work
Not sure if this comes from the changes here, but there's a MAKEFLAGS=w -j -l7.8 --jobserver-fds=3,4 V=1
in the output. Let me try to see who put the w
there.
comment:55 Changed 3 years ago by
- Status changed from needs_work to positive_review
Ok. That's how it's supposed to work https://lists.gnu.org/archive/html/help-make/2016-03/msg00009.html.
comment:56 Changed 3 years ago by
- Milestone changed from sage-8.9 to sage-9.0
moving milestone to 9.0 (after release of 8.9)
comment:57 Changed 3 years ago by
- Branch changed from u/saraedum/28041 to ca243324e4d9706a0c189193beebec09eed33a2b
- Resolution set to fixed
- Status changed from positive_review to closed
Still need to make the changes to the
docker/*.sh
shell scripts.