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 1 commit
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
4 changes: 2 additions & 2 deletions sphinx/util/fileutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +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.')
logger.info(msg, os.fsdecode(source), os.fsdecode(destination),
type='misc', subtype='copy_overwrite')
logger.warning(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:
Expand Down
11 changes: 8 additions & 3 deletions 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 bytes size
picnixz marked this conversation as resolved.
Show resolved Hide resolved
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 All @@ -116,8 +121,8 @@ def copyfile(
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')
logger.warning(msg, os.fsdecode(source), os.fsdecode(dest),
type='misc', subtype='copy_overwrite')

shutil.copyfile(source, dest)
with contextlib.suppress(OSError):
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
1 change: 0 additions & 1 deletion tests/test_util/test_util_fileutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ 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()
Expand Down