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

nixos/plymouth: use bgrt theme #114000

Merged
merged 5 commits into from
Mar 4, 2021
Merged

Conversation

worldofpeace
Copy link
Contributor

@worldofpeace worldofpeace commented Feb 22, 2021

Motivation for this change

Closes #84158.
In a followup I'd like to pick up or encourage @puckipedia towards #88789. LUKS+ZFS is a tentative goal to having plymouth working entirely as expected.

This pull request focuses on getting "FlickerFree Boot" with the BGRT theme, an initiative from Fedora https://fedoraproject.org/wiki/Changes/FlickerFreeBoot, that now (as usual because RedHat values) other projects can benefit from. Most recently (IIRC), Ubuntu is using the master branch of plymouth. Whether plymouth is ready for release is debated, but if you look at this pull request certain polishing upstream is needed, but if you want it we certainly can use it.

I tested this pull request on a machine with using i915 and it worked pretty well, nothing seemed weird.
Here's an actual example of how the bootsplash should look: https://ic.pics.livejournal.com/hansdegoede/13347631/433/433_900.png, with a vendor image, a logo, and spinner.

If you want to see examples in other situations the mockup is useful: https://gitlab.gnome.org/Teams/Design/os-mockups/-/blob/master/boot/boot-progress.png, but they changed the shutdown to be the same as the startup.

Todo's:

  • Need a better logo available

We need something monochromatic, the current image we have looks too big and draws too much attention to itself.
Mentioned this in NixOS/nixos-artwork#58.

  • Switch to the white version of the nixos logo nixos-icons: 2017-03-16 -> 2021-02-24 #114276

  • We need to sync the GDM logo with this logo change for a smooth transition gnome3.gdm: use white nixos logo #114387

  • Testing for AMD+Nvidia users would be helpful but isn't required

  • logo doesn't show up at shutdown
    In initrd we copy these logo's, but on the actual system we
    would need to use environment.etc. However, when I try to do this I get ln permission denied errors. It's also rather confusing that in initrd everything is in $out/share, but on the running system it's /etc. This might require patching all theme files ImageDir to /run/current-system, and using runCommand to get an installable derivation with the logos?
    It seems the source code attempts to change directory to the location of the image files, so no XDG_DATA_DIRS.

    Why doesn't plymouth use the logo path it's configured to use?

Questions:

In pull request the following nix code is added:

boot.initrd.availableKernelModules = [ "i915" ];

also part of the work done by Fedora to get fastboot enabled for Intel users.
This sort of thing is a part of nixos-hardware, but I think ideally nixos really should have some base profiles that nixos-generate-hardware detects and adds like https://github.com/NixOS/nixos-hardware/blob/0cb5491af9da8d6f16540ffc4db24e2361852e46/flake.nix#L77
Perhaps in the release note nixos-hardware can be mentioned or just the configuration for intel users if they're not already using it.

Thanks

@eadwu and @andersk were motivation to finally fix this

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@worldofpeace worldofpeace requested review from a team and samueldr February 22, 2021 13:15
@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Feb 22, 2021
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 11-100 labels Feb 22, 2021
@r-rmcgibbo
Copy link

r-rmcgibbo commented Feb 22, 2021

Result of nixpkgs-review pr 114000 at feb0443e run on aarch64-linux 1

1 package marked as broken and skipped:
  • xmonad_log_applet
26 packages built:
1 suggestion:
  • warning: unclear-gpl

    gpl2 is a deprecated license, check if project uses gpl2Plus or gpl2Only and change meta.license accordingly.

    Near pkgs/os-specific/linux/plymouth/default.nix:115:5:

        |
    115 |     license = licenses.gpl2;
        |     ^
    

Result of nixpkgs-review pr 114000 at feb0443e run on x86_64-linux 1

1 package marked as broken and skipped:
  • xmonad_log_applet
27 packages built:
1 suggestion:
  • warning: unclear-gpl

    gpl2 is a deprecated license, check if project uses gpl2Plus or gpl2Only and change meta.license accordingly.

    Near pkgs/os-specific/linux/plymouth/default.nix:115:5:

        |
    115 |     license = licenses.gpl2;
        |     ^
    

@@ -47,15 +47,15 @@ in
};

themePackages = mkOption {
default = [ nixosBreezePlymouth ];
default = [ ] ++ lib.optional (cfg.theme == "breeze") nixosBreezePlymouth;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
default = [ ] ++ lib.optional (cfg.theme == "breeze") nixosBreezePlymouth;
default = lib.optional (cfg.theme == "breeze") nixosBreezePlymouth;

Copy link
Contributor

Choose a reason for hiding this comment

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

We could just take this opportunity to get rid of nixosBreezePlymouth (I made it). It only exists so there was a default theme that actually displayed the logo and name nicely. If the bgrt theme works for this purpose we can kill it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@michaelpj I still think there could be users that want the breeze-plymouth theme?

Comment on lines +22 to +24
outputs = [
"out"
"dev"
];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
outputs = [
"out"
"dev"
];
outputs = [ "out" "dev" ];

pkgs/os-specific/linux/plymouth/default.nix Show resolved Hide resolved
pkgs/os-specific/linux/plymouth/default.nix Outdated Show resolved Hide resolved
}:

stdenv.mkDerivation rec {
pname = "plymouth";
version = "0.9.4";
version = "2020-12-07";
Copy link
Contributor

Choose a reason for hiding this comment

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

If we use the git describe format, version = "0.9.5-29-gc4ced2a", then the version numbers will remain monotonic if we switch back to a release. This format also works with fetchFromGit*, so we can use rev = version below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wait... Can we maybe open an issue for that? U probably know about that annoying rule we have with "pname = "something-unstable" and verison = YYY-MM-DD. IIRC this is what they do in debian world? Anyways, I think it's worth looking into that but I think it's out of scope of this PR, and I will use what the manual has currently.

Copy link
Contributor

@andersk andersk Feb 24, 2021

Choose a reason for hiding this comment

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

But you haven’t changed pname to "plymouth-unstable", so the monotonicity problem is in scope for this PR.

Edit: never mind, GitHub just wasn’t showing that you changed this later.

Copy link
Member

Choose a reason for hiding this comment

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

Anyways, I think it's worth looking into that but I think it's out of scope of this PR, and I will use what the manual has currently.

The manual is inaccurate because it just mentions name and not pname or version. Changing pname is not an option because it breaks many package definitions and repology.

@davidak
Copy link
Member

davidak commented Feb 23, 2021

AMD GPU

https://youtu.be/88boyDEVKDs

I still see stage1 console output.

@worldofpeace
Copy link
Contributor Author

worldofpeace commented Feb 23, 2021

AMD GPU

https://youtu.be/88boyDEVKDs

I still see stage1 console output.

We still see stage-1 because plymouth can't start that early because we don't have #74842 (comment)

But yeah, I didn't even see the splash in your video. According to https://hansdegoede.livejournal.com/20632.html

The graphics drivers for AMD and Nvidia GPUs reset the hardware when loading, this will cause the display to temporarily go black. There is nothing which can be done about this.

So it will not be perfect for these users, but u should see it. Do you boot in UEFI mode?

@davidak
Copy link
Member

davidak commented Feb 23, 2021

Yes, the splash screen disappeared, but also it's loading time. It seem a lot faster now.

The "NixOS" you see shortly at the start is from GRUB. So it boots in BIOS mode.

Is what you see in the video the expected behavior except not showing the start splash?

I also have an Nvidia card and will test that too.

Let me know if i can help with testing anything else.

@worldofpeace
Copy link
Contributor Author

Yes, the splash screen disappeared, but also it's loading time. It seem a lot faster now.

The "NixOS" you see shortly at the start is from GRUB. So it boots in BIOS mode.

Is what you see in the video the expected behavior except not showing the start splash?

I also have an Nvidia card and will test that too.

Let me know if i can help with testing anything else.

It is expected behavior, BGRT is a UEFI thing. Nvidia in UEFI would also be helpful.

@samueldr
Copy link
Member

The "NixOS" you see shortly at the start is from GRUB. So it boots in BIOS mode.

I'm not sure I follow, (sorry it is a bit tangentially off-topic) GRUB can be used, and is used, both with UEFI and Legacy boot.


As part of fedora's documentation, they list the planned changes they had to make to GRUB, with a user FAQ.

So it should be perfectly usable with GRUB, if we bring in the required patches (assuming they are not in upstream, since development upstream is not quick).


@worldofpeace am I understanding right that this has only been verified and tested with systemd-boot?

Note that it is not an issue if it is the case. But it is something to know.

@worldofpeace
Copy link
Contributor Author

As part of fedora's documentation, they list the planned changes they had to make to GRUB, with a user FAQ.

So it should be perfectly usable with GRUB, if we bring in the required patches (assuming they are not in upstream, since development upstream is not quick).

@worldofpeace am I understanding right that this has only been verified and tested with systemd-boot?
Note that it is not an issue if it is the case. But it is something to know.

Ah, I should have mentioned. I use GRUB. Ahh, I've never actually noticed https://hansdegoede.livejournal.com/19081.html, but it's good that you mention this.

It seems particularly difficult to pick through what Fedora does with grub2 https://src.fedoraproject.org/rpms/grub2/tree/rawhide... They have a 301 patch series 😁 !
A quick find and I spot two patches https://src.fedoraproject.org/rpms/grub2/blob/rawhide/f/0129-Add-auto-hide-menu-support.patch https://src.fedoraproject.org/rpms/grub2/blob/rawhide/f/0170-grub.d-Split-out-boot-success-reset-from-menu-auto-h.patch, so it appears that they have this patched in. It appears from other threads that they have their GRUB done up specifically and there's kernel parameters like the log level etc. that also are being configured.

We should probably split this off as some other tangible sub tasks. I also did just peruse the grub ml archive because I was wondering if they sent these patches upstream, but I'm not very good at that 🐱

copy_bin_and_libs ${plymouth}/bin/plymouth
copy_bin_and_libs ${plymouth}/bin/plymouthd

# Check if the actual requested theme is here
Copy link
Contributor

Choose a reason for hiding this comment

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

👏

@worldofpeace
Copy link
Contributor Author

All my required todo's are now done ✨
More review and testing is welcome, but I believe it's pretty much "complete".


# For themes that are compiled with PLYMOUTH_LOGO_FILE
mkdir -p $out/etc/plymouth
ln -s $logo $out/etc/plymouth/logo.png
Copy link
Member

Choose a reason for hiding this comment

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

Using Nix value directly is IMO clearer than passing it to environment as a middleman:

Suggested change
ln -s $logo $out/etc/plymouth/logo.png
ln -s "${cfg.logo}" "$out/etc/plymouth/logo.png"


# patch out any attempted references to the theme or plymouth's themes directory
# Use -L to copy the directories proper, not the symlinks to them.
Copy link
Member

Choose a reason for hiding this comment

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

Could you also mention why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

extraUtils can't contain references to nix/store, and that's why we're patching. I thought that bit is usually a bit obvious whenever you're working with extraUtilsCommands

pkgs/os-specific/linux/plymouth/default.nix Outdated Show resolved Hide resolved
pkgs/os-specific/linux/plymouth/default.nix Outdated Show resolved Hide resolved
pkgs/os-specific/linux/plymouth/default.nix Outdated Show resolved Hide resolved
pkgs/os-specific/linux/plymouth/default.nix Outdated Show resolved Hide resolved
pkgs/os-specific/linux/plymouth/default.nix Outdated Show resolved Hide resolved
* default to bgrt
* don't use KillMode=none
  https://gitlab.freedesktop.org/plymouth/plymouth/-/issues/123

* multiple outputs
* fix spinfinity logo file
Much better to provide a helpful message than to
get an obscure sed message.
The BGRT theme is probably a close as to "FlickerFree" we can
get without NixOS#74842.
It's more agnostic than the Breeze theme.

We also install all of themes provided by the packages, as it's possible
that one theme needs the ImageDir of another, and they're small files
anyways.

Lastly, how plymouth handles logo and header files is
a total mess, so hopefully when they have an actual release
we won't need to do all this symlinking.
This looks cohesive with the spinner in the bgrt theme.
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/what-are-your-goals-for-21-05/11559/7

@worldofpeace
Copy link
Contributor Author

Planning to merge this in a few days. Has been running totally fine on my machine.

@bbigras
Copy link
Contributor

bbigras commented Mar 4, 2021

Do I only need this to test with nvidia?

{
  boot.plymouth.enable = true;
  initrd.availableKernelModules = [ "nvidia" ];
}

I don't see plymouth while booting. I'm using systemd-boot.

@bbigras
Copy link
Contributor

bbigras commented Mar 4, 2021

I just tested on my laptop (i915). As expected (I think), the splash is only visible after I input the LUKS password and when I shutdown.

It's pretty :) Great work!

I wonder if plymouth could be visible while selecting the nix generation with systemd-boot.

@worldofpeace
Copy link
Contributor Author

the splash is only visible after I input the LUKS password and when I shutdown.

Yep, once we have #88789 we'll have that through plymouth also.

@worldofpeace worldofpeace merged commit 583f1a9 into NixOS:master Mar 4, 2021
@worldofpeace worldofpeace deleted the plymouth-bgrt branch March 4, 2021 23:32
@bbigras
Copy link
Contributor

bbigras commented Mar 4, 2021

Oh, I was able to test with nvidia proprietary with:

{
    initrd.availableKernelModules = [ "nvidia" "nvidia_modeset" "nvidia_uvm" "nvidia_drm" ];

    kernelParams = [
      "nvidia-drm.modeset=1"
      "quiet"
      "splash"
    ];
}

I had some console text, then plymouth was visible, a black screen and then gdm.

Do you want me to open a new issue for that? and maybe wait for the PR to be in unstable.

@worldofpeace
Copy link
Contributor Author

Do you want me to open a new issue for that? and maybe wait for the PR to be in unstable.

"If you have an AMD or Nvidia GPU driving your screen, then this is normal. The graphics drivers for AMD and Nvidia GPUs reset the hardware when loading, this will cause the display to temporarily go black. There is nothing which can be done about this."

https://hansdegoede.livejournal.com/20632.html

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/what-are-your-goals-for-21-05/11559/18

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 11-100
Projects
None yet
Development

Successfully merging this pull request may close these issues.