Fix repository order bug in distribution command (#638)

* Fix bug in distribution command

The distribution command was modifying the list of repos in a given
spack environment which causes problems when resolving package dependencies.
Maintain the original package repository data structure throughout the
distro bundling process so that this does not occur.

* Fix test

* Fix test

* Fix create-env test

Author: jnhealy2

manager/manager_cmds/create_env.py

@@ -46,10 +46,7 @@ def create_env(parser, args):
             manifest.add_user_spec(str(s))
 
     if args.local_source:
-        if spack.spack_version_info[0:3] < (0, 23, 0):
-            manifest.set_config_value("config", "install_tree", {"root": "$env/opt"})
-        else:
-            manifest.set_config_value("config", "install_tree", "$env/opt")
+        manifest.set_config_value("config", "install_tree", {"root": "$env/opt"})
 
     # handle includes, if it is not set up right then nothing gets created
     include_dict = {"machine": args.machine, "dir": theDir, "file": "include.yaml"}

manager/manager_cmds/distribution.py

@@ -4,11 +4,11 @@
 
 import spack.cmd
 import spack.config
+import spack.environment
 import spack.extensions
 import spack.llnl.util.tty as tty
 import spack.util.path
 import spack.util.spack_yaml
-from spack import environment
 from spack.cmd.mirror import create_mirror_for_all_specs, filter_externals
 from spack.llnl.util.filesystem import working_dir
 from spack.main import SpackCommand
@@ -143,11 +143,11 @@ def __init__(self, env, root, includes=None, excludes=None, extra_data=None):
         self._cached_env = None
 
     def __enter__(self):
-        self._cached_env = environment.active_environment()
+        self._cached_env = spack.environment.active_environment()
         tty.msg(f"Concretizing env: {self.environment_to_package.name}....")
         self.environment_to_package.concretize()
         self.environment_to_package.write()
-        environment.deactivate()
+        spack.environment.deactivate()
         self.init_distro_dir()
 
         return self
@@ -156,13 +156,13 @@ def __exit__(self, exc_type, exc_val, exc_tb):
         self.filter_excludes()
         self.remove_unwanted_artifacts()
         if self._cached_env:
-            environment.activate(self._cached_env)
+            spack.environment.activate(self._cached_env)
 
     @property
     def env(self):
         if self._env is None:
             epath = os.path.join(self.path, "environment")
-            self._env = environment.create_in_dir(epath, keep_relative=True)
+            self._env = spack.environment.create_in_dir(epath, keep_relative=True)
         return self._env
 
     def init_distro_dir(self):
@@ -240,24 +240,25 @@ def configure_extensions(self):
                     sconfig("add", f"config:extensions:[{extension}]")
 
     def configure_package_repos(self):
-        repos = set()
         with self.environment_to_package:
+            repos = spack.util.spack_yaml.syaml_dict()
             for scope in valid_env_scopes(self.environment_to_package):
-                for repo in spack.config.get("repos", scope=scope).values():
-                    repos.add(spack.util.path.canonicalize_path(repo))
+                repos.update(spack.config.get("repos", scope=scope))
 
         tty.msg(f"Packing up package repositories to {self.package_repos}....")
         os.makedirs(self.package_repos)
 
-        to_write = {}
-        for repo in repos:
-            name = os.path.basename(repo)
-            to_write[name] = os.path.join(os.path.relpath(self.package_repos, self.env.path), name)
-            shutil.copytree(repo, os.path.join(self.package_repos, name))
+        for name, repo in repos.items():
+            repo = spack.util.path.canonicalize_path(repo)
+            basename = os.path.basename(repo)
+            repos[name] = os.path.join(
+                os.path.relpath(self.package_repos, self.env.path), basename
+            )
+            shutil.copytree(repo, os.path.join(self.package_repos, basename))
 
         tty.msg(f"Adding repositories to env: {self.env.name}....")
         env = get_env_as_dict(self.env)
-        env["spack"]["repos"] = to_write
+        env["spack"]["repos"] = repos
         self._write(env)
 
     def configure_package_settings(self, filter_externals=False):

tests/test_distribution.py

@@ -52,7 +52,7 @@ def {base_name}(parser, args):
 
 def create_repo(path):
     os.makedirs(path)
-    data = {"repo": {"namespace": "test"}}
+    data = {"repo": {"namespace": os.path.basename(path)}}
     with open(os.path.join(path, "repo.yaml"), "w") as f:
         spack.util.spack_yaml.dump(data, f, default_flow_style=False)
     package = os.path.join(path, "packages", "test")
@@ -402,7 +402,6 @@ def test_DistributionPackager_configure_extensions(tmpdir):
     with tmpdir.as_cwd():
         pkgr.configure_extensions()
     mod_content = get_manifest(pkgr.env)
-    print("content", mod_content)
 
     for extension in expected_extensions:
         assert os.path.isdir(os.path.join(pkgr.env.path, extension))
@@ -416,10 +415,17 @@ def test_DistributionPackager_configure_repos(tmpdir):
     the environment into the environment being created and adds a relative path to the
     copied repositories to the spack.yaml that is being constructed.
     """
-    package_repo = os.path.join(tmpdir.strpath, "mock-repo")
+    package_repo = os.path.join(tmpdir.strpath, "mock_repo")
     create_repo(package_repo)
 
-    extra_data = {"repos": {"mock-repo": package_repo}}
+    package_repo2 = os.path.join(tmpdir.strpath, "mock_repo2")
+    create_repo(package_repo2)
+
+    data = spack.util.spack_yaml.syaml_dict()
+    data["mock_repo"] = package_repo
+    data["mock_repo2"] = package_repo2
+
+    extra_data = {"repos": data}
     manifest = os.path.join(tmpdir.strpath, "base-env", "spack.yaml")
     create_spack_manifest(manifest, extra_data=extra_data)
     env = spack.environment.Environment(os.path.dirname(manifest))
@@ -436,8 +442,11 @@ def test_DistributionPackager_configure_repos(tmpdir):
     assert os.path.isdir(expected_repo_dir)
     assert "repos" in result_config["spack"]
 
-    expected_repo = {"mock-repo": f"../{os.path.basename(pkgr.package_repos)}/mock-repo"}
-    assert expected_repo == result_config["spack"]["repos"]
+    expected = spack.util.spack_yaml.syaml_dict()
+    for key, val in data.items():
+        expected[key] = f"../{os.path.basename(pkgr.package_repos)}/{os.path.basename(val)}"
+
+    assert expected == result_config["spack"]["repos"]
 
 
 def test_DistributionPackager_configure_package_settings(tmpdir):
@@ -449,7 +458,7 @@ def test_DistributionPackager_configure_package_settings(tmpdir):
     when `filter_externals` is `True`.
     """
     good = {"all": {"prefer": ["generator=Ninja"]}}
-    bad = {"cmake": {"externals": [{"spec": "[email protected]", "path": "/foo/bar"}]}}
+    bad = {"cmake": {"externals": [{"spec": "[email protected]", "prefix": "/foo/bar"}]}}
 
     extra_data = {"packages": {}}
     extra_data["packages"].update(good)