Standardize site configuration resolution of multiple files

- Dumping site platform_path_maps now sorts the keys alphabetically
- Correct issues with left wins setup for using multiple site json files
and make the tests for this easy to understand by using left/middle/right terminology.

README.md

@@ -122,16 +122,62 @@ configuration files. If the `--site` option is passed to the cli, it is used ins
 the environment variable.
 
 Each of the file paths specified are read and merged into a single site configuration
-dictionary hab uses. The values in each file are merged so the value defined in the
-right most path any given configuration option being used. See
-[Defining Environments](#defining-environments) for how to structure the json to
-prepend, append, set, unset values.
+dictionary hab uses. When using multiple site json files here are some general
+rules to keep in mind.
+
+1. The left most site configuration takes precedence for a given item.
+2. For prepend/append operations on lists, the left site file's paths will placed
+on the outside of the the right site file's paths.
+3. For `platform_path_maps`, only the first key is kept and any duplicates
+are discarded.
+
+See [Defining Environments](#defining-environments) for how to structure the json
+to prepend, append, set, unset values.
 
 Developers can use this to load local site configurations loading their wip code
-instead of the official releases. See the [TestResolvePaths::test_paths](tests/test_site.py)
+instead of the official releases. See [TestResolvePaths::test_paths](tests/test_site.py)
 to see an example of [overriding](tests/site_override.json) the
 [main](tests/site_main.json) site settings.
 
+You can inspect the site settings by using the `hab dump -t s` or
+`hab dump --type site` cli command. See
+[TestMultipleSites::test_left_right and TestMultipleSites::test_right_left](tests/test_site.py)
+for an example of how these rules are applied. Here is a dump of the final result
+of using all [3 site json files](tests/site).
+```bash
+$ cd tests/site
+$ hab --site site_left.json --site site_middle.json --site site_right.json dump --type site -v
+Dump of Site
+-------------------------------------------------------------------
+HAB_PATHS:  C:\blur\dev\hab_\tests\site\site_left.json
+            C:\blur\dev\hab_\tests\site\site_middle.json
+            C:\blur\dev\hab_\tests\site\site_right.json
+config_paths:
+distro_paths:
+ignored_distros:  release, pre
+platforms:  windows, mac, linux
+set_value:  left
+test_paths:  left_prepend
+             middle_prepend
+             right_prepend
+             right_append
+             middle_append
+             left_append
+platform_path_maps:  host:  linux:  host-linux_left
+                                     windows:  host-windows_left
+                     mid:  linux:  mid-linux_middle
+                           windows:  mid-windows_middle
+                     net:  linux:  net-linux_right
+                           windows:  net-windows_right
+                     shared:  linux:  shared-linux_left
+                              windows:  shared-windows_left
+-------------------------------------------------------------------
+```
+Note the order of left/middle/right in the test_paths variable. Also, for
+`platform_path_maps`, `host` is defined in all 3 site files, but only the first
+site file with it defined is used. The other path maps are picked up from the
+site file they are defined in.
+
 ### Python version
 
 Hab uses shell script files instead of an entry_point executable. This allows it

hab/site.py

@@ -87,7 +87,7 @@ def load(self):
         """Iterates over each file in self.path. Replacing the value of each key.
         The last file in the list will have its settings applied even if other
         files define them."""
-        for path in self.paths:
+        for path in reversed(self.paths):
             self.load_file(path)
 
         # Ensure any platform_path_maps are converted to pathlib objects.

hab/utils.py

@@ -122,7 +122,7 @@ def dump_object(obj, label="", width=80, flat_list=False, color=False):
     elif isinstance(obj, (dict, UserDict)):
         rows = []
         lbl = label
-        for k, v in obj.items():
+        for k, v in sorted(obj.items()):
             rows.append(
                 dump_object(
                     v,

tests/site/site_left.json

@@ -0,0 +1,25 @@
+{
+    "prepend": {
+        "test_paths": [
+            "left_prepend"
+        ]
+    },
+    "append": {
+        "test_paths": [
+            "left_append"
+        ],
+        "platform_path_maps": {
+            "host": {
+                "linux": "host-linux_left",
+                "windows": "host-windows_left"
+            },
+            "shared": {
+                "linux": "shared-linux_left",
+                "windows": "shared-windows_left"
+            }
+        }
+    },
+    "set": {
+        "set_value": "left"
+    }
+}

tests/site/site_middle.json

@@ -0,0 +1,25 @@
+{
+    "prepend": {
+        "test_paths": [
+            "middle_prepend"
+        ]
+    },
+    "append": {
+        "test_paths": [
+            "middle_append"
+        ],
+        "platform_path_maps": {
+            "host": {
+                "linux": "host-linux_middle",
+                "windows": "host-windows_middle"
+            },
+            "mid": {
+                "linux": "mid-linux_middle",
+                "windows": "mid-windows_middle"
+            }
+        }
+    },
+    "set": {
+        "set_value": "middle"
+    }
+}

tests/site/site_right.json

@@ -0,0 +1,25 @@
+{
+    "prepend": {
+        "test_paths": [
+            "right_prepend"
+        ]
+    },
+    "append": {
+        "test_paths": [
+            "right_append"
+        ],
+        "platform_path_maps": {
+            "host": {
+                "linux": "host-linux_right",
+                "windows": "host-windows_right"
+            },
+            "net": {
+                "linux": "net-linux_right",
+                "windows": "net-windows_right"
+            }
+        }
+    },
+    "set": {
+        "set_value": "right"
+    }
+}

tests/test_parsing.py

@@ -218,15 +218,15 @@ def standardize(txt):
     pre = ["name:  child", "uri:  not_set/child"]
     post = ["inherits:  True"]
     env = [
-        "environment:  UNSET_VARIABLE:  None",
+        "environment:  FMT_FOR_OS:  a{;}b;c:{PATH!e}{;}d",
         "              TEST:  case",
-        "              FMT_FOR_OS:  a{;}b;c:{PATH!e}{;}d",
+        "              UNSET_VARIABLE:  None",
     ]
 
     env_config = [
-        "environment_config:  unset:  UNSET_VARIABLE",
-        "                     set:  TEST:  case",
-        "                           FMT_FOR_OS:  a{;}b;c:{PATH!e}{;}d",
+        "environment_config:  set:  FMT_FOR_OS:  a{;}b;c:{PATH!e}{;}d",
+        "                           TEST:  case",
+        "                     unset:  UNSET_VARIABLE",
     ]
     cfg = resolver.closest_config("not_set/child")
     header = f"Dump of {type(cfg).__name__}('{cfg.fullpath}')"

tests/test_site.py

@@ -19,6 +19,167 @@ def test_environment_variables(config_root, monkeypatch):
     assert site.paths == paths
 
 
+class TestMultipleSites:
+    """Check that various combinations of site json files results in the correct
+    merged site. The rules are:
+
+    1. The left most site configuration takes precedence for a given item.
+    2. For prepend/append operations on lists, the left site file's paths will
+    placed on the outside of the the right site file's paths.
+    3. For `platform_path_maps`, only the first key is kept and any duplicates
+    are discarded.
+    """
+
+    def host_left(self):
+        return {
+            'linux': PurePosixPath('host-linux_left'),
+            'windows': PureWindowsPath('host-windows_left'),
+        }
+
+    def host_middle(self):
+        return {
+            'linux': PurePosixPath('host-linux_middle'),
+            'windows': PureWindowsPath('host-windows_middle'),
+        }
+
+    def host_right(self):
+        return {
+            'linux': PurePosixPath('host-linux_right'),
+            'windows': PureWindowsPath('host-windows_right'),
+        }
+
+    def mid(self):
+        return {
+            'linux': PurePosixPath('mid-linux_middle'),
+            'windows': PureWindowsPath('mid-windows_middle'),
+        }
+
+    def net(self):
+        return {
+            'linux': PurePosixPath('net-linux_right'),
+            'windows': PureWindowsPath('net-windows_right'),
+        }
+
+    def shared_left(self):
+        return {
+            'linux': PurePosixPath('shared-linux_left'),
+            'windows': PureWindowsPath('shared-windows_left'),
+        }
+
+    def test_left(self, config_root):
+        """Check that site_left.json returns the correct results."""
+        paths = [config_root / "site" / "site_left.json"]
+        site = Site(paths)
+
+        assert site.get("set_value") == ["left"]
+        assert site.get("test_paths") == ["left_prepend", "left_append"]
+
+        check = {'host': self.host_left(), 'shared': self.shared_left()}
+
+        assert site.get("platform_path_maps") == check
+
+    def test_middle(self, config_root):
+        """Check that site_middle.json returns the correct results."""
+        paths = [config_root / "site" / "site_middle.json"]
+        site = Site(paths)
+
+        assert site.get("set_value") == ["middle"]
+        assert site.get("test_paths") == ["middle_prepend", "middle_append"]
+
+        check = {'host': self.host_middle(), 'mid': self.mid()}
+
+        assert site.get("platform_path_maps") == check
+
+    def test_right(self, config_root):
+        """Check that site_right.json returns the correct results."""
+        paths = [config_root / "site" / "site_right.json"]
+        site = Site(paths)
+
+        assert site.get("set_value") == ["right"]
+        assert site.get("test_paths") == ["right_prepend", "right_append"]
+
+        check = {'host': self.host_right(), 'net': self.net()}
+
+        assert site.get("platform_path_maps") == check
+
+    def test_left_right(self, config_root):
+        """Check that site_left.json and site_right.json are merged correctly."""
+        paths = [
+            config_root / "site" / "site_left.json",
+            config_root / "site" / "site_right.json",
+        ]
+        site = Site(paths)
+
+        assert site.get("set_value") == ["left"]
+        assert site.get("test_paths") == [
+            "left_prepend",
+            "right_prepend",
+            "right_append",
+            "left_append",
+        ]
+
+        check = {
+            'host': self.host_left(),
+            'net': self.net(),
+            'shared': self.shared_left(),
+        }
+
+        assert site.get("platform_path_maps") == check
+
+    def test_right_left(self, config_root):
+        """Check that reversing site_left.json and site_right.json get merged
+        correctly."""
+        paths = [
+            config_root / "site" / "site_right.json",
+            config_root / "site" / "site_left.json",
+        ]
+        site = Site(paths)
+
+        assert site.get("set_value") == ["right"]
+        assert site.get("test_paths") == [
+            "right_prepend",
+            "left_prepend",
+            "left_append",
+            "right_append",
+        ]
+
+        check = {
+            'host': self.host_right(),
+            'net': self.net(),
+            'shared': self.shared_left(),
+        }
+
+        assert site.get("platform_path_maps") == check
+
+    def test_left_middle_right(self, config_root):
+        """Check that more than 2 site json files are merged correctly."""
+        paths = [
+            config_root / "site" / "site_left.json",
+            config_root / "site" / "site_middle.json",
+            config_root / "site" / "site_right.json",
+        ]
+        site = Site(paths)
+
+        assert site.get("set_value") == ["left"]
+        assert site.get("test_paths") == [
+            "left_prepend",
+            "middle_prepend",
+            "right_prepend",
+            "right_append",
+            "middle_append",
+            "left_append",
+        ]
+
+        check = {
+            'host': self.host_left(),
+            'mid': self.mid(),
+            'net': self.net(),
+            'shared': self.shared_left(),
+        }
+
+        assert site.get("platform_path_maps") == check
+
+
 class TestResolvePaths:
     def test_path(self, config_root):
         # Check that the correct values are resolved when processing a single result
@@ -30,7 +191,7 @@ def test_path(self, config_root):
 
     def test_paths(self, config_root, helpers):
         # Check that values specified by additional files overwrite the previous values
-        paths = [config_root / "site_main.json", config_root / "site_override.json"]
+        paths = [config_root / "site_override.json", config_root / "site_main.json"]
         site = Site(paths)
         assert site.get("generic_value") is True
         assert site.get("override") == ["site_override.json"]
@@ -53,7 +214,7 @@ def test_paths(self, config_root, helpers):
 
     def test_paths_reversed(self, config_root, helpers):
         # Check that values specified by additional files overwrite the previous values
-        paths = [config_root / "site_override.json", config_root / "site_main.json"]
+        paths = [config_root / "site_main.json", config_root / "site_override.json"]
         site = Site(paths)
         assert site.get("generic_value") is False
         assert site.get("override") == ["site_override.json"]
@@ -276,7 +437,7 @@ def test_merged(self, config_root):
         )
 
         # The right-most site file's settings are loaded when both have the same keys
-        self.assert_override(site)
+        self.assert_main(site)
 
     def test_reversed(self, config_root):
         """Test that the correct platform_path_map settings are resolved when using
@@ -296,4 +457,4 @@ def test_reversed(self, config_root):
         )
 
         # The right-most site file's settings are loaded when both have the same keys
-        self.assert_main(site)
+        self.assert_override(site)