Trac #29097: build/make/Makefile.in: Rename make targets SPKG-clean t…

…o SPKG-uninstall

Cleaning means to remove build artifacts from the directory `@builddir@`
(= SAGE_ROOT).
But these targets *uninstall* packages from `@prefix@=$SAGE_LOCAL`.

Hence we rename `SPKG-clean` targets to `SPKG-uninstall` (but keep
`SPKG-clean` as an alias.) This also unifies the targets between normal
and script packages (the latter used `-uninstall` already).

We also add implicit rules that allow users to uninstall packages that
no longer exist in the source tree. This is preparation for #34203. To
test this, switch to the branch of #33530 and build one of the packages
added, for example `make fastjsonschema`. Switch back to current
`develop` and try to remove it (does not work). Switch to the branch
here and run `make fastjsonschema-uninstall` (works).

Related:
- #29310 - introduced `make doc-uninstall`
- #33705 `make doc-clean` should remove inventory, doctrees
- #29386 Install script packages via `sage-spkg`
- #29626 ./configure --enable-SPKG does not work for optional pip
packages
- #29868 pip-installable packages `sage-doc-html`, `sage-doc-pdf`

URL: https://trac.sagemath.org/29097
Reported by: mkoeppe
Ticket author(s): John Palmieri, Matthias Koeppe
Reviewer(s): Matthias Koeppe, John Palmieri

build/bin/sage-site

@@ -170,7 +170,7 @@ if [ "$1" = '-i' ]; then
     # First, uninstall the packages if -f was given
     if [ "$FORCE_INSTALL" = yes ]; then
         for PKG in $PACKAGES; do
-            $MAKE "$PKG-clean" || true  # Ignore errors
+            $MAKE "$PKG-uninstall" || true  # Ignore errors
         done
     fi
 

build/make/Makefile.in

@@ -114,7 +114,7 @@ INSTALLED_PACKAGE_INSTS = \
 # All previously installed standard/optional/experimental packages that are to be uninstalled
 OPTIONAL_UNINSTALLED_PACKAGES = @SAGE_OPTIONAL_UNINSTALLED_PACKAGES@
 UNINSTALLED_PACKAGES = $(OPTIONAL_UNINSTALLED_PACKAGES)
-UNINSTALLED_PACKAGES_CLEANS = $(UNINSTALLED_PACKAGES:%=%-clean)
+UNINSTALLED_PACKAGES_UNINSTALLS = $(UNINSTALLED_PACKAGES:%=%-uninstall)
 
 # All packages which should be downloaded
 SDIST_PACKAGES = @SAGE_SDIST_PACKAGES@
@@ -167,7 +167,7 @@ $(foreach tree,SAGE_LOCAL SAGE_VENV SAGE_DOCS, \
                $(foreach pkgname,$(INSTALLED_PACKAGES), \
                          $(if $(findstring $(tree),$(trees_$(pkgname))), \
                               $(inst_$(pkgname))))) \
-    $(eval $(tree)_CLEANED_PACKAGE_CLEANS = \
+    $(eval $(tree)_UNINSTALLED_PACKAGE_UNINSTALLS = \
                $(foreach pkgname,$(INSTALLED_PACKAGES), \
                          $(if $(findstring $(tree),$(trees_$(pkgname))), \
                               $(inst_$(pkgname))))))
@@ -190,8 +190,8 @@ else
 PLUS =
 endif
 
-# List of targets that can be run using `sage -i` or `sage -f`
-# These should generally have an associated -clean target for `sage -f` to
+# List of targets that can be run (in addition to names of packages) using `sage -i` or `sage -f`
+# These should generally have an associated -uninstall target for `sage -f` to
 # work correctly
 SAGE_I_TARGETS = sagelib doc
 
@@ -242,24 +242,24 @@ base-toolchain: _clean-broken-gcc base
 
 # All targets except for the base packages and except the documentation
 all-sage: \
-		$(SAGE_LOCAL_INSTALLED_PACKAGE_INSTS) $(SAGE_LOCAL_UNINSTALLED_PACKAGES_CLEANS) \
-		$(SAGE_VENV_INSTALLED_PACKAGE_INSTS)  $(SAGE_VENV_UNINSTALLED_PACKAGES_CLEANS)
+		$(SAGE_LOCAL_INSTALLED_PACKAGE_INSTS) $(SAGE_LOCAL_UNINSTALLED_PACKAGES_UNINSTALLS) \
+		$(SAGE_VENV_INSTALLED_PACKAGE_INSTS)  $(SAGE_VENV_UNINSTALLED_PACKAGES_UNINSTALLS)
 
 # Same but filtered by installation trees:
 all-build-local: toolchain-deps
 	+$(MAKE_REC) all-sage-local
 
-all-sage-local: $(SAGE_LOCAL_INSTALLED_PACKAGE_INSTS) $(SAGE_LOCAL_UNINSTALLED_PACKAGES_CLEANS)
+all-sage-local: $(SAGE_LOCAL_INSTALLED_PACKAGE_INSTS) $(SAGE_LOCAL_UNINSTALLED_PACKAGES_UNINSTALLS)
 
 all-build-venv: toolchain-deps
 	+$(MAKE_REC) all-sage-venv
 
-all-sage-venv:  $(SAGE_VENV_INSTALLED_PACKAGE_INSTS)  $(SAGE_VENV_UNINSTALLED_PACKAGES_CLEANS)
+all-sage-venv:  $(SAGE_VENV_INSTALLED_PACKAGE_INSTS)  $(SAGE_VENV_UNINSTALLED_PACKAGES_UNINSTALLS)
 
 all-build-docs: toolchain-deps
 	+$(MAKE_REC) all-sage-docs
 
-all-sage-docs:  $(SAGE_DOCS_INSTALLED_PACKAGE_INSTS) $(SAGE_DOCS_UNINSTALLED_PACKAGES_CLEANS)
+all-sage-docs:  $(SAGE_DOCS_INSTALLED_PACKAGE_INSTS) $(SAGE_DOCS_UNINSTALLED_PACKAGES_UNINSTALLS)
 
 # Download all packages which should be inside an sdist tarball (the -B
 # option to make forces all targets to be built unconditionally)
@@ -370,6 +370,27 @@ _clean-broken-gcc:
 	   rm -f "$(SAGE_ROOT)/build/make/.clean-broken-gcc"; \
 	   echo "Cleaned up old broken GCC install"; \
 	fi
+
+# Implicit rules for uninstalling packages that no longer exist in the source tree.
+%-SAGE_LOCAL-uninstall:
+	@package=$@; \
+	package=$${package%%-*}; \
+	if [ -d '$(SAGE_LOCAL)' ]; then \
+	    sage-spkg-uninstall $$package; \
+	fi
+
+%-SAGE_VENV-uninstall:
+	@package=$@; \
+	package=$${package%%-*}; \
+	if [ -d '$(SAGE_VENV)' ]; then \
+	    sage-spkg-uninstall $$package $(SAGE_VENV); \
+	fi
+
+%-uninstall:
+	@package=$@; \
+	package=$${package%%-*}; \
+	$(MAKE) $$package-SAGE_LOCAL-uninstall $$package-SAGE_VENV-uninstall
+
 #==============================================================================
 # Setting SAGE_CHECK... variables
 #==============================================================================
@@ -455,7 +476,7 @@ pkg_deps = \
 # <pkgname>-no-deps:
 #     +$(AM_V_at)sage-logger -p '$(SAGE_SPKG) <pkgname>-<pkgvers>' '$(SAGE_LOGS)/<pkgname>-<pkgvers>.log'
 #
-# <pkgname>-clean:
+# <pkgname>-uninstall:
 #     sage-spkg-uninstall <pkgname> '$(SAGE_LOCAL)'
 #
 # So <pkgname>-build-deps installs just the dependencies, while
@@ -470,7 +491,7 @@ pkg_deps = \
 #
 # python3: $(INST)/python3-3.7.3
 #
-# python3-clean:
+# python3-uninstall:
 #     sage-spkg-uninstall python3 '$(SAGE_LOCAL)'
 #
 # Note: In these rules the $(INST)/<pkgname>-<pkgvers> target is used
@@ -512,15 +533,17 @@ $(1)-$(4)-no-deps:
 
 $(1)-no-deps: $(1)-$(4)-no-deps
 
-$(1)-$(4)-clean:
+$(1)-$(4)-uninstall:
 	if [ -d '$$($(4))' ]; then \
 	    sage-spkg-uninstall $(if $(filter $(1),$(TOOLCHAIN_DEPS)),--keep-files) \
 		$(1) '$$($(4))'; \
 	fi
 
-$(1)-clean: $(1)-$(4)-clean
+$(1)-uninstall: $(1)-$(4)-uninstall
+
+$(1)-clean: $(1)-uninstall
 
-.PHONY: $(1) $(1)-clean $(1)-build-deps $(1)-no-deps
+.PHONY: $(1) $(1)-$(4)-uninstall $(1)-uninstall $(1)-clean $(1)-build-deps $(1)-no-deps
 endef #################################################################
 
 $(foreach pkgname, $(NORMAL_PACKAGES),\
@@ -543,7 +566,7 @@ endif
 # <pkgname>: <dependencies>
 #     $(AM_V_at)sage-logger -p 'sage --pip install ...' '$(SAGE_LOGS)/<pkgname>.log'
 #
-# <pkgname>-clean:
+# <pkgname>-uninstall:
 #     -sage --pip uninstall -y ...
 
 # Positional arguments:
@@ -558,10 +581,12 @@ $(1): $(2)
 $(1)-no-deps:
 	$(AM_V_at)sage-logger -p 'sage --pip install -r "$$(SAGE_ROOT)/build/pkgs/$(1)/requirements.txt"' '$$(SAGE_LOGS)/$(1).log'
 
-$(1)-clean:
+$(1)-uninstall:
 	-sage --pip uninstall --isolated --yes --no-input -r '$$(SAGE_ROOT)/build/pkgs/$(1)/requirements.txt'
 
-.PHONY: $(1) $(1)-clean $(1)-build-deps $(1)-no-deps
+$(1)-clean: $(1)-uninstall
+
+.PHONY: $(1) $(1)-uninstall $(1)-clean $(1)-build-deps $(1)-no-deps
 endef
 
 $(foreach pkgname,$(PIP_PACKAGES),\
@@ -587,7 +612,7 @@ endif
 #
 # <pkgname>: $(INST)/<pkgname>-<pkgvers>
 #
-# <pkgname>-clean:
+# <pkgname>-uninstall:
 #     -$(AM_V_at)cd '$SAGE_ROOT' && \\
 #         . '$SAGE_ROOT/src/bin/sage-env-config' && \\
 #         . '$SAGE_ROOT/src/bin/sage-env' && \\
@@ -664,7 +689,7 @@ $(1)-tox-%: FORCE
 		SAGE_SPKG_WHEELS=$$(SAGE_LOCAL)/var/lib/sage/wheels \
 		tox -v -v -v -e $$*
 
-.PHONY: $(1) $(1)-uninstall $(1)-build-deps $(1)-no-deps $(1)-clean
+.PHONY: $(1) $(1)-uninstall $(1)-clean $(1)-build-deps $(1)-no-deps $(1)-clean
 
 endef
 

build/sage_bootstrap/uninstall.py

@@ -1,5 +1,5 @@
 """
-Command-line script for uninstalling an existing Sage spkg from $SAGE_LOCAL.
+Command-line script for uninstalling an existing SPKG from an installation tree ($SAGE_LOCAL, $SAGE_VENV).
 
 This performs two types of uninstallation:
 
@@ -14,9 +14,10 @@
 
     2) New-style uninstallation: More recently installed packages that were
        installed with staged installation have a record of all files installed
-       by that package.  That file is stored in the $SAGE_SPKG_INST directory
-       (typically $SAGE_LOCAL/var/lib/sage/installed) and is created when the
-       spkg is installed.  This is a JSON file containing some meta-data about
+       by that package.  That file is stored in the directory
+       $SAGE_LOCAL/var/lib/sage/installed or $SAGE_VENV/var/lib/sage/installed
+       and is created when the spkg is installed.
+       This is a JSON file containing some meta-data about
        the package, including the list of all files associated with the
        package.  This script removes all these files, including the record
        file.  Any directories that are empty after files are removed from them
@@ -50,14 +51,13 @@
 
 def uninstall(spkg_name, sage_local, keep_files=False, verbose=False):
     """
-    Given a package name and path to SAGE_LOCAL, uninstall that package from
-    SAGE_LOCAL if it is currently installed.
+    Given a package name and path to an installation tree (SAGE_LOCAL or SAGE_VENV),
+    uninstall that package from that tree if it is currently installed.
     """
 
-    # The default path to this directory; however its value should be read
-    # from the environment if possible
+    # The path to the installation records.
+    # See SPKG_INST_RELDIR in SAGE_ROOT/build/make/Makefile.in
     spkg_inst = pth.join(sage_local, 'var', 'lib', 'sage', 'installed')
-    spkg_inst = os.environ.get('SAGE_SPKG_INST', spkg_inst)
 
     # Find all stamp files for the package; there should be only one, but if
     # there is somehow more than one we'll work with the most recent and delete
@@ -131,8 +131,9 @@ def legacy_uninstall(spkg_name, verbose=False):
 
 def modern_uninstall(spkg_name, sage_local, files, verbose=False):
     """
-    Remove all listed files from the given SAGE_LOCAL (all file paths should
-    be assumed relative to SAGE_LOCAL).
+    Remove all listed files from the given installation tree (SAGE_LOCAL or SAGE_VENV).
+
+    All file paths should be assumed relative to the installation tree.
 
     This is otherwise (currently) agnostic about what package is actually
     being uninstalled--all it cares about is removing a list of files.
@@ -259,20 +260,6 @@ def dir_type(path):
     return path
 
 
-def spkg_type(pkg):
-    """
-    A custom argument 'type' for spkgs--checks whether the given package name
-    is a known spkg.
-    """
-    pkgbase = pth.join(PKGS, pkg)
-
-    if not pth.isdir(pkgbase):
-        raise argparse.ArgumentTypeError(
-            "'{0}' is not a known spkg".format(pkg))
-
-    return pkg
-
-
 def make_parser():
     """Returns the command-line argument parser for sage-spkg-uninstall."""
 
@@ -283,10 +270,10 @@ def make_parser():
         epilog='\n'.join(doc_lines[1:]).strip(),
         formatter_class=argparse.RawDescriptionHelpFormatter)
 
-    parser.add_argument('spkg', type=spkg_type, help='the spkg to uninstall')
+    parser.add_argument('spkg', type=str, help='the spkg to uninstall')
     parser.add_argument('sage_local', type=dir_type, nargs='?',
                         default=os.environ.get('SAGE_LOCAL'),
-                        help='the SAGE_LOCAL path (default: the $SAGE_LOCAL '
+                        help='the path of the installation tree (default: the $SAGE_LOCAL '
                              'environment variable if set)')
     parser.add_argument('-v', '--verbose', action='store_true',
                         help='verbose output showing all files removed')
@@ -305,7 +292,7 @@ def run(argv=None):
     args = parser.parse_args(argv if argv is not None else sys.argv[1:])
 
     if args.sage_local is None:
-        print('Error: SAGE_LOCAL must be specified either at the command '
+        print('Error: An installation tree must be specified either at the command '
               'line or in the $SAGE_LOCAL environment variable',
               file=sys.stderr)
         sys.exit(1)

tox.ini

@@ -90,7 +90,7 @@ envlist =
     #
     # Or to rebuild a package with verbose output:
     #
-    #   $ tox -e local-homebrew-macos-standard -- ppl-clean ppl V=1
+    #   $ tox -e local-homebrew-macos-standard -- ppl-uninstall ppl V=1
     #
     # The variant "local-homebrew-macos-usrlocal" uses the global installation in /usr/local
     # instead.  It may install packages or update packages.  It will not remove packages.