[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1036771: marked as done (debian-installer: Wrong colors in GRUB submenus for netboot on x86)

Your message dated Fri, 26 May 2023 07:34:31 +0000
with message-id <E1q2Ry3-005zlQ-AQ@fasolo.debian.org>
and subject line Bug#1036771: fixed in debian-installer 20230526
has caused the Debian Bug report #1036771,
regarding debian-installer: Wrong colors in GRUB submenus for netboot on x86
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
1036771: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1036771
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message --- 
Package: debian-installer
Severity: important

Hi,

[ I'm going to call “dark menu” the “accessible dark contrast” menu;
  also, I've palette-indexed a few screenshots in the hope to reach
  both the BTS and the mailing list. ]

I didn't spot this immediately as it only shows up when using netboot,
e.g. using netboot's mini.iso or netboot-gtk's mini.iso (the same is
very likely via PXE too), under UEFI, with x86 images (i.e. amd64).


Fixing x86: netboot vs. netinst
===============================

The welcome screen is all nice, with a nice background and appropriate
colors. But as soon as one enters a submenu, colors get reset to some
default values, and everything looks bad.

See:
 - netboot-1-welcome.png
 - netboot-2-advanced-submenu.png

The same *doesn't* happen when using netinst instead (all other things
being equal), because in that case the minimal grub.cfg that's generated
by build/util/grub-gencfg during a debian-installer build is replaced by
one generated by debian-cd, via its tools/boot/bookworm/boot-x86:

    # Create grub menu entries to match the isolinux ones
    sed -i '/^menuentry/Q' $CDDIR/boot/grub/grub.cfg;
    $BASEDIR/tools/boot/$DI_CODENAME/parse_isolinux \
        boot$N/isolinux $CDDIR $BASEDIR/data/$DI_CODENAME/grub-theme.in "$DISKINFO_DISTRO" "$DEBIAN_KERNEL" "$DEBVERSION" \
        >> $CDDIR/boot/grub/grub.cfg

I have not investigated further, but it looks like it uses some theme
instead of (or in addition to) hardcoded colors, which makes everything
looks good.

See:
 - netinst-1-welcome.png
 - netinst-2-advanced-submenu.png

This even allows entering the dark menu, getting black and white colors,
and getting back to the regular menu, getting the initial colors back.

Compare:
 - netboot-3-dark-submenu.png [enter]
 - netboot-4-welcome-again.png [exit]

versus:
 - netinst-3-dark-submenu.png [enter]
 - netinst-4-welcome-again.png [exit]


Back to the grub.cfg generated by debian-installer, let's look at what
happens for a netboot-gtk build, we have two grub.cfg:

    build/dest/netboot/gtk/debian-installer/amd64/grub/x86_64-efi/grub.cfg
    build/dest/netboot/gtk/debian-installer/amd64/grub/grub.cfg

The first one calls various `insmod` then sources the second one. Let's
look at the second one (only mentioning the most relevant parts):

    if background_image /isolinux/splash.png; then
      set color_normal=light-gray/black
      set color_highlight=white/black
    elif background_image /splash.png; then
      set color_normal=light-gray/black
      set color_highlight=white/black
    else
      set menu_color_normal=cyan/blue
      set menu_color_highlight=white/blue
    fi
    
    […]
    
    menuentry 'Install' {
        set background_color=black
        linux    /debian-installer/amd64/linux vga=788 --- quiet
        initrd   /debian-installer/amd64/initrd.gz
    }
    submenu --hotkey=a 'Advanced options ...' {
        set menu_color_normal=cyan/blue
        set menu_color_highlight=white/blue
        set gfxpayload=keep
    […]
    
        submenu '... Desktop environment menu ...' {
            set menu_color_normal=cyan/blue
            set menu_color_highlight=white/blue
            set gfxpayload=keep
    
    […]
    
    submenu --hotkey=d 'Accessible dark contrast installer menu ...' {
        set menu_color_normal=white/black
        set menu_color_highlight=yellow/black
        set color_normal=white/black
        set color_highlight=yellow/black
        background_image
        set gfxpayload=keep
        menuentry '... Install' {
            set background_color=black
            linux    /debian-installer/amd64/linux vga=788 theme=dark --- quiet
            initrd   /debian-installer/amd64/initrd.gz
        }
        submenu --hotkey=a '... Advanced options ...' {
            set menu_color_normal=white/black
            set menu_color_highlight=yellow/black
            set color_normal=white/black
            set color_highlight=yellow/black
            background_image
            set gfxpayload=keep
    
    […]

So we have two attempts at loading a background image and applying
specific colors, with a fallback to “usual” colors. But then, in each
and every submenu that is not “dark”, the fallback colors are hardcoded,
which explains why non-dark submenus look bad.

I'm therefore proposing *not* to force those colors in normal submenus,
and to only force colors when in the dark menu (and its submenus).

That would solve non-dark submenus' looking bad, but would not solve the
case of entering then exiting the dark menu: we would stick to black and
white colors. Solving this properly might involve finally addressing
this todo at the top of the grub-gencfg script:

    # TODO: Theme generation from template

and/or forcing “appropriate” colors (see the two background_image calls
at the top of the file and the fallback) in all cases…

All that seems cumbersome and definitely not critical enough to be
investigated before Bookworm. If users enter the dark menu, either they
know what they're doing or they're going to learn what high contrast
looks like until they start the installer, or until they give up and
power-cycle their machine. :)


Using diffoscope against a full build with or without the attached
patch, I've verified that the only changes on amd64 are in:

    netboot/debian-installer/amd64/grub/grub.cfg
    netboot/mini.iso
    netboot/netboot.tar.gz
    netboot/gtk/debian-installer/amd64/grub/grub.cfg
    netboot/gtk/mini.iso
    netboot/gtk/netboot.tar.gz

In all cases, grub.cfg has the hardcoded colors going away. In both
mini.iso cases, the output has a lot of noise due to I think slight
changes in file sizes/block usage (no, I'm not becoming an ISO expert
today).

See how the only affected build targets are netboot and netboot-gtk,
leaving the “CD-oriented” ones alone, meaning no impact on debian-cd
builds. I consider this change to be safe for x86.


Not breaking arm64
==================

Now, build/util/grub-gencfg is used on some other architectures:
 - ia64, which I'll ignore entirely;
 - arm*, which got me worried a little.

Contrary to x86 that has some heavy duty machinery to keep isolinux and
grub menus aligned, arm* seem to ship directly the generated grub.cfg in
the “CD-oriented” bits, and we're seeing the same output with either
netboot* builds or netinst.

See:
 - arm64-grub.png

(And yes, I'm going to pretend I've not seen the wrapping issue!)

This means that the diffoscope output lists more things between two full
arm64 builds, with or without the patch:
 - netboot and netboot-gtk files, as in the am64 case, all fine;
 - netboot/SD-card-images/firmware.*
 - netboot/gtk/SD-card-images/firmware.*
 - images/u-boot/*
 - cdrom/debian-cd_info.tar.gz
 - cdrom/gtk/debian-cd_info.tar.gz

The SD-card-images and u-boot bits seems to only be about a few bytes,
likely a serial number of some kind, and I'm not worried about that.

Seeing debian-cd_info.tar.gz there got me worried, keeping in mind the
original intent was to fix netboot* on x86!

That being said, let's look at a grub.cfg for arm64:

    set menu_color_normal=cyan/blue
    set menu_color_highlight=white/blue
    
    insmod gzio
    
    menuentry 'Install' {
        set background_color=black
        linux    /debian-installer/arm64/linux --- quiet
        initrd   /debian-installer/arm64/initrd.gz
    }
    menuentry 'Graphical install' {
        set background_color=black
        linux    /debian-installer/arm64/linux --- quiet
        initrd   /debian-installer/arm64/gtk/initrd.gz
    }
    submenu --hotkey=a 'Advanced options ...' {
        set menu_color_normal=cyan/blue
        set menu_color_highlight=white/blue
        set gfxpayload=keep
        menuentry '... Graphical expert install' { 
            set background_color=black
            linux    /debian-installer/arm64/linux priority=low ---
            initrd   /debian-installer/arm64/gtk/initrd.gz
        }   
    […]

Here, we see that we have the “usual”/“feedback” colors hardcoded at the
top, there's no background_image call, and there's no condition at all.
So dropping the re-hardcoding of the same colors in submenus doesn't
look like it should break anything (I'm pretty sure I tested that a few
days ago, with my first local netinst arm64 build; but I'll probably
giving it another spin after uploading debian-installer with the patch,
waiting for the various buildd/ftp/release bits to happen).

Also, for some reason, with or without patching the debian-installer
build, the color “reset” after entering/exiting the dark menu just works
on arm64. In passing, I'm seeing a warning/error message when entering
the dark menu, but that might just be some limitation of the
virtualization environment (maybe some unsupported video mode change).


Conclusion
==========

This bug report is more for later reference than anything, to fully
document the reasons, worries, and reassurances about the change, so
that we can get back to it if anything breaks, without having to
rediscover that rabbit hole…


Cheers,
-- 
Cyril Brulebois (kibi@debian.org)            <https://debamax.com/>
D-I release manager -- Release team member -- Freelance Consultant

Attachment: netboot-1-welcome.png
Description: PNG image

Attachment: netboot-2-advanced-submenu.png
Description: PNG image

Attachment: netboot-3-dark-submenu.png
Description: PNG image

Attachment: netboot-4-welcome-again.png
Description: PNG image

Attachment: netinst-1-welcome.png
Description: PNG image

Attachment: netinst-2-advanced-submenu.png
Description: PNG image

Attachment: netinst-3-dark-submenu.png
Description: PNG image

Attachment: netinst-4-welcome-again.png
Description: PNG image

Attachment: arm64-grub.png
Description: PNG image

--- a/build/util/grub-gencfg
+++ b/build/util/grub-gencfg
@@ -119,8 +119,7 @@ sub start_submenu ($;%)
     }
     else
     {
-        print_indented("set menu_color_normal=cyan/blue\n");
-        print_indented("set menu_color_highlight=white/blue\n");
+        # Don't force colors in other submenus
     }
     print_set_theme($theme);
     print_indented("set gfxpayload=keep\n");

--- End Message ---
--- Begin Message --- 
Source: debian-installer
Source-Version: 20230526
Done: Cyril Brulebois <kibi@debian.org>

We believe that the bug you reported is fixed in the latest version of
debian-installer, which is due to be installed in the Debian FTP archive.

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 1036771@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Cyril Brulebois <kibi@debian.org> (supplier of updated debian-installer package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@ftp-master.debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Format: 1.8
Date: Fri, 26 May 2023 09:08:23 +0200
Source: debian-installer
Architecture: source
Version: 20230526
Distribution: unstable
Urgency: medium
Maintainer: Debian Install System Team <debian-boot@lists.debian.org>
Changed-By: Cyril Brulebois <kibi@debian.org>
Closes: 1036215 1036771
Changes:
 debian-installer (20230526) unstable; urgency=medium
 .
   [ Emanuele Rocca ]
   * build/config/x86.cfg: add /splash.png symlink to make GRUB load the
     splash screen when using netboot (Closes: #1036215).
 .
   [ Cyril Brulebois ]
   * Update translation-status for the release.
   * Fix colors in GRUB submenus for x86 netboot (Closes: #1036771).
   * Add CHECK_MINIMAL_VERSION support, so that the release manager can
     combine speedy uploads and peace of mind.
Checksums-Sha1:
 488baa18d3bac1724cb3672cef9f391e9d477be9 4015 debian-installer_20230526.dsc
 c125688b03e48b42ca637c3c7626b74f9a6e242d 1194156 debian-installer_20230526.tar.xz
 8a137139db73f5606c81960931d1b3ca12df8327 12687 debian-installer_20230526_source.buildinfo
Checksums-Sha256:
 380d300a6b0da7aaf0cde3c1129e47fb42d95f19ad79d01555fe5f8e361c2511 4015 debian-installer_20230526.dsc
 c2594f02dac014270b1d6b5823278da4936c70b4d4d4dfe0f1ad3d1bc6363249 1194156 debian-installer_20230526.tar.xz
 823a3e2d3c204a0aef0f7bcd88246275f362a97d315cc9be6cbb5845bfe42a04 12687 debian-installer_20230526_source.buildinfo
Files:
 62b88c04f3d223840920163e917bdb10 4015 devel optional debian-installer_20230526.dsc
 0d2c16ac0bb96e362756a08522820322 1194156 devel optional debian-installer_20230526.tar.xz
 89ba62b361e1e128b3baf94f45412253 12687 devel optional debian-installer_20230526_source.buildinfo

-----BEGIN PGP SIGNATURE-----

iQJEBAEBCgAuFiEEtg6/KYRFPHDXTPR4/5FK8MKzVSAFAmRwXF0QHGtpYmlAZGVi
aWFuLm9yZwAKCRD/kUrwwrNVINqHEACsjKQmmvwVsNJmEQ2mazds4TgBxk6dio/q
4Qlb4vmAN0r9eQTqdzM3q3/XSdfVtCQ7LVU3EDX2n3+bZbfB/K11WarH2RB1l0Ah
ypVVYyBgp1OnPq3jknrk84CeWagBK2EbLvd/kcQWE0X6mTQJz1FGGvmnUuxwPGpg
yi+BGW5Q+4YzIkxIa1SRiwFj6u277LQA/7GfpGCLe5YsAXL0SHueC8HkC1rmbwnw
8R7Z5p30e+2JfIVfeTel2MhbOKEBPc+1Ly4p+Fh1xunoa3KcEia9rU61ixirtj56
WzOpx2J7nNCzZshs13Rpi+21Rc4bBUJkSHQgDJjMgt6vWMgCvwMn0iJPfVeaYni1
e9EYYQW6BJxW+IfBVR4x7matZU9q7p1Kh7+gk+/iY6dCp2WtVi8IRHukUII5lgTT
CnM7wYtR/gf55EdvykbZfyHLG50TakyC5u5uMXHoYUwKdFhYDXAKOGXTKZ0QOfkn
+er+OucHctTpXLDp+pjargE//d4WxMQR08FZ7c5bZ7sjrPeaRp/0ZwNS7vCIO27c
PiAsIrjK7m/btGmua00IWyF/FSYxgDD4UGlQ6PBH6C0KCBrw9g02Pnygtx96LHSC
cdl1in6gcHY17Hds+IN0U5lD4ebfzVacqS468QRWK7k3vikfdaBLg6ML/1PTyOvO
jNzd1bwKqw==
=UVEF
-----END PGP SIGNATURE-----

--- End Message ---
Reply to: