Fix detecting file changes for the overwritten file warning (#12627)

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>

CHANGES.rst

@@ -5,7 +5,7 @@ Bugs fixed
 ----------
 
 * #12096: Warn when files are overwritten in the build directory.
-  Patch by Adam Turner.
+  Patch by Adam Turner and Bénédikt Tran.
 * #12620: Ensure that old-style object description options are respected.
   Patch by Adam Turner.
 * #12601, #12625: Support callable objects in :py:class:`~typing.Annotated` type

sphinx/util/fileutil.py

@@ -75,6 +75,8 @@ def copy_asset_file(source: str | os.PathLike[str], destination: str | os.PathLi
             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.')
+            # See https://github.com/sphinx-doc/sphinx/pull/12627#issuecomment-2241144330
+            # for the rationale for logger.info().
             logger.info(msg, os.fsdecode(source), os.fsdecode(destination),
                         type='misc', subtype='copy_overwrite')
 

sphinx/util/osutil.py

@@ -106,7 +106,12 @@ def copyfile(
         msg = f'{os.fsdecode(source)} does not exist'
         raise FileNotFoundError(msg)
 
-    if not (dest_exists := path.exists(dest)) or not filecmp.cmp(source, dest):
+    if (
+        not (dest_exists := path.exists(dest)) or
+        # comparison must be done using shallow=False since
+        # two different files might have the same size
+        not filecmp.cmp(source, dest, shallow=False)
+    ):
         if __overwrite_warning__ and dest_exists:
             # sphinx.util.logging imports sphinx.util.osutil,
             # so use a local import to avoid circular imports

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

@@ -6,14 +6,14 @@
 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'
+    assert css.read_text() == '/* html_static_path */\n', 'invalid default text'
     # 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'
+    assert css.read_text() == '/* extension styles */\n', 'overwriting failed'
     return []
 
 

tests/test_util/test_util_fileutil.py

@@ -106,18 +106,16 @@ 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, '
+    assert (
+        f'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'
-    )
+    ) in strip_colors(app.status.getvalue())
 
 
 def test_template_basename():