[PATCH] Add remote testing support to the lit config.

[Dan Albert]

Looking better. At first glance, I think there's some more cleanup that can be done in `remote.py`, but I'll finish the review first thing tomorrow.

BTW, I like the `TargetInfo` bit. It looks like that change is actually cleanly separates from the rest of this. That part LGTM, so go ahead and submit that portion as its own commit.


================
Comment at: test/libcxx/test/config.py:116
@@ -115,2 +115,3 @@
             self.execute_external,
+            self.target_executor,
             exec_env=self.env)
----------------
Now that even the base case is a `LocalExecutor`, I think the `target_` prefix of this is misleading.

================
Comment at: test/libcxx/test/config.py:120
@@ +119,3 @@
+    def configure_target_executor(self):
+        exec_str = self.get_lit_conf('target_executor', "None")
+        te = eval(exec_str)
----------------
Default should be `"LocalExecutor"`. Then the if/else can disappear.

================
Comment at: test/libcxx/test/config.py:128
@@ +127,3 @@
+                # that the user wants it at the end, but we have no
+                # way of getting at that easily.
+                selt.lit_config.fatal("Cannot infer how to create a Valgrind "
----------------
`useValgrind` should just go away. If the user wants valgrind, they can add the `ValgrindExecutor` themselves.

================
Comment at: test/libcxx/test/config.py:592
@@ -558,2 +591,3 @@
     def configure_env(self):
-        if sys.platform == 'darwin' and not self.use_system_cxx_lib:
+        if self.target_info.platform() == 'darwin' and not \
+           self.use_system_cxx_lib:
----------------
Use parens rather than the \

And keep the `not` with the thing it's negating.

================
Comment at: test/libcxx/test/format.py:89
@@ -83,1 +88,3 @@
+            return self._evaluate_pass_test(test, tmpBase, execDir, lit_config,
+                                            self.target_executor)
         else:
----------------
No reason to pass this as an argument. Just refer to it as `self.target_executor` in `_evaluate_pass_test`.

================
Comment at: test/libcxx/test/format.py:118
@@ -108,8 +117,3 @@
             if self.exec_env:
-                cmd += ['env']
-                cmd += ['%s=%s' % (k, v) for k, v in self.exec_env.items()]
-            if lit_config.useValgrind:
-                cmd = lit_config.valgrindArgs + cmd
-            cmd += [exec_path]
-            out, err, rc = lit.util.executeCommand(
-                cmd, cwd=os.path.dirname(source_path))
+                env = self.exec_env
+            out, err, rc = target_executor.copy_run(exec_path, [], local_cwd, env)
----------------
The above three lines are just `env = self.exec_env`.

================
Comment at: test/libcxx/test/format.py:118
@@ -108,8 +117,3 @@
             if self.exec_env:
-                cmd += ['env']
-                cmd += ['%s=%s' % (k, v) for k, v in self.exec_env.items()]
-            if lit_config.useValgrind:
-                cmd = lit_config.valgrindArgs + cmd
-            cmd += [exec_path]
-            out, err, rc = lit.util.executeCommand(
-                cmd, cwd=os.path.dirname(source_path))
+                env = self.exec_env
+            out, err, rc = target_executor.copy_run(exec_path, [], local_cwd, env)
----------------
danalbert wrote:
> The above three lines are just `env = self.exec_env`.
Ignore the above. I traced it back and found that the default `self.exec_env` is `{}`, not `None`. Phabricator isn't letting me delete the comment though...

http://reviews.llvm.org/D7380

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/