Distribution extra spack cleanup (#640)

* Minor fixes

fix tests to be robust to non-breaking api changes.
Also, change binary asset check so it is not recursive.
This avoids an infinite loop in some environments.

* When bundling spack with a distribution don't include user files

The distribution command was including files specific to the user's spack environment
were getting bundled along with spack.  This is becasue spack writes into its root
directory as it is being used.  This creates a more exhaustive list of locations
to avoid copying along with spack and more agressively asserts they don't end up in
the final spack installation

Author: jnhealy2

manager/manager_cmds/distribution.py

@@ -1,4 +1,5 @@
 import fnmatch
+import glob
 import os
 import shutil
 
@@ -20,6 +21,9 @@
 level = "long"
 
 
+SPACK_USER_PATTERNS = ["var/*", "opt/*", ".git*", "etc/spack/*"]
+
+
 def add_command(parser, command_dict):
     subparser = parser.add_parser("distribution", help=description)
     subparser.add_argument(
@@ -139,8 +143,7 @@ def valid_env_scopes(env):
 
 def bundle_spack(location):
     tty.msg(f"Packing up Spack installation to {location}....")
-    ignore_these = ["var/spack/environments/*", "opt/*", ".git*", "etc/spack/include.yaml"]
-    copy_files_excluding_pattern(spack_root, location, ignore_these)
+    copy_files_excluding_pattern(spack_root, location, SPACK_USER_PATTERNS)
 
 
 class DistributionPackager:
@@ -205,6 +208,8 @@ def concretize(self):
         self.env.write()
 
     def remove_unwanted_artifacts(self):
+        for pattern in SPACK_USER_PATTERNS:
+            remove_by_pattern(os.path.join(self.spack_dir, pattern))
         for item in os.listdir(self.env.path):
             fullname = os.path.join(self.env.path, item)
             if "spack.yaml" in item:
@@ -413,6 +418,15 @@ def correct_mirror_args(env, args):
         args.source_only = True
 
 
+def remove_by_pattern(pattern):
+    for item in glob.glob(pattern, recursive=True):
+        tty.msg(item)
+        if os.path.isfile(item):
+            os.remove(item)
+        elif os.path.isdir(item):
+            shutil.rmtree(item)
+
+
 def distribution(parser, args):
     env = spack.cmd.require_active_env(cmd_name="manager distribution")
     correct_mirror_args(env, args)

tests/test_distribution.py

@@ -126,6 +126,34 @@ def test_copy_files_excluding_pattern(tmpdir):
     assert len(results) == 3
 
 
+def test_remove_by_pattern(tmpdir):
+    """
+    Test the removal of all files/dirs from a higherarchy that match a passed glob pattern.
+    """
+    root = os.path.join(tmpdir.strpath, "foo")
+    good = [
+        os.path.join(root, "bar", "file.txt"),
+        os.path.join(root, "bar", "bong", "bad.txt"),
+        os.path.join(root, "baz", "file.txt"),
+    ]
+
+    bad_root = os.path.join(root, "bing")
+    bad = [os.path.join(bad_root, "bang", "bad.txt"), os.path.join(bad_root, "bing", "file.txt")]
+
+    for p in good + bad:
+        os.makedirs(os.path.dirname(p), exist_ok=True)
+        with open(p, "w") as f:
+            f.write("content")
+
+    distribution.remove_by_pattern(f"{bad_root}/*")
+
+    for bad_p in bad:
+        assert not os.path.isfile(bad_p)
+    for good_p in good:
+        assert os.path.isfile(good_p)
+    assert os.path.isdir(bad_root)
+
+
 def test_remove_subset_from_dict():
     """
     This test verifies that the `remove_subset_from_dict` function correctly removes specified keys
@@ -453,7 +481,7 @@ def test_concretzie(tmpdir):
     assert os.path.isfile(lockfile)
 
 
-def test_DistributionPackager_remove_unwanted_artifacts(tmpdir):
+def test_DistributionPackager_remove_unwanted_artifacts(tmpdir, monkeypatch):
     """
     This test verifies that `remove_unwanted_artifacts` removes files
     created by concretizing the environment.
@@ -466,13 +494,19 @@ def test_DistributionPackager_remove_unwanted_artifacts(tmpdir):
 
     pkgr = distribution.DistributionPackager(None, root)
     pkgr._env = env
-
     pkgr.concretize()
     assert len(os.listdir(env_dir)) == 3
+    bad_file = os.path.join(pkgr.spack_dir, "foo", "aFile")
+    os.makedirs(os.path.dirname(bad_file))
+    with open(bad_file, "w") as out:
+        out.write("content")
+    monkeypatch.setattr(distribution, "SPACK_USER_PATTERNS", ["foo/*"])
+
     pkgr.remove_unwanted_artifacts()
     env_assets = os.listdir(env_dir)
     assert len(env_assets) == 1
     assert "spack.yaml" in env_assets
+    assert not os.path.isfile(bad_file)
 
 
 def test_DistributionPackager_configure_includes(tmpdir):