¡Hola, Ludo! Thanks for your comments. Reviewing them I found some more issues that I address inline:
Ludovic Courtès <[email protected]> writes: > Miguel Ángel Arruga Vivas <[email protected]> skribis: > >> From 5886bdf74bda59649b3d17b691132d9d030e0fb4 Mon Sep 17 00:00:00 2001 >> From: =?UTF-8?q?Miguel=20=C3=81ngel=20Arruga=20Vivas?= >> <[email protected]> >> Date: Sat, 24 Oct 2020 20:36:21 +0200 >> Subject: [PATCH] system: Do not depend on locale folder generated by >> grub-install. I've changed the title to "system: Generate grub locale directory for grub.cfg.", which is positive, shorter and more descriptive. I have to work on my communicative skills a lot... >> >> * gnu/bootloader/grub.scm (define-module): Use (guix packages). This was a leftover from the modification of Grub itself instead of using computed-file and not needed anymore, removed. >> (grub-locale-folder): New computed folder. >> (grub-configuration-file)[locale-config]: Use grub-locale-folder only when a >> locale is needed. Changed with the new names and added an explanation about the search. > > [...] > >> +(define (grub-locale-folder grub) >> + "Generate a folder with the locales from GRUB." > > s/folder/directory/ :-) Applied, both in the function name and the documentation string. >> + (define builder >> + #~(begin >> + (use-modules (ice-9 ftw)) >> + (let ((locale (string-append #$grub "/share/locale")) >> + (out #$output)) >> + (mkdir out) >> + (chdir out) >> + (for-each (lambda (lang) >> + (let ((file (string-append locale "/" lang >> + "/LC_MESSAGES/grub.mo")) >> + (dest (string-append lang ".mo"))) >> + (when (file-exists? file) >> + (copy-file file dest)))) >> + (scandir locale))))) >> + (computed-file "grub-boot-locale" builder)) > > Maybe just “grub-locales”? This name sounds right, applied too. :-) >> + (let* ((entry (first all-entries)) >> + (device (menu-entry-device entry)) >> + (mount-point (menu-entry-device-mount-point entry)) >> + (bootloader (bootloader-configuration-bootloader config)) >> + (grub (bootloader-package bootloader)) >> + (locale-dir (normalize-file (grub-locale-folder grub) >> + mount-point >> + store-directory-prefix))) > > Nitpick: maybe s/locale-dir/locales/ Applied too, but I moved this call from the let* to ... >> + #~(let ((locale #$(and locale >> + (locale-definition-source >> + (locale-name->definition locale)))) >> + (locale-dir #$(and locale locale-dir))) ... here, to avoid the generation when there is no locale, which shouldn't be a common use case, but it shouldn't depend on the generation of that directory because it won't be used. >> + (when locale >> + (format port "\ >> # Localization configuration. >> -if search --file --set boot_partition /grub/grub.cfg; then >> - set locale_dir=(${boot_partition})/grub/locale >> -else >> - set locale_dir=/boot/grub/locale >> -fi >> -set lang=~a~%" locale)))) >> +~asearch --file --set ~a/[email protected] >> +set locale_dir=~a >> +set lang=~a~%" > > Otherwise LGTM, thanks! Thank you again for the review. Pushed as f445bc65764ffad2ae9f3b382ddb8feb4eeea2fb with these fixes, after running again make check and a subset of make check-system. Happy hacking! Miguel
