Move --repos flag to projects subcommand

The --repos flag is not used by all the subcommands, so there's no
reason for it to be compulsory. This commit moves it from the generic
llvm.py options to the options for the projects subcommand, where it is
only required when adding subprojects.

Change-Id: I7ad10bda6e1b4efeb594b09e5fda200850d99c07

5 files changed, 48 insertions, 43 deletions

diff --git a/helpers/llvm-projs b/helpers/llvm-projs
index 190f379..21b8bea 100755
--- a/helpers/llvm-projs
+++ b/helpers/llvm-projs
@@ -51,7 +51,7 @@ env=$(dirname $LLVM_SRC)
 if [ "$1" = "" ]; then
   echo "Use $prog -h for options"
   echo
-  safe_run python3 $llvmtool --repos $repos --env $env projects
+  safe_run python3 $llvmtool --env $env projects
   exit
 fi
 
@@ -100,7 +100,7 @@ case $opt in
     shift
     ;;
   list)
-    safe_run python3 $llvmtool --repos $repos --env $env projects
+    safe_run python3 $llvmtool --env $env projects
     exit
     ;;
   -h)
@@ -196,4 +196,4 @@ if [ "$remove" != "" ]; then
   remove="-r $remove"
 fi
 
-safe_run python3 $llvmtool --repos $repos --env $env projects $add $remove
+safe_run python3 $llvmtool --env $env projects --repos $repos $add $remove
diff --git a/helpers/llvm-push b/helpers/llvm-push
index 54fee83..b7ad1d1 100755
--- a/helpers/llvm-push
+++ b/helpers/llvm-push
@@ -18,4 +18,4 @@ verify_env
 repos=$LLVM_ROOT/repos
 env=$(dirname $LLVM_SRC)
 
-safe_run python3 $llvmtool --repos $repos --env $env push
+safe_run python3 $llvmtool --env $env push
diff --git a/scripts/llvm.py b/scripts/llvm.py
index 4b18560..27b8cb7 100644
--- a/scripts/llvm.py
+++ b/scripts/llvm.py
@@ -1,6 +1,7 @@
 """This is the main tool for handling llvm builds, bisects etc."""
 
 import os
+from sys import argv
 from sys import exit
 
 from modules.llvm import LLVMSubproject
@@ -103,12 +104,8 @@ options.add_argument(
     '--env',
     required=True,
     help="Path to the environment to update.")
-options.add_argument(
-    '--repos', required=True,
-    help="Path to the directory containing the repositories for all LLVM "
-         "subprojects.")
 
-subcommands = options.add_subparsers()
+subcommands = options.add_subparsers(dest="subcommand")
 
 # Subcommand for adding / removing subprojects
 projs = subcommands.add_parser(
@@ -138,6 +135,10 @@ projs.add_argument(
     choices=valid_subprojects,
     metavar='subproject',
     help="Disable given subprojects.")
+projs.add_argument(
+    '--repos',
+    help="Path to the directory containing the repositories for all LLVM "
+         "subprojects.")
 
 # Subcommand for pushing the current branch to origin
 push = subcommands.add_parser(
@@ -147,4 +148,7 @@ push = subcommands.add_parser(
 push.set_defaults(run_command=push_current_branch)
 
 args = options.parse_args()
+if args.subcommand == "projects" and args.add and not args.repos:
+    projs.error(
+        "When adding a subproject you must also pass the --repos argument")
 args.run_command(args)
diff --git a/tests/cli/llvmtestcase.py b/tests/cli/llvmtestcase.py
index 9b4ac3e..a6f234d 100644
--- a/tests/cli/llvmtestcase.py
+++ b/tests/cli/llvmtestcase.py
@@ -84,18 +84,11 @@ class LLVMTestCase(unittest.TestCase):
     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 values for
-        repos and env that it finds in cls.
+        args. Unless otherwise specified in kwargs, this uses the value for
+        env that it finds in cls.
         """
         command = [cls.python, cls.script]
 
-        repos = cls.repos
-        if "repos" in kwargs:
-            repos = kwargs["repos"]
-        if repos:
-            command.append("--repos")
-            command.append(repos)
-
         env = cls.env
         if "env" in kwargs:
             env = kwargs["env"]
diff --git a/tests/cli/testllvmprojects.py b/tests/cli/testllvmprojects.py
index 30f7ff2..ac1cd23 100644
--- a/tests/cli/testllvmprojects.py
+++ b/tests/cli/testllvmprojects.py
@@ -72,37 +72,23 @@ class Testllvmprojs(LLVMTestCase):
             with cd(repopath):
                 cls.run_quietly(["git", "worktree", "prune"])
 
-    def test_repos_arg_is_compulsory(self):
+    def test_repos_arg_is_compulsory_for_add(self):
         """
-        Test that we must pass in the repos for various combinations of input
-        args.
+        Test that we must pass in the repos when adding a subproject, but not
+        for other combinations of arguments.
         """
         with self.assertRaises(subprocess.CalledProcessError) as context:
-            self.run_with_output(self.llvm_projects(repos=None))
-
-        self.assertRegex(
-            str(context.exception.output),
-            "(.*\n)*.*the following arguments are required:(.*)--repos(.*\n)*")
-
-        with self.assertRaises(subprocess.CalledProcessError) as context:
             self.run_with_output(
-                self.llvm_projects(
-                    "--add", "clang", repos=None))
+                self.llvm_projects("--add", "clang"))
 
         self.assertRegex(
             str(context.exception.output),
-            "(.*\n)*.*the following arguments are required:(.*)--repos(.*\n)*")
-
-        with self.assertRaises(subprocess.CalledProcessError) as context:
-            self.run_with_output(
-                self.llvm_projects(
-                    "--remove",
-                    "clang",
-                    repos=None))
+            "(.*\n)*.*When adding a subproject you must also pass the --repos argument(.*\n)*")
 
-        self.assertRegex(
-            str(context.exception.output),
-            "(.*\n)*.*the following arguments are required:(.*)--repos(.*\n)*")
+        # These should not raise.
+        self.run_with_output(self.llvm_projects())
+        self.run_with_output(
+            self.llvm_projects("--remove", "clang"))
 
     def test_env_arg_is_compulsory(self):
         """
@@ -119,6 +105,7 @@ class Testllvmprojs(LLVMTestCase):
         with self.assertRaises(subprocess.CalledProcessError) as context:
             self.run_with_output(
                 self.llvm_projects(
+                    "--repos", self.repos,
                     "--add", "clang", env=None))
 
         self.assertRegex(
@@ -146,11 +133,14 @@ class Testllvmprojs(LLVMTestCase):
         """
         Test that we can add and remove one or several subprojects.
         """
-        output = self.run_with_output(self.llvm_projects("--add", "clang"))
+        output = self.run_with_output(self.llvm_projects(
+            "--repos", self.repos,
+            "--add", "clang"))
         self.assertRegex(output, "Projects linked:.*\n.*clang.*")
 
         output = self.run_with_output(
             self.llvm_projects(
+                "--repos", self.repos,
                 "--add", "libcxx", "lldb"))
         self.assertRegex(
             output,
@@ -180,6 +170,7 @@ class Testllvmprojs(LLVMTestCase):
         with self.assertRaises(subprocess.CalledProcessError) as context:
             self.run_with_output(
                 self.llvm_projects(
+                    "--repos", self.repos,
                     "--add", "inventedsubproject"))
 
         self.assertRegex(
@@ -203,6 +194,7 @@ class Testllvmprojs(LLVMTestCase):
         """
         output = self.run_with_output(
             self.llvm_projects(
+                "--repos", self.repos,
                 "--add", "clang", "lld", "clang"))
         self.assertRegex(output,
                          "Projects linked:.*\n" +
@@ -230,13 +222,16 @@ class Testllvmprojs(LLVMTestCase):
             os.path.join(self.llvm_src, "projects", "compiler-rt"),
             self.branch)
 
-        output = self.run_with_output(self.llvm_projects("--add", "clang"))
+        output = self.run_with_output(self.llvm_projects(
+            "--repos", self.repos,
+            "--add", "clang"))
         self.assertRegex(output,
                          "Projects linked:.*\n" +
                          ".*clang.*")
 
         output = self.run_with_output(
             self.llvm_projects(
+                "--repos", self.repos,
                 "--add", "compiler-rt", "lld"))
         self.assertRegex(
             output,
@@ -263,6 +258,8 @@ class Testllvmprojs(LLVMTestCase):
         with self.assertRaises(subprocess.CalledProcessError) as context:
             self.run_with_output(
                 self.llvm_projects(
+                    "--repos",
+                    self.repos,
                     "--add",
                     "clang",
                     "--remove",
@@ -286,6 +283,8 @@ class Testllvmprojs(LLVMTestCase):
         with self.assertRaises(subprocess.CalledProcessError) as context:
             self.run_with_output(
                 self.llvm_projects(
+                    "--repos",
+                    self.repos,
                     "--add",
                     "clang",
                     "lld",
@@ -316,6 +315,8 @@ class Testllvmprojs(LLVMTestCase):
         """
         output = self.run_with_output(
             self.llvm_projects(
+                "--repos",
+                self.repos,
                 "--add",
                 "libcxxabi",
                 "--remove",
@@ -333,6 +334,8 @@ class Testllvmprojs(LLVMTestCase):
 
         output = self.run_with_output(
             self.llvm_projects(
+                "--repos",
+                self.repos,
                 "--add",
                 "libunwind",
                 "libcxxabi",
@@ -351,6 +354,8 @@ class Testllvmprojs(LLVMTestCase):
 
         output = self.run_with_output(
             self.llvm_projects(
+                "--repos",
+                self.repos,
                 "--add",
                 "libcxx",
                 "--remove",
@@ -387,11 +392,14 @@ class Testllvmprojs(LLVMTestCase):
         # Make sure that adding projects works
         output = self.run_with_output(
             self.llvm_projects(
+                "--repos", self.repos,
                 "--add", "clang", "lld"))
         self.assertRegex(output, "Projects linked:.*\n.*clang.*\n.*lld.*\n")
 
         output = self.run_with_output(
             self.llvm_projects(
+                "--repos",
+                self.repos,
                 "--add",
                 "libcxx",
                 "libcxxabi",