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

Unexpected template and source code files deployed to site_dir #807

Closed
shichao-an opened this issue Jan 25, 2016 · 11 comments
Closed

Unexpected template and source code files deployed to site_dir #807

shichao-an opened this issue Jan 25, 2016 · 11 comments
Labels

Comments

@shichao-an
Copy link

After upgrading to 0.15.0, the following files get deployed to the site_dir after build:

site/__init__.py
site/__init__.pyc
site/base.html
site/content.html
site/nav.html
site/toc.html

From the gh-pages branch, you can see the last clean commit is:

https://github.com/mkdocs/mkdocs/tree/6b7615eee095c8ccd47a106ca29f2f12314d86f5

The most recent commit has the above files included:

https://github.com/mkdocs/mkdocs/tree/ffe467ab2352ec3a0f690024bd908b7dea0443fc

@d0ugal
Copy link
Member

d0ugal commented Jan 25, 2016

Good spot. I don't think this is actually a problem, so it is a low priority issue but we should try to resolve it.

The tricky thing to realise is, how do we determine which assets should and shouldn't be copied from a theme?

@d0ugal d0ugal added the Bug label Jan 25, 2016
@das-g
Copy link

das-g commented Feb 3, 2016

I don't think this is actually a problem

This is a problem under some specific (but not very unusual) conditions, e.g.:

  • have directory my_project/ in your PYTHONPATH
  • use the Python 3 standard library package html, or any of its subpackages (e.g. html.entities)
  • use mkdocs to generate docs in directory my_project/html/

The gratuitous my_project/html/__init__.py will make the html/ directory a Python package that masks the standard library package of the same name, leading to confusing error messages like

 ImportError: No module named html.entities

@d0ugal
Copy link
Member

d0ugal commented Feb 3, 2016

Okay, wow. That is an unfortunate set of coincidences. In that case, yes it is a problem.

A simple fix to never copy Python files from a theme's source would probably do the job.

@waylan
Copy link
Member

waylan commented Feb 3, 2016

All media files (non markdown and template files) are copied using the mkdocs.utils.copy_media_files function. We can easily add a line to have it also exclude python files, but I'm thinking we only want to exclude python files from the theme dir, not from the docs_dir. After all, a user could have a python source file they want to host for some reason (presumably with a download link pointing to it in their docs). However, in the code, copying the themes and copying the media files in the docs_dir is not really differentiated. We could add an exclude keyword to mkdocs.utils.copy_media_files which accepts a list of file extensions, but I'm not sure we want to go that way.

@d0ugal
Copy link
Member

d0ugal commented Feb 3, 2016

Yeah, ideally we would refactor this a bit and make it smarter. Maybe we create a class that represents a media directory, then a subclass of it could represent a theme - initially that would just exclude Python files, but later it might do more.

... but I am just thinking "out loud", so I've not thought that through fully.

@waylan
Copy link
Member

waylan commented Feb 3, 2016

Right. My thoughts as well. We could do a quick fix (exclude keyword) or we could refactor it and make it right. But then how do we refactor it...

@davidbgk
Copy link

I confirm that it can even be worse than masking the html standard package, see #878 for example.

@waylan
Copy link
Member

waylan commented Mar 30, 2016

It occurs to me that we could require themes to actually be in a subdirectory of the directory which contains the __init__.py file (perhaps called theme; open to suggestions for alternate/better names). Then we could do os.path.join(pkgutil.get_loader(theme.module_name).filename, 'theme') to get the dir of the actual theme and no extra files would be in the actual theme.

So a theme would look like this:

- readthedocs/
    - __init__.py
    - README.md
    - theme/
        - css/
        - js/
        - img/
        - base.html
        - ...

Of course, to do that, all the existing themes would need to be updated and the change would be backward incompatible. However, if such a change were to be made, it should be done now before more third party themes are developed. Note that this would not effect the theme_dir as that is handled separately. Therefore this would only effect themes, not projects - although users would need to update there themes at the same time they update MkDocs. We could issue a warning for a few versions while continuing to support the old scheme. Barring any objection, I'll see what I can come up with.

@davidbgk
Copy link

Or maybe at least check that the target directory is not already in the python path and display a warning about that?

@waylan
Copy link
Member

waylan commented Mar 31, 2016

Hmm, interesting. I just noticed that the OP also reported that the template files (base.html, etc) are also being copied to the site_dir, not just the Python files. I think its clear the we need a different code path for templates than the doc_dir. Simply putting the template in a sub-directory won't address that issue.

waylan added a commit to waylan/mkdocs that referenced this issue Mar 31, 2016
Fixes mkdocs#807. Used the fnmatch lib which supports Unix shell-style
wildcard patterns (not regex). This allows patterns like
`'.*py'`, `'.*'`, etc. A list of excluded patterns are passed
to `mkdocs.utils.copy_media_files`. The pattern `'.*'` is included
by default and not overidable.
waylan added a commit to waylan/mkdocs that referenced this issue Mar 31, 2016
Fixes mkdocs#807. Used the fnmatch lib which supports Unix shell-style
wildcard patterns (not regex). This allows patterns like
`'.*py'`, `'.*'`, etc. A list of excluded patterns are passed
to `mkdocs.utils.copy_media_files`. The pattern `'.*'` is included
by default and not overidable.
@waylan
Copy link
Member

waylan commented Mar 31, 2016

I don't think anyone has the time for working on the the refactor mentioned above anytime soon. Therefore, I pushed a quick fix to #880 which implements the exclude keyword on mkdocs.utils.copy_media_files and added some tests.

Note that the issue with template (HTML) files being copied was introduced in d5f95f2 as a fix for #691. The problem is that that fix should only apply to files in the docs_dir, not files in the theme. My fix addresses this and adds tests for both situations.

das-g added a commit to geometalab/drf-utm-zone-info that referenced this issue Jan 11, 2017
This avoids conflicts with the `html` Python standard library package due to mkdocs/mkdocs#807 in older MkDocs versions. But mostly, it's more clear, anyway.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants