"make doc-clean" should remove inventory, doctrees

[mkoeppe]

Followup to #29310, #33104

We could consider to extend make doc-clean so that it also removes inventory, doctrees.
This should suffice to resolve incremental docbuild problems.

CC: @jhpalmieri @kwankyu

Component: build

Author: John Palmieri

Branch/Commit: 8244126

Reviewer: Matthias Koeppe

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

[jhpalmieri]

comment:1

A related change that may be taken care of, if we follow the proposal on this ticket:

diff --git a/build/make/Makefile.in b/build/make/Makefile.in
index d42b261b3d..75bfa250df 100644
--- a/build/make/Makefile.in
+++ b/build/make/Makefile.in
@@ -340,9 +340,9 @@ doc-html: sagemath_doc_html
 
 # 'doc-html-no-plot': build docs without building the graphics coming
 # from the '.. plot' directive, in case you want to save a few
-# megabytes of disk space. 'doc-clean' is a prerequisite because the
+# megabytes of disk space. 'doc-uninstall' is a prerequisite because the
 # presence of graphics is cached in src/doc/output.
-doc-html-no-plot: doc-clean
+doc-html-no-plot: doc-uninstall
        +$(MAKE_REC) SAGE_DOCBUILD_OPTS="$(SAGE_DOCBUILD_OPTS) --no-plot" doc-html
 
 # Using mathjax is actually the only options, but we keep

[jhpalmieri]

comment:2

For typical Sage packages, what is the difference between make PKG-clean and make PKG-uninstall?

[mkoeppe]

comment:3

For normal Sage packages, make PKG-clean really uninstalls and should be renamed (with deprecation) to make PKG-uninstall (as discussed in #29097).

There's not really much to clean because the build tree for a package is ephemeral - unless sage -i -s was used or a build error occurred. So I suppose at some point in the future we could have make PKG-clean remove the build directories for the package in SAGE_LOCAL/var/tmp/sage/build/.

[jhpalmieri]

comment:5

We can do this:

diff --git a/src/doc/Makefile b/src/doc/Makefile
index 62fb1cafd18..9bf9a1686dc 100644
--- a/src/doc/Makefile
+++ b/src/doc/Makefile
@@ -16,6 +16,8 @@ clean:
        rm -rf en/reference/*/sage
        rm -rf en/reference/sage
        rm -f common/*.pyc
+       rm -rf ../../local/share/doc/sage/inventory
+       rm -rf ../../local/share/doc/sage/doctrees
 
 # Sources generated at build time. (For sources generated at bootstrap time, see bootstrap.)
 doc-src:

I'm not sure how to do this more cleanly without moving the doc-clean target into build/make/Makefile.in, in which case ./configure probably gets run when you run make doc-clean. The problem is that no helpful environment variables are defined in the top-level Makefile.

[jhpalmieri]

Branch: u/jhpalmieri/33705-make-doc-clean

[jhpalmieri]

Author: John Palmieri

[jhpalmieri]

Commit: 589d844

[jhpalmieri]

comment:7

Ready for review.

I don't think we need the change from comment:1, but it does raise a question: if I run make doc-html-no-plot, I get an error:

make[1]: *** No rule to make target `doc-clean', needed by `doc-html-no-plot'.  Stop.

Do make targets (like doc-clean) from the top-level Makefile need to reproduced in build/make/Makefile.in if we want to refer to them?

---

New commits:

2633cc9	build/sage_bootstrap/uninstall.py: Do not refuse to uninstall packages that do not exist in the source tree
dbc61b3	build/sage_bootstrap/uninstall.py: Do not override the stamp file location from environment variable
3a5ea93	build/make/Makefile.in: New implicit targets %-SAGE_LOCAL-uninstall, %-SAGE_VENV-uninstall
65a5604	trac 29097: change "make SPKG-clean" to "make SPKG-uninstall".
4609606	build/make/Makefile.in: Also add implicit rule %-uninstall
e9b30bb	build/sage_bootstrap/uninstall.py: Update comment
fef04d0	build/sage_bootstrap/uninstall.py: Clarify that the sage_local argument is SAGE_LOCAL or SAGE_VENV
9693865	build/make/Makefile.in: Clarify SAGE_I_TARGETS
72695aa	build/sage_bootstrap/uninstall.py: Fix typo in docstring
589d844	trac 33705: "make doc-clean" should delete doctrees and inventory

[sagetrac-git]

Changed commit from 589d844 to aa5ddb6

[sagetrac-git]

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

aa5ddb6

trac 33705: 'make doc-clean' should remove inventory, doctrees

[jhpalmieri]

comment:9

(Ignore the first bunch of commits. I think this should just be based on develop, so that's what I've now done.)

[mkoeppe]

Dependencies: #29097

[mkoeppe]

comment:11

Oh, OK

[mkoeppe]

Changed dependencies from #29097 to none

[mkoeppe]

comment:12

Replying to @jhpalmieri:

if I run make doc-html-no-plot, I get an error:

make[1]: *** No rule to make target `doc-clean', needed by `doc-html-no-plot'.  Stop.

Do make targets (like doc-clean) from the top-level Makefile need to reproduced in build/make/Makefile.in if we want to refer to them?

You can use (cd $(SAGE_ROOT) && $(MAKE) ....)

[mkoeppe]

comment:13

Also, SAGE_LOCAL is not set in the top-level Makefile.

[jhpalmieri]

comment:14

Why is $$SAGE_LOCAL used in the clean target? Should we make this change?

diff --git a/Makefile b/Makefile
index 88107dbb37..e4d36a0308 100644
--- a/Makefile
+++ b/Makefile
@@ -98,13 +98,14 @@ pypi-sdists: sage_setup
 
 SAGE_ROOT = $(CURDIR)
 SAGE_SRC = $(SAGE_ROOT)/src
+SAGE_LOCAL = $(SAGE_ROOT)/local
 
 clean:
        @echo "Deleting package build directories..."
        if [ -f "$(SAGE_SRC)"/bin/sage-env-config ]; then \
            . "$(SAGE_SRC)"/bin/sage-env-config; \
-           if [ -n "$$SAGE_LOCAL" ]; then \
-               rm -rf "$$SAGE_LOCAL/var/tmp/sage/build"; \
+           if [ -n "$(SAGE_LOCAL)" ]; then \
+               rm -rf "$(SAGE_LOCAL)/var/tmp/sage/build"; \
            fi; \
        fi
 

[mkoeppe]

comment:15

SAGE_LOCAL is set by sage-env-config and is not known to make.
So one needs to do the substitution in the shell

[jhpalmieri]

comment:16

Okay, for the purposes of make doc-clean, would it be appropriate to set SAGE_LOCAL to be $(SAGE_ROOT)/local?

[mkoeppe]

comment:17

I think doc-clean should use the same method as clean: Obtain the correct SAGE_LOCAL via sage-env-config

[sagetrac-git]

Changed commit from aa5ddb6 to fe61ce5

[sagetrac-git]

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

fe61ce5

trac 33705: 'make doc-clean' should remove inventory, doctrees

[jhpalmieri]

comment:19

Okay, let's try this. This also fixes the bug with make doc-html-no-plot.

[sagetrac-git]

Changed commit from fe61ce5 to 8244126

[sagetrac-git]

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

8244126

trac 33705: 'make doc-clean' should remove inventory, doctrees

[mkoeppe]

Reviewer: Matthias Koeppe

[jhpalmieri]

comment:22

Thank you!

[vbraun]

Changed branch from u/jhpalmieri/33705-make-doc-clean to 8244126