labath added inline comments.
================
Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:1284
compiler = self.getCompilerBinary()
- version_output = system([[compiler, "--version"]])
- for line in version_output.split(os.linesep):
- m = re.search('version ([0-9.]+)', line)
- if m:
- return m.group(1)
+ version_output = check_output([compiler, "--version"])
+ m = re.search(b'version ([0-9.]+)', version_output)
----------------
DavidSpickett wrote:
> You could use `universal_newlines` here to get the decoded string. It's a bit
> cryptic but saves the decode below.
>
> There is an alias `text` name in 3.7 but requiring that seems ambitious.
how about `error="replace"` on the `check_output` invocation it's equally
cryptic, and does not throw an exception on non-utf output?
(I'm generally unsure about the best way to handle the byte-string duality in
python3 -- whether to try to handle things at the byte level (if they can) or
to convert everything to strings as soon as possible and pretend bytes don't
exist.)
================
Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:1425
+ try:
+ proc = run(cmd, stdin=DEVNULL, stdout=PIPE, stderr=STDOUT,
+ check=True)
----------------
DavidSpickett wrote:
> labath wrote:
> > PSA: this function is available from python-3.5 onwards.
> Any reason to use `run(...check=True)` rather than `check_call`?
>
> https://docs.python.org/3/library/subprocess.html#subprocess.check_call
>
> (`universal_newlines` applies here too if you want)
I must have thought I needed some of this functionality, but now that I look at
it -- I don't. So `check_output` it is.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112212/new/
https://reviews.llvm.org/D112212
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
