Replace --env with --source-dir

Remove the --env argument, which was compulsory for all subcommands, and
replace it with a --source-dir argument, only for those subcommands that
need it.

We do this for 2 reasons:
* Not all subcommands care about the source directory (e.g. llvm.py
  build won't need it)
* llvm.py should not have any knowledge about environments - that
  concept only makes sense for the helper scripts. llvm.py should
  instead receive very specific info about where the source and build
  directories are.

Change-Id: Iffaeef95559e8923bd883d5c51fed8c306287280

4 files changed, 103 insertions, 42 deletions

diff --git a/tests/cli/llvmtestcase.py b/tests/cli/llvmtestcase.py
index a6f234d..db3635f 100644
--- a/tests/cli/llvmtestcase.py
+++ b/tests/cli/llvmtestcase.py
@@ -82,20 +82,9 @@ class LLVMTestCase(unittest.TestCase):
 
     @classmethod
     def command_with_defaults(cls, subcommand, *args, **kwargs):
-        """
-        Build a list representing a llvm subcommand with the given
-        args. Unless otherwise specified in kwargs, this uses the value for
-        env that it finds in cls.
-        """
+        """Build a list representing an llvm subcommand with the given args."""
         command = [cls.python, cls.script]
 
-        env = cls.env
-        if "env" in kwargs:
-            env = kwargs["env"]
-        if env:
-            command.append("--env")
-            command.append(env)
-
         command.append(subcommand)
 
         if len(args):
diff --git a/tests/cli/testllvmconfigure.py b/tests/cli/testllvmconfigure.py
index fd68bde..cfd95d8 100644
--- a/tests/cli/testllvmconfigure.py
+++ b/tests/cli/testllvmconfigure.py
@@ -27,10 +27,21 @@ class Testllvmconfigure(LLVMTestCase):
         cls.llvm_worktree = path.join(cls.env, "llvm")
         cls.add_worktree(cls.llvm_repo, cls.llvm_worktree, "br")
 
+    def test_source_dir_is_compulsory(self):
+        """Test that we get an error if we don't pass the source dir."""
+        with self.assertRaises(CalledProcessError) as context:
+            self.run_with_output(
+                    self.llvm_configure("--build-dir", "anywhere"))
+
+        self.assertRegex(
+            str(context.exception.output),
+            "(.*\n)*the following arguments are required: --source-dir(.*\n)*")
+
     def test_build_dir_is_compulsory(self):
         """Test that we get an error if we don't pass the build dir."""
         with self.assertRaises(CalledProcessError) as context:
-            self.run_with_output(self.llvm_configure())
+            self.run_with_output(
+                    self.llvm_configure("--source-dir", self.llvm_worktree))
 
         self.assertRegex(
             str(context.exception.output),
@@ -39,6 +50,8 @@ class Testllvmconfigure(LLVMTestCase):
     def test_default_args(self):
         """Test that we can get a simple configure command."""
         output = self.run_with_output(self.llvm_configure("--dry-run",
+                                                          "--source-dir",
+                                                          self.llvm_worktree,
                                                           "--build-dir",
                                                           "anywhere"))
         self.assertRegex(output,
@@ -47,6 +60,8 @@ class Testllvmconfigure(LLVMTestCase):
     def test_generator(self):
         """Test that we can specify a custom generator."""
         output = self.run_with_output(self.llvm_configure("--dry-run",
+                                                          "--source-dir",
+                                                          self.llvm_worktree,
                                                           "--build-dir",
                                                           "anywhere",
                                                           "--cmake-generator",
@@ -58,6 +73,7 @@ class Testllvmconfigure(LLVMTestCase):
         """Test that we can specify custom cmake variables."""
         output = self.run_with_output(self.llvm_configure(
             "--dry-run",
+            "--source-dir", self.llvm_worktree,
             "--build-dir", "anywhere",
             "--cmake-def", "VAR1=VAL1",
             "--cmake-def", "VAR2=\"-not --so simple\""))
@@ -74,6 +90,7 @@ class Testllvmconfigure(LLVMTestCase):
         self.assertFalse(path.isdir(buildDir))
 
         self.run_with_output(self.llvm_configure("--dry-run",
+            "--source-dir", self.llvm_worktree,
             "--build-dir", buildDir))
         self.assertFalse(path.isdir(buildDir))
 
@@ -82,11 +99,8 @@ class Testllvmconfigure(LLVMTestCase):
         # source directory. This is fine, since the purpose of this test isn't
         # to check the integration with CMake.
         with self.assertRaises(Exception) as context:
-            print(self.run_with_output(self.llvm_configure("--build-dir",
-                buildDir)))
-
-        self.assertRegex(
-            str(context.exception.output),
-            "(.*\n)*The source directory .* does not appear to contain CMakeLists.txt(.*\n)*")
+            print(self.run_with_output(self.llvm_configure(
+                "--source-dir", self.llvm_worktree,
+                "--build-dir", buildDir)))
 
         self.assertTrue(path.isdir(buildDir))
diff --git a/tests/cli/testllvmprojects.py b/tests/cli/testllvmprojects.py
index ac1cd23..32cead4 100644
--- a/tests/cli/testllvmprojects.py
+++ b/tests/cli/testllvmprojects.py
@@ -79,28 +79,36 @@ class Testllvmprojs(LLVMTestCase):
         """
         with self.assertRaises(subprocess.CalledProcessError) as context:
             self.run_with_output(
-                self.llvm_projects("--add", "clang"))
+                self.llvm_projects(
+                    "--source-dir",
+                    self.llvm_src,
+                    "--add",
+                    "clang"))
 
         self.assertRegex(
             str(context.exception.output),
             "(.*\n)*.*When adding a subproject you must also pass the --repos argument(.*\n)*")
 
         # These should not raise.
-        self.run_with_output(self.llvm_projects())
+        self.run_with_output(self.llvm_projects("--source-dir", self.llvm_src))
         self.run_with_output(
-            self.llvm_projects("--remove", "clang"))
+            self.llvm_projects(
+                "--source-dir",
+                self.llvm_src,
+                "--remove",
+                "clang"))
 
-    def test_env_arg_is_compulsory(self):
+    def test_source_dir_is_compulsory(self):
         """
-        Test that we must pass in the environment for various combinations of
+        Test that we must pass in the source dir for various combinations of
         input args.
         """
         with self.assertRaises(subprocess.CalledProcessError) as context:
-            self.run_with_output(self.llvm_projects(env=None))
+            self.run_with_output(self.llvm_projects())
 
         self.assertRegex(
             str(context.exception.output),
-            "(.*\n)*.*the following arguments are required:(.*)--env(.*\n)*")
+            "(.*\n)*.*the following arguments are required:(.*)--source-dir(.*\n)*")
 
         with self.assertRaises(subprocess.CalledProcessError) as context:
             self.run_with_output(
@@ -110,7 +118,7 @@ class Testllvmprojs(LLVMTestCase):
 
         self.assertRegex(
             str(context.exception.output),
-            "(.*\n)*.*the following arguments are required:(.*)--env(.*\n)*")
+            "(.*\n)*.*the following arguments are required:(.*)--source-dir(.*\n)*")
 
         with self.assertRaises(subprocess.CalledProcessError) as context:
             self.run_with_output(
@@ -119,14 +127,15 @@ class Testllvmprojs(LLVMTestCase):
 
         self.assertRegex(
             str(context.exception.output),
-            "(.*\n)*.*the following arguments are required:(.*)--env(.*\n)*")
+            "(.*\n)*.*the following arguments are required:(.*)--source-dir(.*\n)*")
 
     def test_dump_empty_config(self):
         """
         Test that we're correctly dumping an empty configuration (i.e. no
         projects linked) when running llvmprojs without arguments.
         """
-        output = self.run_with_output(self.llvm_projects())
+        output = self.run_with_output(
+            self.llvm_projects("--source-dir", self.llvm_src))
         self.assertRegex(output, "Projects linked:.*\n.*none.*")
 
     def test_add_remove_subprojects(self):
@@ -134,12 +143,14 @@ class Testllvmprojs(LLVMTestCase):
         Test that we can add and remove one or several subprojects.
         """
         output = self.run_with_output(self.llvm_projects(
+            "--source-dir", self.llvm_src,
             "--repos", self.repos,
             "--add", "clang"))
         self.assertRegex(output, "Projects linked:.*\n.*clang.*")
 
         output = self.run_with_output(
             self.llvm_projects(
+                "--source-dir", self.llvm_src,
                 "--repos", self.repos,
                 "--add", "libcxx", "lldb"))
         self.assertRegex(
@@ -149,7 +160,12 @@ class Testllvmprojs(LLVMTestCase):
             ".*libcxx.*\n" +
             ".*lldb.*")
 
-        output = self.run_with_output(self.llvm_projects("--remove", "libcxx"))
+        output = self.run_with_output(
+            self.llvm_projects(
+                "--source-dir",
+                self.llvm_src,
+                "--remove",
+                "libcxx"))
         self.assertRegex(output,
                          "Projects linked:.*\n" +
                          ".*clang.*\n" +
@@ -157,6 +173,7 @@ class Testllvmprojs(LLVMTestCase):
 
         output = self.run_with_output(
             self.llvm_projects(
+                "--source-dir", self.llvm_src,
                 "--remove", "clang", "lldb"))
         self.assertRegex(output,
                          "Projects linked:.*\n" +
@@ -170,6 +187,7 @@ class Testllvmprojs(LLVMTestCase):
         with self.assertRaises(subprocess.CalledProcessError) as context:
             self.run_with_output(
                 self.llvm_projects(
+                    "--source-dir", self.llvm_src,
                     "--repos", self.repos,
                     "--add", "inventedsubproject"))
 
@@ -180,6 +198,7 @@ class Testllvmprojs(LLVMTestCase):
         with self.assertRaises(subprocess.CalledProcessError) as context:
             self.run_with_output(
                 self.llvm_projects(
+                    "--source-dir", self.llvm_src,
                     "--remove",
                     "inventedsubproject"))
 
@@ -194,6 +213,7 @@ class Testllvmprojs(LLVMTestCase):
         """
         output = self.run_with_output(
             self.llvm_projects(
+                "--source-dir", self.llvm_src,
                 "--repos", self.repos,
                 "--add", "clang", "lld", "clang"))
         self.assertRegex(output,
@@ -203,6 +223,7 @@ class Testllvmprojs(LLVMTestCase):
 
         output = self.run_with_output(
             self.llvm_projects(
+                "--source-dir", self.llvm_src,
                 "--remove", "lld", "lld", "clang"))
         self.assertRegex(output,
                          "Projects linked:.*\n" +
@@ -223,6 +244,7 @@ class Testllvmprojs(LLVMTestCase):
             self.branch)
 
         output = self.run_with_output(self.llvm_projects(
+            "--source-dir", self.llvm_src,
             "--repos", self.repos,
             "--add", "clang"))
         self.assertRegex(output,
@@ -231,6 +253,7 @@ class Testllvmprojs(LLVMTestCase):
 
         output = self.run_with_output(
             self.llvm_projects(
+                "--source-dir", self.llvm_src,
                 "--repos", self.repos,
                 "--add", "compiler-rt", "lld"))
         self.assertRegex(
@@ -242,6 +265,7 @@ class Testllvmprojs(LLVMTestCase):
 
         output = self.run_with_output(
             self.llvm_projects(
+                "--source-dir", self.llvm_src,
                 "--remove", "lldb", "libcxx", "lld"))
         self.assertRegex(
             output,
@@ -258,6 +282,8 @@ class Testllvmprojs(LLVMTestCase):
         with self.assertRaises(subprocess.CalledProcessError) as context:
             self.run_with_output(
                 self.llvm_projects(
+                    "--source-dir",
+                    self.llvm_src,
                     "--repos",
                     self.repos,
                     "--add",
@@ -283,6 +309,8 @@ class Testllvmprojs(LLVMTestCase):
         with self.assertRaises(subprocess.CalledProcessError) as context:
             self.run_with_output(
                 self.llvm_projects(
+                    "--source-dir",
+                    self.llvm_src,
                     "--repos",
                     self.repos,
                     "--add",
@@ -315,6 +343,8 @@ class Testllvmprojs(LLVMTestCase):
         """
         output = self.run_with_output(
             self.llvm_projects(
+                "--source-dir",
+                self.llvm_src,
                 "--repos",
                 self.repos,
                 "--add",
@@ -334,6 +364,8 @@ class Testllvmprojs(LLVMTestCase):
 
         output = self.run_with_output(
             self.llvm_projects(
+                "--source-dir",
+                self.llvm_src,
                 "--repos",
                 self.repos,
                 "--add",
@@ -354,6 +386,8 @@ class Testllvmprojs(LLVMTestCase):
 
         output = self.run_with_output(
             self.llvm_projects(
+                "--source-dir",
+                self.llvm_src,
                 "--repos",
                 self.repos,
                 "--add",
@@ -377,49 +411,62 @@ class Testllvmprojs(LLVMTestCase):
         """
         # Create a separate environment
         new_env = mkdtemp()
+        new_llvm_src = os.path.join(new_env, "llvm")
         new_branch = "br" + str(uuid4())
-        self.add_worktree(self.get_subproj_repo("llvm"),
-                          os.path.join(new_env, "llvm"), new_branch)
+        self.add_worktree(
+            self.get_subproj_repo("llvm"),
+            new_llvm_src,
+            new_branch)
 
         # Check that we start with a clean slate in both the new environment and
         # the one that's already set up
-        output = self.run_with_output(self.llvm_projects())
+        output = self.run_with_output(self.llvm_projects("--source-dir",
+                                                         self.llvm_src))
         self.assertRegex(output, "Projects linked:.*\n.*none.*")
 
-        output = self.run_with_output(self.llvm_projects(env=new_env))
+        output = self.run_with_output(self.llvm_projects("--source-dir",
+                                                         new_llvm_src))
         self.assertRegex(output, "Projects linked:.*\n.*none.*")
 
         # Make sure that adding projects works
         output = self.run_with_output(
             self.llvm_projects(
+                "--source-dir", self.llvm_src,
                 "--repos", self.repos,
                 "--add", "clang", "lld"))
         self.assertRegex(output, "Projects linked:.*\n.*clang.*\n.*lld.*\n")
 
         output = self.run_with_output(
             self.llvm_projects(
+                "--source-dir",
+                new_llvm_src,
                 "--repos",
                 self.repos,
                 "--add",
                 "libcxx",
-                "libcxxabi",
-                env=new_env))
+                "libcxxabi"))
         self.assertRegex(output,
                          "Projects linked:.*\n.*libcxx.*\n.*libcxxabi.*\n.*")
 
-        output = self.run_with_output(self.llvm_projects())
+        output = self.run_with_output(
+            self.llvm_projects("--source-dir", self.llvm_src))
         self.assertRegex(output, "Projects linked:.*\n.*clang.*\n.*lld.*\n")
 
         # Make sure that removing projects works
-        output = self.run_with_output(self.llvm_projects("--remove", "lld"))
+        output = self.run_with_output(
+            self.llvm_projects(
+                "--source-dir", self.llvm_src,
+                "--remove", "lld"))
         self.assertRegex(output, "Projects linked:.*\n.*clang.*\n")
 
         output = self.run_with_output(self.llvm_projects("--remove", "libcxx",
-                                                         env=new_env))
+                                                         "--source-dir",
+                                                         new_llvm_src))
         self.assertRegex(output,
                          "Projects linked:.*\n.*libcxxabi.*\n.*")
 
-        output = self.run_with_output(self.llvm_projects())
+        output = self.run_with_output(
+            self.llvm_projects("--source-dir", self.llvm_src))
         self.assertRegex(output, "Projects linked:.*\n.*clang.*\n")
 
         shutil.rmtree(new_env)
diff --git a/tests/cli/testllvmpush.py b/tests/cli/testllvmpush.py
index 1d9b8ab..a704185 100644
--- a/tests/cli/testllvmpush.py
+++ b/tests/cli/testllvmpush.py
@@ -87,6 +87,15 @@ class Testllvmpush(LLVMTestCase):
             with cd(repopath):
                 cls.run_quietly(["git", "worktree", "prune"])
 
+    def test_source_dir_ir_compulsory(self):
+        """Test that we get an error if we don't pass the source dir."""
+        with self.assertRaises(subprocess.CalledProcessError) as context:
+            output = self.run_with_output(self.llvm_push())
+
+        self.assertRegex(
+            str(context.exception.output),
+            "(.*\n)*the following arguments are required: --source-dir(.*\n)*")
+
     def test_push(self):
         with cd(self.llvm_src):
             self.create_dummy_commit("Test llvm push")
@@ -101,7 +110,8 @@ class Testllvmpush(LLVMTestCase):
             with cd(worktreePath):
                 self.create_dummy_commit("Test {} push".format(subproj))
 
-        pushed = self.run_with_output(self.llvm_push())
+        pushed = self.run_with_output(
+            self.llvm_push("--source-dir", self.llvm_src))
         remote_branch = "linaro-local/{}/{}".format(self.user, self.branch)
 
         self.assertRegex(
@@ -147,7 +157,8 @@ class Testllvmpush(LLVMTestCase):
                 self.create_dummy_commit("Test {} push".format(subproj))
 
         with self.assertRaises(subprocess.CalledProcessError) as context:
-            output = self.run_with_output(self.llvm_push())
+            output = self.run_with_output(
+                self.llvm_push("--source-dir", self.llvm_src))
 
         # Check that we error out because lld is on a different branch
         self.assertRegex(str(context.exception.output),