Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[system_setup] Allow skipping locale-gen command #1303

Merged
merged 1 commit into from
May 31, 2022
Merged

[system_setup] Allow skipping locale-gen command #1303

merged 1 commit into from
May 31, 2022

Conversation

CarlosNihelton
Copy link
Collaborator

@CarlosNihelton CarlosNihelton commented May 30, 2022

Language packs post-install hook executes locale-gen.
For certain languages a matrix of locales is generated.
Re-running locale-gen regenerates not only the locale we wanted, but the whole matrix, causing further delays. One example of such locale matrix generation happening after installing a language pack can be seen below:

image

We can save up to 40s (tested on my laptop) by skipping locale-gen if apt succeeded.
Still useful to run it unconditionally in dry-run since apt won't execute any post-install hook.

@CarlosNihelton CarlosNihelton requested a review from didrocks May 30, 2022 20:53
@CarlosNihelton CarlosNihelton marked this pull request as ready for review May 30, 2022 21:05
Copy link
Collaborator

@didrocks didrocks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still useful to run it unconditionally in dry-run since apt won't execute any post-install hook.
Sorry, I don’t understand this one. Basically, what‘s the point of running it in dry-run as we haven’t installed any new packages? Is the base system not self-sufficient? (and it’s dry-run anyway, so shouldn’t touch the system).

Also, this contradicts this comment you added as part of the PR, which implies the reverse (skipping it only in live run):

   # langpacks post-install hooks run locale-gen.
   # It makes sense to skip it in live run.

Sorry, I didn't express well what I intended to convey. The post install hook of various language-packs I looked into the sources invoke the script /usr/share/locales/install-language-pack:

#!/bin/sh -e

if [ -z "$1" ]; then
    echo "Usage: $0 <language code> <class> [<version>]"
    exit 0
fi

# install locales for base packages (not for gnome/kde)
if [ -z "$2" ]; then
    # Update requested locales if locales-all is not installed
    if dpkg-query -s locales-all >/dev/null 2>&1 ; then
        echo "locales-all installed, skipping locales generation"
    else
        /usr/sbin/locale-gen --keep-existing "$1"
    fi
fi

# ensure that .desktop caches are up to date
dpkg-trigger gmenucache || true

As you can see, unless locales-all is installed, it invokes locale-gen. If apt suceed running, we don't need to invoke locale-gen a second time. Yet, it's useful to preserve this controller ability to run locale-gen for the cases when apt fails.
In dry-run we don't install language packs, so apt will not run post-install hooks and there we use a 'mocked' environment to generate locales into and verify that we generated what was intended. Thus it's useful to run locale-gen in this controller in that situation.

@didrocks
Copy link
Collaborator

Sorry, it seems there is some formatting failure, I wrote above:

Sorry, I don’t understand this one. Basically, what‘s the point of running it in dry-run as we haven’t installed any new packages? Is the base system not self-sufficient? (and it’s dry-run anyway, so shouldn’t touch the system).

@CarlosNihelton
Copy link
Collaborator Author

Sorry, it seems there is some formatting failure, I wrote above:

Sorry, I don’t understand this one. Basically, what‘s the point of running it in dry-run as we haven’t installed any new packages? Is the base system not self-sufficient? (and it’s dry-run anyway, so shouldn’t touch the system).

The point would be asserting that locale would be installed either way. Nothing is written to the system, but to .subiquity directory tree. But I realized that we won't check the locale definition files that were generated, so I'm dropping those lines.

@didrocks
Copy link
Collaborator

But you still have the option on the function call itself, (using =ok btw?), it’s not needed anymore, correct?

@CarlosNihelton
Copy link
Collaborator Author

But you still have the option on the function call itself, (using =ok btw?), it’s not needed anymore, correct?

My take on that is: if apt fails to install language packs we still want to generate locale for the language user selected, to ensure date-time formatting, currency and other stuff works as she expects, even though the LC_MESSAGES come in English. Would you say those should be all or nothing?

@didrocks
Copy link
Collaborator

Hum, I see your point and I don’t have any arguments against it, so ok, that’s good enough to me :) Mind adding a small comment about this then and then, we can merge it, thanks!

lang-packs post-install hook executes locale-gen.
For certain languages a matrix of locales is generated.
Re-running locale-gen regenerates not only the locale we wanted,
but the whole matrix, causing further delays.

We can save up to 40s (tested on my laptop) by skipping locale-gen if
apt succeeded.

Still useful to run it unconditionally i dry-run since apt won't execute
any post-install hook.
@CarlosNihelton
Copy link
Collaborator Author

Hum, I see your point and I don’t have any arguments against it, so ok, that’s good enough to me :) Mind adding a small comment about this then and then, we can merge it, thanks!

There you go!

@didrocks didrocks merged commit 95bee59 into canonical:main May 31, 2022
@CarlosNihelton CarlosNihelton deleted the wsl-locgen-speedup branch May 31, 2022 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants