Dry run-ify LLVMSourceConfig

Make it possible to use LLVMSourceConfig in dry run mode. This is useful
because otherwise we can't have a dry run on *any* scripts that use an
LLVMSourceConfig, since it might throw errors that wouldn't occur in
non-dry mode.

This is different from non-dry run in the following ways:
* we do not perform any validation of the source directory (since in dry
  run mode it is reasonable to expect that the directory doesn't even
  exist yet)
* we do not try to add or remove any subprojects on disk

Instead, we do try to keep track of our updates (which projects are
added and which are removed). We don't however look at what may exist on
disk since that increases the complexity of the code significantly and
it is very likely that scripts will create a source directory from
scratch rather than use one where subprojects have already been added.
We may want to fix that after a bit of refactoring.

Change-Id: Ic12c292b9cdc932aa5de3fa780af5e608dcba01c

3 files changed, 158 insertions, 40 deletions

diff --git a/modules/llvm.py b/modules/llvm.py
index 298076d..953b513 100644
--- a/modules/llvm.py
+++ b/modules/llvm.py
@@ -73,7 +73,7 @@ class LLVMSourceConfig(object):
     functionality for adding / removing them.
     """
 
-    def __init__(self, proj, sourcePath,
+    def __init__(self, proj, sourcePath, dry,
                  subprojs=LLVMSubproject.get_all_subprojects()):
         """ Create a source configuration.
 
@@ -83,6 +83,13 @@ class LLVMSourceConfig(object):
             Temporary project directory (used mostly for logging).
         sourcePath
             Must point to a valid LLVM source tree.
+        dry
+            Whether or not we are running in dry run mode. If we are, then we do
+            not try to add or remove any subprojects, but we try to remember
+            which updates we performed. We don't currently check the subprojects
+            that are enabled on disk, since for the dry run mode we don't
+            perform any validation whatsoever on the source directory. This may
+            change in the future.
         subprojs : dictionary
             Dictionary containing a number of LLVMSubproject objects.
             By default, this contains all the LLVM subprojects as returned by
@@ -92,17 +99,22 @@ class LLVMSourceConfig(object):
             class's methods. It is an error to invoke them with a subproj that
             does not exist in this dictionary.
         """
-        sourcePath = str(sourcePath)
-        if not os.path.isdir(sourcePath):
-            raise EnvironmentError("Invalid path to LLVM source tree")
+        if not dry:
+            sourcePath = str(sourcePath)
+            if not os.path.isdir(sourcePath):
+                raise EnvironmentError("Invalid path to LLVM source tree")
+            self.proj = proj
+            self.llvmSourceTree = Worktree(proj, sourcePath)
+        else:
+            self.subprojsEnabledInDryRun = []
 
-        self.proj = proj
-        self.llvmSourceTree = Worktree(proj, sourcePath)
+        self.sourcePath = sourcePath
+        self.dry = dry
         self.subprojs = subprojs
 
     def get_path(self):
         """Get the path corresponding to this source config."""
-        return self.llvmSourceTree.repodir
+        return self.sourcePath
 
     def get_enabled_subprojects(self):
         """Get a list of the subprojects enabled in this configuration."""
@@ -169,7 +181,7 @@ class LLVMSourceConfig(object):
                 raise RuntimeError("Error while processing {}".format(subproj)) from exc
 
         # Visit LLVM last, in case getting the enabled subprojects errors out.
-        action(self.llvmSourceTree.repodir)
+        action(self.sourcePath)
 
     def for_each_subproj(self, action):
         """Perform the given action for each subproject excluding LLVM.
@@ -196,7 +208,7 @@ class LLVMSourceConfig(object):
     def __get_subproj_cmake_path(self, subprojName):
         """Get the full path to subprojName in this source tree."""
         subproj = self.__get_subproj_object(subprojName)
-        return subproj.get_cmake_path(self.llvmSourceTree.repodir)
+        return subproj.get_cmake_path(self.sourcePath)
 
     def __is_enabled(self, subprojName):
         """
@@ -205,6 +217,9 @@ class LLVMSourceConfig(object):
         a directory for the subproject exists but does not satisfy those
         conditions, an EnvironmentError is thrown.
         """
+        if self.dry:
+            return subprojName in self.subprojsEnabledInDryRun
+
         subprojPath = self.__get_subproj_cmake_path(subprojName)
 
         if not os.path.isdir(subprojPath):
@@ -239,6 +254,10 @@ class LLVMSourceConfig(object):
             # Subproject has already been added, nothing to do.
             return
 
+        if self.dry:
+            self.subprojsEnabledInDryRun.append(subprojName)
+            return
+
         if os.path.exists(path):
             raise EnvironmentError(
                 "{} already exists but is not a valid subproject directory."
@@ -260,6 +279,11 @@ class LLVMSourceConfig(object):
 
     def __remove_subproject(self, subprojName):
         """Remove a given subproject from this build configuration."""
+        if self.dry:
+            if self.__is_enabled(subprojName):
+                self.subprojsEnabledInDryRun.remove(subprojName)
+            return
+
         path = self.__get_subproj_cmake_path(subprojName)
 
         if not os.path.isdir(path):
diff --git a/scripts/llvm.py b/scripts/llvm.py
index cce0ef4..3a4954c 100644
--- a/scripts/llvm.py
+++ b/scripts/llvm.py
@@ -52,7 +52,7 @@ def projects(args):
 
     llvm_worktree_root = args.sources
     llvm_repos_root = args.repos
-    config = LLVMSourceConfig(proj, llvm_worktree_root)
+    config = LLVMSourceConfig(proj, llvm_worktree_root, dry=False)
 
     if not args.add and not args.remove:
         # Nothing to change, just print the current configuration
@@ -79,7 +79,7 @@ def push_current_branch(args):
     proj = Proj()
 
     llvm_worktree_root = args.sources
-    config = LLVMSourceConfig(proj, llvm_worktree_root)
+    config = LLVMSourceConfig(proj, llvm_worktree_root, dry=False)
 
     llvm_worktree = Clone(proj, llvm_worktree_root)
     local_branch = llvm_worktree.getbranch()
@@ -99,7 +99,7 @@ def configure_build(args):
     proj = Proj()
 
     llvm_worktree_root = args.sources
-    sourceConfig = LLVMSourceConfig(proj, llvm_worktree_root)
+    sourceConfig = LLVMSourceConfig(proj, llvm_worktree_root, args.dry)
 
     buildConfig = LLVMBuildConfig(sourceConfig, args.build)
 
diff --git a/tests/unittests/testllvmsourceconfig.py b/tests/unittests/testllvmsourceconfig.py
index 0baf7e3..3bdc4ed 100644
--- a/tests/unittests/testllvmsourceconfig.py
+++ b/tests/unittests/testllvmsourceconfig.py
@@ -60,7 +60,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
 
     def test_get_path(self):
         sourcePath = self.temporaryLLVM.repodir
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
         self.assertEqual(config.get_path(), sourcePath)
 
     def test_detect_enabled_all(self):
@@ -76,7 +76,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
                 self.temporaryLLVMbranch)
             self.assertTrue(os.path.isdir(path), "Failed to create worktree")
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
         enabled = config.get_enabled_subprojects()
 
         self.assertEqual(set(subprojs), set(enabled),
@@ -98,7 +98,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
         path = os.path.join(sourcePath, "tools", "libcxx")
         os.makedirs(path)
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
         enabled = config.get_enabled_subprojects()
 
         self.assertEqual(
@@ -120,7 +120,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
                 self.temporaryLLVMbranch)
             self.assertTrue(os.path.isdir(path), "Failed to create worktree")
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
         enabled = config.get_enabled_subprojects()
 
         self.assertTrue("lld" in enabled, "Failed to detect lld")
@@ -135,7 +135,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
         path = subprojs["compiler-rt"].get_cmake_path(sourcePath)
         os.makedirs(path)
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
         with self.assertRaises(EnvironmentError) as context:
             config.get_enabled_subprojects()
 
@@ -157,7 +157,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
             branch)
         self.assertTrue(os.path.isdir(path), "Failed to create worktree")
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
         with self.assertRaises(EnvironmentError) as context:
             config.get_enabled_subprojects()
 
@@ -166,7 +166,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
                                                                                    self.temporaryLLVMbranch))
 
     def test_add_invalid_subproject(self):
-        config = LLVMSourceConfig(self.proj, self.temporaryLLVM.repodir)
+        config = LLVMSourceConfig(self.proj, self.temporaryLLVM.repodir, False)
         subproj = "not-an-llvm-subproject"
         subprojPath = self.originalLLVM.repodir  # Dummy path
 
@@ -179,7 +179,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
     def test_add_each_subproject(self):
         sourcePath = self.temporaryLLVM.repodir
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
 
         subprojs = LLVMSubproject.get_all_subprojects()
         for subproj in subprojs:
@@ -191,7 +191,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
     def test_add_all_subprojects(self):
         sourcePath = self.temporaryLLVM.repodir
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
 
         to_add = {}
         subprojs = LLVMSubproject.get_all_subprojects()
@@ -209,7 +209,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
     def test_add_some_subprojects(self):
         sourcePath = self.temporaryLLVM.repodir
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
 
         to_add = {}
         to_add["clang"] = self.__get_subproj_repo("clang")
@@ -228,7 +228,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
     def test_add_existing_subprojects(self):
         sourcePath = self.temporaryLLVM.repodir
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
 
         subprojs = LLVMSubproject.get_all_subprojects()
         existingPath = subprojs["lldb"].get_cmake_path(sourcePath)
@@ -253,7 +253,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
         """
         sourcePath = self.temporaryLLVM.repodir
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
 
         clangRepo = self.__get_subproj_repo("clang")
         with cd(clangRepo.repodir):
@@ -277,7 +277,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
         sourcePath = self.temporaryLLVM.repodir
         branch = "different-than-" + self.temporaryLLVMbranch
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
 
         subprojs = LLVMSubproject.get_all_subprojects()
         existingPath = subprojs["lldb"].get_cmake_path(sourcePath)
@@ -303,7 +303,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
         sourcePath = self.temporaryLLVM.repodir
         branch = "different-than-" + self.temporaryLLVMbranch
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
 
         subprojs = LLVMSubproject.get_all_subprojects()
         existingPath = subprojs["lldb"].get_cmake_path(sourcePath)
@@ -344,7 +344,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
             clangPath,
             self.temporaryLLVMbranch)
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
         config.update(remove=["lld"])
 
         self.assertFalse(os.path.isdir(lldPath), "Failed to remove subproject")
@@ -363,7 +363,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
                 path,
                 self.temporaryLLVMbranch)
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
         config.update(remove=["compiler-rt", "lld"])
 
         for subproj in ["compiler-rt", "lld"]:
@@ -390,7 +390,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
                 self.temporaryLLVMbranch)
             self.assertTrue(os.path.isdir(path), "Failed to create worktree")
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
         config.update(remove=list(subprojs.keys()))
 
         for subproj in list(subprojs.keys()):
@@ -413,7 +413,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
                 self.temporaryLLVMbranch)
             self.assertTrue(os.path.isdir(path), "Failed to create worktree")
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
 
         for subproj in list(subprojs.keys()):
             config.update(remove=[subproj])
@@ -437,7 +437,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
                 path,
                 self.temporaryLLVMbranch)
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
         config.update(remove=["compiler-rt", "lld", "lld", "compiler-rt"])
 
         for subproj in ["compiler-rt", "lld"]:
@@ -452,7 +452,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
 
     def test_remove_invalid_subproject(self):
         sourcePath = self.temporaryLLVM.repodir
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
 
         subproj = "not-an-llvm-subproject"
 
@@ -476,7 +476,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
 
         clangPath = subprojs["clang"].get_cmake_path(sourcePath)
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
         config.update(remove=["clang"])
 
         self.assertFalse(
@@ -490,7 +490,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
         subprojs = LLVMSubproject.get_all_subprojects()
         lldbRepo = self.__get_subproj_repo("lldb")
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
 
         config.update({"lldb" : lldbRepo})
         self.assertTrue(
@@ -520,7 +520,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
                 path,
                 self.temporaryLLVMbranch)
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
         config.update({
                 "libcxx" : self.__get_subproj_repo("libcxx"),
                 "libcxxabi" : self.__get_subproj_repo("libcxxabi")},
@@ -548,7 +548,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
         lldRepo = self.__get_subproj_repo("lld")
         libunwindRepo = self.__get_subproj_repo("libunwind")
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
         with self.assertRaises(ValueError) as context:
             config.update(
                     { "clang" : clangRepo, "lld" : lldRepo, "libunwind" :
@@ -574,7 +574,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
     def test_for_each_enabled(self):
         sourcePath = self.temporaryLLVM.repodir
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
 
         logPath = unittest.mock.MagicMock()
         config.for_each_enabled(logPath)
@@ -603,7 +603,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
         """Test that we rethrow exceptions correctly."""
         sourcePath = self.temporaryLLVM.repodir
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
         subprojs = config.subprojs
         enabled = ["clang", "compiler-rt", "lld", "lldb", "libunwind"]
 
@@ -629,7 +629,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
     def test_for_each_subproj(self):
         sourcePath = self.temporaryLLVM.repodir
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
 
         subprojs = config.subprojs
         enabled = ["clang", "compiler-rt", "lld", "lldb", "libunwind"]
@@ -664,7 +664,7 @@ class TestLLVMSourceConfig(unittest.TestCase):
         """Test that we rethrow exceptions correctly."""
         sourcePath = self.temporaryLLVM.repodir
 
-        config = LLVMSourceConfig(self.proj, sourcePath)
+        config = LLVMSourceConfig(self.proj, sourcePath, False)
         subprojs = config.subprojs
         enabled = ["clang", "compiler-rt", "lld", "lldb", "libunwind"]
 
@@ -700,6 +700,100 @@ class TestLLVMSourceConfig(unittest.TestCase):
         self.assertRegex(str(context.exception.__cause__),
                          "An error has been!!1(.*\n)*")
 
+    def test_dry_run_none_enabled(self):
+        """
+        Test that when running in dry run mode, we don't error out if the
+        source directory is bogus, and also that we report no enabled
+        subprojects.
+        """
+        sourcePath = "path-to-llvm-source-tree"
+
+        config = LLVMSourceConfig(self.proj, sourcePath, True)
+        enabled = config.get_enabled_subprojects()
+
+        self.assertEqual([], enabled)
+
+    def test_dry_run_add_subprojects(self):
+        """
+        Test that in dry run mode we don't perform any side effects, but we
+        remember the subprojects that were added.
+        """
+        sourcePath = "path-to-llvm-source-tree"
+
+        config = LLVMSourceConfig(self.proj, sourcePath, True)
+        subprojs = config.subprojs
+
+        config.update({"clang" : "path-to-clang-repo",
+                       "compiler-rt" : "path-to-compiler-rt-repo"})
+        self.assertFalse(
+            os.path.isdir(subprojs["clang"].get_cmake_path(sourcePath)))
+        self.assertFalse(
+            os.path.isdir(subprojs["compiler-rt"].get_cmake_path(sourcePath)))
+
+        enabled = config.get_enabled_subprojects()
+        self.assertEqual(set(["clang", "compiler-rt"]),
+                         set(config.get_enabled_subprojects()))
+
+    def test_dry_run_remove_subprojects(self):
+        """ Test that in dry run mode we don't remove existing subprojects."""
+        sourcePath = self.temporaryLLVM.repodir
+
+        subprojs = LLVMSubproject.get_all_subprojects()
+        enabled = ["clang", "compiler-rt", "lld", "lldb", "libunwind"]
+
+        for subproj in enabled:
+            path = subprojs[subproj].get_cmake_path(sourcePath)
+            worktree = Worktree.create(
+                self.proj,
+                self.__get_subproj_repo(subproj),
+                path,
+                self.temporaryLLVMbranch)
+
+        config = LLVMSourceConfig(self.proj, sourcePath, True)
+        config.update({}, ["clang", "lld"])
+
+        for subproj in enabled:
+            self.assertTrue(
+                os.path.isdir(subprojs[subproj].get_cmake_path(sourcePath)))
+
+        self.assertNotIn("clang", config.get_enabled_subprojects())
+        self.assertNotIn("lld", config.get_enabled_subprojects())
+
+    def test_dry_run_add_then_remove(self):
+        """ Test that in dry run mode we correctly keep track of updates."""
+        sourcePath = "path-to-llvm-source-tree"
+
+        config = LLVMSourceConfig(self.proj, sourcePath, True)
+        subprojs = config.subprojs
+
+        config.update({"clang" : "path-to-clang-repo",
+                       "compiler-rt" : "path-to-compiler-rt-repo"})
+        enabled = config.get_enabled_subprojects()
+        self.assertEqual(set(["clang", "compiler-rt"]),
+                         set(config.get_enabled_subprojects()))
+
+        config.update({"lld" : "path-to-lld-repo"}, ["compiler-rt"])
+        self.assertEqual(set(["clang", "lld"]),
+                         set(config.get_enabled_subprojects()))
+
+    def test_dry_run_for_each_enabled(self):
+        """ Test that for_each_enabled works in dry run mode."""
+        sourcePath = "path-to-llvm-source-tree"
+
+        config = LLVMSourceConfig(self.proj, sourcePath, True)
+        subprojs = config.subprojs
+
+        config.update({"clang": "path-to-clang-repo",
+                       "compiler-rt": "path-to-compiler-rt-repo"})
+
+        action = MagicMock()
+        config.for_each_enabled(action)
+
+        action.assert_has_calls(
+            [call(subprojs["clang"].get_cmake_path(sourcePath)),
+             call(subprojs["compiler-rt"].get_cmake_path(sourcePath))],
+            any_order=True)
+
 
 if __name__ == "__main__":
     unittest.main()