From 395b478b1b4184eeb5c923c74b8325c00d382354 Mon Sep 17 00:00:00 2001
From: Massimiliano Culpo <massimiliano.culpo@gmail.com>
Date: Sat, 15 Aug 2020 04:31:55 +0200
Subject: spack config update (bugfix): packages.yaml with empty attributes
 (#18057)

Before this PR, packages.yaml files that contained an
empty "paths" or "modules" attribute were not updated
correctly, since the update function was not reporting
them as changed after the update.

This PR fixes that issue and adds a unit test to
avoid regression.
---
 lib/spack/spack/schema/packages.py | 10 +++++++++-
 lib/spack/spack/test/cmd/config.py | 21 +++++++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/lib/spack/spack/schema/packages.py b/lib/spack/spack/schema/packages.py
index 16a8a223ef..2efe00991a 100644
--- a/lib/spack/spack/schema/packages.py
+++ b/lib/spack/spack/schema/packages.py
@@ -128,6 +128,14 @@ def update(data):
     changed = False
     for cfg_object in data.values():
         externals = []
+
+        # If we don't have these deprecated attributes, continue
+        if not any(x in cfg_object for x in ('paths', 'modules')):
+            continue
+
+        # If we arrive here we need to make some changes i.e.
+        # we need to remove and eventually convert some attributes
+        changed = True
         paths = cfg_object.pop('paths', {})
         for spec, prefix in paths.items():
             externals.append({
@@ -141,6 +149,6 @@ def update(data):
                 'modules': [str(module)]
             })
         if externals:
-            changed = True
             cfg_object['externals'] = externals
+
     return changed
diff --git a/lib/spack/spack/test/cmd/config.py b/lib/spack/spack/test/cmd/config.py
index 97b76dbee1..112d88f342 100644
--- a/lib/spack/spack/test/cmd/config.py
+++ b/lib/spack/spack/test/cmd/config.py
@@ -551,6 +551,27 @@ packages:
     assert '# Another comment after the outdated section' in text
 
 
+@pytest.mark.regression('18050')
+def test_config_update_works_for_empty_paths(mutable_config):
+    # Create an outdated config file with empty "paths" and "modules"
+    scope = spack.config.default_modify_scope()
+    cfg_file = spack.config.config.get_config_filename(scope, 'packages')
+    with open(cfg_file, mode='w') as f:
+        f.write("""
+packages:
+  cmake:
+    paths: {}
+    modules: {}
+    buildable: False
+""")
+
+    # Try to update it, it should not raise errors
+    output = config('update', '-y', 'packages')
+
+    # This ensures that we updated the configuration
+    assert '[backup=' in output
+
+
 def check_update(data):
     """Check that the data from the packages_yaml_v015
     has been updated.
-- 
cgit v1.2.3-70-g09d2