Opened 9 years ago

Closed 9 years ago

Last modified 8 years ago

#10823 closed enhancement (fixed)

environment variable SAGE_SPKG_INSTALL_DOCS to build and install spkg docs

Reported by: jason Owned by: tbd
Priority: major Milestone: sage-4.7
Component: packages: standard Keywords:
Cc: drkirkby, kcrisman Merged in: sage-4.7.alpha1
Authors: Jason Grout Reviewers: David Kirkby
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by ddrake)

There are lots of times when it would be convenient to have the documentation of various spkgs installed in a local or system Sage installation. For example, it seems that I'm always wishing that I had that at an airport or on an airplane. At one point a long time ago, we had an extradocs spkg, but it was never maintained. So here is a proposal:

When building an spkg, if the SAGE_SPKG_INSTALL_DOCS environment variable is yes, then the docs are built (if available in the spkg) and are installed in $SAGE_ROOT/local/share/doc/<SPKG NAME>/

For example, numpy includes the docs with the sources. I'd like to insert the following at the bottom of the numpy spkg-install:

if [ "x$SAGE_SPKG_INSTALL_DOCS" = xyes ] ; then
    cd doc
    make html
    if [ $? -ne 0 ]; then
    echo "Error building numpy docs."
    exit 1
    fi
    mkdir -p $SAGE_ROOT/local/share/doc/numpy
    mv build/html $SAGE_ROOT/local/share/doc/numpy
fi

This builds the numpy docs and makes a directory $SAGE_ROOT/local/share/doc/numpy/html/ that contains the standalone html documentation for numpy.

Here are a few updated spkgs:

Attachments (2)

trac-10823-doc.patch (3.3 KB) - added by jason 9 years ago.
trac-10823-doc-2.patch (1.1 KB) - added by jason 9 years ago.
apply on top of previous patches

Download all attachments as: .zip

Change History (36)

comment:1 Changed 9 years ago by jason

  • Description modified (diff)

comment:2 Changed 9 years ago by jason

  • Description modified (diff)

comment:3 Changed 9 years ago by jason

  • Description modified (diff)

comment:4 Changed 9 years ago by jason

  • Description modified (diff)
  • Status changed from new to needs_review

comment:5 Changed 9 years ago by jason

  • Description modified (diff)

comment:6 follow-up: Changed 9 years ago by jason

  • Cc drkirkby added

CCing our resident shell script expert for comments: David, is there anything else that should be done in the above snippet of shell script?

comment:7 Changed 9 years ago by jason

  • Description modified (diff)

comment:8 Changed 9 years ago by jason

  • Description modified (diff)

comment:9 Changed 9 years ago by jason

  • Description modified (diff)

comment:10 in reply to: ↑ 6 Changed 9 years ago by drkirkby

Replying to jason:

CCing our resident shell script expert for comments: David, is there anything else that should be done in the above snippet of shell script?

I would indent the

    echo "Error building numpy docs."
    exit 1

for clarity. Apart from that, it looks OK. It will put the docs in a directory

$SAGE_ROOT/local/share/doc/numpy/html

not

$SAGE_ROOT/local/share/doc/numpy

but I think that's a good idea, as some docs might be in pdf, others in html etc.

I think it would be more normal to copy the files, rather than move them, so the build directory remains intact. (It usually gets deleted anyway, but there are ways of saving the contents).

Overall, I think this is a good proposal. Lots of programs used by Sage have documentation which we don't use. There may be other tools needed to generate some of the files - (GNU info for example). In cases like that, it might be worth checking if the right tool existed.

comment:11 Changed 9 years ago by drkirkby

PS, I think the Sage users (or is it developers?) guide needs to be modified to document the variable. One of the manuals, (I forget which) has all the environment variables documented on one page. So I think the changes to the documentation need to be reviewed here, which will be a trivial review.

Then each .spkg with the changes would need to be on separate tickets.

comment:12 Changed 9 years ago by kcrisman

  • Cc kcrisman added

comment:13 Changed 9 years ago by jason

  • Description modified (diff)

comment:14 Changed 9 years ago by jason

Okay, the doc patches are up here and the description notes which tickets are for the spkgs that I put up earlier.

comment:15 Changed 9 years ago by jason

  • Description modified (diff)

comment:16 Changed 9 years ago by jason

  • Authors set to Jason Grout

Okay, that's it for now, so feel free to review this and the spkgs noted in the description!

comment:17 follow-ups: Changed 9 years ago by drkirkby

  • Status changed from needs_review to needs_work

Sorry for the delay. I spent about an hour replying to this yesterday when my wife accidentally pulled the power cord from my laptop, which did not have a battery in it at the time. So it was all lost.

From what I recall

  • 'make' should be $MAKE
  • There should be a link added to this page http://www.sagemath.org/doc/ (which is generated from the docs in Sage) to show the place where the new documentation can be found.
  • The line with echo "SAGE_LOCAL undefined ... exiting"; in doc/en/developer/producing_spkgs.rst has an extra semi-colon on the end. Since that is in the same file as here, that might as well be fixed at the same time. Also replace 'make' by $MAKE there.
  • The ticket description shows make html but the patch make doc. There is no standard way of creating documentation. Hence the developers guide should make it clearer that this section of spk-install will probably need changing depending on the package, and should not be copied verbatim and expected to work.
  • There's a possibility that building some of the documentation may require external programs like GNU info, LaTeX. makeinfo etc. This needs to be mentioned in the docs.
  • The patch says the documentation gets installed in the file $SAGE_ROOT/local/share/doc/SPKG_NAME/. I think there should be in front of $SPKG_NAME - just as with SAGE_ROOT. Also, a more accurate description is that the docs are installed in one or more directories below $SAGE_ROOT/local/share/doc/SPKG_NAME/

Clearly adding the code to the packages is quite simple, but unless it is well documented it's not going to be much use. People must be able to find the documentation, which I think will be most easily done if on the main page http://www.sagemath.org/doc/

Have you given thought to what is to be done if one can build both HTML and PDF docs? I suggesting building both might be best, but I don't know.

Overall though, I think this is a good idea.

Dave

comment:18 Changed 9 years ago by drkirkby

  • Reviewers set to David Kirkby

comment:19 in reply to: ↑ 17 Changed 9 years ago by drkirkby

Replying to drkirkby:

  • The patch says the documentation gets installed in the file $SAGE_ROOT/local/share/doc/SPKG_NAME/. I think there should be in front of $SPKG_NAME - just as with SAGE_ROOT. Also, a more accurate description is that the docs are installed in one or more directories below $SAGE_ROOT/local/share/doc/SPKG_NAME/

I meant to say there was a $ sign missing. So it should be

$SAGE_ROOT/local/share/doc/$SPKG_NAME/, with files stored in one or more directories below $SAGE_ROOT/local/share/doc/$SPKG_NAME/

Dave

comment:20 in reply to: ↑ 17 ; follow-up: Changed 9 years ago by jason

Replying to drkirkby:

Sorry for the delay. I spent about an hour replying to this yesterday when my wife accidentally pulled the power cord from my laptop, which did not have a battery in it at the time. So it was all lost.

Thanks very much for your time and thoughtful care in reviewing this (twice, even!)

From what I recall

  • 'make' should be $MAKE
  • The ticket description shows make html but the patch make doc. There is no standard way of creating documentation. Hence the developers guide should make it clearer that this section of spk-install will probably need changing depending on the package, and should not be copied verbatim and expected to work.

You're right that I just put example instructions up there. Instead, I'll just put a comment (I've modified the description accordingly).

  • The line with echo "SAGE_LOCAL undefined ... exiting"; in doc/en/developer/producing_spkgs.rst has an extra semi-colon on the end. Since that is in the same file as here, that might as well be fixed at the same time. Also replace 'make' by $MAKE there.

Okay, though it's not really in the direct scope of this ticket, since I'm modifying the file anyway, I'll throw this in too.

  • There should be a link added to this page http://www.sagemath.org/doc/ (which is generated from the docs in Sage) to show the place where the new documentation can be found.

Clearly adding the code to the packages is quite simple, but unless it is well documented it's not going to be much use. People must be able to find the documentation, which I think will be most easily done if on the main page http://www.sagemath.org/doc/

Do you happen to know what file this is generated from? I'd also consider this a separate ticket, since there are questions about how to deal with this on sagemath.org, and the fact that the link probably ought not be there if we don't generate the docs, maybe.

  • There's a possibility that building some of the documentation may require external programs like GNU info, LaTeX. makeinfo etc. This needs to be mentioned in the docs.

Fix coming up.

  • The patch says the documentation gets installed in the file $SAGE_ROOT/local/share/doc/SPKG_NAME/. I think there should be in front of $SPKG_NAME - just as with SAGE_ROOT. Also, a more accurate description is that the docs are installed in one or more directories below $SAGE_ROOT/local/share/doc/SPKG_NAME/

I didn't put the $ since $SPKG_NAME is not a variable that works in the spkg-install file. My hope was to help them realize that they needed to manually fill in SPKG_NAME. I'll put a note to that effect.

Have you given thought to what is to be done if one can build both HTML and PDF docs? I suggesting building both might be best, but I don't know.

Personally, in the spkgs I modified, I just built the html documentation unless the default package documentation-building command built pdf too (I believe pari was that way). I think both might be best, but having just html documentation built requires less (latex is not required, IIRC).

Overall though, I think this is a good idea.

Thanks!

comment:21 in reply to: ↑ 20 Changed 9 years ago by jason

Replying to jason:

Replying to drkirkby:

  • There should be a link added to this page http://www.sagemath.org/doc/ (which is generated from the docs in Sage) to show the place where the new documentation can be found.

Clearly adding the code to the packages is quite simple, but unless it is well documented it's not going to be much use. People must be able to find the documentation, which I think will be most easily done if on the main page http://www.sagemath.org/doc/

This is now #10853

  • There's a possibility that building some of the documentation may require external programs like GNU info, LaTeX. makeinfo etc. This needs to be mentioned in the docs.

Fix coming up.

Done.

  • The patch says the documentation gets installed in the file $SAGE_ROOT/local/share/doc/SPKG_NAME/. I think there should be in front of $SPKG_NAME - just as with SAGE_ROOT. Also, a more accurate description is that the docs are installed in one or more directories below $SAGE_ROOT/local/share/doc/SPKG_NAME/

I didn't put the $ since $SPKG_NAME is not a variable that works in the spkg-install file. My hope was to help them realize that they needed to manually fill in SPKG_NAME. I'll put a note to that effect.

PACKAGE_NAME is used quite a bit in other places in that file, so I used that. That is understood in that file to be a placeholder to be replaced for a specific package.

comment:22 Changed 9 years ago by jason

  • Status changed from needs_work to needs_review

Things should be ready for review...

comment:23 Changed 9 years ago by drkirkby

I take your point about $SPKG_NAME. Leaving it as SPKG_NAME is the right thing to do.

The file devel/sage-main/doc/en/website/templates/index.html looks to be the one that generates http://www.sagemath.org/doc/ We can discuss that on #10853

I forgot, what is shown as lines 221 to 224 in doc/en/developer/producing_spkgs.rst are indented one space too much.

I don't have a strong feeling on the following, and are happy to give it a positive review either way, but something that should at least be considered.

Should doc/en/developer/producing_spkgs.rst state the developer should check if any required software is actually installed? Or is it better to let the installation fail if the program is not installed? I assume there should be a helpful message like "latex not found" generated by a package that needs it.

It might be helpful to add something like this to the sample spkg-install file, inside your if [ "x$SAGE_SPKG_INSTALL_DOCS" = xyes ] ; then.

# Before trying to build the documentation, check if any needed programs are 
# present. In the example below we check for 'latex', but this will depend on the package.
# Some packages may need no extra tools installed, others may require some. 
# We use 'command -v' for testing this, and not 'which' since 'which' is not
# portable, whereas 'command -v' is defined by POSIX 
# if [ `command -v latex` ] ; then 
#   echo "Good, latex was found, so building the documentation"
# else
#   echo "Sorry, can't build the documentation for SPKG_NAME as latex is not installed"
# fi

As I say, I don't have a strong opinion.

Fix the indentation and I'll give this a positive review, or make the developer do more work in testing for the required software. You choose!

Dave

comment:24 follow-up: Changed 9 years ago by kcrisman

Question: How much does this add to the size of a Sage download? It's at least conceivable that it would be not be negligible.

comment:25 in reply to: ↑ 24 ; follow-up: Changed 9 years ago by drkirkby

Replying to kcrisman:

Question: How much does this add to the size of a Sage download? It's at least conceivable that it would be not be negligible.

If we continue as now, not building the documentation, then only a 1000 or so bytes for the patches.

If we changed the way we built binaries, and included all the documentation, then I could believe the download size would increase significantly - I would guess someone in the range 10 to 100 MB.

Dave

comment:26 in reply to: ↑ 25 Changed 9 years ago by kcrisman

Replying to drkirkby:

Replying to kcrisman:

Question: How much does this add to the size of a Sage download? It's at least conceivable that it would be not be negligible.

If we continue as now, not building the documentation, then only a 1000 or so bytes for the patches.

Sorry for the spam - I guess I didn't realize this would be an *optional* variable!

Changed 9 years ago by jason

comment:27 Changed 9 years ago by jason

Okay, I attached a new patch which should take care of all of David's concerns.

Thanks for the careful review!

comment:28 Changed 9 years ago by drkirkby

  • Status changed from needs_review to positive_review

This is fine, so positive review. I'll look to review the individual packages that make use of the environment variable, but for now at least it is properly documented in both the Installation and Developer guides.

comment:29 Changed 9 years ago by drkirkby

  • Status changed from positive_review to needs_work

I'm putting this back to needs_work, as I realised after looking at a revised Matplotlib package (#10828) that simply copying the "docs" directory might in some cases copy a lot of unnecessary files. The spkg-install should tell developers to remove unnecessary files from $SAGE_ROOT/local/share/doc/SPKG_NAME. Again this will depend on the package, but something like:

find "$SAGE_ROOT/local/share/doc/SPKG_NAME" -name '*.py'  -exec rm -f {} \;

should get rid of the .py files for example.

Dave

Changed 9 years ago by jason

apply on top of previous patches

comment:30 Changed 9 years ago by jason

  • Status changed from needs_work to needs_review

I added a further note to the documentation.

comment:31 Changed 9 years ago by drkirkby

  • Status changed from needs_review to positive_review

That's fine.

comment:32 Changed 9 years ago by jdemeyer

  • Merged in set to sage-4.7.alpha1
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:33 Changed 9 years ago by ddrake

  • Description modified (diff)

comment:34 Changed 8 years ago by jason

See #11197 for a follow-up ticket for implementing building docs after Sage is built.

Note: See TracTickets for help on using tickets.