Repository navigation
make sphinx Breaks when Certain Untracked Files are Present #2404
Description
make sphinxfails if I have some file like "pep-0684-old.rst" (the filename regex in pep_sphinx_extensions/pep_zero_generator/pep_index_generator.py isn't precise enough)make sphinxfails if I have a venv dir named "venv-spam" sitting at the top of the repo (tries to parse a vendor.txt file inside the venv)
In both cases I'd expect it to work fine, ignoring the relevant files.
Activity
AA-Turner commentedon Mar 9, 2022
The first case is easier to solve, the second is not possible as far as I'm aware -- Sphinx has no way to specify a whitelist of files -- the
exclude_fileslist is the only mechanism and as such must exhaustivley list things. Sorry.A
ericsnowcurrently commentedon Mar 10, 2022
I'm not familiar with how Sphinx works. It is surprising, though, that it can't handle the case. IIUC, it is blindly walking the entire directory tree and doesn't support a wildcard whitelist. What does it need to recurse in the PEP repo?
AA-Turner commentedon Mar 10, 2022
You're right. And I'd ask the same question on default recursion. One thing that might be possible is adding
venv*toexclude_filesinconf.py& hoping Sphinx does string-matching on paths, though untested.A
CAM-Gerlach commentedon Mar 10, 2022
Sorry for the delay responding; I've been dealing with urgent family issues most of today.
Good catch. It seems like the issue is ultimately due to having to try to render files with the legacy
.txtextension along with.rst, which results in irrelevant.txtfiles being picked up. (Sidenote: it seems like they were mass renamed before, but that was reverted, IIRC).However, since full Unix-style glob syntax is supported in
exclude_patterns, we can exclude any.txtfiles in subdirectories (like this example), and any in the root directory that don't match the exact pattern used for.txtPEPs (i.e. the equivalent of the regex^pep?[0-9]{4}\.txt$), which fully resolves the issue for the general case.I implemented and tested this on some sample files in the root and subdirectories (including your example, and a few with varying degrees of similar names), and it excluded all of them, while not excluding any valid PEPs. Therefore, I opened PR #2405 which fixes this.
hugovk commentedon Mar 11, 2022
I think the big difference here to "normal" Sphinx docs builds is we're reading the source documentation files from the root, and trying to ignore certain support and any other random user files.
Whereas normal Sphinx builds virtually always read source files from a subdirectory such as
docs:Is it feasible to move our source PEP files into a subdirectory?
Can/should Sphinx be made to ignore
non-SVNnon-CVSnon-source-controlled files? (Edit: fixed typo!)AA-Turner commentedon Mar 11, 2022
Last time mass moves were done, it was reverted due to the GitHub interface: #462 (comment)
This is still the case: https://github.com/python/peps/commits/main/.github/workflows/render.yml (the blame UI does track moves, though -- a fun inconsistency).
A
encukou commentedon Mar 11, 2022
Please don't. The text in this repo isn't about the project in the repo, it is the project.
CAM-Gerlach commentedon Mar 11, 2022
@hugovk see #2405 for a general-case fix.
The other even bigger difference, that is more directly the problem in this case, is that for legacy reasons, the older PEPs have the
txtextension, and with all the content files in the repo root, without some cleverexclude_patterns(added in #2405), there's no easy way for Sphinx to tell what it should consider content and what it should ignore without hardcoding everything.To be clear, given the GitHub UI limitations and general churn I don't believe its worth changing now (when, at least for this problem, a relatively straightforward fix is available).
However, for what it's worth, I don't really understand this reasoning. In almost any sort of medium to large project, source content goes in a subdir—for Python projects,
src/projectname(or justprojectname, with the attended downsides for autodiscovering, testing, etc of the implicit layout); for docs repos, in adocsor similar subdir; for static sites, source content typically goes in acontentor similar subdir, etc.On this repo in particular, it harms UX, on GitHub and locally, since page after page of PEPs have to be scrolled through to read the Readme on GitHub, or get from the files that have names before
pto those after, and it makes it very difficult for anyone not intimately familiar with everything on the repo to know what files are infra, reademe-type documents, etc., and which are files related to PEP content—there are a couple that despite tracking down the history, I'm still not 100% sure. Not to mention, it leads to tooling problems like this.ncoghlan commentedon Mar 12, 2022
I think the issue I'm seeing trying to build locally is this one (rather than #2402), since the linting works fine, but the rendering is complaining about old files inside the
.gitfolder:I'll try the #2415 branch to see if that resolves the issue.
ncoghlan commentedon Mar 12, 2022
Confirmed - if I check out #2415 and merge in the current main branch (to pick up the various #2402 fixes), then local Sphinx rendering starts working for me (as it isn't picking up the old PEP drafts in the .git history).
hugovk commentedon Mar 12, 2022
Thanks for testing!
Whichever of #2405 and #2415 we go for (I'm leaning towards #2405), let's explicitly exclude
.git. Please see PR #2423.I'll also repeat this question, my original typo made no sense :)
AA-Turner commentedon Mar 12, 2022
Maybe. A lot more work though. Also two approaches (files tracked by
git, or files not in.gitignore). I think for the initial implementation here we shouldn't do so.A
CAM-Gerlach commentedon Mar 12, 2022
See comments there; in short, this is only needed if we really want a very short-term stopgap until one of those PRs is merged.
This does seem like a lot more work, even more so than #2415 , which solves the issue much more precisely.