Merge pull request #2744 from tk0miya/refactor_copying_assets

Refactor copying assets

CHANGES

@@ -21,6 +21,8 @@ Incompatible changes
 * #2454: The filename of sourcelink is now changed.  The value of
   `html_sourcelink_suffix` will be appended to the original filename (like
   ``index.rst.txt``).
+* ``sphinx.util.copy_static_entry()`` is now deprecated.
+  Use ``sphinx.util.fileutil.copy_asset()`` instead.
 
 
 Features added

sphinx/builders/applehelp.py

@@ -18,11 +18,11 @@ import shlex
 
 from sphinx.builders.html import StandaloneHTMLBuilder
 from sphinx.config import string_classes
-from sphinx.util import copy_static_entry
 from sphinx.util.osutil import copyfile, ensuredir, make_filename
 from sphinx.util.console import bold
+from sphinx.util.fileutil import copy_asset
 from sphinx.util.pycompat import htmlescape
-from sphinx.util.matching import compile_matchers
+from sphinx.util.matching import Matcher
 from sphinx.errors import SphinxError
 
 import plistlib
@@ -107,17 +107,15 @@ class AppleHelpBuilder(StandaloneHTMLBuilder):
         self.finish_tasks.add_task(self.build_helpbook)
 
     def copy_localized_files(self):
-        source_dir = path.join(self.confdir,
-                               self.config.applehelp_locale + '.lproj')
+        source_dir = path.join(self.confdir, self.config.applehelp_locale + '.lproj')
         target_dir = self.outdir
 
         if path.isdir(source_dir):
             self.info(bold('copying localized files... '), nonl=True)
 
-            ctx = self.globalcontext.copy()
-            matchers = compile_matchers(self.config.exclude_patterns)
-            copy_static_entry(source_dir, target_dir, self, ctx,
-                              exclude_matchers=matchers)
+            excluded = Matcher(self.config.exclude_patterns + ['**/.*'])
+            copy_asset(source_dir, target_dir, excluded,
+                       context=self.globalcontext, renderer=self.templates)
 
             self.info('done')
 

sphinx/builders/changes.py

@@ -15,12 +15,12 @@ from os import path
 from six import iteritems
 
 from sphinx import package_dir
-from sphinx.util import copy_static_entry
 from sphinx.locale import _
 from sphinx.theming import Theme
 from sphinx.builders import Builder
 from sphinx.util.osutil import ensuredir, os_path
 from sphinx.util.console import bold
+from sphinx.util.fileutil import copy_asset_file
 from sphinx.util.pycompat import htmlescape
 
 
@@ -138,12 +138,10 @@ class ChangesBuilder(Builder):
                 f.write(self.templates.render('changes/rstsource.html', ctx))
         themectx = dict(('theme_' + key, val) for (key, val) in
                         iteritems(self.theme.get_options({})))
-        copy_static_entry(path.join(package_dir, 'themes', 'default',
-                                    'static', 'default.css_t'),
-                          self.outdir, self, themectx)
-        copy_static_entry(path.join(package_dir, 'themes', 'basic',
-                                    'static', 'basic.css'),
-                          self.outdir, self)
+        copy_asset_file(path.join(package_dir, 'themes', 'default', 'static', 'default.css_t'),
+                        self.outdir, context=themectx, renderer=self.templates)
+        copy_asset_file(path.join(package_dir, 'themes', 'basic', 'static', 'basic.css'),
+                        self.outdir)
 
     def hl(self, text, version):
         text = htmlescape(text)

sphinx/builders/html.py

@@ -27,12 +27,13 @@ from docutils.frontend import OptionParser
 from docutils.readers.doctree import Reader as DoctreeReader
 
 from sphinx import package_dir, __display_version__
-from sphinx.util import jsonimpl, copy_static_entry, copy_extra_entry
+from sphinx.util import jsonimpl
 from sphinx.util.i18n import format_date
 from sphinx.util.osutil import SEP, os_path, relative_uri, ensuredir, \
     movefile, copyfile
 from sphinx.util.nodes import inline_all_toctrees
-from sphinx.util.matching import patmatch, compile_matchers
+from sphinx.util.fileutil import copy_asset
+from sphinx.util.matching import patmatch, Matcher, DOTFILES
 from sphinx.config import string_classes
 from sphinx.locale import _, l_
 from sphinx.search import js_index
@@ -613,21 +614,19 @@ class StandaloneHTMLBuilder(Builder):
 
         # then, copy over theme-supplied static files
         if self.theme:
-            themeentries = [path.join(themepath, 'static')
-                            for themepath in self.theme.get_dirchain()[::-1]]
-            for entry in themeentries:
-                copy_static_entry(entry, path.join(self.outdir, '_static'),
-                                  self, ctx)
+            for theme_path in self.theme.get_dirchain()[::-1]:
+                entry = path.join(theme_path, 'static')
+                copy_asset(entry, path.join(self.outdir, '_static'), excluded=DOTFILES,
+                           context=ctx, renderer=self.templates)
         # then, copy over all user-supplied static files
-        staticentries = [path.join(self.confdir, spath)
-                         for spath in self.config.html_static_path]
-        matchers = compile_matchers(self.config.exclude_patterns)
-        for entry in staticentries:
+        excluded = Matcher(self.config.exclude_patterns + ["**/.*"])
+        for static_path in self.config.html_static_path:
+            entry = path.join(self.confdir, static_path)
             if not path.exists(entry):
                 self.warn('html_static_path entry %r does not exist' % entry)
                 continue
-            copy_static_entry(entry, path.join(self.outdir, '_static'), self,
-                              ctx, exclude_matchers=matchers)
+            copy_asset(entry, path.join(self.outdir, '_static'), excluded,
+                       context=ctx, renderer=self.templates)
         # copy logo and favicon files if not already in static path
         if self.config.html_logo:
             logobase = path.basename(self.config.html_logo)
@@ -650,14 +649,15 @@ class StandaloneHTMLBuilder(Builder):
     def copy_extra_files(self):
         # copy html_extra_path files
         self.info(bold('copying extra files... '), nonl=True)
-        extraentries = [path.join(self.confdir, epath)
-                        for epath in self.config.html_extra_path]
-        matchers = compile_matchers(self.config.exclude_patterns)
-        for entry in extraentries:
+        excluded = Matcher(self.config.exclude_patterns)
+
+        for extra_path in self.config.html_extra_path:
+            entry = path.join(self.confdir, extra_path)
             if not path.exists(entry):
                 self.warn('html_extra_path entry %r does not exist' % entry)
                 continue
-            copy_extra_entry(entry, self.outdir, matchers)
+
+            copy_asset(entry, self.outdir, excluded)
         self.info('done')
 
     def write_buildinfo(self):
@@ -1069,6 +1069,7 @@ class SerializingHTMLBuilder(StandaloneHTMLBuilder):
         self.theme = None       # no theme necessary
         self.templates = None   # no template bridge necessary
         self.init_translator_class()
+        self.init_templates()
         self.init_highlighter()
 
     def get_target_uri(self, docname, typ=None):

sphinx/util/__init__.py

@@ -18,7 +18,7 @@ import posixpath
 import traceback
 import unicodedata
 from os import path
-from codecs import open, BOM_UTF8
+from codecs import BOM_UTF8
 from collections import deque
 
 from six import iteritems, text_type, binary_type
@@ -32,6 +32,7 @@ import jinja2
 import sphinx
 from sphinx.errors import PycodeError, SphinxParallelError, ExtensionError
 from sphinx.util.console import strip_colors
+from sphinx.util.fileutil import copy_asset_file
 from sphinx.util.osutil import fs_encoding
 
 # import other utilities; partly for backwards compatibility, so don't
@@ -148,7 +149,7 @@ class FilenameUniqDict(dict):
 
 def copy_static_entry(source, targetdir, builder, context={},
                       exclude_matchers=(), level=0):
-    """Copy a HTML builder static_path entry from source to targetdir.
+    """[DEPRECATED] Copy a HTML builder static_path entry from source to targetdir.
 
     Handles all possible cases of files, directories and subdirectories.
     """
@@ -158,16 +159,7 @@ def copy_static_entry(source, targetdir, builder, context={},
             if matcher(relpath):
                 return
     if path.isfile(source):
-        target = path.join(targetdir, path.basename(source))
-        if source.lower().endswith('_t') and builder.templates:
-            # templated!
-            fsrc = open(source, 'r', encoding='utf-8')
-            fdst = open(target[:-2], 'w', encoding='utf-8')
-            fdst.write(builder.templates.render_string(fsrc.read(), context))
-            fsrc.close()
-            fdst.close()
-        else:
-            copyfile(source, target)
+        copy_asset_file(source, targetdir, context, builder.templates)
     elif path.isdir(source):
         if not path.isdir(targetdir):
             os.mkdir(targetdir)
@@ -181,37 +173,6 @@ def copy_static_entry(source, targetdir, builder, context={},
                               builder, context, level=level+1,
                               exclude_matchers=exclude_matchers)
 
-
-def copy_extra_entry(source, targetdir, exclude_matchers=()):
-    """Copy a HTML builder extra_path entry from source to targetdir.
-
-    Handles all possible cases of files, directories and subdirectories.
-    """
-    def excluded(path):
-        relpath = relative_path(os.path.dirname(source), path)
-        return any(matcher(relpath) for matcher in exclude_matchers)
-
-    def copy_extra_file(source_, targetdir_):
-        if not excluded(source_):
-            target = path.join(targetdir_, os.path.basename(source_))
-            copyfile(source_, target)
-
-    if os.path.isfile(source):
-        copy_extra_file(source, targetdir)
-        return
-
-    for root, dirs, files in os.walk(source):
-        reltargetdir = os.path.join(targetdir, relative_path(source, root))
-        for dir in dirs[:]:
-            if excluded(os.path.join(root, dir)):
-                dirs.remove(dir)
-            else:
-                target = os.path.join(reltargetdir, dir)
-                if not path.exists(target):
-                    os.mkdir(target)
-        for file in files:
-            copy_extra_file(os.path.join(root, file), reltargetdir)
-
 _DEBUG_HEADER = '''\
 # Sphinx version: %s
 # Python version: %s (%s)

sphinx/util/fileutil.py

@@ -0,0 +1,80 @@
+# -*- coding: utf-8 -*-
+"""
+    sphinx.util.fileutil
+    ~~~~~~~~~~~~~~~~~~~~
+
+    File utility functions for Sphinx.
+
+    :copyright: Copyright 2007-2016 by the Sphinx team, see AUTHORS.
+    :license: BSD, see LICENSE for details.
+"""
+import os
+import codecs
+import posixpath
+from docutils.utils import relative_path
+from sphinx.util.osutil import copyfile, ensuredir, walk
+
+
+def copy_asset_file(source, destination, context=None, renderer=None):
+    """Copy an asset file to destination.
+
+    On copying, it expands the template variables if context argument is given and
+    the asset is a template file.
+
+    :param source: The path to source file
+    :param destination: The path to destination file or directory
+    :param context: The template variables. If not given, template files are simply copied
+    :param renderer: The template engine
+    """
+    if not os.path.exists(source):
+        return
+
+    if os.path.exists(destination) and os.path.isdir(destination):
+        # Use source filename if destination points a directory
+        destination = os.path.join(destination, os.path.basename(source))
+
+    if source.lower().endswith('_t') and context:
+        if renderer is None:
+            msg = 'Template engine is not initialized. Failed to render %s' % source
+            raise RuntimeError(msg)
+
+        with codecs.open(source, 'r', encoding='utf-8') as fsrc:
+            with codecs.open(destination[:-2], 'w', encoding='utf-8') as fdst:
+                fdst.write(renderer.render_string(fsrc.read(), context))
+    else:
+        copyfile(source, destination)
+
+
+def copy_asset(source, destination, excluded=lambda path: False, context=None, renderer=None):
+    """Copy asset files to destination recursively.
+
+    On copying, it expands the template variables if context argument is given and
+    the asset is a template file.
+
+    :param source: The path to source file or directory
+    :param destination: The path to destination directory
+    :param excluded: The matcher to determine the given path should be copied or not
+    :param context: The template variables. If not given, template files are simply copied
+    :param renderer: The template engine
+    """
+    if not os.path.exists(source):
+        return
+
+    ensuredir(destination)
+    if os.path.isfile(source):
+        copy_asset_file(source, destination, context, renderer)
+        return
+
+    for root, dirs, files in walk(source):
+        reldir = relative_path(source, root)
+        for dir in dirs[:]:
+            if excluded(posixpath.join(reldir, dir)):
+                dirs.remove(dir)
+            else:
+                ensuredir(posixpath.join(destination, reldir, dir))
+
+        for filename in files:
+            if not excluded(posixpath.join(reldir, filename)):
+                copy_asset_file(posixpath.join(root, filename),
+                                posixpath.join(destination, reldir),
+                                context, renderer)

sphinx/util/matching.py

@@ -62,6 +62,27 @@ def compile_matchers(patterns):
     return [re.compile(_translate_pattern(pat)).match for pat in patterns]
 
 
+class Matcher(object):
+    """A pattern matcher for Multiple shell-style glob patterns.
+
+    Note: this modifies the patterns to work with copy_asset().
+          For example, "**/index.rst" matches with "index.rst"
+    """
+
+    def __init__(self, patterns):
+        expanded = [pat[3:] for pat in patterns if pat.startswith('**/')]
+        self.patterns = compile_matchers(patterns + expanded)
+
+    def __call__(self, string):
+        return self.match(string)
+
+    def match(self, string):
+        return any(pat(string) for pat in self.patterns)
+
+
+DOTFILES = Matcher(['**/.*'])
+
+
 _pat_cache = {}
 
 

tests/roots/test-html_assets/conf.py

@@ -1,6 +1,9 @@
 # -*- coding: utf-8 -*-
 
 master_doc = 'index'
+project = 'Sphinx'
+version = '1.4.4'
 
+html_static_path = ['static', 'subdir']
 html_extra_path = ['extra', 'subdir']
 exclude_patterns = ['**/_build', '**/.htpasswd']

tests/roots/test-html_assets/extra/API.html_t

@@ -0,0 +1 @@
+{{ project }}-{{ version }}

tests/roots/test-html_assets/static/API.html_t

@@ -0,0 +1 @@
+{{ project }}-{{ version }}

tests/test_build_html.py

@@ -976,10 +976,23 @@ def test_jsmath(app, status, warning):
     assert '<div class="math">\na + 1 &lt; b</div>' in content
 
 
-@with_app(buildername='html', testroot='html_extra_path')
-def test_html_extra_path(app, status, warning):
+@with_app(buildername='html', testroot='html_assets')
+def test_html_assets(app, status, warning):
     app.builder.build_all()
 
+    # html_static_path
+    assert not (app.outdir / '_static' / '.htaccess').exists()
+    assert not (app.outdir / '_static' / '.htpasswd').exists()
+    assert (app.outdir / '_static' / 'API.html').exists()
+    assert (app.outdir / '_static' / 'API.html').text() == 'Sphinx-1.4.4'
+    assert (app.outdir / '_static' / 'css/style.css').exists()
+    assert (app.outdir / '_static' / 'rimg.png').exists()
+    assert not (app.outdir / '_static' / '_build/index.html').exists()
+    assert (app.outdir / '_static' / 'background.png').exists()
+    assert not (app.outdir / '_static' / 'subdir' / '.htaccess').exists()
+    assert not (app.outdir / '_static' / 'subdir' / '.htpasswd').exists()
+
+    # html_extra_path
     assert (app.outdir / '.htaccess').exists()
     assert not (app.outdir / '.htpasswd').exists()
     assert (app.outdir / 'API.html_t').exists()
@@ -987,6 +1000,8 @@ def test_html_extra_path(app, status, warning):
     assert (app.outdir / 'rimg.png').exists()
     assert not (app.outdir / '_build/index.html').exists()
     assert (app.outdir / 'background.png').exists()
+    assert (app.outdir / 'subdir' / '.htaccess').exists()
+    assert not (app.outdir / 'subdir' / '.htpasswd').exists()
 
 
 @with_app(buildername='html', confoverrides={'html_sourcelink_suffix': ''})

tests/test_util_fileutil.py

@@ -0,0 +1,114 @@
+# -*- coding: utf-8 -*-
+"""
+    test_util_fileutil
+    ~~~~~~~~~~~~~~~~~~
+
+    Tests sphinx.util.fileutil functions.
+
+    :copyright: Copyright 2007-2016 by the Sphinx team, see AUTHORS.
+    :license: BSD, see LICENSE for details.
+"""
+from sphinx.util.fileutil import copy_asset, copy_asset_file
+from sphinx.jinja2glue import BuiltinTemplateLoader
+
+from mock import Mock
+from util import with_tempdir
+
+
+class DummyTemplateLoader(BuiltinTemplateLoader):
+    def __init__(self):
+        BuiltinTemplateLoader.__init__(self)
+        builder = Mock()
+        builder.config.templates_path = []
+        builder.app.translater = None
+        self.init(builder)
+
+
+@with_tempdir
+def test_copy_asset_file(tmpdir):
+    renderer = DummyTemplateLoader()
+
+    # copy normal file
+    src = (tmpdir / 'asset.txt')
+    src.write_text('# test data')
+    dest = (tmpdir / 'output.txt')
+
+    copy_asset_file(src, dest)
+    assert dest.exists()
+    assert src.text() == dest.text()
+
+    # copy template file
+    src = (tmpdir / 'asset.txt_t')
+    src.write_text('# {{var1}} data')
+    dest = (tmpdir / 'output.txt_t')
+
+    copy_asset_file(src, dest, {'var1': 'template'}, renderer)
+    assert not dest.exists()
+    assert (tmpdir / 'output.txt').exists()
+    assert (tmpdir / 'output.txt').text() == '# template data'
+
+    # copy template file to subdir
+    src = (tmpdir / 'asset.txt_t')
+    src.write_text('# {{var1}} data')
+    subdir1 = (tmpdir / 'subdir')
+    subdir1.makedirs()
+
+    copy_asset_file(src, subdir1, {'var1': 'template'}, renderer)
+    assert (subdir1 / 'asset.txt').exists()
+    assert (subdir1 / 'asset.txt').text() == '# template data'
+
+    # copy template file without context
+    src = (tmpdir / 'asset.txt_t')
+    subdir2 = (tmpdir / 'subdir2')
+    subdir2.makedirs()
+
+    copy_asset_file(src, subdir2)
+    assert not (subdir2 / 'asset.txt').exists()
+    assert (subdir2 / 'asset.txt_t').exists()
+    assert (subdir2 / 'asset.txt_t').text() == '# {{var1}} data'
+
+
+@with_tempdir
+def test_copy_asset(tmpdir):
+    renderer = DummyTemplateLoader()
+
+    # prepare source files
+    source = (tmpdir / 'source')
+    source.makedirs()
+    (source / 'index.rst').write_text('index.rst')
+    (source / 'foo.rst_t').write_text('{{var1}}.rst')
+    (source / '_static').makedirs()
+    (source / '_static' / 'basic.css').write_text('basic.css')
+    (source / '_templates').makedirs()
+    (source / '_templates' / 'layout.html').write_text('layout.html')
+    (source / '_templates' / 'sidebar.html_t').write_text('sidebar: {{var2}}')
+
+    # copy a single file
+    assert not (tmpdir / 'test1').exists()
+    copy_asset(source / 'index.rst', tmpdir / 'test1')
+    assert (tmpdir / 'test1').exists()
+    assert (tmpdir / 'test1/index.rst').exists()
+
+    # copy directories
+    destdir = tmpdir / 'test2'
+    copy_asset(source, destdir, context=dict(var1='bar', var2='baz'), renderer=renderer)
+    assert (destdir / 'index.rst').exists()
+    assert (destdir / 'foo.rst').exists()
+    assert (destdir / 'foo.rst').text() == 'bar.rst'
+    assert (destdir / '_static' / 'basic.css').exists()
+    assert (destdir / '_templates' / 'layout.html').exists()
+    assert (destdir / '_templates' / 'sidebar.html').exists()
+    assert (destdir / '_templates' / 'sidebar.html').text() == 'sidebar: baz'
+
+    # copy with exclusion
+    def excluded(path):
+        return ('sidebar.html' in path or 'basic.css' in path)
+
+    destdir = tmpdir / 'test3'
+    copy_asset(source, destdir, excluded,
+               context=dict(var1='bar', var2='baz'), renderer=renderer)
+    assert (destdir / 'index.rst').exists()
+    assert (destdir / 'foo.rst').exists()
+    assert not (destdir / '_static' / 'basic.css').exists()
+    assert (destdir / '_templates' / 'layout.html').exists()
+    assert not (destdir / '_templates' / 'sidebar.html').exists()

tests/test_util_matching.py

@@ -0,0 +1,91 @@
+# -*- coding: utf-8 -*-
+"""
+    test_util_matching
+    ~~~~~~~~~~~~~~~~~~
+
+    Tests sphinx.util.matching functions.
+
+    :copyright: Copyright 2007-2016 by the Sphinx team, see AUTHORS.
+    :license: BSD, see LICENSE for details.
+"""
+from sphinx.util.matching import compile_matchers, Matcher
+
+
+def test_compile_matchers():
+    # exact matching
+    pat = compile_matchers(['hello.py']).pop()
+    assert pat('hello.py')
+    assert not pat('hello-py')
+    assert not pat('subdir/hello.py')
+
+    # wild card (*)
+    pat = compile_matchers(['hello.*']).pop()
+    assert pat('hello.py')
+    assert pat('hello.rst')
+
+    pat = compile_matchers(['*.py']).pop()
+    assert pat('hello.py')
+    assert pat('world.py')
+    assert not pat('subdir/hello.py')
+
+    # wild card (**)
+    pat = compile_matchers(['hello.**']).pop()
+    assert pat('hello.py')
+    assert pat('hello.rst')
+    assert pat('hello.py/world.py')
+
+    pat = compile_matchers(['**.py']).pop()
+    assert pat('hello.py')
+    assert pat('world.py')
+    assert pat('subdir/hello.py')
+
+    pat = compile_matchers(['**/hello.py']).pop()
+    assert not pat('hello.py')
+    assert pat('subdir/hello.py')
+    assert pat('subdir/subdir/hello.py')
+
+    # wild card (?)
+    pat = compile_matchers(['hello.?']).pop()
+    assert pat('hello.c')
+    assert not pat('hello.py')
+
+    # pattern ([...])
+    pat = compile_matchers(['hello[12\\].py']).pop()
+    assert pat('hello1.py')
+    assert pat('hello2.py')
+    assert pat('hello\\.py')
+    assert not pat('hello3.py')
+
+    pat = compile_matchers(['hello[^12].py']).pop()  # "^" is not negative identifier
+    assert pat('hello1.py')
+    assert pat('hello2.py')
+    assert pat('hello^.py')
+    assert not pat('hello3.py')
+
+    # negative pattern ([!...])
+    pat = compile_matchers(['hello[!12].py']).pop()
+    assert not pat('hello1.py')
+    assert not pat('hello2.py')
+    assert not pat('hello/.py')  # negative pattern does not match to "/"
+    assert pat('hello3.py')
+
+    # non patterns
+    pat = compile_matchers(['hello[.py']).pop()
+    assert pat('hello[.py')
+    assert not pat('hello.py')
+
+    pat = compile_matchers(['hello[].py']).pop()
+    assert pat('hello[].py')
+    assert not pat('hello.py')
+
+    pat = compile_matchers(['hello[!].py']).pop()
+    assert pat('hello[!].py')
+    assert not pat('hello.py')
+
+
+def test_Matcher():
+    matcher = Matcher(['hello.py', '**/world.py'])
+    assert matcher('hello.py')
+    assert not matcher('subdir/hello.py')
+    assert matcher('world.py')
+    assert matcher('subdir/world.py')