DavidSpickett added inline comments.
================ Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:1422 + def runBuildCommands(self, commands): + for cmd in commands: + self.trace(shlex.join(cmd)) ---------------- Is this ever going to have more than one command to run? Seems like the source is the function above that puts a single command into a list. ================ 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) ---------------- labath wrote: > 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.) > and does not throw an exception on non-utf output I didn't consider that so yeah this looks fine. 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