sagemath/sage

Update/fix top-level makefile target fast-rebuild-clean (used in Docker build)

fchapoton opened this issue · 18 comments

fix something that breaks the build of docker:

https://gitlab.com/sagemath/sage/-/jobs/870630473

CC: @mkoeppe @dimpase @jhpalmieri @saraedum

Component: scripts

Author: Matthias Koeppe

Branch/Commit: 045d01b

Reviewer: Frédéric Chapoton

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

Commit: 45e9bd1

New commits:

45e9bd1trying to fix our Makefile
comment:2

I presume the result of this fix should be visible soon on the GitLab runner, right?

comment:3

no, because this target inside the makefile is only triggered by the docker-building script, so only for new develop branches (as far as I understand this mess)

comment:4

Note that tab characters are part of Makefile syntax

comment:5

src/build is not used anymore. It is now build/pkgs/sagelib/src/build.

comment:6

then please make a better branch

comment:7

Overall this make target is problematic and it would be best to clean it up more thoroughly.

  • It not only cleans (as the name of the target suggests) but actually uninstalls sagelib (in a rather brutal way)
  • This is the same defect as noted in #21775 (make distclean: Don't delete $SAGE_ROOT/local)
  • Also, it refers to local directly instead of using SAGE_LOCAL (which can be configured using --prefix)
  • The latter is important for #29536 - make docker images from GitHub CI workflow and regular Sage Docker images interoperable

Changed commit from 45e9bd1 to 045d01b

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

045d01bMakefile (fast-rebuild-clean): Use new location of src/build, move uninstallation to Dockerfile
comment:10

Try this branch please

Author: Matthias Koeppe

comment:11

looks good, but I cannot test

comment:12

Replying to @fchapoton:

looks good, but I cannot test

Some GitLab pipelines are running and complete, e.g.
this one https://gitlab.com/sagemath/dev/trac/-/pipelines/221142937
for #30925

The pipeline for the current ticket is in the queue:
https://gitlab.com/sagemath/dev/trac/-/pipelines/221294945

I have no idea how many people are using these, and what it takes to test functionality of the current ticket there.

Reviewer: Frédéric Chapoton

comment:13

ok, let it be. It looks harmless enough, and all it could do would be to help fix the docker building..

Changed branch from public/30960 to 045d01b