ci: add trailing whitespace test

[wchargin]

Summary:
Prevents regressions of nice fixes like #2364.

Test Plan:
Run the script, and note that it passes with no output. Then remove
README.md from the exceptions list and add nonexistent.txt, and
observe that the script fails with:

Superfluous trailing whitespace:
README.md:10:For in-depth information on the Graph Visualizer, see this tutorial: $
README.md:18:through setting up and using TensorBoard. There's an associated $
README.md:36:For more details, see $
README.md:69:For this, you need $

Stale exceptions (no whitespace problems; prune exceptions list):
nonexistent.txt

wchargin-branch: whitespace-hygiene-test

[nfelt]

Since realistically we're not going to have newlines in filenames committed to our git repo, it seems simpler and more obvious to just do for line in stdout.splitlines(): filename_raw, line_number_raw, line_number = line.split(b"\0", 2).

[wchargin]

IMHO, the correct version is sufficiently simple as to be
unobjectionable. But in the interest of compromise, I can switch this
over to the incorrect-but-fewer-lines version. (I’d rather cede this
point, which is under our control, than the other, which is under the
control of whoever clones the repository.)

[nfelt]

os.chdir(toplevel.rstrip())?

[wchargin]

This is again for whitespace-correctness—nothing wrong with

$ git clone -q gh:tensorflow/tensorboard "tensorboard "
$ cd "tensorboard "
$ pwd | xxd
00000000: 2f74 6d70 2f74 6d70 2e49 426d 7057 5446  /tmp/tmp.IBmpWTF
00000010: 7573 712f 7465 6e73 6f72 626f 6172 6420  usq/tensorboard 
00000020: 0a                                       .
$ python -c '
> import os
> import subprocess
> toplevel = subprocess.check_output(["git", "rev-parse", "--show-toplevel"])
> os.chdir(toplevel.rstrip())
> '
Traceback (most recent call last):
  File "<string>", line 5, in <module>
OSError: [Errno 2] No such file or directory: '/tmp/tmp.IBmpWTFusq/tensorboard'

It really costs us nothing here to do the right thing, and I’d love to
avoid proliferating even more programs that behave incorrectly on
filenames that have whitespace.

[nfelt]

Are any of these ones that should actually be longstanding exceptions? If not, why not just commit the whitespace fixes to these now and avoid ever checking in the exceptions list support and stale exceptions detection?

[wchargin]

Are any of these ones that should actually be longstanding exceptions?

Nope. (The only potential genuine exception that I could think of would
be Markdown hard-breaks, which should rarely be used anyway.)

why not just commit the whitespace fixes to these now and avoid ever
checking in the exceptions list support

Two benefits:

• we avoid a megacommit that kills all the blame (existing such
  commits, like e1adbef and friends, are perennially frustrating in
  everyday TensorBoard development); and
• in the case that we do need a genuine exception (say, some strange
  data format for a plugin’s test data), the path of least resistance
  will be to mark it as an exception rather than just disabling the
  test because there’s an urgent deadline.

[nfelt]

Running your script shows that there are only 88 affected individual lines. I don't think an 88-line delta is a megacommit and it's certainly nothing like a 11,000 line one. Blame impact would be localized only to affected lines.

(Though, even if we were talking about reformatting en masse (e.g. buildifiering all our build files) I think it's better to just get it over with. Doing it one touched line at a time will never actually fix everything so you never achieve the uniformity that's the desired goal anyway. Doing it file-by-file means that you still clobber the blame but now there's hundreds of such commits instead of one.)

The second benefit is entirely speculative.

[wchargin]

Good point: I was scared off by the file count and didn’t look at the
delta. That does make it much less bad. I’m convinced; will send out a
fix.

The second benefit is entirely speculative.

Certainly.