[PATCH] D67643: [lit] Extend internal diff to support `-` argument

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: llvm/utils/lit/lit/builtin_commands/diff.py:27-29
+    # FIXME: How can we restart stdin if the encoding is wrong?  How can we
+    # read stdin with a different encoding or in binary mode in a way that's
+    # compatible with python 2 and 3?
----------------
This function seems fragile to me. It reads each file three times:
1. without an encoding (system default?), may fail
2. with a utf-8 encoding, may fail
3. inside compareTwo*Files, reads them yet again

I think the best way to be portable between Python 2 & 3 will probably be to work in bytes as much as possible.

I think you can use this pattern to get a new file descriptor for stdin that reads bytes:
```
fileno = sys.stdin.fileno()
if fileno is not None:
    new_stdin = os.fdopen(os.dup(fileno), 'rb') # read in binary
```

Here's how I would do it:
1. read both inputs completely in binary (can't fail)
2. try decoding the entire file in a few encodings (try default, try utf-8, if that fails, stick with bytes and diff_bytes)
3. split file on u'\n' or b'\n' as appropriate, perhaps stripping trailing '\r' if present, depending on flags

Does that seem reasonable?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67643/new/

https://reviews.llvm.org/D67643