[PATCH] D29822: [LNT] Get cc info from CMake cache instead of from command-line
Matthias Braun via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 10 11:01:13 PST 2017
MatzeB accepted this revision.
MatzeB added a comment.
This revision is now accepted and ready to land.
LGTM, comments below
================
Comment at: lnt/tests/test_suite.py:588
+ output = subprocess.check_output(*args, **kwargs)
+ sys.stdout.write(output)
+ return output
----------------
Wouldn't it make more sense to keep check_call and add check_output as an additional function? The check_output() write() idiom won't output anything until the command is finished and will probably show all of stderr before stdout instead of intermixing them.
================
Comment at: lnt/tests/test_suite.py:645-647
for item in extra_cmake_defs:
k, v = item.split('=', 1)
defs[k] = v
----------------
It feels a bit odd that the 2nd loop is nearly a duplication of the first one, just that it isn't... How about:
- Adding default=[] for the cmake_defines `add_option()` call.
- Merging the two loops: `for item in self.opts.cmake_defines + extra_cmake_defs: ...`
https://reviews.llvm.org/D29822
More information about the llvm-commits
mailing list