Possible bug: misc.copy_overwrite warning

[gabalafou]

Sphinx 8 introduced a change to not overwrite user-supplied data by default when copying. It changed two utility functions, copyfile and copy_asset_file. They now skip any files that would result in a file overwrite and emit a "misc.copy_overwrite" warning unless called with force=True.

Nbsphinx calls sphinx.util.copyfile directly and it calls copy_asset_file indirectly via sphinx.util.fileutil.copy_asset, but it does not pass force=True.

I am not familiar enough with Sphinx or nbsphinx to know if force=True should be passed. All I know is this issue surfaced as a bug in my own development workflow. I was working on the PyData Sphinx Theme repo, which aborts its docs build if it encounters a warning that it does not expect.

Here is the full warning:

WARNING: Aborted attempted copy from /dev/pydata-sphinx-theme/docs/_build/html/.doctrees/nbsphinx/examples/pydata.ipynb to /dev/pydata-sphinx-theme/docs/_build/html/examples/pydata.ipynb (the destination path has existing data). [misc.copy_overwrite]

The reason why I suspect that this might be a bug in nbsphinx is that the PyData Theme docs use a number of other extensions and none of them trigger this warning; it's only nbsphinx.

[mgeier]

Thanks for reporting this!

Which version of nbsphinx did you experience this with?

There was a regression in Sphinx which wasn't fixed, but instead they added the warning you mentioned (see also sphinx-doc/sphinx#12096 (comment)).
I implemented a work-around for the regression in #808, which was released in the latest nbsphinx release 0.9.5.

If you still get the warning with the latest release, please provide a minimal example how to reproduce this.

if force=True should be passed

I don't think it should be passed, because we don't actually want to overwrite users' files. We want to provide files that the user can then overwrite if desired.

UPDATE: I think I was confused about this, see my comment #818 (comment) below.

[gabalafou]

I'm still seeing this with nbsphinx version 0.9.5. (I checked the version by adding a print(nbsphinx.__version__) into conf.py)

I'll work on a minimal example.

[mgeier]

After reading your original description again, I realized that I completely missed that this is not about CSS/JS (which has a work-around in #808), but about copying .ipynb files, which is a totally different situation (probably unrelated to the above-mentioned Sphinx regression), and which are not supposed to be overwritten by users!

I guess this problem does not happen when building from scratch, but only when a previous build is present in the build directory?
Maybe in this case we should use force=True after all?

[mgeier]

The notebook files are copied here:

nbsphinx/src/nbsphinx/__init__.py

Lines 1707 to 1713 in 1fda481

	notebooks = app.env.nbsphinx_notebooks.values()
	for notebook in status_iterator(
	notebooks, 'copying notebooks ... ',
	'brown', len(notebooks)):
	sphinx.util.copyfile(
	os.path.join(app.env.nbsphinx_auxdir, notebook),
	os.path.join(app.builder.outdir, notebook))

I think we could add force=True in our call, but it would be good to check for backwards compatibility, because this argument was added fairly recently (in sphinx-doc/sphinx#12647, Sphinx 8.0.0). In older versions this flag is probably not necessary because the default was to overwrite files.

@gabalafou Would you like to make a PR?

[gabalafou]

Sure, I'll give it a try :)

[gabalafou]

Just for the record, I created a minimal repro gist for this bug

[gabalafou]

To reproduce the bug, after you set up your sphinx folder following the minimal repro gist, then you make html, then you modify the helloworld.ipynb file (for example, change the title to "foo"), then make html again and you will get the warning (misc.copy_overwrite). You will also notice that if you try serve the built folder, and try to access helloworld.ipynb from the server, you will get the older version—in other words you will not see the change you made (the title will still say "Hi" instead of "foo").