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

Adjust 'make docs' to do an incremental build #16835

Merged
merged 6 commits into from Dec 14, 2020

Conversation

mppf
Copy link
Member

@mppf mppf commented Dec 9, 2020

Resolves #16826

Note, #16862 /
chapel-lang/sphinxcontrib-chapeldomain#45 is
required for this change to function properly - but this branch has those
changes.

This PR adjusts make docs to do an incremental sphinx build. For now,
the chpldoc command always runs, but the scripts only update the source
files for sphinx-build if they are different.

This brings a make docs after a minor change to about 10 seconds.

Details:

  • Adjusts util/config/update-if-different to include a mode argument
    (--update or --copy) and adjusts the existing calls to it to use
    the --update argument.
  • Added --copy mode to util/config/update-if-different that will
    copy all updated source files (recursively) and delete things in the
    dst directory not present in src.
  • Does the module docs build steps by first constructing the rst sources
    in a subdirectory of build/doc/ and then using update-if-different --copy from here (so that removed files can be correctly handled)
  • Updated Makefiles for make docs to avoid deleting "build" directory
    (since we can use it again, incrementally)
  • Removes a lot of command echoing for this process. One can use make SHELL='sh -x' docs to see the commands again.

Reviewed by @ben-albrecht and @lydia-duncan - thanks!

  • full local testing

mppf added a commit to chapel-lang/sphinxcontrib-chapeldomain that referenced this pull request Dec 9, 2020
Fix problem: dictionary changed size during iteration

The issue comes up when using sphinx to do an incremental
docs build (see chapel-lang/chapel#16835 )

I encountered an error along these lines:
```
Exception occurred:
  File "CHPL_HOME/third-party/chpl-venv/install/chpldeps/sphinxcontrib/chapeldomain/__init__.py", line 810, in clear_doc
    for fullname, (fn, x) in self.data['objects'].items():
RuntimeError: dictionary changed size during iteration
The full traceback has been saved in /var/folders/b6/1lwlz22n5d53878_x52vp3kr0007pg/T/sphinx-err-xabz0j9_.log, if you want to report the issue to the developers.
Please also report this if it was a user error, so that a better error message can be provided next time.
A bug report can be filed in the tracker at <https://github.com/sphinx-doc/sphinx/issues>. Thanks!
```

To fix, this PR adjusts `clear_doc` to save the keys to remove in a list and then remove them.

Reviewed by @lydia-duncan - thanks!
Copy link
Member

@lydia-duncan lydia-duncan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor typo, but otherwise I think this looks good (though it'll be great for Ben to take a look as well)

util/config/update-if-different Outdated Show resolved Hide resolved
e-kayrakli added a commit that referenced this pull request Dec 14, 2020
Upgrade sphinxcontrib version, fix some spec highligting issues

This PR:

- Upgrades sphinxcontrib-chapeldomain from 0.0.17 to 0.0.19, which includes:
  - A more complete pygments highlighter: chapel-lang/sphinxcontrib-chapeldomain#41
  - A patch for enabling incremental builds: chapel-lang/sphinxcontrib-chapeldomain#45
    With this change we can now merge #16835

- Fix some of the codeblocks in the specs to have the correct highlighting.
  Previously, these didn't specify a specific highlighting language, because our
  highlighters weren't able to parse the snippets. The new highlighter is able
  to parse them, so this PR just makes them `.. code-block:: chapel`

  See #14623

[Reviewed by @mppf and @lydia-duncan]

Test:
- [x] `third-party/chpl-venv` builds
- [x] `make docs` generates correct highlighting for the changed cases
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
Now 'make docs' should correctly handle the case
where a file is deleted.

---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
Copy link
Member

@ben-albrecht ben-albrecht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall.

I got a docs build error when updating from a really old branch due to man page existing, but I was unable to reproduce it (and it doesn't really make sense to me).

I also spot-checked various edge cases to make sure things were always getting rebuilt if changed.

I have a minor doc request inline. Thanks!

---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
---
Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
@mppf mppf merged commit 06cf289 into chapel-lang:master Dec 14, 2020
@mppf mppf deleted the make-docs-incremental branch December 14, 2020 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

incremental docs build
3 participants