diff --git a/CHANGES.rst b/CHANGES.rst index 66150d2624a..1686f2cbc8a 100644 --- a/CHANGES.rst +++ b/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 diff --git a/sphinx/util/fileutil.py b/sphinx/util/fileutil.py index b79fd0fc5ca..10bfc3bd610 100644 --- a/sphinx/util/fileutil.py +++ b/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') diff --git a/sphinx/util/osutil.py b/sphinx/util/osutil.py index 474fc5a2bb7..c5a856b22c7 100644 --- a/sphinx/util/osutil.py +++ b/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 diff --git a/tests/roots/test-util-copyasset_overwrite/myext.py b/tests/roots/test-util-copyasset_overwrite/myext.py index b4fd59749fb..544057c1fc3 100644 --- a/tests/roots/test-util-copyasset_overwrite/myext.py +++ b/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 [] diff --git a/tests/test_util/test_util_fileutil.py b/tests/test_util/test_util_fileutil.py index 4603257034a..2071fc3fade 100644 --- a/tests/test_util/test_util_fileutil.py +++ b/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():