From 750be2b5237c5e64c17503a60471fb0956dfb48c Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Thu, 21 Nov 2024 18:37:56 +0000 Subject: [PATCH 1/3] Add config inheritance for personality field It is often the case that we want to build an image which just applies a few minor changes to an existing personality. For instance, the apps we want for most Spanish images are the same, but we have a few country-specific applications, and we often wish to build a sub-variant for a partner or region of a particular country. Currently we need to copy the full list of apps & settings from our es.ini personality config into (for instance) es_MX.ini; and then again from es_MX.ini into (e.g.) es_MX_aguascalientes.ini to add some region-specific apps or resources Implement a form of inheritance where (e.g.) building the es_MX personality reads es.ini and es_MX.ini, in that order; and (e.g.) building the es_MX_aguascalientes personality reads es.ini, es_MX.ini and es_MX_aguascalientes.ini, again in that order. This matches the convention that we have of naming personalities for the locale, and then adding further underscore-separated suffixes for subvariants thereof. This is only implemented for the personality field. This is where we do most of our customisation and is the place where inheritance would be most useful. https://phabricator.endlessm.com/T34731 --- run-build | 43 ++++++++++++++++++++++++--------- tests/test_image_builder.py | 48 +++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 11 deletions(-) diff --git a/run-build b/run-build index 4141e522..7b54054e 100755 --- a/run-build +++ b/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 diff --git a/tests/test_image_builder.py b/tests/test_image_builder.py index b7dd188c..086f6832 100644 --- a/tests/test_image_builder.py +++ b/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 From eac4b8976abce30fe5d947e489caf91344a68d1f Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Fri, 22 Nov 2024 10:46:19 +0000 Subject: [PATCH 2/3] Make Endless Key collections, Kolibri channels & node IDs merged fields Now that we have inheritance in personality configs, it becomes useful to be able to say that (for example) es_MX should add 1 specific channel to the selection inherited from es. Since commit 59980bce, values in merged fields keep the order in which they were first encountered. As a result, if (for example) the es personality specifies channels B, D, C (in that order) and es_MX adds channel A, then the resulting order is B, D, C, A. The order is significant: it is the order that channels are shown in the Kolibri Learn tab. So in this case it is not possible to move channel A to the top of the list without overriding the whole list of channels (which is possible but loses the benefit of this change). In practice, I only found two cases where this happens; in one case I hardcoded the previous order, and in the other case I just accepted the slight change. https://phabricator.endlessm.com/T34731 --- config/defaults.ini | 11 +++++++---- config/personality/en.ini | 2 +- config/personality/es.ini | 2 +- lib/eib.py | 4 ++++ 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/config/defaults.ini b/config/defaults.ini index b03dadf3..8e8a93c7 100644 --- a/config/defaults.ini +++ b/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 diff --git a/config/personality/en.ini b/config/personality/en.ini index 157305f5..2528685f 100644 --- a/config/personality/en.ini +++ b/config/personality/en.ini @@ -27,7 +27,7 @@ apps_add = com.endlessnetwork.sciencesnacks [endlesskey] -collections = +collections_add = artist athlete curious diff --git a/config/personality/es.ini b/config/personality/es.ini index 17b91245..ee822a3b 100644 --- a/config/personality/es.ini +++ b/config/personality/es.ini @@ -45,6 +45,6 @@ apps_add = ar.com.pilas_engine.App [endlesskey] -collections = +collections_add = spanish spanish-extras diff --git a/lib/eib.py b/lib/eib.py index 2baaf010..fa657069 100644 --- a/lib/eib.py +++ b/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'), ] From 217484c78847d32e1ba60deb053bdbbd4b7f327b Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Fri, 22 Nov 2024 11:45:56 +0000 Subject: [PATCH 3/3] fr: Default to Paris timezone For projects in Haiti we have a (private) fr_HT personality. Checking our metrics, the majority of fr activations are in metropolitan France (the hexagonal bit of France that is just across the channel from the UK, rather than overseas regions). --- config/personality/fr.ini | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/config/personality/fr.ini b/config/personality/fr.ini index f50158d7..abeb85ba 100644 --- a/config/personality/fr.ini +++ b/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