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

validate_html_static_path raises exception when static path and outdir are in different drives #6759

Closed
bizzfitch opened this issue Oct 22, 2019 · 2 comments

Comments

@bizzfitch
Copy link

bizzfitch commented Oct 22, 2019

Describe the bug
Hey sphinx team. Made a github account just for this, pretty excited to be potentially contributing to a project I use all the time

I think the bug is caused by the implementation for #1464

Bug is pretty much outlined by the title. In the code for sphinx.builders.html, the validate_html_static_path title will raise an error if the static path is in a different drive than the outdir. I assume this will also happen in validate_html_extra_path. Specifically, the outdir is using a UNC directory (like \network\some\path), but this happens with any two drives.

This occurs because these functions make a call to os.path.commonpath, which raises an error if the arguments are in different drives. Previously, sphinx had no problem with writing the output to a different drive.

To Reproduce
Steps to reproduce the behavior:
Easy enough to reproduce, no need for any real tinkering, since the error occurs before any real run happens.

$ git clone https://github.com/.../some_project
$ cd some_project
$ pip install -r requirements.txt
$ cd docs
$ make html BUILDDIR="D:\any\other\drive"
$ # we don't get past the init stage of the application

Expected behavior
In 2.1.2, neither of these functions exist, so the documents are successfully written in a different
drive than the source files. This is important for my project in particular, but seems like it's probably a common enough use case.

Your project
Can't submit the particular project. Hope this is okay, it's a project for work. I don't believe there's anything specific to the project that affects the bug, I reproduced it easily at home.

Environment info

  • OS: Windows 10
  • Python version: 3.7.4
  • Sphinx version: 2.2.0
  • Sphinx extensions: sphinx.ext.autodoc, sphinx.ext.doctest, sphinx.ext.mathjax, sphinx.ext.napoleon, sphxarg.ext
  • Extra tools: sphinx_rtd_theme

Additional context
I've solved it temporarily by simply putting the calls to commonpath in try-except blocks. I think this is fine unless there is a specific reason this cross-drive writing shouldn't be allowed.

@bizzfitch
Copy link
Author

I don't know if it's kosher to suggest a fix myself, but here's what I did to get around this. Not elegant, obviously.

Original:

def validate_html_static_path(app: Sphinx, config: Config) -> None:
    """Check html_static_paths setting."""
    for entry in config.html_static_path[:]:
        static_path = path.normpath(path.join(app.confdir, entry))
        if not path.exists(static_path):
            logger.warning(__('html_static_path entry %r does not exist'), entry)
            config.html_static_path.remove(entry)
        elif path.commonpath([app.outdir, static_path]) == app.outdir:
            logger.warning(__('html_static_path entry %r is placed inside outdir'), entry)
            config.html_static_path.remove(entry)

Proposed fix:

def validate_html_static_path(app: Sphinx, config: Config) -> None:
    """Check html_static_paths setting."""
    for entry in config.html_static_path[:]:
        static_path = path.normpath(path.join(app.confdir, entry))
        if not path.exists(static_path):
            logger.warning(__('html_static_path entry %r does not exist'), entry)
            config.html_static_path.remove(entry)
        else:
            try:
                commonpath = path.commonpath([app.outdir, static_path])
            except ValueError:
                continue
            if commonpath  == app.outdir:
                logger.warning(__('html_static_path entry %r is placed inside outdir'), entry)
                config.html_static_path.remove(entry)

bizzfitch pushed a commit to bizzfitch/sphinx that referenced this issue Oct 24, 2019
…s no longer throws an error if the paths are in different directories
@tk0miya tk0miya added this to the 2.2.1 milestone Oct 26, 2019
tk0miya pushed a commit to tk0miya/sphinx that referenced this issue Oct 26, 2019
…s no longer throws an error if the paths are in different directories
tk0miya added a commit that referenced this issue Oct 26, 2019
Fixes #6759: validation of html static paths and extra paths no longe…
@tk0miya
Copy link
Member

tk0miya commented Oct 26, 2019

Now your fix is released Sphinx-2.2.1. Thank you for your work!

@tk0miya tk0miya closed this as completed Oct 26, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants