[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