Merge pull request #169 from endlessm/T34731

Add config inheritance for personality field

config/defaults.ini

@@ -263,7 +263,7 @@ regular_users_can_manage_content = false
 # it out of the images until it gets updated to match the new Kolibri
 # experience on Endless OS.
 #
-# install_channels =
+# install_channels_add =
 #   # How to get started with Kolibri on Endless OS
 #   e8a879742b2249a0a4b890f9903916f7
 
@@ -278,17 +278,20 @@ regular_users_can_manage_content = false
 # exclude_node_ids - A list of content node IDs to exclude. Use this to
 #   exclude content nested beneath content nodes that are being included.
 #
+# These are merged fields, so use an _add or _del suffix as desired to adjust
+# the lists.
+#
 # Example:
 #
 # [kolibri-e8a879742b2249a0a4b890f9903916f7]
-# include_node_ids =
+# include_node_ids_add =
 #   # English [topic]
 #   3b909a18242c48208dbc49d06bc48162
 #   # Español [topic]
 #   6e8f60c6b9c841969853d48f4eff22cf
 #   # Français [topic]
 #   0b70a374af244baaa2b795530c2c0b55
-# exclude_node_ids =
+# exclude_node_ids_add =
 #   # Kolibri 0.12.2 User Guide for Admins [document]
 #   5bb37c1832c8489ab2940f31588305f6
 
@@ -303,7 +306,7 @@ kolibri_pkgspec = https://github.com/learningequality/kolibri/releases/download/
 # Which Endless Key collections to preload in the image.
 # Must match the name of one of the collections shipped with the Endless Key
 # (ex. artist, explorer, spanish etc).
-collections =
+collections_add =
 
 # By default, only the content specified in each collection will be included.
 # Set this to true to include all content from all channels in the requested

config/personality/en.ini

@@ -27,7 +27,7 @@ apps_add =
   com.endlessnetwork.sciencesnacks
 
 [endlesskey]
-collections =
+collections_add =
   artist
   athlete
   curious

config/personality/es.ini

@@ -45,6 +45,6 @@ apps_add =
   ar.com.pilas_engine.App
 
 [endlesskey]
-collections =
+collections_add =
   spanish
   spanish-extras

config/personality/fr.ini

@@ -1,10 +1,8 @@
 # fr personality settings
 
 [image]
-# FIXME: Language codes (locales) and timezone are country specific, so
-# just choose Haiti for now since that's where we'll release first.
 language = fr_FR.utf8
-timezone = America/Port-au-Prince
+timezone = Europe/Paris
 
 [flatpak]
 locales = fr

lib/eib.py

@@ -116,6 +116,7 @@ class ImageConfigParser(configparser.ConfigParser):
         ('buildroot', 'packages'),
         ('check', 'hooks'),
         ('content', 'hooks'),
+        ('endlesskey', 'collections'),
         ('error', 'hooks'),
         ('flatpak', 'locales'),
         ('flatpak-remote-*', 'apps'),
@@ -126,6 +127,9 @@ class ImageConfigParser(configparser.ConfigParser):
         ('image', 'icon_grid'),
         ('image', 'settings'),
         ('image', 'settings_locks'),
+        ('kolibri', 'install_channels'),
+        ('kolibri-*', 'exclude_node_ids'),
+        ('kolibri-*', 'include_node_ids'),
         ('manifest', 'hooks'),
         ('publish', 'hooks'),
     ]

run-build

@@ -387,6 +387,27 @@ class ImageBuilder(object):
             datetime.datetime.utcnow().strftime('%y%m%d-%H%M%S')
         )
 
+    @staticmethod
+    def _get_prefixes(value):
+        """Yields all prefixes of value split by _, including the whole string.
+
+        >>> list(ImageBuilder._get_prefixes("en_GB_orkney"))
+        ["en", "en_GB", "en_GB_orkney"]
+        """
+        parts = value.split("_")
+        for i in range(1, len(parts) + 1):
+            yield "_".join(parts[:i])
+
+    def _get_attr_values(self, attr):
+        """Yields one or more values for attr. For most properties this is a single
+        value, but for "personality" this yields a series of values to implement
+        inheritance."""
+        value = getattr(self, attr)
+        if attr == "personality":
+            yield from self._get_prefixes(value)
+        else:
+            yield value
+
     def _get_config_paths(self, dir_path, dir_ns):
         config_files = []
 
@@ -398,20 +419,20 @@ class ImageBuilder(object):
         config_attrs = ('product', 'branch', 'arch', 'platform',
                         'personality')
         for attr in config_attrs:
-            path = os.path.join(dir_path, attr,
-                                getattr(self, attr) + '.ini')
-            namespace = dir_ns + '_'.join((attr, getattr(self, attr)))
-            config_files.append((path, namespace))
+            for value in self._get_attr_values(attr):
+                path = os.path.join(dir_path, attr, value + '.ini')
+                namespace = dir_ns + '_'.join((attr, value))
+                config_files.append((path, namespace))
 
         # Add combinations of per-attribute config directories
         for attr1, attr2 in itertools.combinations(config_attrs, 2):
-            ini_name = (getattr(self, attr1) + '-' +
-                        getattr(self, attr2) + '.ini')
-            path = os.path.join(dir_path, attr1 + '-' + attr2, ini_name)
-            namespace = (dir_ns +
-                         '_'.join((attr1, attr2, getattr(self, attr1),
-                                   getattr(self, attr2))))
-            config_files.append((path, namespace))
+            for (value1, value2) in itertools.product(self._get_attr_values(attr1),
+                                                      self._get_attr_values(attr2)):
+                ini_name = (value1 + '-' + value2 + '.ini')
+                path = os.path.join(dir_path, attr1 + '-' + attr2, ini_name)
+                namespace = (dir_ns +
+                             '_'.join((attr1, attr2, value1, value2)))
+                config_files.append((path, namespace))
 
         return config_files
 

tests/test_image_builder.py

@@ -281,6 +281,54 @@ def _run_test():
     _run_test()
 
 
+def test_config_inheritance(make_builder, tmp_path, tmp_builder_paths, caplog):
+    """Test that personality en_GB_orkney also loads settings from en and en_GB."""
+    configdir = tmp_path / 'config'
+    personalitydir = configdir / 'personality'
+    personalitydir.mkdir(parents=True, exist_ok=True)
+
+    en = personalitydir / 'en.ini'
+    en.write_text(dedent("""\
+    [image]
+    language = en_US.utf8
+
+    [flatpak-remote-eos-apps]
+    apps_add =
+      com.endlessm.encyclopedia.en
+      com.endlessm.football.en
+    """))
+
+    en_GB = personalitydir / 'en_GB.ini'
+    en_GB.write_text(dedent("""\
+    [image]
+    language = en_GB.utf8
+
+    [flatpak-remote-eos-apps]
+    # Football means something else in the UK
+    apps_del =
+      com.endlessm.football.en
+    apps_add =
+      com.endlessm.football.en_GB
+    """))
+
+    en_GB_orkney = personalitydir / 'en_GB_orkney.ini'
+    en_GB_orkney.write_text(dedent("""\
+    [flatpak-remote-eos-apps]
+    apps_add =
+      com.endlessm.orkneyingasaga
+    """))
+
+    builder = make_builder(configdir=str(configdir), personality="en_GB_orkney")
+    builder.configure()
+
+    assert builder.config['image']['language'] == "en_GB.utf8"
+    assert builder.config['flatpak-remote-eos-apps']['apps'] == "\n".join([
+      "com.endlessm.encyclopedia.en",
+      "com.endlessm.football.en_GB",
+      "com.endlessm.orkneyingasaga",
+    ])
+
+
 def test_localdir(make_builder, tmp_path, tmp_builder_paths, caplog):
     """Test use of local settings directory"""
     # Build without localdir