Make search_doc independent of sage_setup

[antonio-rojas]

search_doc currently imports stuff from sage_setup, which shouldn't be needed by sage at runtime. It also relies on the sage doc source to be present, which is not necessarily true for distro packages.
This patch replaces the current exhaustive check that all translated docs have been built by a simple check that $SAGE_DOC/html exists.

CC: @timokau @kiwifb @saraedum

Component: documentation

Keywords: packaging

Author: Antonio Rojas

Branch/Commit: b3abf9c

Reviewer: François Bissey

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

[antonio-rojas]

Branch: u/arojas/make_search_doc_independent_of_sage_setup

[antonio-rojas]

Changed keywords from none to packaging

[antonio-rojas]

Commit: 86c3afb

[antonio-rojas]

Author: Antonio Rojas

[antonio-rojas]

New commits:

86c3afb

Make search_doc work if sage_setup is not installed

[kiwifb]

comment:3

Thanks for copying me. Those bits are a part of something I just remove from sage https://github.com/cschwan/sage-on-gentoo/blob/master/sci-mathematics/sage/files/sage-8.4-sagedoc.patch . I wonder if it is time for it to disappear in sage proper too.

Why do I remove that bit? This check at runtime that the documentation has been built - it may have been helpful at some point during early sage development but I don't believe it should still be the case. Furthermore, this effectively depends on the presence (fake or real) of sage's documentation source. And of course we disapprove of that in distro. Finally if the search fails, it suggests you do stuff that is not supported for user on a distro (sage -docbuild).

[antonio-rojas]

comment:4

It would be great to get rid of all that code, yes. If devs agree to that I'll happily withdraw this.

[jhpalmieri]

comment:5

@kiwifb: Is there some good way of detecting whether you are on a distro, and then conditionally evaluate the code? My view has been for a long time that a Sage installation (any software installation, really) is not complete without documentation, so I want some sort of check. (I don't think you're advocating removing that big chunk of code always, just in your distro, but if we can make things work for both settings, why not?) Can we set some environment variable, or touch some file, or do something else to indicate that it's a distro and not a from-source or from-binary installation?

@antonio-rojas: By the way, an alternative to the branch here: rather than to duplicate code and variable definitions from sage_setup.docbuild, is to define the relevant variables in sage.misc.sagedoc or sage.env, and then import them in sage_setup.docbuild. Duplication is usually a bad idea: harder to maintain and harder to diagnose problems. (sage_setup.docbuild already has some imports from the Sage library.)

[kiwifb]

comment:6

I understand the concern about the documentation (or at least some documentation - in sage-on-gentoo I allow for some language selection with the caveat that only english is complete). The code is useful for someone writing new code with the associated documentation but it is difficult to justify it at runtime for everyone.

There no current way to say we are a distro, although we could come up with one :)

Overall I think the concern that documentation is present could be checked by a doctest rather than anytime you do a search. I am OK for sage's source code to be referenced during a doctest.

[jhpalmieri]

Changed branch from u/arojas/make_search_doc_independent_of_sage_setup to u/jhpalmieri/make_search_doc_independent_of_sage_setup

[jhpalmieri]

comment:8

Okay, how about something like this? I tried to write it so it would work if SAGE_DOC_SRC is missing or if sage_setup is missing. The doctests are marked 'optional - dochtml'. I also added doctests so that we find out when the hardcoded lists of languages and documents become outdated (when building with access to the full Sage source code).

---

New commits:

41bc9de

trac 26227: do not check whether the documentation exists every time

[jhpalmieri]

Changed commit from 86c3afb to 41bc9de

[kiwifb]

comment:9

I am not too crazy about it, I'd like other people opinions and some reflection time before completely dismissing the approach. But my first reaction is as follow

• I don't like HARDCODED* variables in particular documents.
• Of course it make sense have it tagged dochtml - good thinking.
• In sage-on-gentoo I allow installing a subset of languages (with english being compulsory if you build/install documentation at all) I am not liking the idea of a doctest failing because I don't have languages that are not relevant to me. I guess that's why I don't like HARDCODED_DOCUMENTS, but I could live with that.

I guess more generally I didn't think doctesting that some or all documentation has been built/installed would need that much machinery that in turn needs its own problematic doctests. Is the added complexity worth it?

But thank you for showing what you think it should look like.

[kiwifb]

comment:10

Some more thoughts.

Using sources at runtime is forbidden but testing is ideally run before install time and certainly before packaging for binary distro. We don't expect users installing binary packages to be able to run the test suite. Apart from some rare cases it is just not present to be run anyway.

So relying on SAGE_DOC_SRC at testing is fine. Nevertheless there is a general feeling that the list of language may belong to env.py rather than somewhere in sage_setup. Something more satisfactory could be constructed there.

There are many moving parts and I am considering giving it a shot from the current branch and some thinking time.

[kiwifb]

comment:11

Ping to other distro guys, do you have a definition for SAGE_DOC_SRC in sage/env.py for your distro. If you do what is it? In sage-on-gentoo SAGE_DOC_SRC=SAGE_DOC at runtime (but not at build time obviously).

[jhpalmieri]

comment:12

Replying to @kiwifb:

I guess more generally I didn't think doctesting that some or all documentation has been built/installed would need that much machinery that in turn needs its own problematic doctests. Is the added complexity worth it?

To really test whether the built documentation is present, you need access to SAGE_DOC_SRC and sage_setup to compile the lists of languages and documents. So if the doctests should pass without access to those, you need

• sensible versions of the variables,
• and a way to detect when those sensible versions are no longer sensible (because someone has translated the tutorial to Arabic, for example).

If you don't care about doctests passing, or if you only care about doctests passing if you have intact SAGE_DOC_SRC and sage_setup, then it doesn't need to be as complicated. (References to sage_setup could be avoided, but I think you need SAGE_DOC_SRC.) Since some people want to distribute Sage without any built documentation and have it pass doctests, I don't know what is reasonable to require for doctests to pass. Maybe that's where the discussion should start: under what circumstances should doctests pass?

Does it at least make sense to have a separate function (as in my branch) that just tests whether the documentation is built, along with a doctest?

[kiwifb]

comment:13

Replying to @jhpalmieri:

Replying to @kiwifb:

I guess more generally I didn't think doctesting that some or all documentation has been built/installed would need that much machinery that in turn needs its own problematic doctests. Is the added complexity worth it?

To really test whether the built documentation is present, you need access to SAGE_DOC_SRC and sage_setup to compile the lists of languages and documents. So if the doctests should pass without access to those, you need

• sensible versions of the variables,
• and a way to detect when those sensible versions are no longer sensible (because someone has translated the tutorial to Arabic, for example).

Yes.

If you don't care about doctests passing, or if you only care about doctests passing if you have intact SAGE_DOC_SRC and sage_setup, then it doesn't need to be as complicated. (References to sage_setup could be avoided, but I think you need SAGE_DOC_SRC.) Since some people want to distribute Sage without any built documentation and have it pass doctests, I don't know what is reasonable to require for doctests to pass. Maybe that's where the discussion should start: under what circumstances should doctests pass?

I must say I am puzzled by my colleagues insisting the tests should pass without the doc. I guess technically I have it run OK with just some languages of the documentation but I was not expecting it to pass with none. But having the ability to test the html documentation separately is not absurd. I agree you should at least have "something that look like SAGE_DOC_SRC". Again, like I said using sources for testing purposes is OK.

Does it at least make sense to have a separate function (as in my branch) that just tests whether the documentation is built, along with a doctest?

Yes, I hadn't gone as far as thinking of a new function when I brought you the idea of doctesting but it make sense for it to exist and to be the thing tested. The only issue I may have is: "is it useful to have this available at runtime?" Which then turns into where is it appropriate to put this.

[kiwifb]

comment:14

And if it is not needed at runtime, my reaction is to shove it into sage_setup.

[antonio-rojas]

comment:15

Replying to @kiwifb:

Some more thoughts.

Using sources at runtime is forbidden but testing is ideally run before install time and certainly before packaging for binary distro. We don't expect users installing binary packages to be able to run the test suite. Apart from some rare cases it is just not present to be run anyway.

I don't agree with this. Binary packages have a different folder structure than the one in sage-the-distribution and it's important to make sure that everything works as intended, and the way to check that is by running the test suite, and it needs to be done when the package is installed and the source files are no longer available. This helps detecting runtime dependencies on source files that should be removed, and can't be detected by running the test suite on sage-the-distribution.

[jhpalmieri]

comment:16

• Should there be a doctest to test whether the documentation has been built? If I am installing Sage myself, I know whether I tried to build the docs, and if I did, then I know whether it succeeded. So people who build from source or use a Sage binary don't need this doctest. What about for distros? If we want such a doctest, should it be in sage_setup or the main Sage library? If in the main Sage library, can it assume that sage_setup and SAGE_DOC_SRC are intact?

• If a user tries to search the documentation, should they be warned if the documentation is missing or incomplete?

[antonio-rojas]

comment:17

Replying to @jhpalmieri:

• Should there be a doctest to test whether the documentation has been built? If I am installing Sage myself, I know whether I tried to build the docs, and if I did, then I know whether it succeeded.

The same applies to packagers who build distro packages, so I don't see why that case should be treated differently.

• If a user tries to search the documentation, should they be warned if the documentation is missing or incomplete?

We ship docs as a separate, optional package, so such a warning would be quite useful.

[timokau]

comment:18

Replying to @antonio-rojas:

Replying to @kiwifb:

Some more thoughts.

Using sources at runtime is forbidden but testing is ideally run before install time and certainly before packaging for binary distro. We don't expect users installing binary packages to be able to run the test suite. Apart from some rare cases it is just not present to be run anyway.

I don't agree with this. Binary packages have a different folder structure than the one in sage-the-distribution and it's important to make sure that everything works as intended, and the way to check that is by running the test suite, and it needs to be done when the package is installed and the source files are no longer available. This helps detecting runtime dependencies on source files that should be removed, and can't be detected by running the test suite on sage-the-distribution.

I agree with arojas. I think testing is ideally run after install time, under identical conditions as if the package was installed on a users machine. Because that is what we want to test: That sage is fully functional for a user. What does it help when sage works after build but may break after installation?

I personally just ship the sage source code. Why do you not want to do that? The only negative I see is the disk space, which is not that much.

[jhpalmieri]

comment:19

Replying to @antonio-rojas:

• If a user tries to search the documentation, should they be warned if the documentation is missing or incomplete?

We ship docs as a separate, optional package, so such a warning would be quite useful.

Does your main distribution include SAGE_DOC_SRC? If not, how can you meaningfully measure whether the documentation is present? My branch tries to address this; is it overkill?

[timokau]

comment:20

Replying to @jhpalmieri:

• Should there be a doctest to test whether the documentation has been built? If I am installing Sage myself, I know whether I tried to build the docs, and if I did, then I know whether it succeeded. So people who build from source or use a Sage binary don't need this doctest. What about for distros? If we want such a doctest, should it be in sage_setup or the main Sage library? If in the main Sage library, can it assume that sage_setup and SAGE_DOC_SRC are intact?

I think a doctest that makes sure sage can find its documentation is very valuable. However it shouldn't be mandatory to build sage. That is why I originally introduced optional - dochtml. Currently I'm building sage without docs and then testing it without dochtml. I then build the docs seperately and run only the dochtml tests for that. Users can decide weather or not to install the docs. Unfortunately, that will be somewhat undermined by this change.

• If a user tries to search the documentation, should they be warned if the documentation is missing or incomplete?

That would be nice.

[timokau]

comment:21

Replying to @jhpalmieri:

Replying to @antonio-rojas:

• If a user tries to search the documentation, should they be warned if the documentation is missing or incomplete?

We ship docs as a separate, optional package, so such a warning would be quite useful.

Does your main distribution include SAGE_DOC_SRC? If not, how can you meaningfully measure whether the documentation is present? My branch tries to address this; is it overkill?

I ship two packages: sage and sageWithDoc. The second one is just a wrapper around sage that sets SAGE_DOC_SRC.

[antonio-rojas]

comment:22

Replying to @jhpalmieri:

Replying to @antonio-rojas:

• If a user tries to search the documentation, should they be warned if the documentation is missing or incomplete?

We ship docs as a separate, optional package, so such a warning would be quite useful.

Does your main distribution include SAGE_DOC_SRC? If not, how can you meaningfully measure whether the documentation is present? My branch tries to address this; is it overkill?

We do define SAGE_DOC_SRC because it is still required by Sage at runtime (ideally it shouldn't). For us, simply checking whether $SAGE_DOC/html is present would be enough.

[timokau]

comment:23

We do define it too, but just a dummy value:

SAGE_DOC_SRC=${SAGE_DOC_SRC_OVERRIDE:-dummy}

Where the wrapper may set SAGE_DOC_SRC_OVERRIDE.

[jhpalmieri]

comment:24

Maybe you should rip out all of the stuff that François mentions in comment:3, and instead check for the presence of SAGE_DOC/html, printing a warning (but no instructions about how to build the docs) if that is missing. If you want to get fancy, then do it like Sage's deprecation warnings: only print the warning once per session. That could go on a separate ticket, though.

I'm assuming that distributions will either include no documentation or will include (almost) all of it. With this approach, if for some reason someone decided to only include the stuff in Italian, then search_doc will not print any warnings but will not work well.

[kiwifb]

comment:25

Replying to @jhpalmieri:

Maybe you should rip out all of the stuff that François mentions in comment:3, and instead check for the presence of SAGE_DOC/html, printing a warning (but no instructions about how to build the docs) if that is missing. If you want to get fancy, then do it like Sage's deprecation warnings: only print the warning once per session. That could go on a separate ticket, though.

I'm assuming that distributions will either include no documentation or will include (almost) all of it. With this approach, if for some reason someone decided to only include the stuff in Italian, then search_doc will not print any warnings but will not work well.

Sorry for the delay. My job had to take priority over the last few days. It seems like arojas and timokau have a different concept of what should be tested and when compared to me and I suspect Debian. That is unexpected but not a show stopper to me. People should be able to do what they think they need to do unless it is outrageous in some ways.

I would totally be in favor of John's last proposal. Rip this particular code and check whether the (or some) documentation is available when searching.

[antonio-rojas]

Changed branch from u/jhpalmieri/make_search_doc_independent_of_sage_setup to u/arojas/make_search_doc_independent_of_sage_setup

[antonio-rojas]

Changed commit from 41bc9de to b3abf9c

[antonio-rojas]

New commits:

ea41775	Merge remote-tracking branch 'origin/develop' into t/26227/make_search_doc_independent_of_sage_setup
746949d	Revert "trac 26227: do not check whether the documentation exists every time"
b3abf9c	Replace exhaustive check for built docs by a simple check that $SAGE_DOC/html exists

[antonio-rojas]

comment:28

Done as suggested in comment:25

[kiwifb]

Reviewer: François Bissey

[kiwifb]

comment:29

Good enough for me.

[vbraun]

Changed branch from u/arojas/make_search_doc_independent_of_sage_setup to b3abf9c