Add make targets to generate documentation #122
Conversation
| sudo apt-get update -y | ||
| sudo apt-get install -y texlive-latex-base texlive-latex-extra texlive-fonts-recommended latex2html pandoc | ||
| # our latex files have \usepackage{html} because we create webpages on our systems |
There was a problem hiding this comment.
I suspect the project's GitHub workflow still needs this section. Have you tested docs generation without these steps in a Docker container?
There was a problem hiding this comment.
Yes, and it worked okay. But I'm not extremely confident in my test setup
There was a problem hiding this comment.
However, I think the right thing to do is to simplify this PR and only add the make targets. The github workflow can be modified later. The only thing I actually care about at the moment is getting the pdf files in the release tarball. Unfortunately, adding the docs to the tarball right now will break ci.yml, since that workflow does not install pdflatex, so 'make dist' and 'make distcheck' will fail.
There was a problem hiding this comment.
what is the driving reason behind your concern to see the pdf files in the release tarball? We do distribute them in the release tarballs from our website, and the GitHub Actions CI does include them as release artifacts (or will include them when we create our next release). Is there a particular use case in the CFITSIO community that specifically needs the user manual PDF files inside the tarball coming from GitHub?
There was a problem hiding this comment.
I want 'make dist' to give me a usable artifact, which would include the .pdf files. Mostly, I find it very odd that the github workflow is being used to generate the release. 'make dist' is the well-known mechanism for generating a release and the vendor lock-in to github seems suboptimal. It should be easy for the user to generate the documentation from the .tex sources without needing to interact with github.
.github/workflows/documentation.yml
Outdated
| pandoc fpackguide.md -V linkcolor=blue -o fpackguide.pdf | ||
| ./configure | ||
| make cfitsio.pdf fitsio.pdf quick.pdf fpackguide.pdf |
There was a problem hiding this comment.
Why not simplify further and add a make docs target to generate all four PDFs?
There was a problem hiding this comment.
Also, could you just generate the PDFs inside the docs subdirectory? That would eliminate the need for most of the changes below. It's a minor thing. Probably not a big deal.
There was a problem hiding this comment.
I thought about "make docs", but I don't like the name clash with the directory docs/. Perhaps "make doc" or "make pdf" would be appropriate.
To generate the pdf in docs/, we need to either add a lot of boiler plate to Makefile.am or use a recursive make setup. It seems simpler to just use the VPATH build (arguably as bad as recursive make!) and build them in the top-level build directory.
There was a problem hiding this comment.
This is causing a build breakage in ci, because if we add this change then 'make dist' and 'make distcheck' will try to build the documentation. (So the *.pdf files are in the distribution tarball). But the workflow on develop does not install pdflatex. The github workflow for this PR will always run from develop, so we need to first merge a change to .githhub/workflows/ci.yml which will install pdflatex in the github runner. Or...we just create the make recipe to generate the pdf files but do not include them in the release tarball. This feels a bit chicken-and-eggy. I will modify this PR so that there are no changes to .github/workflow and we just instantiate the make recipes to build the docs.
|
Summary of recent changes:
Reverted all changes to github workflows. Unfortunately, we cannot include the docs in the distribution tarball because 'make dist' will fail in ci.yml until pdflatex is installed in that workflow. |
Add suffix rules to build PDF files from .tex (pdflatex) and .md (pandoc) sources. Add phony 'doc' target to build documentation. Add VPATH for docs/ to support out-of-tree builds. Add generated files to CLEANFILES. Issue: HEASARC#120
esabol
left a comment
esabol
left a comment
There was a problem hiding this comment.
Looks good to me! Thank you!
|
... although, tbh, I'd still prefer for the PDFs to be generated in or moved to the |
3 participants
It seems cleaner to have a make recipe to generate the documentation since this will allow users to more easily build from source.
Issue: #120