From 40c4c81c192c03f3a3a6d60396c3a50c0ea44234 Mon Sep 17 00:00:00 2001
From: "John W. Parent" <45471568+johnwparent@users.noreply.github.com>
Date: Wed, 6 Sep 2023 14:44:00 -0400
Subject: "spack config add": support values with ":" (#39831)

This is a fixed version of b72a268

* That commit would discard the final key component (so if you set
  "config:install_tree:root", it would discard "root" and just set
  install tree).
* When setting key:"value", with the quotes, that commit would
  discard the quotes, which would confuse the system if adding a
  value like "{example}" (the "{" character indicates a dictionary).
  This commit retains the quotes.
---
 lib/spack/spack/config.py      | 33 +++++++++++++++++++++++++--------
 lib/spack/spack/test/config.py | 19 +++++++++++++++++++
 2 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index b3fb5648ac..86e8981a18 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -857,12 +857,12 @@ def add_from_file(filename, scope=None):
 def add(fullpath, scope=None):
     """Add the given configuration to the specified config scope.
     Add accepts a path. If you want to add from a filename, use add_from_file"""
-
     components = process_config_path(fullpath)
 
     has_existing_value = True
     path = ""
     override = False
+    value = syaml.load_config(components[-1])
     for idx, name in enumerate(components[:-1]):
         # First handle double colons in constructing path
         colon = "::" if override else ":" if path else ""
@@ -883,14 +883,14 @@ def add(fullpath, scope=None):
             existing = get_valid_type(path)
 
             # construct value from this point down
-            value = syaml.load_config(components[-1])
             for component in reversed(components[idx + 1 : -1]):
                 value = {component: value}
             break
 
+    if override:
+        path += "::"
+
     if has_existing_value:
-        path, _, value = fullpath.rpartition(":")
-        value = syaml.load_config(value)
         existing = get(path, scope=scope)
 
     # append values to lists
@@ -1231,11 +1231,17 @@ def merge_yaml(dest, source, prepend=False, append=False):
     return copy.copy(source)
 
 
-#
-# Process a path argument to config.set() that may contain overrides ('::' or
-# trailing ':')
-#
 def process_config_path(path):
+    """Process a path argument to config.set() that may contain overrides ('::' or
+    trailing ':')
+
+    Note: quoted value path components will be processed as a single value (escaping colons)
+        quoted path components outside of the value will be considered ill formed and will
+        raise.
+        e.g. `this:is:a:path:'value:with:colon'` will yield:
+
+            [this, is, a, path, value:with:colon]
+    """
     result = []
     if path.startswith(":"):
         raise syaml.SpackYAMLError("Illegal leading `:' in path `{0}'".format(path), "")
@@ -1263,6 +1269,17 @@ def process_config_path(path):
             front.append = True
 
         result.append(front)
+
+        quote = "['\"]"
+        not_quote = "[^'\"]"
+
+        if re.match(f"^{quote}", path):
+            m = re.match(rf"^({quote}{not_quote}+{quote})$", path)
+            if not m:
+                raise ValueError("Quotes indicate value, but there are additional path entries")
+            result.append(m.group(1))
+            break
+
     return result
 
 
diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py
index d3b5a544ba..f7bf7d7569 100644
--- a/lib/spack/spack/test/config.py
+++ b/lib/spack/spack/test/config.py
@@ -277,6 +277,25 @@ def test_add_config_path(mutable_config):
     compilers = spack.config.get("packages")["all"]["compiler"]
     assert "gcc" in compilers
 
+    # Try quotes to escape brackets
+    path = "config:install_tree:projections:cmake:\
+'{architecture}/{compiler.name}-{compiler.version}/{name}-{version}-{hash}'"
+    spack.config.add(path)
+    set_value = spack.config.get("config")["install_tree"]["projections"]["cmake"]
+    assert set_value == "{architecture}/{compiler.name}-{compiler.version}/{name}-{version}-{hash}"
+
+    # NOTE:
+    # The config path: "config:install_tree:root:<path>" is unique in that it can accept multiple
+    # schemas (such as a dropped "root" component) which is atypical and may lead to passing tests
+    # when the behavior is in reality incorrect.
+    # the config path below is such that no subkey accepts a string as a valid entry in our schema
+
+    # try quotes to escape colons
+    path = "config:build_stage:'C:\\path\\to\\config.yaml'"
+    spack.config.add(path)
+    set_value = spack.config.get("config")["build_stage"]
+    assert "C:\\path\\to\\config.yaml" in set_value
+
 
 @pytest.mark.regression("17543,23259")
 def test_add_config_path_with_enumerated_type(mutable_config):
-- 
cgit v1.2.3-70-g09d2