Infra: Fix non-PEP txt files being included and erroring out the build

[CAM-Gerlach]

Presently, as described in #2404 , Sphinx attempts to render any .txt files not explicitly excluded in exclude_patterns of conf.py, in the root or any subdirectories. This results in build failure if any are found, particularly in untracked files which we don't control (e.g. vendor.txt in venvs).

Per the Sphinx docs we can use full Unix shell-style glob syntax, so adding the glob **/*.txt to exclude_patterns neatly fixes most instance of this problem by ensuring that .txt files are not matched unless they are in the root directory (as all the valid PEPs are).

To handle any remaining cases, i.e. non-PEP .txt files that are in the root directory and not explicitly excluded, adding *[!0-9].txt,*[!0-9]?.txt, *[!0-9]??.txt, *[!0-9]???.txt, *[!p]?????.txt, *[!e]??????.txt, and *[!p]???????.txt ensures that to be included, .txt files must match the equivalent of the regex ^pep?[0-9]{4}\.txt$, which is only used for PEPs.

This solution was tested on some sample files in the root and subdirectories, including the reported venv-spam/vendor.txt, and several with varying degrees of names somewhat similar to PEPs, and it excluded all of them, while not excluding any valid PEPs.

It also obviates the need for some of our other explicit excludes, but I opted to leave them for now

Fix #2404

[AA-Turner]

I am -1 on this; whilst a nice idea I think it overcomplicates this list massively for a edge-case.

If glob syntax is supported, why not just add *env* to the blacklist?

A

[CAM-Gerlach]

The cited example and the stopgap fix is one specific case, but the actual problem @ericsnowcurrently describes in #2404 is general: any .txt file added anywhere in this repo, and any .txt file added by any tool at a user- or repo-level .gitignored directory, file or path will be interpreted as a reST PEP file and attempted to be built/rendered accordingly (which will likely hard fail the build), unless explicitly deny-listed.

Adding *env* is merely a stopgap fix to one very specific case (venvs/virtualenvs that happen to be named *env*) of a much more general problem, leaving any other instance of the above not explicitly special-cased into the exclude_patterns unsolved. Instead, this PR implements the "allow-list" approach for legacy .txt PEPs that @ericsnowcurrently suggests using the existing supported exclude_pattern syntax.

I'm not sure I see how adding a handful of globs to an exclude list, while eliminating many of the existing special cases in the process (and avoid having to add an unbounded number more in the future), "massively" increases complexity. Since it as confirmed per my testing, it effectively eliminates any chance of a false negative while producing no false positives, and no more txt PEPs will be added, I don't see a practical downside that would motivate us to not simply fix this the right way from the start.

[AA-Turner]

I'm not sure I see how adding a handful of globs to an exclude list ... "massively" increases complexity.

The list does not get much longer, but it is really hard (at least for me) to mentally parse what the patterns are meant to be matching or not matching -- the approach I took in 2415 has more code "behind the veil", but keeps conf.py very simple / understandable.

So not complexity in terms of what is actually written, but complexity in terms of having to spend several minutes understanding what is going on with the patterns.

A

[CAM-Gerlach]

Yeah, I can certainly see how it could be confusing, but I could just explain it more clearly in the code comments. However, I'd be okay with your revised approach in #2415 provided it is actively planned to be upstreamed in Sphinx, because otherwise it adds way more complexity we have to maintain indefinitely than a few exclude patterns here.

[CAM-Gerlach]

Looks like closing PRs with other PRs is working again; it wasn't for a time.