From e7b797a0c452fd430b380e6235f51e08a5e0d20f Mon Sep 17 00:00:00 2001
From: Dom Heinzeller <dom.heinzeller@icloud.com>
Date: Mon, 2 Oct 2023 17:22:43 -0600
Subject: [PATCH 1/6] Make specs for containers configurable, clean up
 arguments for spack stack create [env|ctr]

---
 .../stack/cmd/stack_cmds/create.py            | 83 +++++++++++++------
 .../spack-stack/stack/container_env.py        | 59 +++++++------
 .../spack-stack/stack/stack_paths.py          |  1 +
 3 files changed, 90 insertions(+), 53 deletions(-)

diff --git a/lib/jcsda-emc/spack-stack/stack/cmd/stack_cmds/create.py b/lib/jcsda-emc/spack-stack/stack/cmd/stack_cmds/create.py
index ef93d78b45218e..d038839958245f 100644
--- a/lib/jcsda-emc/spack-stack/stack/cmd/stack_cmds/create.py
+++ b/lib/jcsda-emc/spack-stack/stack/cmd/stack_cmds/create.py
@@ -51,44 +51,86 @@ def container_config_help():
     help_string = "Pre-configured container." + os.linesep
     help_string += "Available options are: " + os.linesep
     for config in container_configs:
+        if config == "README.md":
+            continue
         help_string += "\t" + config.rstrip(".yaml") + os.linesep
     return help_string
 
 
+def container_specs_help():
+    _, _, specs_lists = next(os.walk(stack_path("configs", "containers", "specs")))
+    help_string = "List of specs to build in container." + os.linesep
+    help_string += "Available options are: " + os.linesep
+    for specs_list in specs_lists:
+        help_string += "\t" + specs_list.rstrip(".yaml") + os.linesep
+    return help_string
+
+
 def setup_common_parser_args(subparser):
     """Shared CLI args for container and environment subcommands"""
+
     subparser.add_argument(
-        "--template",
-        type=str,
+        "--overwrite",
+        action="store_true",
         required=False,
-        dest="template",
-        default="empty",
-        help=template_help(),
+        default=False,
+        help="Overwrite existing environment if it exists." " Warning this is dangerous.",
+    )
+
+
+def setup_ctr_parser(subparser):
+    """create container-specific parsing options"""
+
+    subparser.add_argument("--container", 
+        required=True,
+        help=container_config_help()
+    )
+
+    subparser.add_argument("--specs",
+        required=True,
+        help=container_specs_help()
     )
 
     subparser.add_argument(
-        "--name",
+        "--dir",
         type=str,
         required=False,
-        default=None,
-        help='Environment name, defaults to "{}".'.format(default_env_name),
+        default=default_env_path,
+        help="Environment will be placed in <dir>/container/."
+        " Default is {}/container/.".format(default_env_path)
     )
 
+    setup_common_parser_args(subparser)
+
+
+def setup_env_parser(subparser):
+    """create environment-specific parsing options"""
+    setup_common_parser_args(subparser)
+
     subparser.add_argument(
         "--dir",
         type=str,
         required=False,
         default=default_env_path,
-        help="Environment will be placed in <dir>/<name>/."
-        " Default is {}/<name>/.".format(default_env_path),
+        help="Environment will be placed in <dir>/<env-name>/."
+        " Default is {}/<env-name>/.".format(default_env_path),
     )
 
     subparser.add_argument(
-        "--overwrite",
-        action="store_true",
+        "--name",
+        type=str,
         required=False,
-        default=False,
-        help="Overwrite existing environment if it exists." " Warning this is dangerous.",
+        default=None,
+        help='Environment name, defaults to <template>.<site>',
+    )
+
+    subparser.add_argument(
+        "--template",
+        type=str,
+        required=False,
+        dest="template",
+        default="empty",
+        help=template_help(),
     )
 
     subparser.add_argument(
@@ -106,17 +148,6 @@ def setup_common_parser_args(subparser):
         help="Include upstream environment (/path/to/spack-stack-x.y.z/envs/unified-env/install)",
     )
 
-
-def setup_ctr_parser(subparser):
-    """create container-specific parsing options"""
-    subparser.add_argument("container", help=container_config_help())
-
-    setup_common_parser_args(subparser)
-
-
-def setup_env_parser(subparser):
-    """create environment-specific parsing options"""
-    setup_common_parser_args(subparser)
     subparser.add_argument(
         "--site", type=str, required=False, default=default_site(), help=site_help()
     )
@@ -145,7 +176,7 @@ def setup_create_parser(subparser):
 def container_create(args):
     """Create pre-configured container"""
 
-    container = StackContainer(args.container, args.template, args.name, args.dir, args.packages)
+    container = StackContainer(args.container, args.dir, args.specs)
 
     env_dir = container.env_dir
     if os.path.exists(env_dir):
diff --git a/lib/jcsda-emc/spack-stack/stack/container_env.py b/lib/jcsda-emc/spack-stack/stack/container_env.py
index e510846982b5a4..6ec3e0710cfff2 100644
--- a/lib/jcsda-emc/spack-stack/stack/container_env.py
+++ b/lib/jcsda-emc/spack-stack/stack/container_env.py
@@ -6,7 +6,7 @@
 from spack.extensions.stack.stack_paths import (
     common_path,
     container_path,
-    template_path,
+    container_specs_path,
 )
 
 
@@ -16,48 +16,50 @@ class StackContainer:
     its packages.yaml versions then writes out a merged file.
     """
 
-    def __init__(self, container, template, name, dir, base_packages) -> None:
-        self.template = template
+    def __init__(self, container, dir, specs) -> None:
         self.container = container
+        self.specs = specs
 
-        test_path = os.path.join(container_path, container + ".yaml")
+        test_path = os.path.join(container_path, self.container + ".yaml")
         if os.path.exists(test_path):
             self.container_path = test_path
         elif os.path.isabs(container):
-            self.container_path = container
+            self.container_path = self.container
         else:
             raise Exception("Invalid container {}".format(self.container))
 
-        if os.path.isabs(self.template):
-            self.template_path = self.template
-        elif os.path.exists(os.path.join(template_path, self.template)):
-            self.template_path = os.path.join(template_path, self.template)
+        test_path = os.path.join(container_specs_path, self.specs + ".yaml")
+        if os.path.exists(test_path):
+            self.specs_path = test_path
+        elif os.path.isabs(specs):
+            self.specs_path = self.specs
         else:
-            raise Exception("Invalid application template")
-
-        self.name = name if name else "{}".format(container)
+            raise Exception("Invalid specs list {}".format(self.specs))
 
+        self.name = self.container
         self.dir = dir
         self.env_dir = os.path.join(self.dir, self.name)
-        if base_packages:
-            self.base_packages = base_packages
-        else:
-            self.base_packages = os.path.join(common_path, "packages.yaml")
+        self.base_packages = os.path.join(common_path, "packages.yaml")
 
     def write(self):
         """Merge base packages and app's spack.yaml into
         output container file.
         """
-        template_env = os.path.join(self.template_path, "spack.yaml")
-        with open(template_env, "r") as f:
-            # Spack uses :: to override settings.
-            # but it's not understood when used in a spack.yaml
+        #template_env = os.path.join(self.template_path, "spack.yaml")
+        #with open(template_env, "r") as f:
+        #    # Spack uses :: to override settings.
+        #    # but it's not understood when used in a spack.yaml
+        #    filedata = f.read()
+        #    filedata = filedata.replace("::", ":")
+        #    template_yaml = syaml.load_config(filedata)
+
+        with open(self.container_path, "r") as f:
             filedata = f.read()
             filedata = filedata.replace("::", ":")
-            template_yaml = syaml.load_config(filedata)
+            container_yaml = syaml.load_config(filedata)
 
-        with open(self.container_path, "r") as f:
-            container_yaml = syaml.load_config(f)
+        with open(self.specs_path, "r") as f:
+            specs_yaml = syaml.load_config(f)
 
         # Create copy so we can modify it
         original_yaml = copy.deepcopy(container_yaml)
@@ -74,11 +76,14 @@ def write(self):
             container_yaml["spack"]["packages"], packages_yaml["packages"]
         )
 
-        container_yaml = spack.config.merge_yaml(container_yaml, template_yaml)
-        # Merge the original back in so it takes precedence
-        container_yaml = spack.config.merge_yaml(container_yaml, original_yaml)
+        if "specs" not in container_yaml["spack"]:
+            container_yaml["spack"]["specs"] = {}
+
+        container_yaml["spack"]["specs"] = spack.config.merge_yaml(
+            container_yaml["spack"]["specs"], specs_yaml["specs"]
+        )
 
-        container_yaml["spack"]["container"]["labels"]["app"] = self.template
+        container_yaml["spack"]["container"]["labels"]["app"] = self.specs
 
         os.makedirs(self.env_dir, exist_ok=True)
 
diff --git a/lib/jcsda-emc/spack-stack/stack/stack_paths.py b/lib/jcsda-emc/spack-stack/stack/stack_paths.py
index 534109c385dc2b..889a0d36751c77 100644
--- a/lib/jcsda-emc/spack-stack/stack/stack_paths.py
+++ b/lib/jcsda-emc/spack-stack/stack/stack_paths.py
@@ -22,4 +22,5 @@ def stack_path(*paths):
 common_path = stack_path('configs', 'common')
 site_path = stack_path('configs', 'sites')
 container_path = stack_path('configs', 'containers')
+container_specs_path = stack_path('configs', 'containers', 'specs')
 template_path = stack_path('configs', 'templates')

From 6c02740816c2f82451ca295b25a0760d3f61ab2d Mon Sep 17 00:00:00 2001
From: Dom Heinzeller <dom.heinzeller@icloud.com>
Date: Mon, 2 Oct 2023 21:13:21 -0600
Subject: [PATCH 2/6] Fix style errors in spack-stack extensions

---
 .../spack-stack/stack/cmd/stack_cmds/create.py   | 14 ++++----------
 lib/jcsda-emc/spack-stack/stack/container_env.py | 16 +---------------
 lib/jcsda-emc/spack-stack/stack/stack_paths.py   | 14 +++++++-------
 3 files changed, 12 insertions(+), 32 deletions(-)

diff --git a/lib/jcsda-emc/spack-stack/stack/cmd/stack_cmds/create.py b/lib/jcsda-emc/spack-stack/stack/cmd/stack_cmds/create.py
index d038839958245f..5073bb5c160e6a 100644
--- a/lib/jcsda-emc/spack-stack/stack/cmd/stack_cmds/create.py
+++ b/lib/jcsda-emc/spack-stack/stack/cmd/stack_cmds/create.py
@@ -81,15 +81,9 @@ def setup_common_parser_args(subparser):
 def setup_ctr_parser(subparser):
     """create container-specific parsing options"""
 
-    subparser.add_argument("--container", 
-        required=True,
-        help=container_config_help()
-    )
+    subparser.add_argument("--container", required=True, help=container_config_help())
 
-    subparser.add_argument("--specs",
-        required=True,
-        help=container_specs_help()
-    )
+    subparser.add_argument("--specs", required=True, help=container_specs_help())
 
     subparser.add_argument(
         "--dir",
@@ -97,7 +91,7 @@ def setup_ctr_parser(subparser):
         required=False,
         default=default_env_path,
         help="Environment will be placed in <dir>/container/."
-        " Default is {}/container/.".format(default_env_path)
+        " Default is {}/container/.".format(default_env_path),
     )
 
     setup_common_parser_args(subparser)
@@ -121,7 +115,7 @@ def setup_env_parser(subparser):
         type=str,
         required=False,
         default=None,
-        help='Environment name, defaults to <template>.<site>',
+        help="Environment name, defaults to <template>.<site>",
     )
 
     subparser.add_argument(
diff --git a/lib/jcsda-emc/spack-stack/stack/container_env.py b/lib/jcsda-emc/spack-stack/stack/container_env.py
index 6ec3e0710cfff2..c3f26c9988e4ef 100644
--- a/lib/jcsda-emc/spack-stack/stack/container_env.py
+++ b/lib/jcsda-emc/spack-stack/stack/container_env.py
@@ -3,11 +3,7 @@
 
 import spack
 import spack.util.spack_yaml as syaml
-from spack.extensions.stack.stack_paths import (
-    common_path,
-    container_path,
-    container_specs_path,
-)
+from spack.extensions.stack.stack_paths import common_path, container_path, container_specs_path
 
 
 class StackContainer:
@@ -45,13 +41,6 @@ def write(self):
         """Merge base packages and app's spack.yaml into
         output container file.
         """
-        #template_env = os.path.join(self.template_path, "spack.yaml")
-        #with open(template_env, "r") as f:
-        #    # Spack uses :: to override settings.
-        #    # but it's not understood when used in a spack.yaml
-        #    filedata = f.read()
-        #    filedata = filedata.replace("::", ":")
-        #    template_yaml = syaml.load_config(filedata)
 
         with open(self.container_path, "r") as f:
             filedata = f.read()
@@ -61,9 +50,6 @@ def write(self):
         with open(self.specs_path, "r") as f:
             specs_yaml = syaml.load_config(f)
 
-        # Create copy so we can modify it
-        original_yaml = copy.deepcopy(container_yaml)
-
         with open(self.base_packages, "r") as f:
             filedata = f.read()
             filedata = filedata.replace("::", ":")
diff --git a/lib/jcsda-emc/spack-stack/stack/stack_paths.py b/lib/jcsda-emc/spack-stack/stack/stack_paths.py
index 889a0d36751c77..62ddd132b2776f 100644
--- a/lib/jcsda-emc/spack-stack/stack/stack_paths.py
+++ b/lib/jcsda-emc/spack-stack/stack/stack_paths.py
@@ -5,7 +5,7 @@
 # Hidden file in top-level spack-stack dir so this module can
 # find relative config files. Assuming Spack is a submodule of
 # spack-stack.
-check_file = '.spackstack'
+check_file = ".spackstack"
 
 
 # Find spack-stack directory assuming this Spack instance
@@ -14,13 +14,13 @@ def stack_path(*paths):
     stack_dir = os.path.dirname(spack.paths.spack_root)
 
     if not os.path.exists(os.path.join(stack_dir, check_file)):
-        raise Exception('Not a submodule of spack-stack')
+        raise Exception("Not a submodule of spack-stack")
 
     return os.path.join(stack_dir, *paths)
 
 
-common_path = stack_path('configs', 'common')
-site_path = stack_path('configs', 'sites')
-container_path = stack_path('configs', 'containers')
-container_specs_path = stack_path('configs', 'containers', 'specs')
-template_path = stack_path('configs', 'templates')
+common_path = stack_path("configs", "common")
+site_path = stack_path("configs", "sites")
+container_path = stack_path("configs", "containers")
+container_specs_path = stack_path("configs", "containers", "specs")
+template_path = stack_path("configs", "templates")

From afd8a10fc443654a9a1cc39d70787339fdc0ea06 Mon Sep 17 00:00:00 2001
From: Dom Heinzeller <dom.heinzeller@icloud.com>
Date: Mon, 2 Oct 2023 21:27:23 -0600
Subject: [PATCH 3/6] Remove unused 'import copy' from
 lib/jcsda-emc/spack-stack/stack/container_env.py

---
 lib/jcsda-emc/spack-stack/stack/container_env.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/jcsda-emc/spack-stack/stack/container_env.py b/lib/jcsda-emc/spack-stack/stack/container_env.py
index c3f26c9988e4ef..1e8685cd372058 100644
--- a/lib/jcsda-emc/spack-stack/stack/container_env.py
+++ b/lib/jcsda-emc/spack-stack/stack/container_env.py
@@ -1,4 +1,3 @@
-import copy
 import os
 
 import spack

From 734e565d70d2e77e7e409ee676ebe530f75a30c1 Mon Sep 17 00:00:00 2001
From: Dom Heinzeller <dom.heinzeller@icloud.com>
Date: Tue, 3 Oct 2023 09:14:21 -0600
Subject: [PATCH 4/6] Update unit tests for creating containers

---
 lib/jcsda-emc/spack-stack/tests/test_stack_create.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/jcsda-emc/spack-stack/tests/test_stack_create.py b/lib/jcsda-emc/spack-stack/tests/test_stack_create.py
index 71e5678f67c3dc..41d3155f9fb749 100644
--- a/lib/jcsda-emc/spack-stack/tests/test_stack_create.py
+++ b/lib/jcsda-emc/spack-stack/tests/test_stack_create.py
@@ -71,9 +71,11 @@ def test_sites(site):
 
 @pytest.mark.extension("stack")
 @pytest.mark.parametrize("container", all_containers())
-@pytest.mark.filterwarnings("ignore::UserWarning")
+@pytest.mark.parametrize("spec", all_specs())
+#@pytest.mark.filterwarnings("ignore::UserWarning")
 def test_containers(container):
-    if not container:
+    if not container or not spec:
         return
     container_wo_ext = os.path.splitext(container)[0]
-    stack_create("create", "ctr", container_wo_ext, "--dir", test_dir, "--overwrite")
+    spec_wo_ext = os.path.splitext(spec)[0]
+    stack_create("create", "ctr", "--container", container_wo_ext, "--spec", spec_wo_ext, "--dir", test_dir, "--overwrite")

From bea11118605a39c5067548e25c2a27fa79f99be3 Mon Sep 17 00:00:00 2001
From: Dom Heinzeller <dom.heinzeller@icloud.com>
Date: Tue, 3 Oct 2023 09:48:45 -0600
Subject: [PATCH 5/6] Bug fix for unit tests and formatting changes in
 lib/jcsda-emc/spack-stack/tests/test_stack_create.py

---
 .../spack-stack/tests/test_stack_create.py    | 27 ++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/lib/jcsda-emc/spack-stack/tests/test_stack_create.py b/lib/jcsda-emc/spack-stack/tests/test_stack_create.py
index 41d3155f9fb749..5e85df121dd98d 100644
--- a/lib/jcsda-emc/spack-stack/tests/test_stack_create.py
+++ b/lib/jcsda-emc/spack-stack/tests/test_stack_create.py
@@ -51,6 +51,17 @@ def all_containers():
         return None
 
 
+def all_specs():
+    specs_path = stack_path("configs", "containers", "specs")
+    if specs_path:
+        _, _, specs = next(os.walk(specs_path))
+        # Exclude files like "README.md"
+        specs = [x for x in specs if x.endswith(".yaml")]
+        return specs
+    else:
+        return None
+
+
 @pytest.mark.extension("stack")
 @pytest.mark.parametrize("template", all_templates())
 @pytest.mark.filterwarnings("ignore::UserWarning")
@@ -72,10 +83,20 @@ def test_sites(site):
 @pytest.mark.extension("stack")
 @pytest.mark.parametrize("container", all_containers())
 @pytest.mark.parametrize("spec", all_specs())
-#@pytest.mark.filterwarnings("ignore::UserWarning")
-def test_containers(container):
+# @pytest.mark.filterwarnings("ignore::UserWarning")
+def test_containers(container, spec):
     if not container or not spec:
         return
     container_wo_ext = os.path.splitext(container)[0]
     spec_wo_ext = os.path.splitext(spec)[0]
-    stack_create("create", "ctr", "--container", container_wo_ext, "--spec", spec_wo_ext, "--dir", test_dir, "--overwrite")
+    stack_create(
+        "create",
+        "ctr",
+        "--container",
+        container_wo_ext,
+        "--spec",
+        spec_wo_ext,
+        "--dir",
+        test_dir,
+        "--overwrite",
+    )

From 1f4e7c5e6e3fe9e6a9a4b61e7a979107f87a2233 Mon Sep 17 00:00:00 2001
From: Dom Heinzeller <dom.heinzeller@icloud.com>
Date: Tue, 3 Oct 2023 15:52:58 -0600
Subject: [PATCH 6/6] Use better logic to filter README.md in
 lib/jcsda-emc/spack-stack/stack/cmd/stack_cmds/create.py

---
 lib/jcsda-emc/spack-stack/stack/cmd/stack_cmds/create.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/jcsda-emc/spack-stack/stack/cmd/stack_cmds/create.py b/lib/jcsda-emc/spack-stack/stack/cmd/stack_cmds/create.py
index 5073bb5c160e6a..b9611393f02049 100644
--- a/lib/jcsda-emc/spack-stack/stack/cmd/stack_cmds/create.py
+++ b/lib/jcsda-emc/spack-stack/stack/cmd/stack_cmds/create.py
@@ -48,11 +48,11 @@ def template_help():
 
 def container_config_help():
     _, _, container_configs = next(os.walk(stack_path("configs", "containers")))
+    # Exclude files like "README.md"
+    container_configs = [x for x in container_configs if x.endswith(".yaml")]
     help_string = "Pre-configured container." + os.linesep
     help_string += "Available options are: " + os.linesep
     for config in container_configs:
-        if config == "README.md":
-            continue
         help_string += "\t" + config.rstrip(".yaml") + os.linesep
     return help_string