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

fix sphinx startup guide to not to fail on rtd build as per #2569 #5753

Merged
merged 3 commits into from Jun 6, 2019

Conversation

@wilvk
Copy link
Contributor

@wilvk wilvk commented Jun 1, 2019

It's great to have a Github issue outlining how to work around an issue in the startup documentation ( see #2569), but it looks bad when this hasn't been integrated back into the documentation (for about 5 months when there was a breaking change), as ReadTheDocs is all about the docs - especially docs for starting/intro tutorials.

As per #2569 this PR specifies the missing step to get docs building correctly first time in RTD, without having to search for answers to build failures.

@stsewd
Copy link
Member

@stsewd stsewd commented Jun 1, 2019

Thanks for notice this, since sphinx 2.0, index is the default value https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-master_doc

Also, this isn't only for sphinx + markdown, but sphinx in general. And users can still set master_doc to a different value and have an index.rst or index.md file.

So, I think is worth a more general note here. At the beginning of the tutorial there is mention of the index file, probably we should put a note that users should make sure they have an index file.

@wilvk
Copy link
Contributor Author

@wilvk wilvk commented Jun 1, 2019

Thanks for getting back to me so quickly.
The issue I was getting was that in the RTD build process, it was failing on:

Running Sphinx v1.8.5
loading translations [en]... done
making output directory...
building [mo]: targets for 0 po files that are out of date
building [readthedocs]: targets for 1 source files that are out of date
updating environment: 1 added, 0 changed, 0 removed
reading sources... [100%] index


Traceback (most recent call last):
  File "/home/docs/checkouts/readthedocs.org/user_builds/tide/envs/latest/lib/python3.7/site-packages/sphinx/cmd/build.py", line 304, in build_main
    app.build(args.force_all, filenames)
  File "/home/docs/checkouts/readthedocs.org/user_builds/tide/envs/latest/lib/python3.7/site-packages/sphinx/application.py", line 341, in build
    self.builder.build_update()
  File "/home/docs/checkouts/readthedocs.org/user_builds/tide/envs/latest/lib/python3.7/site-packages/sphinx/builders/__init__.py", line 347, in build_update
    len(to_build))
  File "/home/docs/checkouts/readthedocs.org/user_builds/tide/envs/latest/lib/python3.7/site-packages/sphinx/builders/__init__.py", line 360, in build
    updated_docnames = set(self.read())
  File "/home/docs/checkouts/readthedocs.org/user_builds/tide/envs/latest/lib/python3.7/site-packages/sphinx/builders/__init__.py", line 472, in read
    self.env.doc2path(self.config.master_doc))
sphinx.errors.SphinxError: master file /home/docs/checkouts/readthedocs.org/user_builds/tide/checkouts/latest/docs/contents.rst not found

Sphinx error:
master file /home/docs/checkouts/readthedocs.org/user_builds/tide/checkouts/latest/docs/contents.rst not found

Where initially I had an index.rst but not a contents.rst.

I'm using:
pip Shinx=2.0.1
pip 19.1.1
Python 3.7.2

I think the main issue here is that RTD is using Shpinx 1.8.5 in it's build process but the latest pip install is 2.0.1. Wouldn't it make sense to outline that the fix in this PR is to work around that 1.8.5 is looking for content.rst where 2.0.1 is looking for index.rst? ... or can the version of Sphynx be set in the RTD build process, in which case it would be better to set it to 2.0.1 (IMHO).

@stsewd
Copy link
Member

@stsewd stsewd commented Jun 1, 2019

@wilvk
Copy link
Contributor Author

@wilvk wilvk commented Jun 2, 2019

So, given the discrepancy in the default pip version and the default RTD version of Sphinx, wouldn't the proposed change in this PR make the most sense for a person with no experience to get up to speed quickly?

I'm not sure how to word your suggested changes to this PR as having an index file won't stop the error in the build - it's looking for a content file by default for that version of Sphinx.

@stsewd
Copy link
Member

@stsewd stsewd commented Jun 2, 2019

Well, the original problem, building locally your docs should give you the same error, since you didn't have a contents.md file and master_doc = 'contents'.

If the build doesn't work locally, it shouldn't in read the docs.

Also, the next step of the guide is https://docs.readthedocs.io/en/stable/intro/import-guide.html, which makes mention of:

Some documentation projects require additional configuration to build such as specifying a certain version of Python or installing additional dependencies. You can configure these settings in a readthedocs.yml file. See our Configuration File docs for more details.

I think we should extend that to mention if users want to build with another sphinx version, they should use a requirements.txt file

@wilvk
Copy link
Contributor Author

@wilvk wilvk commented Jun 3, 2019

Ok, I've made the changes as requested.

@stsewd
Copy link
Member

@stsewd stsewd commented Jun 3, 2019

I just notice that we have this in

https://github.com/rtfd/readthedocs.org/blob/master/docs/builds.rst

.. note::

    Regardless of whether you build your docs with Sphinx or MkDocs,
    we recommend you pin the version of Sphinx or Mkdocs you want us to use.
    You can do this the same way other
    :doc:`dependencies are specified <guides/specifying-dependencies>`.
    Some examples of pinning versions might be ``sphinx<2.0`` or ``mkdocs>=1.0``.

Which we link from

https://github.com/rtfd/readthedocs.org/blob/master/docs/intro/import-guide.rst

Check out our :doc:`/builds` page to learn more about how Read the Docs builds your docs,
and to troubleshoot any issues that arise.

So, maybe we should put the same text to make this more visible or reorganize the docs, not sure. What do you think should be a good approach here?

@wilvk
Copy link
Contributor Author

@wilvk wilvk commented Jun 4, 2019

I'll have a read and let you know my thoughts. On face-value it seems like a bigger piece of work than just getting the quickstart guide working correctly but will let you know.

Copy link
Member

@ericholscher ericholscher left a comment

This seems fine for now. We can refactor this later. 👍

@ericholscher ericholscher merged commit 5c3536a into readthedocs:master Jun 6, 2019
1 check passed
@ericholscher
Copy link
Member

@ericholscher ericholscher commented Jun 6, 2019

Thanks @wilvk!

@wilvk wilvk deleted the doc_fix_newbies branch Aug 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants