Re: [OE-core] [PATCH v2] oeqa/selftest/locales: Create selftest for locales generation

[Louis Rannou <lrannou@baylibre.com>]

On 01/02/2023 18:57, Richard Purdie wrote:
> On Wed, 2023-02-01 at 16:02 +0100, Louis Rannou wrote:
>>
>> On 01/02/2023 14:49, Richard Purdie wrote:
>>> On Fri, 2023-01-27 at 16:33 +0100, Louis Rannou wrote:
>>>> From: Louis Rannou <lrannou@baylibre.com>
>>>>
>>>> [YOCTO #9070]
>>>>
>>>> Add a new selftest to validate the locales generation. This selftest builds a
>>>> complete target with GLIBC_GENERATE_LOCALES, IMAGE_LINGUAS,
>>>> ENABLE_BINARY_LOCALE_GENERATION set.
>>>>
>>>> The tests has two cases: GLIBC_GENERATE_LOCALES=1 and
>>>> GLIBC_GENERATE_LOCALES=0. When set to 1, we check that the locales exist in the
>>>> local-archive and if it does exist for the glibc. When set to 0, we check it
>>>> does not.
>>>>
>>>> Signed-off-by: Louis Rannou <lrannou@baylibre.com>
>>>> ---
>>>>    meta/lib/oeqa/selftest/cases/locales.py | 81 +++++++++++++++++++++++++
>>>>    1 file changed, 81 insertions(+)
>>>>    create mode 100644 meta/lib/oeqa/selftest/cases/locales.py
>>>>
>>>> diff --git a/meta/lib/oeqa/selftest/cases/locales.py b/meta/lib/oeqa/selftest/cases/locales.py
>>>> new file mode 100644
>>>> index 0000000000..f053cc8e31
>>>> --- /dev/null
>>>> +++ b/meta/lib/oeqa/selftest/cases/locales.py
>>>> @@ -0,0 +1,81 @@
>>>> +#
>>>> +# SPDX-License-Identifier: MIT
>>>> +#
>>>> +
>>>> +from oeqa.selftest.case import OESelftestTestCase
>>>> +from oeqa.core.decorator import OETestTag
>>>> +from oeqa.utils.commands import bitbake, runqemu
>>>> +
>>>> +class LocalesTest(OESelftestTestCase):
>>>> +
>>>> +    @OETestTag("runqemu")
>>>> +    def test_locales_on(self):
>>>> +        """
>>>> +        Summary: Test the locales are generated
>>>> +        Expected: 1. Check the locale exist in the locale-archive
>>>> +                  2. Check the locale exist for the glibc
>>>> +                  3. Check the locale can be generated
>>>> +        Product: oe-core
>>>> +        Author: Louis Rannou <lrannou@baylibre.com>
>>>> +        AutomatedBy: Louis Rannou <lrannou@baylibre.com>
>>>> +        """
>>>> +
>>>> +        features = []
>>>> +        features.append('EXTRA_IMAGE_FEATURES = "empty-root-password allow-empty-password allow-root-login"')
>>>> +        features.append('IMAGE_INSTALL:append = " glibc-utils localedef"')
>>>> +        features.append('GLIBC_GENERATE_LOCALES = "en_US.UTF-8 fr_FR.UTF-8"')
>>>> +        features.append('IMAGE_LINGUAS:append = " en-us fr-fr"')
>>>> +        features.append('ENABLE_BINARY_LOCALE_GENERATION = "1"')
>>>> +        self.write_config("\n".join(features))
>>>> +
>>>> +        # Build a core-image-minimal
>>>> +        bitbake('core-image-minimal')
>>>> +
>>>> +        with runqemu("core-image-minimal", ssh=False, runqemuparams='nographic') as qemu:
>>>> +            cmd = "locale -a"
>>>> +            status, output = qemu.run_serial(cmd)
>>>> +            # output must includes fr_FR or fr_FR.UTF-8
>>>> +            self.assertEqual(status, 1, msg='localedef test failed: output: %s' % output)
>>>> +            self.assertIn("fr_FR", output, msg='locale -a test failed: output: %s' % output)
>>>> +
>>>> +            cmd = "localedef --list-archive -v"
>>>> +            status, output = qemu.run_serial(cmd)
>>>> +            # output must includes fr_FR.utf8
>>>> +            self.assertEqual(status, 1, msg='localedef test failed: output: %s' % output)
>>>> +            self.assertIn("fr_FR.utf8", output, msg='locale -a test failed: output: %s' % output)
>>>> +
>>>> +    @OETestTag("runqemu")
>>>> +    def test_locales_off(self):
>>>> +        """
>>>> +        Summary: Test the locales are not generated
>>>> +        Expected: 1. Check the locale does not exist in the locale-archive
>>>> +                  2. Check the locale does not exist for the glibc
>>>> +                  3. Check the locale cannot be generated
>>>> +        Product: oe-core
>>>> +        Author: Louis Rannou <lrannou@baylibre.com>
>>>> +        AutomatedBy: Louis Rannou <lrannou@baylibre.com>
>>>> +        """
>>>> +
>>>> +        features = []
>>>> +        features.append('EXTRA_IMAGE_FEATURES = "empty-root-password allow-empty-password allow-root-login"')
>>>> +        features.append('IMAGE_INSTALL:append = " glibc-utils localedef"')
>>>> +        features.append('GLIBC_GENERATE_LOCALES = "en_US.UTF-8 fr_FR.UTF-8"')
>>>> +        features.append('IMAGE_LINGUAS:append = " en-us fr-fr"')
>>>> +        features.append('ENABLE_BINARY_LOCALE_GENERATION = "0"')
>>>> +        self.write_config("\n".join(features))
>>>> +
>>>> +        # Build a core-image-minimal
>>>> +        bitbake('core-image-minimal')
>>>> +
>>>> +        with runqemu("core-image-minimal", ssh=False, runqemuparams='nographic') as qemu:
>>>> +            cmd = "locale -a"
>>>> +            status, output = qemu.run_serial(cmd)
>>>> +            # output must not includes fr_FR or fr_FR.UTF-8
>>>> +            self.assertEqual(status, 1, msg='localedef test failed: output: %s' % output)
>>>
>>> This should really say locale instead of localedef here to be clear
>>> which command the error is coming from.
>>>
>>>> +            self.assertNotIn("fr_FR", output, msg='locale -a test failed: output: %s' % output)
>>>> +
>>>> +            cmd = "localedef --list-archive -v"
>>>> +            status, output = qemu.run_serial(cmd)
>>>> +            # output must not includes fr_FR.utf8
>>>> +            self.assertEqual(status, 1, msg='localedef test failed: output: %s' % output)
>>>> +            self.assertNotIn("fr_FR.utf8", output, msg='locale -a test failed: output: %s' % output)
>>>
>>> We did run this through tests on the autobuilder and the tests did
>>> pass. I did wonder rather than duplicate the whole test with a line
>>> change, could we share the code between the two?
>> I think the things we can share are only about the features
>> configuration and the bitbake command. Going further would require to
>> create an if/else statement for the assert. I think this will not
>> simplify the code.
>>
>>>
>>> I then start looking at the code and I'm a bit worried the locales are
>>> not present when binary locale generation is turned off as I think they
>>> should be. Going from memory, I thought that they should be generated
>>> on target at first boot in that case so they should be present in both
>>> cases.
>>
>> I am not sure to understand. But if you suppose there should be
>> something in /usr/share/i18n/locales, there is nothing there.
>>
>>>
>>> To keep things simple, lets merge the first case in and file a bug for
>>> the other case? I think we'll need to investigate that and check
>>> whether we're broken something somewhere.
>>
>> Ok, I'll send a v3 with the first part only. Let's forget about the code
>> sharing before we get clear about what the opposite test will be.
> 
> I was able to take a quick look at this. The locale generation on
> target is broken, it throws errors upon boot. I was able to make it
> work with this patch:
> 
> diff --git a/meta/recipes-core/glibc/glibc-locale.inc b/meta/recipes-core/glibc/glibc-locale.inc
> index 7f70b3ca4f..c39007b573 100644
> --- a/meta/recipes-core/glibc/glibc-locale.inc
> +++ b/meta/recipes-core/glibc/glibc-locale.inc
> @@ -60,7 +60,7 @@ DESCRIPTION:localedef = "glibc: compile locale definition files"
>   # FILES:glibc-gconv will not be automatically extended in multilib.
>   # Explicitly add ${MLPREFIX} for FILES:glibc-gconv.
>   FILES:${MLPREFIX}glibc-gconv = "${libdir}/gconv/*"
> -FILES:localedef = "${bindir}/localedef"
> +FILES:localedef = "${bindir}/localedef ${libdir}/locale"
>   
>   LOCALETREESRC = "${COMPONENTS_DIR}/${PACKAGE_ARCH}/glibc-stash-locale"
>   
> @@ -87,6 +87,7 @@ do_install() {
>          find ${D}${libdir} -type d -empty -delete
>          copy_locale_files ${datadir}/locale 0644
>          install -m 0644 ${LOCALETREESRC}/SUPPORTED ${WORKDIR}/SUPPORTED
> +       install -d ${D}${libdir}/locale
>   }
>   
>   inherit libc-package
> 
> could you see if the same test will now work with that patch applied?
> I'm happy to have the basic test but if we can fix this and stop it
> regressing that would be even better.

The patch does not change the result. I have actually nothing in the 
image/usr/lib/locale directory.

I am not sure to understand... If I add to my image the packages 
glibc-localedata-fr-fr and glibc-charmap-utf-8, then I have something in 
/usr/share/i18n/locales and /usr/share/i18n/charmaps, and then I can 
generate the locales with `localedef -c -i fr_FR -f UTF-8 fr_FR`

In the end, I believe this is correct that I have nothing in 
{libdir}/locale, but perhaps the settings IMAGE_LINGUAS (or 
GLIBC_GENERATE_LOCALES ?) should specify a rdepend to the locale/charmap 
packages, so the locales would exist in the image, but would not be 
generated at build.

Regards,
Louis