Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix warnings when files are overwritten in the build directory. #12627

Merged
merged 3 commits into from
Jul 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions sphinx/util/fileutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down
7 changes: 6 additions & 1 deletion sphinx/util/osutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions tests/roots/test-util-copyasset_overwrite/myext.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 []


Expand Down
8 changes: 3 additions & 5 deletions tests/test_util/test_util_fileutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down