sandbox/_config.py, element.py: Refactor SandboxConfig

This commit changes SandboxConfig such that it now has a simple constructor
and a new SandboxConfig.new_from_node() classmethod to load it from a YAML
configuration node. The new version of SandboxConfig now uses type annotations.

SandboxConfig also now sports a to_dict() method to help in serialization in
artifacts, this replaces SandboxConfig.get_unique_key() since it does exactly
the same thing, but uses the same names as expected in the YAML configuration
to achieve it.

The element.py code has been updated to use the classmethod, and to
use the to_dict() method when constructing cache keys.

This refactor is meant to allow instantiating a SandboxConfig without
any MappingNode, such that we can later load a SandboxConfig from an
Artifact instead of from an parsed Element.

This commit also updates the cache keys in the cache key test, as
the cache key format is slightly changed by the to_dict() method.

31 files changed, 132 insertions, 61 deletions

diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index 6c4e45d03..595017052 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -347,7 +347,7 @@ class Element(Plugin):
         # Extract Sandbox config
         sandbox_config = self.__extract_sandbox_config(project, load_element)
         self.__variables.expand(sandbox_config)
-        self.__sandbox_config = SandboxConfig(sandbox_config, context.platform)
+        self.__sandbox_config = SandboxConfig.new_from_node(sandbox_config, platform=context.platform)
 
     def __lt__(self, other):
         return self.name < other.name
@@ -2249,7 +2249,7 @@ class Element(Plugin):
                 "element-plugin-key": self.get_unique_key(),
                 "element-plugin-name": self.get_kind(),
                 "element-plugin-version": self.BST_ARTIFACT_VERSION,
-                "sandbox": self.__sandbox_config.get_unique_key(),
+                "sandbox": self.__sandbox_config.to_dict(),
                 "environment": cache_env,
                 "public": self.__public.strip_node_info(),
             }
diff --git a/src/buildstream/sandbox/_config.py b/src/buildstream/sandbox/_config.py
index 114274190..6f8323453 100644
--- a/src/buildstream/sandbox/_config.py
+++ b/src/buildstream/sandbox/_config.py
@@ -1,5 +1,5 @@
 #
-#  Copyright (C) 2018 Codethink Limited
+#  Copyright (C) 2020 Codethink Limited
 #
 #  This program is free software; you can redistribute it and/or
 #  modify it under the terms of the GNU Lesser General Public
@@ -16,52 +16,123 @@
 #
 #  Authors:
 #        Jim MacArthur <jim.macarthur@codethink.co.uk>
+#        Tristan Van Berkom <tristan.vanberkom@codethink.co.uk>
+#
 
+from typing import TYPE_CHECKING, Dict, Optional, Union
 from .._platform import Platform
 
+if TYPE_CHECKING:
+    from ..node import Node, MappingNode
+
 
 # SandboxConfig
 #
-# A container for sandbox configuration data. We want the internals
-# of this to be opaque, hence putting it in its own private file.
+# The Sandbox configuration parameters, this object carries configuration
+# required to instantiate the correct type of sandbox, and assert that
+# the local or remote worker sandbox has the capabilities required.
+#
+# Args:
+#    build_os: The build OS name
+#    build_arch: A canonical machine architecture name, as defined by Platform.canonicalize_arch()
+#    build_uid: The UID for the sandbox process
+#    build_gid: The GID for the sandbox process
+#
+# If the build_uid or build_gid is unspecified, then the underlying sandbox implementation
+# does not guarantee what UID/GID will be used, but generally UID/GID 0 will be used in a
+# sandbox implementation which supports UID/GID control.
+#
+# If the build_uid or build_gid is specified, then the UID/GID is guaranteed to match
+# the specified UID/GID, if the underlying sandbox implementation does not support UID/GID
+# control, then an error will be raised when attempting to configure the sandbox.
+#
 class SandboxConfig:
-    def __init__(self, sandbox_config, platform):
-        host_arch = platform.get_host_arch()
-        host_os = platform.get_host_os()
+    def __init__(
+        self, *, build_os: str, build_arch: str, build_uid: Optional[int] = None, build_gid: Optional[int] = None
+    ):
+        self.build_os = build_os
+        self.build_arch = build_arch
+        self.build_uid = build_uid
+        self.build_gid = build_gid
 
-        sandbox_config.validate_keys(["build-uid", "build-gid", "build-os", "build-arch"])
+    # to_dict():
+    #
+    # Represent the SandboxConfig as a dictionary.
+    #
+    # This dictionary will be stored in the corresponding artifact
+    # whenever an artifact is cached. When loading an element from
+    # an artifact, then this dict will be loaded as a MappingNode
+    # and interpreted by SandboxConfig.new_from_node().
+    #
+    # This function is also used to contribute to the owning element's cache key.
+    #
+    # Returns:
+    #    A dictionary representation of this SandboxConfig
+    #
+    def to_dict(self) -> Dict[str, Union[str, int]]:
 
-        build_os = sandbox_config.get_str("build-os", default=None)
-        if build_os:
-            self.build_os = build_os.lower()
-        else:
-            self.build_os = host_os
+        # Assign mandatory portions of the sandbox configuration
+        #
+        # /!\ No additional mandatory members can ever be added to
+        #     the sandbox configuration, as that would result in
+        #     breaking cache key stability.
+        #
+        sandbox_dict: Dict[str, Union[str, int]] = {"build-os": self.build_os, "build-arch": self.build_arch}
 
-        build_arch = sandbox_config.get_str("build-arch", default=None)
-        if build_arch:
-            self.build_arch = Platform.canonicalize_arch(build_arch)
-        else:
-            self.build_arch = host_arch
+        # Assign optional portions of the sandbox configuration
+        #
+        # /!\ In order to preserve cache key stability, these attributes
+        #     are only ever added to the dictionary if they have been
+        #     explicitly set, unset values must not affect the dictionary.
+        #
+        if self.build_uid is not None:
+            sandbox_dict["build-uid"] = self.build_uid
+        if self.build_gid is not None:
+            sandbox_dict["build-gid"] = self.build_gid
 
-        self.build_uid = sandbox_config.get_int("build-uid", None)
-        self.build_gid = sandbox_config.get_int("build-gid", None)
+        return sandbox_dict
 
-    # get_unique_key():
+    # new_from_node():
+    #
+    # Instantiate a new SandboxConfig from YAML configuration.
     #
-    # This returns the SandboxConfig's contribution
-    # to an element's cache key.
+    # If the Platform is specified, then we expect to be loading
+    # from project definitions, and some defaults will be derived
+    # from the Platform. Otherwise, we expect to be loading from
+    # a cached artifact, and values are expected to exist on the
+    # given node.
+    #
+    # Args:
+    #    config: The YAML configuration node
+    #    platform: The host Platform instance, or None
     #
     # Returns:
-    #    (dict): A dictionary to add to an element's cache key
+    #    A new SandboxConfig instance
     #
-    def get_unique_key(self):
+    @classmethod
+    def new_from_node(cls, config: "MappingNode[Node]", *, platform: Optional[Platform] = None) -> "SandboxConfig":
+        config.validate_keys(["build-uid", "build-gid", "build-os", "build-arch"])
 
-        unique_key = {"os": self.build_os, "arch": self.build_arch}
+        build_os: str
+        build_arch: str
 
-        if self.build_uid is not None:
-            unique_key["build-uid"] = self.build_uid
+        if platform:
+            tmp = config.get_str("build-os", None)
+            if tmp:
+                build_os = tmp.lower()
+            else:
+                build_os = platform.get_host_os()
 
-        if self.build_gid is not None:
-            unique_key["build-gid"] = self.build_gid
+            tmp = config.get_str("build-arch", None)
+            if tmp:
+                build_arch = Platform.canonicalize_arch(tmp)
+            else:
+                build_arch = platform.get_host_arch()
+        else:
+            build_os = config.get_str("build-os")
+            build_arch = config.get_str("build-arch")
+
+        build_uid = config.get_int("build-uid", None)
+        build_gid = config.get_int("build-gid", None)
 
-        return unique_key
+        return cls(build_os=build_os, build_arch=build_arch, build_uid=build_uid, build_gid=build_gid)
diff --git a/tests/cachekey/project/elements/build1.expected b/tests/cachekey/project/elements/build1.expected
index 369036a16..8939fe401 100644
--- a/tests/cachekey/project/elements/build1.expected
+++ b/tests/cachekey/project/elements/build1.expected
@@ -1 +1 @@
-e47d9b32ba894f1d454b66d8d30d3d20226b54c327e8b6a3893c9c55ff02378c
\ No newline at end of file
+2529ccdc2fb103a69562bd022616ddb19cf6e9de84d8cfbc7840153f4ddc0731
\ No newline at end of file
diff --git a/tests/cachekey/project/elements/build2.expected b/tests/cachekey/project/elements/build2.expected
index 6e9dfd075..db52547e3 100644
--- a/tests/cachekey/project/elements/build2.expected
+++ b/tests/cachekey/project/elements/build2.expected
@@ -1 +1 @@
-ffe535e1ac311448584d9f0b17f51bb85c8cc9bd3c081a411f9c3ea464f5fcd7
\ No newline at end of file
+1b8ebfc60feb2cdb72997c67114b60e54fb16f9ac32ee1d244ecd3adede4f435
\ No newline at end of file
diff --git a/tests/cachekey/project/elements/build3.expected b/tests/cachekey/project/elements/build3.expected
index 7b3aabe9f..868549ae6 100644
--- a/tests/cachekey/project/elements/build3.expected
+++ b/tests/cachekey/project/elements/build3.expected
@@ -1 +1 @@
-c8bae677ec6af58543e4b779a295c9f6bee0c1eface38d9be62a66c247a68ba5
\ No newline at end of file
+b2ec847601220bead3bff667a9b059156f4cb4c0a057afbc0598b9d93f8ded55
\ No newline at end of file
diff --git a/tests/cachekey/project/elements/compose1.expected b/tests/cachekey/project/elements/compose1.expected
index fd0ca75fd..cc7f7d427 100644
--- a/tests/cachekey/project/elements/compose1.expected
+++ b/tests/cachekey/project/elements/compose1.expected
@@ -1 +1 @@
-3a033cd6b00a0b7e113f2f78f67e73923bdadad33e7d8825fabb0e75f0386417
\ No newline at end of file
+7c814c5c137e1792da1a3d039ead391a755aa158b18298efe67fa71f70ace2ca
\ No newline at end of file
diff --git a/tests/cachekey/project/elements/compose2.expected b/tests/cachekey/project/elements/compose2.expected
index 5ab72fe76..c01eb0f8d 100644
--- a/tests/cachekey/project/elements/compose2.expected
+++ b/tests/cachekey/project/elements/compose2.expected
@@ -1 +1 @@
-ec2cf079f662a497af5687d2458489fe3ad83810a2db1815ea489fb30f416fc9
\ No newline at end of file
+1da1fc95645027f8bd30e26b73639f1cc0b196a4ed96e745b96ae923b84b7929
\ No newline at end of file
diff --git a/tests/cachekey/project/elements/compose3.expected b/tests/cachekey/project/elements/compose3.expected
index 5a6620a09..7dd15f6a2 100644
--- a/tests/cachekey/project/elements/compose3.expected
+++ b/tests/cachekey/project/elements/compose3.expected
@@ -1 +1 @@
-d24fd1c4beca8b40aad24e8bd4a6106e1278e193b10872e1ed33b0829eb4831a
\ No newline at end of file
+5529233330d798bc39b898b90d20b2763d6512b76ec2215081b3f1083fd5d2b5
\ No newline at end of file
diff --git a/tests/cachekey/project/elements/compose4.expected b/tests/cachekey/project/elements/compose4.expected
index 2c4ab4c48..e70405388 100644
--- a/tests/cachekey/project/elements/compose4.expected
+++ b/tests/cachekey/project/elements/compose4.expected
@@ -1 +1 @@
-37dd09028bd0a95e64b83df32a1446932f81a72c4c9a6a2cac5c0165ac61e29b
\ No newline at end of file
+b4785d3aea6c4218420c582487a0fcb1fe988b898b96c01693fe5645c24f054a
\ No newline at end of file
diff --git a/tests/cachekey/project/elements/compose5.expected b/tests/cachekey/project/elements/compose5.expected
index 425c55e96..ebcbac9af 100644
--- a/tests/cachekey/project/elements/compose5.expected
+++ b/tests/cachekey/project/elements/compose5.expected
@@ -1 +1 @@
-4f2eea4571545a5ce896b13320aa7e6c55ca874237fcb7ebeea017f8456e1b5c
\ No newline at end of file
+8d0cb89fc95dd7114a64a35ff47228f16b039484da688854f21a8651e0fefa4b
\ No newline at end of file
diff --git a/tests/cachekey/project/elements/import1.expected b/tests/cachekey/project/elements/import1.expected
index d9e854ed6..c7fac5326 100644
--- a/tests/cachekey/project/elements/import1.expected
+++ b/tests/cachekey/project/elements/import1.expected
@@ -1 +1 @@
-f27187b2e1b43a5d4d1855fc46fabe256d561d49d6aff2186a52d4d75315fe19
\ No newline at end of file
+a5914d072494e9a592a44ceb97a553c0473c15b239d1a827aca796a1a0fe51f1
\ No newline at end of file
diff --git a/tests/cachekey/project/elements/import2.expected b/tests/cachekey/project/elements/import2.expected
index 263684b4e..715006230 100644
--- a/tests/cachekey/project/elements/import2.expected
+++ b/tests/cachekey/project/elements/import2.expected
@@ -1 +1 @@
-815467a6401a32162cfbcff05f11ff366f9f2d11338bc89d9cf41715da7db89c
\ No newline at end of file
+b2a6e4d687832bacd7b00765813d8392153902e34df3d91e3926189c81fea42e
\ No newline at end of file
diff --git a/tests/cachekey/project/elements/import3.expected b/tests/cachekey/project/elements/import3.expected
index 24d6a031e..5d925d248 100644
--- a/tests/cachekey/project/elements/import3.expected
+++ b/tests/cachekey/project/elements/import3.expected
@@ -1 +1 @@
-3fef8b7e07efe9b9599e32ce72b1f0f8ab31accd61788c94c57951770ab184c2
\ No newline at end of file
+365b4af1b444bd113a3641fa7b22268040fb448c42ef351ecf8d9a09fa702262
\ No newline at end of file
diff --git a/tests/cachekey/project/elements/script1.expected b/tests/cachekey/project/elements/script1.expected
index 5d9799f1a..1f6579ffd 100644
--- a/tests/cachekey/project/elements/script1.expected
+++ b/tests/cachekey/project/elements/script1.expected
@@ -1 +1 @@
-1220b2849910f60f864c2c325cda301809dcb9b730281862448508fbeae5fb91
\ No newline at end of file
+5346f804ba9ac6fc642971c5673a2f5520c52706cfa1e698de7a94ffe361df84
\ No newline at end of file
diff --git a/tests/cachekey/project/sources/bzr1.expected b/tests/cachekey/project/sources/bzr1.expected
index 4e66d6c12..6d194a24a 100644
--- a/tests/cachekey/project/sources/bzr1.expected
+++ b/tests/cachekey/project/sources/bzr1.expected
@@ -1 +1 @@
-ce45db2c82a50eec39416dc857c7a676121a5276b396ea6c7917b123a932ea34
\ No newline at end of file
+4ee74db778d7d1ad83ce680320961ffa08655c87e8316aee369d8f943a56286d
\ No newline at end of file
diff --git a/tests/cachekey/project/sources/git1.expected b/tests/cachekey/project/sources/git1.expected
index 7c6974590..8d9e051e6 100644
--- a/tests/cachekey/project/sources/git1.expected
+++ b/tests/cachekey/project/sources/git1.expected
@@ -1 +1 @@
-f02ac8cc516819ee72f56bdc3c1a6e4bb3a154a506a361a8d34a63b37dfa8dc7
\ No newline at end of file
+8f3da148eb9fcd637537094ea309fc187767d375e87c5e585da49e40efbeb6e8
\ No newline at end of file
diff --git a/tests/cachekey/project/sources/git2.expected b/tests/cachekey/project/sources/git2.expected
index 4fa7db829..abed12ce6 100644
--- a/tests/cachekey/project/sources/git2.expected
+++ b/tests/cachekey/project/sources/git2.expected
@@ -1 +1 @@
-59964ee437e38e37dbbb78362118937e90defd89bdf358667394a35f6ed2ba46
\ No newline at end of file
+5a1a828102a76fc1ec84b9d012b578320e17267e6b0104f2b3f79490d5081d04
\ No newline at end of file
diff --git a/tests/cachekey/project/sources/git3.expected b/tests/cachekey/project/sources/git3.expected
index 91cb0f882..27dc857c5 100644
--- a/tests/cachekey/project/sources/git3.expected
+++ b/tests/cachekey/project/sources/git3.expected
@@ -1 +1 @@
-10dd7b3883cfe8fd8f7f29dd258098080227ee73c53ff825542d62d204f1e6c3
\ No newline at end of file
+9ace68b14a015497e88d0cee916145e0109dc406188ab4d2f7f69942e4081d82
\ No newline at end of file
diff --git a/tests/cachekey/project/sources/local1.expected b/tests/cachekey/project/sources/local1.expected
index 961785aa5..387231668 100644
--- a/tests/cachekey/project/sources/local1.expected
+++ b/tests/cachekey/project/sources/local1.expected
@@ -1 +1 @@
-13acaa66115bd1d374b236b0ddd2cef804df75b3aa28878f4200efa7c758b14c
\ No newline at end of file
+4881a9a467073e35f94b01fef5b3a30292d1a9501a9872f94873c2ce7a411fb4
\ No newline at end of file
diff --git a/tests/cachekey/project/sources/local2.expected b/tests/cachekey/project/sources/local2.expected
index 762e7bc48..5f29fcc76 100644
--- a/tests/cachekey/project/sources/local2.expected
+++ b/tests/cachekey/project/sources/local2.expected
@@ -1 +1 @@
-214238ba50ead82896e27d95e833c3df990f9e3ab728fdb0e44a3b07f986ab3c
\ No newline at end of file
+2f4fd0b15ca13e30acd846eb842e7c4fccc77111a29c19133edc91a5e2c22731
\ No newline at end of file
diff --git a/tests/cachekey/project/sources/patch1.expected b/tests/cachekey/project/sources/patch1.expected
index 4593895c5..d542f5027 100644
--- a/tests/cachekey/project/sources/patch1.expected
+++ b/tests/cachekey/project/sources/patch1.expected
@@ -1 +1 @@
-3b607c5657b600ab0e2c8b5ea09ed26fc65b3899b67d107676cf687b2cde374d
\ No newline at end of file
+d8ff899dff9b951bea1fb57cecd9e9e0beff8d7ab84c9855394d0ba911e1bd78
\ No newline at end of file
diff --git a/tests/cachekey/project/sources/patch2.expected b/tests/cachekey/project/sources/patch2.expected
index 221c9c779..00a5565de 100644
--- a/tests/cachekey/project/sources/patch2.expected
+++ b/tests/cachekey/project/sources/patch2.expected
@@ -1 +1 @@
-1911adbe62cbd7d9f0ded708530274c7f591798bd0a0b5f7e8358f7fcda3066a
\ No newline at end of file
+7717f45302278888f90c7ffd51b5507bb170856bb1100c53fe17b5ed36716fc8
\ No newline at end of file
diff --git a/tests/cachekey/project/sources/patch3.expected b/tests/cachekey/project/sources/patch3.expected
index 1ad6d9fce..07caeeec4 100644
--- a/tests/cachekey/project/sources/patch3.expected
+++ b/tests/cachekey/project/sources/patch3.expected
@@ -1 +1 @@
-175ff52608a55a6af98466822e22f401c46b2659116ccfeee6dc78e09249da78
\ No newline at end of file
+aea86d3b816e30cb388e5632ca6ae4b308084a35b9311197bc879c9c8f4bc01b
\ No newline at end of file
diff --git a/tests/cachekey/project/sources/pip1.expected b/tests/cachekey/project/sources/pip1.expected
index 59f11863b..7b06ef700 100644
--- a/tests/cachekey/project/sources/pip1.expected
+++ b/tests/cachekey/project/sources/pip1.expected
@@ -1 +1 @@
-20ac777cc56dfeafcf4543e0d9ed31108b32075c8a3e9d25d3deec16fbf7f246
\ No newline at end of file
+d78eaa9658a21b207b972a4876f52b4119ec5418b1a2894156fab7961bb06630
\ No newline at end of file
diff --git a/tests/cachekey/project/sources/remote1.expected b/tests/cachekey/project/sources/remote1.expected
index 9970589f3..ff778e162 100644
--- a/tests/cachekey/project/sources/remote1.expected
+++ b/tests/cachekey/project/sources/remote1.expected
@@ -1 +1 @@
-4d13955e7eeb0f0d77b0a3a72e77acd8636bdc8284712975593742d0667f88f7
\ No newline at end of file
+ea79defd2b026dfd76579b44c45795f02e71fa21ad86fa37b6f718d8570b5c17
\ No newline at end of file
diff --git a/tests/cachekey/project/sources/remote2.expected b/tests/cachekey/project/sources/remote2.expected
index 139e4bf74..3b4dcf1e2 100644
--- a/tests/cachekey/project/sources/remote2.expected
+++ b/tests/cachekey/project/sources/remote2.expected
@@ -1 +1 @@
-5f904fd43ca49f268af3141afe73116f1260800918a738afeaf51a94f99fffb2
\ No newline at end of file
+57c59523777f5da561809a5707c76d3bbc667b3f5d21cb262abdfc70e9679589
\ No newline at end of file
diff --git a/tests/cachekey/project/sources/tar1.expected b/tests/cachekey/project/sources/tar1.expected
index 89a492067..cf970120d 100644
--- a/tests/cachekey/project/sources/tar1.expected
+++ b/tests/cachekey/project/sources/tar1.expected
@@ -1 +1 @@
-f516c30a7a493acd74e49c4ac91cf697dfd32e8821d4230becac100cdac7df64
\ No newline at end of file
+b43e8a77bf8f89febcf4f40c3cde01155201fdb9ce08fa02d8b9d7b0a6538d1f
\ No newline at end of file
diff --git a/tests/cachekey/project/sources/tar2.expected b/tests/cachekey/project/sources/tar2.expected
index e9928d767..5bf390493 100644
--- a/tests/cachekey/project/sources/tar2.expected
+++ b/tests/cachekey/project/sources/tar2.expected
@@ -1 +1 @@
-eba29361b50cf14128dbf34362635bc2170474c8bb13c916638b2531174bf04a
\ No newline at end of file
+ca1385011d42e2ede76d3f7a6e88ca1bd972ab2a35ce0d30a74f5260ac066731
\ No newline at end of file
diff --git a/tests/cachekey/project/sources/zip1.expected b/tests/cachekey/project/sources/zip1.expected
index 41292f2a2..a8bf1143b 100644
--- a/tests/cachekey/project/sources/zip1.expected
+++ b/tests/cachekey/project/sources/zip1.expected
@@ -1 +1 @@
-a759e15073fdeed9224e41af4c094464e268bb8ced95dedf6e6dc35ec524d003
\ No newline at end of file
+8c154d2d9cb7c2dd9228c79f04fc15e3e4554698b44bf9d01de28b712aad13b4
\ No newline at end of file
diff --git a/tests/cachekey/project/sources/zip2.expected b/tests/cachekey/project/sources/zip2.expected
index e723f4852..18b1ac850 100644
--- a/tests/cachekey/project/sources/zip2.expected
+++ b/tests/cachekey/project/sources/zip2.expected
@@ -1 +1 @@
-d9c5a347340a387c4cecf24c29ad4b3c524773e8224683380221a62ec820abd9
\ No newline at end of file
+a38037f5ac8658d8a2174e78be822a9e04083441759bbdb1dd18a2009ae9e256
\ No newline at end of file
diff --git a/tests/cachekey/project/target.expected b/tests/cachekey/project/target.expected
index 9ea9991e9..91c58491e 100644
--- a/tests/cachekey/project/target.expected
+++ b/tests/cachekey/project/target.expected
@@ -1 +1 @@
-cb3b1bbf3d8b7be1a0ee7305c1056e95225f84b2c2b5189cfee69a66dcbd0786
\ No newline at end of file
+6f21fe36670a3a6e09597f12797fc7a42567a437c598b36d0c7792471929d57c
\ No newline at end of file