Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add latexpdf to tox #2575

Closed
stevepiercy opened this issue May 15, 2016 · 6 comments

Comments

Projects
None yet
2 participants
@stevepiercy
Copy link
Member

commented May 15, 2016

Consider adding latexpdf to the list of builders in tox, which would serve as a check that PDF should build at Read the Docs.

Any thoughts?

@bertjwregeer

This comment has been minimized.

Copy link
Member

commented May 15, 2016

I currently have latexpdf installed on only some of my development machines. Adding it to tox means we also need to make sure that all of our builds have latexpdf available, and I speak from experience when I say that getting latex and everything installed and working correctly is a major pain in the behind.

I am a -1 on this. I wouldn't mind adding it as a non-default. There is an issue with the PDF book generation and the TOC, in that you have to run latexpdf multiple times for it to generate a correct TOC. Read the Docs doesn't do so, and thus there is no TOC or it uses an outdated TOC from a previous run (when I checked in the past, this may have been fixed).

@stevepiercy

This comment has been minimized.

Copy link
Member Author

commented May 23, 2016

I agree with not a default for the reasons you cite and because the benefit is not worth the loss of time waiting for builds.

To make this optional, would I just duplicate and edit the docs stanza and add a line for pdf, or is there more to it? The following "works", but I don't know if it is in fact optional. tox -e docs seems to leave out pdf, so I think it's good.

#tox.ini
# ...
    docs,pdf,pep8,
# ...
[testenv:pdf]
basepython = python3.5
whitelist_externals = make
commands =
    pip install pyramid[pdf]
    make -C docs latexpdf BUILDDIR={envdir} "SPHINXOPTS=-W -E"
#setup.py
# ...
      extras_require={
          'testing': testing_extras,
          'docs': docs_extras,
          'pdf': docs_extras,
          },
# ...

I haven't seen the TOC issue in RTD for a while, but I check maybe a couple times per year, and only then when someone points it out to me.

@bertjwregeer

This comment has been minimized.

Copy link
Member

commented May 23, 2016

Don't add another extras_require, docs is fine here since that already installs all the needed parts.

Also, adding it to the default list is exactly what you don't want to do...

@bertjwregeer

This comment has been minimized.

Copy link
Member

commented May 23, 2016

Sorry, let me make that clearer. The envlist under [tox] is the default environments to run when one is not explicitly provided on the command line.

@stevepiercy

This comment has been minimized.

Copy link
Member Author

commented May 23, 2016

Thanks for the clarification. I reverted setup.py, removed pdf from envlist in tox.ini, and edited tox.ini as follows:

[testenv:pdf]
basepython = python3.5
whitelist_externals = make
commands =
    pip install pyramid[docs]
    make -C docs latexpdf BUILDDIR={envdir} "SPHINXOPTS=-W -E"

Invoked by tox -e pdf seems to work fine and makes PDF builds optional.

@bertjwregeer

This comment has been minimized.

Copy link
Member

commented May 23, 2016

That works for me.

stevepiercy added a commit to stevepiercy/pyramid that referenced this issue May 23, 2016

stevepiercy added a commit to stevepiercy/pyramid that referenced this issue May 23, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.