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

Fixing double insertion of encoding in locale.gen/locale.conf #1421

Merged
merged 6 commits into from
Aug 12, 2022
Merged

Conversation

Torxed
Copy link
Member

@Torxed Torxed commented Aug 12, 2022

PR Description:

Since we migrated to the new menu system, we started give more and accurate options throughout the installation (since we have a better search and way of presenting).
This meant that locales were offered "as is", meaning we offered both:

sv_SE.UTF-8 UTF-8
sv_SE ISO-8859-1

Previously we would only have offered sv_SE, and separately offered UTF-8 or ISO-8859-1 in another step.
Since we still offered both steps, but also both options - you see where this is going.

The "fix" is a small one, it simply checks if there's a . in the locale part, splits that and overrides UTF-8 if the split one differes from UTF-8 (which is our default value).

Any modifiers like sr_RS@latin UTF-8 should be kept intact as well, and placed in the order mentioned here: Locale#Generating_locales: language[_territory][.codeset][@modifier].

Tests and Checks

  • I have tested the code!

@Torxed Torxed linked an issue Aug 12, 2022 that may be closed by this pull request
@Torxed
Copy link
Member Author

Torxed commented Aug 12, 2022

Tested, and the installer can now handle locale being set with included encoding.
screenshot

There's still the issue of dual entries in locale.gen, which isn't a huge issue as it only adds latency when generating the locales. But it's undesirable so we'll correct that next.

Conf used:

{
    "HSM": null,
    "__separator__": null,
    "additional-repositories": [],
    "archinstall-language": "English",
    "audio": "pipewire",
    "bootloader": "systemd-bootctl",
    "config_version": "2.5.0",
    "debug": false,
    "desktop-environment": "awesome",
    "gfx_driver": "Nvidia (proprietary)",
    "harddrives": [
        "/dev/sda"
    ],
    "hostname": "laptop",
    "kernels": [
        "linux"
    ],
    "keyboard-layout": "sv-latin1",
    "mirror-region": {
        "Sweden": {
            "http://ftp.acc.umu.se/mirror/archlinux/$repo/os/$arch": true,
            "http://ftp.lysator.liu.se/pub/archlinux/$repo/os/$arch": true,
            "http://ftp.myrveln.se/pub/linux/archlinux/$repo/os/$arch": true,
            "http://ftpmirror.infania.net/mirror/archlinux/$repo/os/$arch": true,
            "https://ftp.acc.umu.se/mirror/archlinux/$repo/os/$arch": true,
            "https://ftp.ludd.ltu.se/mirrors/archlinux/$repo/os/$arch": true,
            "https://ftp.lysator.liu.se/pub/archlinux/$repo/os/$arch": true,
            "https://ftp.myrveln.se/pub/linux/archlinux/$repo/os/$arch": true,
            "https://mirror.osbeck.com/archlinux/$repo/os/$arch": true
        }
    },
    "mount_point": null,
    "nic": {
        "dhcp": true,
        "dns": null,
        "gateway": null,
        "iface": null,
        "ip": null,
        "type": "iso"
    },
    "no_pkg_lookups": false,
    "ntp": true,
    "offline": false,
    "packages": [
        "nano",
        "wget",
        "git",
        "util-linux",
        "rsync",
        "python",
        "sublime-text-4"
    ],
    "parallel downloads": 3,
    "plugin": "https://hvornum.se/aur_plugin.py",
    "profile": {
        "path": "/root/archinstall-git/archinstall/profiles/desktop.py"
    },
    "save_config": null,
    "script": "guided",
    "silent": false,
    "swap": true,
    "sys-encoding": "UTF-8",
    "sys-language": "en_US.UTF-8",
    "timezone": "Europe/Stockholm",
    "version": "2.5.0",
    "custom-commands" : [
        "runuser -l anton -c 'mkdir -p /home/anton/github/'",
        "runuser -l anton -c 'git clone https://github.com/Torxed/Torxed.git /home/anton/github/Torxed'",
        "rsync -tr /home/anton/github/Torxed/.filesystem_overlays/laptop/usr/bin/* /usr/bin/",
        "rsync -tr /home/anton/github/Torxed/.filesystem_overlays/common/usr/bin/* /usr/bin/",
        "chmod +x /usr/bin/*"
    ]
}

@Torxed
Copy link
Member Author

Torxed commented Aug 12, 2022

And here's a conf with the @modifier in works:
screenshot2

{
    "HSM": null,
    "__separator__": null,
    "additional-repositories": [],
    "archinstall-language": "English",
    "audio": "pipewire",
    "bootloader": "systemd-bootctl",
    "config_version": "2.5.0",
    "debug": false,
    "desktop-environment": "awesome",
    "gfx_driver": "Nvidia (proprietary)",
    "harddrives": [
        "/dev/sda"
    ],
    "hostname": "laptop",
    "kernels": [
        "linux"
    ],
    "keyboard-layout": "sv-latin1",
    "mirror-region": {
        "Sweden": {
            "http://ftp.acc.umu.se/mirror/archlinux/$repo/os/$arch": true,
            "http://ftp.lysator.liu.se/pub/archlinux/$repo/os/$arch": true,
            "http://ftp.myrveln.se/pub/linux/archlinux/$repo/os/$arch": true,
            "http://ftpmirror.infania.net/mirror/archlinux/$repo/os/$arch": true,
            "https://ftp.acc.umu.se/mirror/archlinux/$repo/os/$arch": true,
            "https://ftp.ludd.ltu.se/mirrors/archlinux/$repo/os/$arch": true,
            "https://ftp.lysator.liu.se/pub/archlinux/$repo/os/$arch": true,
            "https://ftp.myrveln.se/pub/linux/archlinux/$repo/os/$arch": true,
            "https://mirror.osbeck.com/archlinux/$repo/os/$arch": true
        }
    },
    "mount_point": null,
    "nic": {
        "dhcp": true,
        "dns": null,
        "gateway": null,
        "iface": null,
        "ip": null,
        "type": "iso"
    },
    "no_pkg_lookups": false,
    "ntp": true,
    "offline": false,
    "packages": [
        "nano",
        "wget",
        "git",
        "util-linux",
        "rsync",
        "python",
        "sublime-text-4"
    ],
    "parallel downloads": 3,
    "plugin": "https://hvornum.se/aur_plugin.py",
    "profile": {
        "path": "/root/archinstall-git/archinstall/profiles/desktop.py"
    },
    "save_config": null,
    "script": "guided",
    "silent": false,
    "swap": true,
    "sys-encoding": "ISO-8859-15",
    "sys-language": "de_DE@euro",
    "timezone": "Europe/Stockholm",
    "version": "2.5.0",
    "custom-commands" : [
        "runuser -l anton -c 'mkdir -p /home/anton/github/'",
        "runuser -l anton -c 'git clone https://github.com/Torxed/Torxed.git /home/anton/github/Torxed'",
        "rsync -tr /home/anton/github/Torxed/.filesystem_overlays/laptop/usr/bin/* /usr/bin/",
        "rsync -tr /home/anton/github/Torxed/.filesystem_overlays/common/usr/bin/* /usr/bin/",
        "chmod +x /usr/bin/*"
    ]
}

It causes other issues when we try to decode things from the output log as UTF-8, for obvious reasons.
So globally, we should honor whatever locale is set/generated when running custom-commands after the minimal install is done (which includes setting the locale at the end):
screenshot

…e more correct locale generation introduced in this PR.
@Torxed
Copy link
Member Author

Torxed commented Aug 12, 2022

Fixed the encoding issue by using .decode('UTF-8', errors='backslashreplace') in the one place I knew this issue would arise. I think this PR is ready to go in. I'll leave it open for a few hours for people to take a gander :)

Copy link
Member

@grazzolini grazzolini left a comment

Choose a reason for hiding this comment

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

LGTM as a temporary fix.

@Torxed Torxed merged commit b1ab5ba into master Aug 12, 2022
@Torxed Torxed deleted the fix-1200 branch August 12, 2022 20:36
@Torxed
Copy link
Member Author

Torxed commented Aug 12, 2022

Perfect, I've ran some 40+ installations with this now and it seems to hold up.
I'll run a few more manual one since most of the ones I've done is automated.

@svartkanin
Copy link
Collaborator

@Torxed would you mind sharing your automated installation setup, it might be quite useful since I'm tackling some upgrades to the profiles atm :)

@Torxed
Copy link
Member Author

Torxed commented Aug 13, 2022

@Torxed would you mind sharing your automated installation setup, it might be quite useful since I'm tackling some upgrades to the profiles atm :)

Sure, it's as simple as uploading a conf.json, disk.json and creds.json to a webserver and then:

python ~/github/Scripts/python/archtest.py --bridge br0 --internet eth0 --conf https://hvornum.se/laptop_conf.json --creds https://hvornum.se/laptop_creds.json --disk-layout https://hvornum.se/laptop_disk.json --silent --repo https://github.com/Torxed/archinstall --branch fix-1200 --new-drives --rebuild

--internet just tells it which interface your machine uses to face the internet, --bridge is created in order to bridge a tap (--tap) parameter to the bridge and VM so the VM doesn't have to use NAT (slows down stuff some times).

--creds, --conf, --disk-layout and --silent is passed to archinstall inside the ISO. Which archtest.py creates and sets up. It also clones archinstall with the given --branch as well as --repo <url> (if you want to use your fork).

--new-drives creates two new Qemu disk images, one 15GB and one 40GB (I think it is), you can use --boot hdd after one install to boot up the installed system, just remember to remove --new-drives and --rebuild.

The script archtest.py is by far not perfect, it probably has some poor assumptions of packages that I always have installed that might be missing on your setup. But it should be trivial to tweak around.

@svartkanin
Copy link
Collaborator

Nice I'll give that a go!

@Torxed
Copy link
Member Author

Torxed commented Aug 13, 2022

Let me know how it went.
Remember that you don't need to --rebuild between push:es to your repo.
Just reboot the ISO or Ctrl-D to force a re-login and it will perform git pull as it's first step. It's built into the auto-run, and this is the auto-run string: https://github.com/Torxed/Scripts/blob/6501150748f0f252a1f271fb94dda57a2e0626fd/python/archtest.py#L375-L391

autorun_string = "[[ -z $DISPLAY && $XDG_VTNR -eq 1 ]] &&"
autorun_string += ' sh -c "cd /root/archinstall-git;'
autorun_string += ' git config --global pull.rebase false;'
autorun_string += ' git pull;'
autorun_string += ' cp examples/guided.py ./;'
autorun_string += ' python guided.py'
# Append options to archinstall (aka guided.py)
if args.conf:
	autorun_string += f' --conf {args.conf}'
if args.disk_layout:
	autorun_string += f' --disk_layout {args.disk_layout}'
if args.creds:
	autorun_string += f' --creds {args.creds}'
if args.silent:
	autorun_string += f' --silent'

autorun_string += '";\n'

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.

The end of /etc/locale.gen is appended with two en_US.UTF-8 UTF-8.
3 participants