Deprecate env.update()!

To make BuildEnvironment simple, the responsibility of converting
source files to doctrees is moved to Builder class.

CHANGES

@@ -28,6 +28,8 @@ Deprecated
 * ``Sphinx.add_source_parser()`` has changed;  the *suffix* argument has
   been deprecated.  Please use ``Sphinx.add_source_suffix()`` instead.
 * ``sphinx.util.docutils.directive_helper()`` is deprecated.
+* All ``env.update()``, ``env._read_serial()`` and ``env._read_parallel()`` are
+  deprecated.  Please use ``builder.read()`` instead.
 
 Features added
 --------------

sphinx/builders/__init__.py

@@ -16,7 +16,9 @@ from typing import TYPE_CHECKING
 from docutils import nodes
 
 from sphinx.deprecation import RemovedInSphinx20Warning
+from sphinx.environment import BuildEnvironment
 from sphinx.environment.adapters.asset import ImageAdapter
+from sphinx.errors import SphinxError
 from sphinx.util import i18n, import_object, logging, status_iterator
 from sphinx.util.build_phase import BuildPhase
 from sphinx.util.console import bold  # type: ignore
@@ -344,7 +346,7 @@ class Builder(object):
 
         # while reading, collect all warnings from docutils
         with logging.pending_warnings():
-            updated_docnames = set(self.env.update(self.config, self.srcdir, self.doctreedir))
+            updated_docnames = set(self.read())
 
         doccount = len(updated_docnames)
         logger.info(bold('looking for now-outdated files... '), nonl=1)
@@ -403,6 +405,106 @@ class Builder(object):
         # wait for all tasks
         self.finish_tasks.join()
 
+    def read(self):
+        # type: () -> List[unicode]
+        """(Re-)read all files new or changed since last update.
+
+        Store all environment docnames in the canonical format (ie using SEP as
+        a separator in place of os.path.sep).
+        """
+        updated, reason = self.env.update_config(self.config, self.srcdir, self.doctreedir)
+
+        logger.info(bold('updating environment: '), nonl=True)
+
+        self.env.find_files(self.config, self)
+        added, changed, removed = self.env.get_outdated_files(updated)
+
+        # allow user intervention as well
+        for docs in self.app.emit('env-get-outdated', self, added, changed, removed):
+            changed.update(set(docs) & self.env.found_docs)
+
+        # if files were added or removed, all documents with globbed toctrees
+        # must be reread
+        if added or removed:
+            # ... but not those that already were removed
+            changed.update(self.env.glob_toctrees & self.env.found_docs)
+
+        if changed:
+            logger.info('[%s] ', reason, nonl=True)
+        logger.info('%s added, %s changed, %s removed',
+                    len(added), len(changed), len(removed))
+
+        # clear all files no longer present
+        for docname in removed:
+            self.app.emit('env-purge-doc', self.env, docname)
+            self.env.clear_doc(docname)
+
+        # read all new and changed files
+        docnames = sorted(added | changed)
+        # allow changing and reordering the list of docs to read
+        self.app.emit('env-before-read-docs', self.env, docnames)
+
+        # check if we should do parallel or serial read
+        if parallel_available and len(docnames) > 5 and self.app.parallel > 1:
+            par_ok = self.app.is_parallel_allowed('read')
+        else:
+            par_ok = False
+
+        if par_ok:
+            self._read_parallel(docnames, nproc=self.app.parallel)
+        else:
+            self._read_serial(docnames)
+
+        if self.config.master_doc not in self.env.all_docs:
+            raise SphinxError('master file %s not found' %
+                              self.env.doc2path(self.config.master_doc))
+
+        for retval in self.app.emit('env-updated', self.env):
+            if retval is not None:
+                docnames.extend(retval)
+
+        return sorted(docnames)
+
+    def _read_serial(self, docnames):
+        # type: (List[unicode]) -> None
+        for docname in status_iterator(docnames, 'reading sources... ', "purple",
+                                       len(docnames), self.app.verbosity):
+            # remove all inventory entries for that file
+            self.app.emit('env-purge-doc', self.env, docname)
+            self.env.clear_doc(docname)
+            self.env.read_doc(docname, self.app)
+
+    def _read_parallel(self, docnames, nproc):
+        # type: (List[unicode], int) -> None
+        # clear all outdated docs at once
+        for docname in docnames:
+            self.app.emit('env-purge-doc', self.env, docname)
+            self.env.clear_doc(docname)
+
+        def read_process(docs):
+            # type: (List[unicode]) -> unicode
+            self.env.app = self.app
+            for docname in docs:
+                self.env.read_doc(docname, self.app)
+            # allow pickling self to send it back
+            return BuildEnvironment.dumps(self.env)
+
+        def merge(docs, otherenv):
+            # type: (List[unicode], unicode) -> None
+            env = BuildEnvironment.loads(otherenv)
+            self.env.merge_info_from(docs, env, self.app)
+
+        tasks = ParallelTasks(nproc)
+        chunks = make_chunks(docnames, nproc)
+
+        for chunk in status_iterator(chunks, 'reading sources... ', "purple",
+                                     len(chunks), self.app.verbosity):
+            tasks.add_task(read_process, chunk, merge)
+
+        # make sure all threads have finished
+        logger.info(bold('waiting for workers...'))
+        tasks.join()
+
     def write(self, build_docnames, updated_docnames, method='update'):
         # type: (Iterable[unicode], Sequence[unicode], unicode) -> None
         if build_docnames is None or build_docnames == ['__all__']:

sphinx/environment/__init__.py

@@ -26,21 +26,19 @@ from six import BytesIO, itervalues, class_types, next
 from six.moves import cPickle as pickle
 
 from sphinx import addnodes, versioning
-from sphinx.deprecation import RemovedInSphinx20Warning
+from sphinx.deprecation import RemovedInSphinx20Warning, RemovedInSphinx30Warning
 from sphinx.environment.adapters.indexentries import IndexEntries
 from sphinx.environment.adapters.toctree import TocTree
 from sphinx.errors import SphinxError, ExtensionError
 from sphinx.io import read_doc
 from sphinx.transforms import SphinxTransformer
-from sphinx.util import get_matching_docs, FilenameUniqDict, status_iterator
+from sphinx.util import get_matching_docs, FilenameUniqDict
 from sphinx.util import logging, rst
-from sphinx.util.console import bold  # type: ignore
 from sphinx.util.docutils import sphinx_domains, WarningStream
 from sphinx.util.i18n import find_catalog_files
 from sphinx.util.matching import compile_matchers
 from sphinx.util.nodes import is_translatable
 from sphinx.util.osutil import SEP, ensuredir
-from sphinx.util.parallel import ParallelTasks, parallel_available, make_chunks
 from sphinx.util.websupport import is_commentable
 
 if TYPE_CHECKING:
@@ -499,106 +497,6 @@ class BuildEnvironment(object):
 
         return added, changed, removed
 
-    def update(self, config, srcdir, doctreedir):
-        # type: (Config, unicode, unicode) -> List[unicode]
-        """(Re-)read all files new or changed since last update.
-
-        Store all environment docnames in the canonical format (ie using SEP as
-        a separator in place of os.path.sep).
-        """
-        updated, reason = self.update_config(config, srcdir, doctreedir)
-
-        logger.info(bold('updating environment: '), nonl=True)
-
-        self.find_files(config, self.app.builder)
-        added, changed, removed = self.get_outdated_files(updated)
-
-        # allow user intervention as well
-        for docs in self.app.emit('env-get-outdated', self, added, changed, removed):
-            changed.update(set(docs) & self.found_docs)
-
-        # if files were added or removed, all documents with globbed toctrees
-        # must be reread
-        if added or removed:
-            # ... but not those that already were removed
-            changed.update(self.glob_toctrees & self.found_docs)
-
-        if changed:
-            logger.info('[%s] ', reason, nonl=True)
-        logger.info('%s added, %s changed, %s removed',
-                    len(added), len(changed), len(removed))
-
-        # clear all files no longer present
-        for docname in removed:
-            self.app.emit('env-purge-doc', self, docname)
-            self.clear_doc(docname)
-
-        # read all new and changed files
-        docnames = sorted(added | changed)
-        # allow changing and reordering the list of docs to read
-        self.app.emit('env-before-read-docs', self, docnames)
-
-        # check if we should do parallel or serial read
-        if parallel_available and len(docnames) > 5 and self.app.parallel > 1:
-            par_ok = self.app.is_parallel_allowed('read')
-        else:
-            par_ok = False
-
-        if par_ok:
-            self._read_parallel(docnames, self.app, nproc=self.app.parallel)
-        else:
-            self._read_serial(docnames, self.app)
-
-        if config.master_doc not in self.all_docs:
-            raise SphinxError('master file %s not found' %
-                              self.doc2path(config.master_doc))
-
-        for retval in self.app.emit('env-updated', self):
-            if retval is not None:
-                docnames.extend(retval)
-
-        return sorted(docnames)
-
-    def _read_serial(self, docnames, app):
-        # type: (List[unicode], Sphinx) -> None
-        for docname in status_iterator(docnames, 'reading sources... ', "purple",
-                                       len(docnames), self.app.verbosity):
-            # remove all inventory entries for that file
-            app.emit('env-purge-doc', self, docname)
-            self.clear_doc(docname)
-            self.read_doc(docname, app)
-
-    def _read_parallel(self, docnames, app, nproc):
-        # type: (List[unicode], Sphinx, int) -> None
-        # clear all outdated docs at once
-        for docname in docnames:
-            app.emit('env-purge-doc', self, docname)
-            self.clear_doc(docname)
-
-        def read_process(docs):
-            # type: (List[unicode]) -> unicode
-            self.app = app
-            for docname in docs:
-                self.read_doc(docname, app)
-            # allow pickling self to send it back
-            return BuildEnvironment.dumps(self)
-
-        def merge(docs, otherenv):
-            # type: (List[unicode], unicode) -> None
-            env = BuildEnvironment.loads(otherenv)
-            self.merge_info_from(docs, env, app)
-
-        tasks = ParallelTasks(nproc)
-        chunks = make_chunks(docnames, nproc)
-
-        for chunk in status_iterator(chunks, 'reading sources... ', "purple",
-                                     len(chunks), self.app.verbosity):
-            tasks.add_task(read_process, chunk, merge)
-
-        # make sure all threads have finished
-        logger.info(bold('waiting for workers...'))
-        tasks.join()
-
     def check_dependents(self, app, already):
         # type: (Sphinx, Set[unicode]) -> Iterator[unicode]
         to_rewrite = []  # type: List[unicode]
@@ -945,3 +843,23 @@ class BuildEnvironment(object):
         for domain in self.domains.values():
             domain.check_consistency()
         self.app.emit('env-check-consistency', self)
+
+    # --------- METHODS FOR COMPATIBILITY --------------------------------------
+
+    def update(self, config, srcdir, doctreedir):
+        # type: (Config, unicode, unicode) -> List[unicode]
+        warnings.warn('env.update() is deprecated. Please use builder.read() instead.',
+                      RemovedInSphinx30Warning)
+        return self.app.builder.read()
+
+    def _read_serial(self, docnames, app):
+        # type: (List[unicode], Sphinx) -> None
+        warnings.warn('env._read_serial() is deprecated. Please use builder.read() instead.',
+                      RemovedInSphinx30Warning)
+        return self.app.builder._read_serial(docnames)
+
+    def _read_parallel(self, docnames, app, nproc):
+        # type: (List[unicode], Sphinx, int) -> None
+        warnings.warn('env._read_parallel() is deprecated. Please use builder.read() instead.',
+                      RemovedInSphinx30Warning)
+        return self.app.builder._read_parallel(docnames, nproc)

tests/test_builder.py

@@ -0,0 +1,56 @@
+# -*- coding: utf-8 -*-
+"""
+    test_builder
+    ~~~~~~~~
+
+    Test the Builder class.
+
+    :copyright: Copyright 2007-2018 by the Sphinx team, see AUTHORS.
+    :license: BSD, see LICENSE for details.
+"""
+import pytest
+
+
+@pytest.mark.sphinx('dummy', srcdir="test_builder")
+def test_incremental_reading(app):
+    # first reading
+    updated = app.builder.read()
+    assert set(updated) == app.env.found_docs == set(app.env.all_docs)
+
+    # test if exclude_patterns works ok
+    assert 'subdir/excluded' not in app.env.found_docs
+
+    # before second reading, add, modify and remove source files
+    (app.srcdir / 'new.txt').write_text('New file\n========\n')
+    app.env.all_docs['contents'] = 0  # mark as modified
+    (app.srcdir / 'autodoc.txt').unlink()
+
+    # second reading
+    updated = app.builder.read()
+
+    # "includes" and "images" are in there because they contain references
+    # to nonexisting downloadable or image files, which are given another
+    # chance to exist
+    assert set(updated) == set(['contents', 'new', 'includes', 'images'])
+    assert 'autodoc' not in app.env.all_docs
+    assert 'autodoc' not in app.env.found_docs
+
+
+@pytest.mark.sphinx('dummy')
+def test_env_read_docs(app):
+    """By default, docnames are read in alphanumeric order"""
+    def on_env_read_docs_1(app, env, docnames):
+        pass
+
+    app.connect('env-before-read-docs', on_env_read_docs_1)
+
+    read_docnames = app.builder.read()
+    assert len(read_docnames) > 2 and read_docnames == sorted(read_docnames)
+
+    def on_env_read_docs_2(app, env, docnames):
+        docnames.remove('images')
+
+    app.connect('env-before-read-docs', on_env_read_docs_2)
+
+    read_docnames = app.builder.read()
+    assert len(read_docnames) == 2

tests/test_environment.py

@@ -12,38 +12,15 @@ import pytest
 
 from sphinx.builders.html import StandaloneHTMLBuilder
 from sphinx.builders.latex import LaTeXBuilder
-from sphinx.testing.util import SphinxTestApp, path
-
-app = env = None
 
 
-@pytest.fixture(scope='module', autouse=True)
+@pytest.mark.sphinx('dummy')
-def setup_module(rootdir, sphinx_test_tempdir):
+def test_images(app):
-    global app, env
+    app.build()
-    srcdir = sphinx_test_tempdir / 'root-envtest'
-    if not srcdir.exists():
-        (rootdir / 'test-root').copytree(srcdir)
-    app = SphinxTestApp(srcdir=srcdir)
-    env = app.env
-    yield
-    app.cleanup()
-
-
-# Tests are run in the order they appear in the file, therefore we can
-# afford to not run update() in the setup but in its own test
-
-def test_first_update():
-    updated = env.update(app.config, app.srcdir, app.doctreedir)
-    assert set(updated) == env.found_docs == set(env.all_docs)
-    # test if exclude_patterns works ok
-    assert 'subdir/excluded' not in env.found_docs
-
-
-def test_images():
     assert ('image file not readable: foo.png'
             in app._warning.getvalue())
 
-    tree = env.get_doctree('images')
+    tree = app.env.get_doctree('images')
     htmlbuilder = StandaloneHTMLBuilder(app)
     htmlbuilder.set_environment(app.env)
     htmlbuilder.init()
@@ -67,44 +44,10 @@ def test_images():
              'svgimg.pdf', 'img.foo.png'])
 
 
-def test_second_update():
+@pytest.mark.sphinx('dummy')
-    # delete, add and "edit" (change saved mtime) some files and update again
+def test_object_inventory(app):
-    env.all_docs['contents'] = 0
+    app.build()
-    root = path(app.srcdir)
+    refs = app.env.domaindata['py']['objects']
-    # important: using "autodoc" because it is the last one to be included in
-    # the contents.txt toctree; otherwise section numbers would shift
-    (root / 'autodoc.txt').unlink()
-    (root / 'new.txt').write_text('New file\n========\n')
-    updated = env.update(app.config, app.srcdir, app.doctreedir)
-    # "includes" and "images" are in there because they contain references
-    # to nonexisting downloadable or image files, which are given another
-    # chance to exist
-    assert set(updated) == set(['contents', 'new', 'includes', 'images'])
-    assert 'autodoc' not in env.all_docs
-    assert 'autodoc' not in env.found_docs
-
-
-def test_env_read_docs():
-    """By default, docnames are read in alphanumeric order"""
-    def on_env_read_docs_1(app, env, docnames):
-        pass
-
-    app.connect('env-before-read-docs', on_env_read_docs_1)
-
-    read_docnames = env.update(app.config, app.srcdir, app.doctreedir)
-    assert len(read_docnames) > 2 and read_docnames == sorted(read_docnames)
-
-    def on_env_read_docs_2(app, env, docnames):
-        docnames.remove('images')
-
-    app.connect('env-before-read-docs', on_env_read_docs_2)
-
-    read_docnames = env.update(app.config, app.srcdir, app.doctreedir)
-    assert len(read_docnames) == 2
-
-
-def test_object_inventory():
-    refs = env.domaindata['py']['objects']
 
     assert 'func_without_module' in refs
     assert refs['func_without_module'] == ('objects', 'function')
@@ -121,8 +64,8 @@ def test_object_inventory():
     assert 'func_in_module' not in refs
     assert 'func_noindex' not in refs
 
-    assert env.domaindata['py']['modules']['mod'] == \
+    assert app.env.domaindata['py']['modules']['mod'] == \
         ('objects', 'Module synopsis.', 'UNIX', False)
 
-    assert env.domains['py'].data is env.domaindata['py']
+    assert app.env.domains['py'].data is app.env.domaindata['py']
-    assert env.domains['c'].data is env.domaindata['c']
+    assert app.env.domains['c'].data is app.env.domaindata['c']

tests/test_intl.py

@@ -525,7 +525,8 @@ def test_gettext_buildr_ignores_only_directive(app):
 def test_gettext_dont_rebuild_mo(make_app, app_params, build_mo):
     # --- don't rebuild by .mo mtime
     def get_number_of_update_targets(app_):
-        updated = app_.env.update(app_.config, app_.srcdir, app_.doctreedir)
+        app_.env.find_files(app_.config, app_.builder)
+        _, updated, _ = app_.env.get_outdated_files(config_changed=False)
         return len(updated)
 
     args, kwargs = app_params
@@ -706,12 +707,14 @@ def test_html_rebuild_mo(app):
     app.build()
     # --- rebuild by .mo mtime
     app.builder.build_update()
-    updated = app.env.update(app.config, app.srcdir, app.doctreedir)
+    app.env.find_files(app.config, app.builder)
+    _, updated, _ = app.env.get_outdated_files(config_changed=False)
     assert len(updated) == 0
 
     mtime = (app.srcdir / 'xx' / 'LC_MESSAGES' / 'bom.mo').stat().st_mtime
     (app.srcdir / 'xx' / 'LC_MESSAGES' / 'bom.mo').utime((mtime + 5, mtime + 5))
-    updated = app.env.update(app.config, app.srcdir, app.doctreedir)
+    app.env.find_files(app.config, app.builder)
+    _, updated, _ = app.env.get_outdated_files(config_changed=False)
     assert len(updated) == 1