locale_gen: fix/improvements (#9238)

* locale_gen: fix/improvements

* fix sanity

* add RV doc

* add integration test forcing mechanism=debian

- test is failing

* fix RETURN doc

* reformat yaml

* comment out the test for ubuntu_mode=True

* multiple changes:

- add changelog fragment
- improved docs

* normalize docs after rebasing

* Update changelogs/fragments/9131-locale-gen-rewrite.yml

* apply recommendations from review

* Update plugins/modules/locale_gen.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update changelogs/fragments/9238-locale-gen-rewrite.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/locale_gen.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/locale_gen.py

Co-authored-by: Felix Fontein <felix@fontein.de>

---------

Co-authored-by: Felix Fontein <felix@fontein.de>

changelogs/fragments/9238-locale-gen-rewrite.yml

@@ -0,0 +1,13 @@
+minor_changes:
+  - "locale_gen - invert the logic to determine ``ubuntu_mode``, making it look first for ``/etc/locale.gen`` (set ``ubuntu_mode`` to ``False``)
+    and only then looking for ``/var/lib/locales/supported.d/`` (set ``ubuntu_mode`` to ``True``)
+    (https://github.com/ansible-collections/community.general/pull/9238,
+    https://github.com/ansible-collections/community.general/issues/9131,
+    https://github.com/ansible-collections/community.general/issues/8487)."
+  - >
+    locale_gen - new return value ``mechanism`` to better express the semantics of the ``ubuntu_mode``, with the possible values being either
+    ``glibc`` (``ubuntu_mode=False``) or ``ubuntu_legacy`` (``ubuntu_mode=True``) (https://github.com/ansible-collections/community.general/pull/9238).
+deprecated_features:
+  - >
+    locale_gen - ``ubuntu_mode=True``, or ``mechanism=ubuntu_legacy`` is deprecated and will be removed in community.general 13.0.0
+    (https://github.com/ansible-collections/community.general/pull/9238).

plugins/modules/locale_gen.py

@@ -12,7 +12,7 @@ DOCUMENTATION = r"""
 module: locale_gen
 short_description: Creates or removes locales
 description:
-  - Manages locales by editing /etc/locale.gen and invoking C(locale-gen).
+  - Manages locales in Debian and Ubuntu systems.
 author:
   - Augustus Kling (@AugustusKling)
 extends_documentation_fragment:
@@ -33,11 +33,20 @@ options:
   state:
     type: str
     description:
-      - Whether the locale shall be present.
+      - Whether the locales shall be present.
     choices: [absent, present]
     default: present
 notes:
-  - This module does not support RHEL-based systems.
+  - If C(/etc/locale.gen) exists, the module will assume to be using the B(glibc) mechanism, else if C(/var/lib/locales/supported.d/)
+    exists it will assume to be using the B(ubuntu_legacy) mechanism, else it will raise an error.
+  - When using glibc mechanism, it will manage locales by editing C(/etc/locale.gen) and running C(locale-gen).
+  - When using ubuntu_legacy mechanism, it will manage locales by editing C(/var/lib/locales/supported.d/local) and then running
+    C(locale-gen).
+  - Please note that the code path that uses ubuntu_legacy mechanism has not been tested for a while, because Ubuntu is already using
+    the glibc mechanism. There is no support for that, given our inability to test it. Therefore, that mechanism is B(deprecated)
+    and will be removed in community.general 13.0.0.
+  - Currently the module is B(only supported for Debian and Ubuntu) systems.
+  - This module requires the package C(locales) installed in Debian and Ubuntu systems.
 """
 
 EXAMPLES = r"""
@@ -54,6 +63,18 @@ EXAMPLES = r"""
     state: present
 """
 
+RETURN = r"""
+mechanism:
+  description: Mechanism used to deploy the locales.
+  type: str
+  choices:
+    - glibc
+    - ubuntu_legacy
+  returned: success
+  sample: glibc
+  version_added: 10.2.0
+"""
+
 import os
 import re
 
@@ -63,7 +84,10 @@ from ansible_collections.community.general.plugins.module_utils.mh.deco import c
 from ansible_collections.community.general.plugins.module_utils.locale_gen import locale_runner, locale_gen_runner
 
 
-class LocaleGen(StateModuleHelper):
+ETC_LOCALE_GEN = "/etc/locale.gen"
+VAR_LIB_LOCALES = "/var/lib/locales/supported.d"
+VAR_LIB_LOCALES_LOCAL = os.path.join(VAR_LIB_LOCALES, "local")
+SUPPORTED_LOCALES = "/usr/share/i18n/SUPPORTED"
 LOCALE_NORMALIZATION = {
     ".utf8": ".UTF-8",
     ".eucjp": ".EUC-JP",
@@ -76,9 +100,9 @@ class LocaleGen(StateModuleHelper):
     ".gb18030": ".GB18030",
     ".euctw": ".EUC-TW",
 }
-    LOCALE_GEN = "/etc/locale.gen"
-    LOCALE_SUPPORTED = "/var/lib/locales/supported.d/"
 
+
+class LocaleGen(StateModuleHelper):
     output_params = ["name"]
     module = dict(
         argument_spec=dict(
@@ -90,15 +114,36 @@ class LocaleGen(StateModuleHelper):
     use_old_vardict = False
 
     def __init_module__(self):
-        self.vars.set("ubuntu_mode", False)
+        self.MECHANISMS = dict(
-        if os.path.exists(self.LOCALE_SUPPORTED):
+            ubuntu_legacy=dict(
+                available=SUPPORTED_LOCALES,
+                apply_change=self.apply_change_ubuntu_legacy,
+            ),
+            glibc=dict(
+                available=SUPPORTED_LOCALES,
+                apply_change=self.apply_change_glibc,
+            ),
+        )
+
+        if os.path.exists(ETC_LOCALE_GEN):
+            self.vars.ubuntu_mode = False
+            self.vars.mechanism = "glibc"
+        elif os.path.exists(VAR_LIB_LOCALES):
             self.vars.ubuntu_mode = True
+            self.vars.mechanism = "ubuntu_legacy"
+            self.module.deprecate(
+                "On this machine mechanism=ubuntu_legacy is used. This mechanism is deprecated and will be removed from"
+                " in community.general 13.0.0. If you see this message on a modern Debian or Ubuntu version,"
+                " please create an issue in the community.general repository",
+                version="13.0.0", collection_name="community.general"
+            )
         else:
-            if not os.path.exists(self.LOCALE_GEN):
+            self.do_raise('{0} and {1} are missing. Is the package "locales" installed?'.format(
-                self.do_raise("{0} and {1} are missing. Is the package \"locales\" installed?".format(
+                VAR_LIB_LOCALES, ETC_LOCALE_GEN
-                    self.LOCALE_SUPPORTED, self.LOCALE_GEN
             ))
 
+        self.runner = locale_runner(self.module)
+
         self.assert_available()
         self.vars.set("is_present", self.is_present(), output=False)
         self.vars.set("state_tracking", self._state_name(self.vars.is_present), output=False, change=True)
@@ -115,18 +160,14 @@ class LocaleGen(StateModuleHelper):
         checking either :
         * if the locale is present in /etc/locales.gen
         * or if the locale is present in /usr/share/i18n/SUPPORTED"""
-        __regexp = r'^#?\s*(?P<locale>\S+[\._\S]+) (?P<charset>\S+)\s*$'
+        regexp = r'^\s*#?\s*(?P<locale>\S+[\._\S]+) (?P<charset>\S+)\s*$'
-        if self.vars.ubuntu_mode:
+        locales_available = self.MECHANISMS[self.vars.mechanism]["available"]
-            __locales_available = '/usr/share/i18n/SUPPORTED'
-        else:
-            __locales_available = '/etc/locale.gen'
 
-        re_compiled = re.compile(__regexp)
+        re_compiled = re.compile(regexp)
-        with open(__locales_available, 'r') as fd:
+        with open(locales_available, 'r') as fd:
             lines = fd.readlines()
         res = [re_compiled.match(line) for line in lines]
-            if self.verbosity >= 4:
+        self.vars.set("available_lines", lines, verbosity=4)
-                self.vars.available_lines = lines
 
         locales_not_found = []
         for locale in self.vars.name:
@@ -138,7 +179,7 @@ class LocaleGen(StateModuleHelper):
         locales_not_found = self.locale_get_not_present(locales_not_found)
 
         if locales_not_found:
-            self.do_raise("The following locales you've entered are not available on your system: {0}".format(', '.join(locales_not_found)))
+            self.do_raise("The following locales you have entered are not available on your system: {0}".format(', '.join(locales_not_found)))
 
     def is_present(self):
         return not self.locale_get_not_present(self.vars.name)
@@ -160,13 +201,13 @@ class LocaleGen(StateModuleHelper):
     def fix_case(self, name):
         """locale -a might return the encoding in either lower or upper case.
         Passing through this function makes them uniform for comparisons."""
-        for s, r in self.LOCALE_NORMALIZATION.items():
+        for s, r in LOCALE_NORMALIZATION.items():
             name = name.replace(s, r)
         return name
 
-    def set_locale(self, names, enabled=True):
+    def set_locale_glibc(self, names, enabled=True):
         """ Sets the state of the locale. Defaults to enabled. """
-        with open("/etc/locale.gen", 'r') as fr:
+        with open(ETC_LOCALE_GEN, 'r') as fr:
             lines = fr.readlines()
 
         locale_regexes = []
@@ -185,10 +226,10 @@ class LocaleGen(StateModuleHelper):
                 lines[i] = search.sub(replace, lines[i])
 
         # Write the modified content back to the file
-        with open("/etc/locale.gen", 'w') as fw:
+        with open(ETC_LOCALE_GEN, 'w') as fw:
             fw.writelines(lines)
 
-    def apply_change(self, targetState, names):
+    def apply_change_glibc(self, targetState, names):
         """Create or remove locale.
 
         Keyword arguments:
@@ -196,13 +237,13 @@ class LocaleGen(StateModuleHelper):
         names -- Names list including encoding such as de_CH.UTF-8.
         """
 
-        self.set_locale(names, enabled=(targetState == "present"))
+        self.set_locale_glibc(names, enabled=(targetState == "present"))
 
         runner = locale_gen_runner(self.module)
         with runner() as ctx:
             ctx.run()
 
-    def apply_change_ubuntu(self, targetState, names):
+    def apply_change_ubuntu_legacy(self, targetState, names):
         """Create or remove locale.
 
         Keyword arguments:
@@ -218,9 +259,9 @@ class LocaleGen(StateModuleHelper):
                 ctx.run()
         else:
             # Delete locale involves discarding the locale from /var/lib/locales/supported.d/local and regenerating all locales.
-            with open("/var/lib/locales/supported.d/local", "r") as fr:
+            with open(VAR_LIB_LOCALES_LOCAL, "r") as fr:
                 content = fr.readlines()
-            with open("/var/lib/locales/supported.d/local", "w") as fw:
+            with open(VAR_LIB_LOCALES_LOCAL, "w") as fw:
                 for line in content:
                     locale, charset = line.split(' ')
                     if locale not in names:
@@ -234,10 +275,7 @@ class LocaleGen(StateModuleHelper):
     def __state_fallback__(self):
         if self.vars.state_tracking == self.vars.state:
             return
-        if self.vars.ubuntu_mode:
+        self.MECHANISMS[self.vars.mechanism]["apply_change"](self.vars.state, self.vars.name)
-            self.apply_change_ubuntu(self.vars.state, self.vars.name)
-        else:
-            self.apply_change(self.vars.state, self.vars.name)
 
 
 def main():

tests/integration/targets/locale_gen/tasks/main.yml

@@ -12,7 +12,8 @@
   ansible.builtin.meta: end_play
   when: ansible_distribution not in ('Ubuntu', 'Debian')
 
-- include_tasks: basic.yml
+- name: Run tests auto-detecting mechanism
+  ansible.builtin.include_tasks: basic.yml
   loop: "{{ locale_list_basic }}"
   loop_control:
     loop_var: locale_basic