Opened 17 months ago

Last modified 3 months ago

#29097 new enhancement

build/make/Makefile.in: Rename make targets SPKG-clean to SPKG-uninstall

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.4
Component: build Keywords:
Cc: dimpase, embray, jhpalmieri, slabbe, mjo, schilly Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: u/jhpalmieri/clean2uninstall (Commits, GitHub, GitLab) Commit: 5203e5043ab29eb9fcf88256e8164ebe1d7bf0f3
Dependencies: #29793, #29868, #29386 Stopgaps:

Status badges

Description (last modified by mkoeppe)

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

(Part of this has been done in #29386.)

Likewise "doc-output-clean" should be renamed to "doc-uninstall".

Also useful would be phony targets of the form SPKG-deps that would install all prerequisite targets of SPKG, but not SPKG itself.

Also related: #29626 ./configure --enable-SPKG does not work for optional pip packages

See also: #29868 pip-installable packages sage-doc-html, sage-doc-pdf

Change History (17)

comment:1 Changed 15 months ago by mkoeppe

  • Description modified (diff)

comment:2 Changed 14 months ago by mkoeppe

  • Milestone changed from sage-9.1 to sage-9.2

comment:3 Changed 14 months ago by mkoeppe

  • Cc jhpalmieri slabbe added
  • Description modified (diff)

comment:4 Changed 14 months ago by mkoeppe

  • Cc mjo added

comment:5 Changed 12 months ago by mkoeppe

  • Dependencies set to #29793

comment:6 Changed 12 months ago by jhpalmieri

There is no reason for any deprecation here, is there? I have a branch that I'm testing out now to do the change "clean -> uninstall", but not the new phony targets SPKG-deps.

comment:7 Changed 12 months ago by jhpalmieri

  • Branch set to u/jhpalmieri/clean2uninstall

comment:8 Changed 12 months ago by jhpalmieri

  • Commit set to 5203e5043ab29eb9fcf88256e8164ebe1d7bf0f3

Here is a branch to test out. I wasn't sure about the instruction when docbuilding fails:

    Note: incremental documentation builds sometimes cause spurious
    error messages. To be certain that these are real errors, run
    "make doc-clean" first and try again.

There could be artifacts that make doc-uninstall won't clean up, so I think I want to keep this as make doc-clean. I don't know if it should also suggest running ./bootstrap, but that's a separate issue.


New commits:

5203e50trac 29097: change "make SPKG-clean" to "make SPKG-uninstall".

comment:9 follow-up: Changed 12 months ago by mjo

There could be artifacts that make doc-uninstall won't clean up, so I think I want to keep this as make doc-clean

Right now doc-clean does both doc-src-clean doc-output-clean. I suggest,

  1. Rename doc-output-clean to doc-uninstall (already done).
  2. Drop doc-clean
  3. Rename doc-src-clean to doc-clean
  4. Change the suggestion to run both doc-clean and doc-uninstall

That way the targets wind up with the right names, and the suggestion doesn't effectively change.

NB: in this hunk the doc-src-clean was dropped (I don't know which is correct):

-doc-html-no-plot: doc-clean $(DOC_DEPENDENCIES)
+doc-html-no-plot: doc-uninstall $(DOC_DEPENDENCIES)

comment:10 in reply to: ↑ 9 Changed 12 months ago by jhpalmieri

Replying to mjo:

There could be artifacts that make doc-uninstall won't clean up, so I think I want to keep this as make doc-clean

Right now doc-clean does both doc-src-clean doc-output-clean. I suggest,

  1. Rename doc-output-clean to doc-uninstall (already done).
  2. Drop doc-clean
  3. Rename doc-src-clean to doc-clean
  4. Change the suggestion to run both doc-clean and doc-uninstall

That way the targets wind up with the right names, and the suggestion doesn't effectively change.

My only reluctance is that I am very used to typing make doc-clean when working with tickets involving docbuilding. I don't know how many others are in the same habit, but we would all have to change that habit.

NB: in this hunk the doc-src-clean was dropped (I don't know which is correct):

-doc-html-no-plot: doc-clean $(DOC_DEPENDENCIES)
+doc-html-no-plot: doc-uninstall $(DOC_DEPENDENCIES)

This should be okay: the reason to clean the docs when toggling doc-html-no-plot is because inclusion (or not) of plots is cached in the doc output (in local/share/doc/sage/inventory or maybe .../doctrees). So doc-uninstall should be good enough. It works for me, at least.

comment:11 Changed 10 months ago by mkoeppe

  • Cc schilly added

For the doc-... cleaning/uninstalling targets, I would like to understand first whether everything that we put in $SAGE_LOCAL/share/doc/sage/ should really be installed there.

Are doctrees and inventory needed in the installation at all? Do other Python packages install inventory files? Or is this just something that is reused and updated in an incremental docbuild?

comment:12 Changed 10 months ago by mkoeppe

  • Description modified (diff)

comment:13 Changed 10 months ago by jhpalmieri

The inventory files are intended to allow for cross-references among different pieces of the documentation: see https://www.sphinx-doc.org/en/master/usage/extensions/intersphinx.html and set_intersphinx_mapping in src/sage/docs/conf.py. It allows the tutorial (for example) to refer to the reference manual. For example, the reference :ref:`sage.rings.padics.tutorial` in tour_numtheory.rst in the tutorial points to the reference manual, and I think this uses the inventory files to construct the link when building the documentation.

According to https://www.sphinx-doc.org/en/master/man/sphinx-build.html, the doctrees are used to cache the parsed source files before writing output. Once the output has been written, they aren't needed, so I guess they could be moved somewhere else, and I'm guessing the same for the inventory files, but I think they are all needed when building the docs.

comment:14 Changed 10 months ago by mkoeppe

  • Dependencies changed from #29793 to #29793, #29868

comment:15 Changed 10 months ago by mkoeppe

  • Milestone changed from sage-9.2 to sage-9.3

comment:16 Changed 9 months ago by mkoeppe

  • Dependencies changed from #29793, #29868 to #29793, #29868, #29386
  • Description modified (diff)

comment:17 Changed 3 months ago by mkoeppe

  • Milestone changed from sage-9.3 to sage-9.4

Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review of ticket status, priority, and last modification date.

Note: See TracTickets for help on using tickets.