Page MenuHomeFreeBSD

Bootloader code base (lua scripts ) clean up and extension with a theming system
Needs RevisionPublic

Authored by ghislain_smartix.llc on Apr 19 2023, 8:36 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Apr 26, 8:54 PM
Unknown Object (File)
Sat, Apr 20, 12:42 PM
Unknown Object (File)
Mon, Apr 8, 9:27 PM
Unknown Object (File)
Mar 4 2024, 8:10 AM
Unknown Object (File)
Dec 22 2023, 11:21 PM
Unknown Object (File)
Dec 10 2023, 9:15 PM
Unknown Object (File)
Oct 30 2023, 10:03 AM
Unknown Object (File)
Sep 28 2023, 10:08 AM

Details

Reviewers
manu
jrtc27
Group Reviewers
Loader
Summary

The scripts controlling the booting procedure were previously quite difficult to read and understand.
In addition, some routines were repetitive and the algorithms they represented quite confusing depending on the context.
Another limitation that this pull-request tries to lift is the ease with which brands, logos and menus can be drawn and colorized.

Prior to this new theming system being introduced, colors on logos, brands etc... had to be manually set using fixed strings that contained TERMINAL ESCAPE SEQUENCES. These control characters instruct the terminal on where and how to display colors. The new system reduces the mechanical aspect of the colorization by introducing
a set of libraries that take care of the colorization automatically. With this new system in place the bootloader menu composed of three main
sections ('the brand', 'the menu' and 'the logo' sections) can be completely or partially themed/colorized. Each section has a separate
bootloader variable that allows its independent manipulation/control.

The new bootloader variables introduced are:

  • loader_menu_theme : controls the colorization ability of the menu
  • loader_brand_theme : controls the colorization ability of the brand
  • loader_logo_theme : controls the colorization ability of the logo

On top of these important variables, the scripts responsible for the display have been extended and certain parts rewritten to accommodate this theming system.The library now makes it very easy to add and remove logos, brands etc...

Work is in progress to theme the terminal after the operating system has successfully booted to provide a complete environment. The same concept applies but within the operating system we have a lot more control so interesting ideas are being explored.

Some but not all the benefits that these features aim to provide are:

  • Good utilization of your terminal capabilities. One benefit of the theming system is that it tries to make good utilization of your terminal capabilities.
  • Flexibility in the color code system. This feature also has the purpose to help the administrator visually separate, represent and identify environments, localities, regions, system functions (QA, Test, Production environment).
  • Environments separation, representation and identification through a flexible color code system ( theming system )

Note:

This change extends the commit represented by this hash ( 546f18f3dadc ). One new feature that needs both attention and discussion is the place where the loader looks for scripts and the naming convention used in the process. For the naming convention, The old method was to look for file names that started with either 'logo-' or 'brand-'. The commit mentioned above added modifications that changed the naming convention to 'gfx-' for both logos and brands. One of the disadvantages of this naming convention arises when there are several brands and/or logos; for this test about 20 'gfx-files' were used. When there are this many files, as a developer, it becomes quite difficult to work with these files because you have to remember them by name and mentally match the name to the content (logo or brand). When coding, this convention makes tracing and debugging a bit tedious; error messages are a little to generic. So to solve the problem, at least for developers, this commit re-implements the old naming convention in a completely new way so as to make both naming conventions compatible. Now file names can be prefixed with either 'logo-','brand-' or 'gfx-' without an issue. In addition, another functionality that this commit includes is the ability to go through a list of predefined directories in search for potential loader script files.

The current convention keeps all the script files in /boot/lua. To help extend the convention keep the tree a bit organized, two sub-directories (/boot/lua/logo/ and /boot/lua/brand) were added and the new functionality now looks in these three directories for bootloader scripts by convention.

A final point of discussion is the rewording of the menu. This commit tries to help clarify some of the menu options by adding few words that help in that sense.

Test Plan

{F59833704}

A short video clip has been uploaded to help illustrate the concept and also as a way to demonstrate that testing has been done.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

The scripts controlling the booting procedure were previously quite difficult to read and understand.
In addition, some routines were repetitive and the algorithms they represented quite confusing depending on the context.
Another limitation that this pull-request tries to lift is the ease with which brands, logos and menus can be drawn and colorized.

Prior to this new theming system being introduced, colors on logos, brands etc... had to be manually set using fixed strings that contained TERMINAL ESCAPE SEQUENCES. These control characters instruct the terminal on where and how to display colors. The new system reduces the mechanical aspect of the colorization by introducing
a set of libraries that take care of the colorization automatically. With this new system in place the bootloader menu composed of three main
sections ('the brand', 'the menu' and 'the logo' sections) can be completely or partially themed/colorized. Each section has a separate
bootloader variable that allows its independent manipulation/control.

The new bootloader variables introduced are:

loader_menu_theme : controls the colorization ability of the menu

loader_brand_theme : controls the colorization ability of the brand

loader_logo_theme : controls the colorization ability of the logo

On top of these important variables, the scripts responsible for the display have been extended and certain parts rewritten to accommodate this theming system.The library now makes it very easy to add and remove logos, brands etc...

Work is in progress to theme the terminal after the operating system has successfully booted to provide a complete environment. The same concept applies but within the operating system we have a lot more control so interesting ideas are being explored.

Some but not all the benefits that these features aim to provide are:

Good utilization of your terminal capabilities. One benefit of the theming system is that it tries to make good utilization of your terminal capabilities.

Flexibility in the color code system. This feature also has the purpose to help the administrator visually separate, represent and identify environments, localities, regions, system functions (QA, Test, Production environment).

Environments separation, representation and identification through a flexible color code system ( theming system )

Note:

This change extends the commit represented by this hash ( 546f18f3dadc ). One new feature that needs both attention and discussion is the place where the loader looks for scripts and the naming convention used in the process. For the naming convention, The old method was to look for file names that started with either 'logo-' or 'brand-'. The commit mentioned above added modifications that changed the naming convention to 'gfx-' for both logos and brands. One of the disadvantages of this naming convention arises when there are several brands and/or logos; for this test about 20 'gfx-files' were used. When there are this many files, as a developer, it becomes quite difficult to work with these files because you have to remember them by name and mentally match the name to the content (logo or brand). When coding, this convention makes tracing and debugging a bit tedious; error messages are a little to generic. So to solve the problem, at least for developers, this commit re-implements the old naming convention in a completely new way so as to make both naming conventions compatible. Now file names can be prefixed with either 'logo-','brand-' or 'gfx-' without an issue. In addition, another functionality that this commit includes is the ability to go through a list of predefined directories in search for potential loader script files.

The current convention keeps all the script files in /boot/lua. To help extend the convention keep the tree a bit organized, two sub-directories (/boot/lua/logo/ and /boot/lua/brand) were added and the new functionality now looks in these three directories for bootloader scripts by convention.

A final point of discussion is the rewording of the menu. This commit tries to help clarify some of the menu options by adding few words that help in that sense.

jrtc27 requested changes to this revision.Apr 19 2023, 6:25 PM
jrtc27 added a subscriber: jrtc27.
jrtc27 added inline comments.
stand/lua/menu.lua
275

I'll let others comment on all the other changes rather than sharing my own thoughts which I know are shared, but this breaks making the message configurable, as done for installer images (which you can see in the fact that your SmartixOS installer says [B]oot into Multi user rather than [B]oot Installer like FreeBSD's.

This revision now requires changes to proceed.Apr 19 2023, 6:25 PM
stand/lua/menu.lua
275

Thanks for the prompt reply. Well, I will be looking closely into it while others take time to review the changes. But in case this breaks certain configuration messages, the optimistic side of me wants to believe that few lines of code after the discussion we are already having should help fix the issue to some extent.

stand/lua/menu.lua
275

If you want people to properly review your changes I suggest you read https://wiki.freebsd.org/Phabricator and follow what it says. In particular this is not a revision that does one thing and one thing only, nor have you included more than the default few lines of context. Plus skimming the changes here shows a bunch of gratuitous reorganising and renaming of questionable value. Each logical part of the revision needs to be its own revision and be adequately justified.

stand/lua/menu.lua
275

Thanks for the pointer. Having this information scattered in the Developer Primer and other places make finding the most relevant information a bit slow. your suggestions have been noted for future code review submissions.

I agree that, these changes touch on several files at once but they were all inter-related; and changing one aspect/file necessarily implied touching on few others. My apologies for any extra work this may have created or will be creating in case things move along.

The change in the names of certain variables and the refactoring in certain places were mostly added for clarity; a term that may be relative of course but the purpose was to help boost developers productivity by reducing the time it takes to figure out the logic and the use of certain variables.

Just a quick comment... I've left a longer one in reply...

This change is a little too large to be effectively reviewed. I'll spend some time tomorrow suggesting ways that you can make smaller reviews that have a good chance of getting into the tree more quickly...

stand/lua/menu.lua
275

I'm struggling myself with how to give this sort of feedback. The current code review is too large for me to give meaningful commentary on it. It mixes a number of style changes (some good, some bad) with a number of functional additions and some changes to the APIs that need some careful study (which given the size of this review is a lot harder than it needs to be and would likely take more time than I have to devote to this).

I understand that having things scattered a bit is frustrating. We've been trying to make sure all the information is at least there, but it could use a pass to make sure that it's easy to find.

There's a lot of good things in this review. I'd like to focus on those, but will likely do so by suggesting you split this or that feature out into its own review so people can focus on the change, use their time more efficiently and give you better feedback for what needs to change and to maximize chances things will wind up in the tree more quickly rather than less so. I've taken a first look, but am out of time for today (my teenager has her spring concert tonight in 2 hours, and it's almost time to leave for dinner). I'll try to get back to this tomorrow though.

stand/lua/menu.lua
275

Carefully noted! Thanks for the feedback.

The documentation still rocks despite everything; we can do better as a community though and i have no doubt this is one of the chief goals.

Looking very much forward to resolving all the issues. A bit late to send out warn wishes but i hope you had a blast with your teenager; engineering is great but family is everything.