Merge branch 'master' into issue-12585-extra/objectsinv-resolution-am…

…biguity-false-positive-reduction

Conflicts:
	CHANGES.rst (manual adjustment)

CHANGES.rst

@@ -1,17 +1,28 @@
-Release 7.4.6 (in development)
+Release 7.4.7 (in development)
 ==============================
 
 Bugs fixed
 ----------
 
+* #12096: Warn when files are overwritten in the build directory.
+  Patch by Adam Turner.
+* #12620: Ensure that old-style object description options are respected.
+  Patch by Adam Turner.
+* #12587: Do not warn when potential ambiguity detected during Intersphinx
+  resolution occurs due to duplicate targets that differ case-insensitively.
+  Patch by James Addison.
+
+Release 7.4.6 (released Jul 18, 2024)
+=====================================
+
+Bugs fixed
+----------
+
 * #12859, #9743, #12609: autosummary: Do not add the package prefix when
   generating autosummary directives for modules within a package.
   Patch by Adam Turner.
 * #12613: Reduce log severity for ambiguity detection during inventory loading.
   Patch by James Addison.
-* #12587: Do not warn when potential ambiguity detected during Intersphinx
-  resolution occurs due to duplicate targets that differ case-insensitively.
-  Patch by James Addison.
 
 Release 7.4.5 (released Jul 16, 2024)
 =====================================

sphinx/__init__.py

@@ -1,6 +1,6 @@
 """The Sphinx documentation toolchain."""
 
-__version__ = '7.4.6'
+__version__ = '7.4.7'
 __display_version__ = __version__  # used for command line version
 
 # Keep this file executable as-is in Python 3!
@@ -27,7 +27,7 @@
 #:
 #: .. versionadded:: 1.2
 #:    Before version 1.2, check the string ``sphinx.__version__``.
-version_info = (7, 4, 6, 'beta', 0)
+version_info = (7, 4, 7, 'beta', 0)
 
 package_dir = os.path.abspath(os.path.dirname(__file__))
 

sphinx/builders/html/__init__.py

@@ -815,8 +815,8 @@ def copy_stemmer_js(self) -> None:
 
     def copy_theme_static_files(self, context: dict[str, Any]) -> None:
         def onerror(filename: str, error: Exception) -> None:
-            logger.warning(__('Failed to copy a file in html_static_file: %s: %r'),
-                           filename, error)
+            msg = __("Failed to copy a file in the theme's 'static' directory: %s: %r")
+            logger.warning(msg, filename, error)
 
         if self.theme:
             for entry in reversed(self.theme.get_theme_dirs()):
@@ -1142,7 +1142,8 @@ def js_tag(js: _JavaScript | str) -> str:
             source_name = path.join(self.outdir, '_sources',
                                     os_path(ctx['sourcename']))
             ensuredir(path.dirname(source_name))
-            copyfile(self.env.doc2path(pagename), source_name)
+            copyfile(self.env.doc2path(pagename), source_name,
+                     __overwrite_warning__=False)
 
     def update_page_context(self, pagename: str, templatename: str,
                             ctx: dict[str, Any], event_arg: Any) -> None:

sphinx/directives/__init__.py

@@ -220,21 +220,26 @@ def run(self) -> list[Node]:
         node['domain'] = self.domain
         # 'desctype' is a backwards compatible attribute
         node['objtype'] = node['desctype'] = self.objtype
+
+        # Copy old option names to new ones
+        # xref RemovedInSphinx90Warning
+        # deprecate noindex, noindexentry, and nocontentsentry in Sphinx 9.0
+        if 'no-index' not in self.options and 'noindex' in self.options:
+            self.options['no-index'] = self.options['noindex']
+        if 'no-index-entry' not in self.options and 'noindexentry' in self.options:
+            self.options['no-index-entry'] = self.options['noindexentry']
+        if 'no-contents-entry' not in self.options and 'nocontentsentry' in self.options:
+            self.options['no-contents-entry'] = self.options['nocontentsentry']
+
         node['no-index'] = node['noindex'] = no_index = (
             'no-index' in self.options
-            # xref RemovedInSphinx90Warning
-            # deprecate noindex in Sphinx 9.0
-            or 'noindex' in self.options)
+        )
         node['no-index-entry'] = node['noindexentry'] = (
             'no-index-entry' in self.options
-            # xref RemovedInSphinx90Warning
-            # deprecate noindexentry in Sphinx 9.0
-            or 'noindexentry' in self.options)
+        )
         node['no-contents-entry'] = node['nocontentsentry'] = (
             'no-contents-entry' in self.options
-            # xref RemovedInSphinx90Warning
-            # deprecate nocontentsentry in Sphinx 9.0
-            or 'nocontentsentry' in self.options)
+        )
         node['no-typesetting'] = ('no-typesetting' in self.options)
         if self.domain:
             node['classes'].append(self.domain)

sphinx/domains/javascript.py

@@ -309,7 +309,7 @@ class JSModule(SphinxDirective):
     def run(self) -> list[Node]:
         mod_name = self.arguments[0].strip()
         self.env.ref_context['js:module'] = mod_name
-        no_index = 'no-index' in self.options or 'noindex' in self.options
+        no_index = 'no-index' in self.options
 
         content_nodes = self.parse_content_to_nodes(allow_section_headings=True)
 

sphinx/domains/python/__init__.py

@@ -452,7 +452,7 @@ def run(self) -> list[Node]:
         domain = cast(PythonDomain, self.env.get_domain('py'))
 
         modname = self.arguments[0].strip()
-        no_index = 'no-index' in self.options or 'noindex' in self.options
+        no_index = 'no-index' in self.options
         self.env.ref_context['py:module'] = modname
 
         content_nodes = self.parse_content_to_nodes(allow_section_headings=True)

sphinx/util/fileutil.py

@@ -8,12 +8,15 @@
 
 from docutils.utils import relative_path
 
+from sphinx.util import logging
 from sphinx.util.osutil import copyfile, ensuredir
 
 if TYPE_CHECKING:
     from sphinx.util.template import BaseRenderer
     from sphinx.util.typing import PathMatcher
 
+logger = logging.getLogger(__name__)
+
 
 def _template_basename(filename: str | os.PathLike[str]) -> str | None:
     """Given an input filename:
@@ -30,7 +33,8 @@ def _template_basename(filename: str | os.PathLike[str]) -> str | None:
 
 def copy_asset_file(source: str | os.PathLike[str], destination: str | os.PathLike[str],
                     context: dict[str, Any] | None = None,
-                    renderer: BaseRenderer | None = None) -> None:
+                    renderer: BaseRenderer | None = None,
+                    *, __overwrite_warning__: bool = True) -> None:
     """Copy an asset file to destination.
 
     On copying, it expands the template variables if context argument is given and
@@ -56,17 +60,36 @@ def copy_asset_file(source: str | os.PathLike[str], destination: str | os.PathLi
             renderer = SphinxRenderer()
 
         with open(source, encoding='utf-8') as fsrc:
-            destination = _template_basename(destination) or destination
-            with open(destination, 'w', encoding='utf-8') as fdst:
-                fdst.write(renderer.render_string(fsrc.read(), context))
+            template_content = fsrc.read()
+        rendered_template = renderer.render_string(template_content, context)
+
+        if (
+            __overwrite_warning__
+            and os.path.exists(destination)
+            and template_content != rendered_template
+        ):
+            # Consider raising an error in Sphinx 8.
+            # Certainly make overwriting user content opt-in.
+            # xref: RemovedInSphinx80Warning
+            # xref: https://github.com/sphinx-doc/sphinx/issues/12096
+            msg = ('Copying the rendered template %s to %s will overwrite data, '
+                   'as a file already exists at the destination path '
+                   'and the content does not match.')
+            logger.info(msg, os.fsdecode(source), os.fsdecode(destination),
+                        type='misc', subtype='copy_overwrite')
+
+        destination = _template_basename(destination) or destination
+        with open(destination, 'w', encoding='utf-8') as fdst:
+            fdst.write(rendered_template)
     else:
-        copyfile(source, destination)
+        copyfile(source, destination, __overwrite_warning__=__overwrite_warning__)
 
 
 def copy_asset(source: str | os.PathLike[str], destination: str | os.PathLike[str],
                excluded: PathMatcher = lambda path: False,
                context: dict[str, Any] | None = None, renderer: BaseRenderer | None = None,
-               onerror: Callable[[str, Exception], None] | None = None) -> None:
+               onerror: Callable[[str, Exception], None] | None = None,
+               *, __overwrite_warning__: bool = True) -> None:
     """Copy asset files to destination recursively.
 
     On copying, it expands the template variables if context argument is given and
@@ -88,7 +111,8 @@ def copy_asset(source: str | os.PathLike[str], destination: str | os.PathLike[st
 
     ensuredir(destination)
     if os.path.isfile(source):
-        copy_asset_file(source, destination, context, renderer)
+        copy_asset_file(source, destination, context, renderer,
+                        __overwrite_warning__=__overwrite_warning__)
         return
 
     for root, dirs, files in os.walk(source, followlinks=True):
@@ -104,7 +128,8 @@ def copy_asset(source: str | os.PathLike[str], destination: str | os.PathLike[st
                 try:
                     copy_asset_file(posixpath.join(root, filename),
                                     posixpath.join(destination, reldir),
-                                    context, renderer)
+                                    context, renderer,
+                                    __overwrite_warning__=__overwrite_warning__)
                 except Exception as exc:
                     if onerror:
                         onerror(posixpath.join(root, filename), exc)

sphinx/util/osutil.py

@@ -88,7 +88,12 @@ def copytimes(source: str | os.PathLike[str], dest: str | os.PathLike[str]) -> N
         os.utime(dest, (st.st_atime, st.st_mtime))
 
 
-def copyfile(source: str | os.PathLike[str], dest: str | os.PathLike[str]) -> None:
+def copyfile(
+    source: str | os.PathLike[str],
+    dest: str | os.PathLike[str],
+    *,
+    __overwrite_warning__: bool = True,
+) -> None:
     """Copy a file and its modification times, if possible.
 
     :param source: An existing source to copy.
@@ -101,7 +106,19 @@ def copyfile(source: str | os.PathLike[str], dest: str | os.PathLike[str]) -> No
         msg = f'{os.fsdecode(source)} does not exist'
         raise FileNotFoundError(msg)
 
-    if not path.exists(dest) or not filecmp.cmp(source, dest):
+    if not (dest_exists := path.exists(dest)) or not filecmp.cmp(source, dest):
+        if __overwrite_warning__ and dest_exists:
+            # sphinx.util.logging imports sphinx.util.osutil,
+            # so use a local import to avoid circular imports
+            from sphinx.util import logging
+            logger = logging.getLogger(__name__)
+
+            msg = ('Copying the source path %s to %s will overwrite data, '
+                   'as a file already exists at the destination path '
+                   'and the content does not match.')
+            logger.info(msg, os.fsdecode(source), os.fsdecode(dest),
+                        type='misc', subtype='copy_overwrite')
+
         shutil.copyfile(source, dest)
         with contextlib.suppress(OSError):
             # don't do full copystat because the source may be read-only

tests/roots/test-util-copyasset_overwrite/conf.py

@@ -0,0 +1,7 @@
+import os
+import sys
+sys.path.insert(0, os.path.abspath('.'))
+
+extensions = ['myext']
+html_static_path = ['user_static']
+html_theme = 'basic'

tests/roots/test-util-copyasset_overwrite/myext.py

@@ -0,0 +1,22 @@
+from pathlib import Path
+
+from sphinx.util.fileutil import copy_asset
+
+
+def _copy_asset_overwrite_hook(app):
+    css = app.outdir / '_static' / 'custom-styles.css'
+    # html_static_path is copied by default
+    assert css.read_text() == '/* html_static_path */\n'
+    # warning generated by here
+    copy_asset(
+        Path(__file__).parent.joinpath('myext_static', 'custom-styles.css'),
+        app.outdir / '_static',
+    )
+    # This demonstrates the overwriting
+    assert css.read_text() == '/* extension styles */\n'
+    return []
+
+
+def setup(app):
+    app.connect('html-collect-pages', _copy_asset_overwrite_hook)
+    app.add_css_file('custom-styles.css')

tests/roots/test-util-copyasset_overwrite/myext_static/custom-styles.css

@@ -0,0 +1 @@
+/* extension styles */

tests/roots/test-util-copyasset_overwrite/user_static/custom-styles.css

@@ -0,0 +1 @@
+/* html_static_path */

tests/test_util/test_util_fileutil.py

@@ -2,7 +2,10 @@
 
 from unittest import mock
 
+import pytest
+
 from sphinx.jinja2glue import BuiltinTemplateLoader
+from sphinx.util import strip_colors
 from sphinx.util.fileutil import _template_basename, copy_asset, copy_asset_file
 
 
@@ -103,6 +106,20 @@ def excluded(path):
     assert not (destdir / '_templates' / 'sidebar.html').exists()
 
 
+@pytest.mark.xfail(reason='Filesystem chicanery(?)')
+@pytest.mark.sphinx('html', testroot='util-copyasset_overwrite')
+def test_copy_asset_overwrite(app):
+    app.build()
+    warnings = strip_colors(app.warning.getvalue())
+    src = app.srcdir / 'myext_static' / 'custom-styles.css'
+    dst = app.outdir / '_static' / 'custom-styles.css'
+    assert warnings == (
+        f'WARNING: Copying the source path {src} to {dst} will overwrite data, '
+        'as a file already exists at the destination path '
+        'and the content does not match.\n'
+    )
+
+
 def test_template_basename():
     assert _template_basename('asset.txt') is None
     assert _template_basename('asset.txt.jinja') == 'asset.txt'