In the original lualoader project, 'escapef' and 'escapeb' were chosen for
'escape fg' and 'escape bg'. We've carried on this naming convention, and as
our use of attributes grow the likeliness of 'escapeb'/'resetb' being
confused upon glance for 'escape bold'/'reset bold' increases.
Fix this by renaming these four functions to {escape,reset}{fg,bg} rather
than {escape,reset}{f,b} for clarity.
Reported by: dteske
See: comments in the hook module about intended usage, as well as the
introduced use for config.reloaded.
Use the newly introduced hook module to define a "config.reloaded" hook.
This is currently used to register core's clearKernelCache as a reload hook
to avoid a circular dependency and fix this functionality- it didn't
actually work out, and it isn't immediately obvious how it slipped into src.
Other hook types will be introduced into the core lualoader as useful hook
points are identified.
Previously, we sent a CSI 0m sequence to reset attributes, which also reset
the color scheme if the terminal defaults didn't match what we're expecting.
Go all-in and reset the color scheme, too, just in case.
Reported by: emaste
The console may have been set for different colors before lualoader kicks
in; notably, a black-on-white color scheme is not necessarily what we're
expecting.
While here, make color.default() a composition of color.escape() instead of
rewriting the escape sequence to make it more obvious what it's achieving: a
white-on-black color scheme with no attributes set.
Reported by: emaste, whose eyes may rest easily
With autodetection turned on, hitting the filesystem everytime we need to
calculate choices for the kernel carousel is kind of slow. Cache once on the
first listing and reload it anytime the config is reloaded in case any of
the loader.conf(5) changes that affect this (kernel, kernels,
kernels_autodetect) have changed. This also picks up the case where we've
changed currdev and the autodetected kernels could change.
cli_execute was changed to return the status, cascade that to
cli_execute_unparsed.
This fixes a lot of false "Failed to execute" errors following r330620; no
failures actually occurred, but [module]_error would've then promptly
executed (and also "failed")
This applies to:
- exec
- [module]_before
- [module]_error
- [module]_after
Before this commit, these used loader.perform to execute them as a pure,
unsalted loader command. This means that they were not able to take
advantage of any Lua-salted loader commands, like boot and autoboot, or pure
Lua loader commands (functions attached to the 'cli' module).
They now have access to the full arsenal, just shy of being able to execute
arbitrary Lua.
loader.interpret should not be used for executing loader commands from an
untrusted source (e.g. environment vars) as it will allow execution of
arbitrary Lua. Replace it with a call to the recently introduced
cli_execute_unparsed, which parses it out as a loader command and then
dispatches it as a loader command. This effectively filters out arbitrary
Lua.
This will be used for scenarios where the command to execute is coming in
via the environment (from, for example, loader.conf(5)) and is thus not
necessarily trusted.
cli_execute_unparsed will immediately be used for handling
module_{before,after,error} as well as menu_timeout_command. We still want
to offer these variables the ability to execute Lua-intercepted loader
commands, but we don't want them to be able to execute arbitrary Lua.
Reviewed by: imp
Differential Revision: https://reviews.freebsd.org/D14580
Back when I "fixed" the loading of kernel/modules to be deferred until
booting, I inadvertently broke the ability to manually load a set of kernels
and modules in case of something bad having happened. lualoader would
instead happily load whatever is specified in loader.conf(5) and go about
the boot, leading to a panic loop as you try to rediscover a way to stop the
panicky efirt module from loading and fail miserably.
Reported by: me, sadly
loader.command(...) will return whatever the executed function returns, so
follow suit and return whatever loader.command() returned or whatever the
Lua function returns.
- All of our default positions were offset from forth
- Our menu frame size was smaller than in forth
- Logo/brand drawing had an off-by-one, drawing one column lower on the
screen than they should have been.
- While here, switch a print() to printc() as it's expected that logos may
contain color and other escpae sequences that we'll need to honor.
It may be set to "left" or "right" -- any other value will cause the title
to be centered.
I've chosen to position these things just inside the vertical borders,
rather than overlapping the corners. This is an arbitrary choice and easily
amendable if this looks terrible.
Rather than before the menu is drawn. The drawer is going to reset the
crusor position as soon as it draws anything anyways, so doing it before
serves no purpose. Setting it after is needed so we don't clobber the menu
when we start booting.
r330282 registered loader.printc as printc, so use it instead. This makes
sense for a couple reasons, the major point being that it reads a little bit
easier and pairs nicely with the global 'print'.
Similar cases can not really be made for other loader.* functions as most of
them are either highly specific to our use-case or usually available in
other modules, such as `os`. printc does not have a standard implementation
in the Lua world(*), so we have a little more leeway with it, and it's kind
of a special case of the globally available 'print'.
(*) I've been in the Lua world for all of two weeks, so this could be wrong.
- Add drawer.frame_styles to map out the kinds of characters we need for the
different loader_menu_frame values
- Respect loader_menu_frame, default to double[*]
- (imp) Use loader.printc instead of print- print adds a newline to the
output, which is not the right thing we want to be doing.
- (imp) Draw horizontal frames a little more efficiently- setting the cursor
after every line segment is horribly inefficient, especially on serial
consoles. Halve the number of characters written at the expense of an
additional loop to draw the bottom frame, which is likely more efficient
in the long run for some of less ideal scenarios.
[*] menu.4th(8) claims that the default here was single, but unset
loader_menu_frame yielded double and we didn't have any overrides in the
default loader.conf(5), so double it is.
Distribution will be done after all of the lualoader manpages are created.
Reviewed by: rpokala
Differential Revision: https://reviews.freebsd.org/D14480
Distribution will be done after all of the lualoader manpages are created.
Reviewed by: rpokala
Differential Revision: https://reviews.freebsd.org/D14479
Rather than hardcoding these things. This could lead to some form of loader
localization later, but the main goal at the moment is to get a clear view
of the strings we're outputting and strive to use more string.format() and
less wild concatenation all over the place.
We've included an extra '0' in there (which might get removed later, but
it's maintained for the moment for legacy purposes) which oftentimes
indicate that the following number should be treated as octal. This is not
the case, so note that to prevent future confusion (of myself and others).
Our module bits ended up more stable than I anticipated, so this turns out
to be no longer useful.
If things like this need to come back, we should do it in a separate 'debug'
module to serve as a collection of debugging aides. As a rule, this 'debug'
module would *not* be allowed as a requirement of any other modules in-tree.
- Add screen.default_x and screen.default_y to determine where
screen.defcursor resets the cursor to.
- Use screen.setcursor in screen.defcursor instead of rewriting the escape
sequence.
- Use screen.default_y when resetting the cursor after writing the new
twiddle character, add a comment verbally describing the position just in
case.
It worked on my test setup, but is clearly non-functional on others.
Further examination of check-password.4th showed that it actually reset the
cursor to 0,25 every time and overwrote the previous password prompt. Do
that, and also clear the "Incorrect Password" text if the correct password
gets entered.
twiddle_pos didn't need to be a module-scope local, since it's going to get
reset with every read anyways- it was left-over from other things.
screen.movecursor with a y=-1 setting was from a test of movecursor,
resulting in the twiddle characters being drawn going up the console and
looking quite funky.
This gives some form of feedback while typing, and matches-(ish*) Forth
behavior. The cursor generally rests two column after the password prompt,
then the twiddle is drawn three columns later and the cursor reset to
resting position after being drawn.
I've removed the note about re-evaluating it for security considerations and
instead set it up as a module-local variable that we can set later depending
on environment or something. It's set to false with no chance of changing at
the moment.
*As close as I can tell from reading check-password.4th, because I don't
have an easy test (or deployed) setup for forth loader to check how close
it is. Please do mention if it's not close enough.
This is motivated by a want to reduce heap usage if the menu is being
skipped. Currently, the menu module must be loaded regardless of whether
it's being skipped or not, which adds a cool ~50-100KB worth of memory
usage.
Move the menu skip logic out to core (and remove a debug print), then check
in loader.lua if we should be skipping the menu and avoid loading the menu
module entirely if so. This keeps our memory usage below ~115KB for a boot
with the menu stripped.
Also worth noting: with this change, we no longer explicitly invoke autoboot
if we're skipping the menu. Instead, we let the standard loader behavior
apply: try to autoboot if we need to, then drop to a loader prompt if not or
if the autoboot sequence is interrupted. The only thing we still handle
before dropping to the loader autoboot sequence is loadelf(), so that we can
still apply any of our kernel loading behavior.
screen was also guilty of not-so-great argument names, but it was also
guilty of handling color sequences on its own. Change those bits to using
the color module instead.
As a side note, between color and screen, I'm not 100% sure that returning
the color_value is the right thing to do if we won't generate the escape
sequences. This should be re-evaluated at a later time, and they should
likely return nil instead.
Instead of a single-letter parameter ('m'), use something a little more
descriptive and meaningful: 'menudef' ("menu definition") -- these functions
expect to be passed a menudef, so call it what it is.
While here, throw an assertion in that we have a handler for the selected
menu item. This is more of a debugging aide so that it's more obvious when
one is testing a menudef that they've added an entry item that we don't
handle.
This is an improvement over the past behavior of ignoring the unknown menu
entry.
This cleans up the odd approach to menu drawing. Instead of tracking
validity, we track the menu that was drawn on the screen. Whenever we draw a
menu, we'll set this to that menu.
Anything that invalidates the screen should go ahead and trigger an explicit
redraw, rather than finding a wy to set screen_invalid.
The currently drawn menu is then reset in menu.run as we exit the menu
system, so that dropping to the loader prompt or leaving menu.run() will
just behave as expected without doing redundant work every time we leave a
menu.
In the common case, this will effectively do nothing as the menu will get
redrawn as we leave submenus regardless of whether the screen has been
marked invalid or not
However, upon escape to the loader prompt, one could do either of the
following to re-enter the menu system:
-- Method 1
require('menu').run()
-- Method 2
require('menu').process(menu.default)
With method 1, the menu will get redrawn anyways as we do this before
autoboot checking upon entry. With method 2, however, the menu will not be
redrawn without this invalidation.
Both methods are acceptable for re-entering the menu system, although the
latter method in the local module for processing new and interesting menus
is more expected.
cli_execute is likely the only exception that we should make, due to it
being a global. We don't really need other globals, so this won't really end
up an epidemic.
There's no reason for autoboot handling to be mixed in with menu processing.
It is a distinct process that should only be done once when entering the
menu system.
menu.process has been modified to take an initial keypress to process and to
only draw the screen initially if it's been invalidated. The keypress is
kind of a kludge, although it could be argued to be a potentially useful
kludge if there are other processes that may need to feed a keypress into
the menu system.
In general, every menu redraw is going to require a screen clear and cursor
reset. Each redraw also has the potential to invalidate the alias table, so
we move the alias table being used out into a module variable. This allows
third party consumers to also inspect or update the alias table if they need
to.
While here, stop searching the alias table once we've found a match.
This is driven by an urge to separate out the bits that really only need to
happen when the menu system starts up. Key points:
- menu.process now does the bulk of menu handling. It retains autoboot
handling for dubious reasons, and it no longer accepts a 'nil' menu to
process as 'the default'. Its return value is insignificant.
- The MENU_SUBMENU handler now returns nothing. If menu.process has exited,
then we continue processing menu items on the parent menu as expected.
- menu.run is now the entry point of the menu system. It checks whether the
menu should be skipped, processes the default menu, then returns.
These indices were assigned the same values as they would've been implicitly
assigned anyways.
While here, throw terminating commas after the last value of tables.
It should use the common parser, but it should not be processed like a
standard file. Rewite check_nextboot to read the file in, check whether it
should continue, then parse as needed.
This allows us to throw the recently introduced check_and_halt callback
swiftly out the window.
config.parse is now purely a parser, rather than a whole proccessor. The
standard process for loading a config file has been split out into
config.processFile.
This clears the way for having nextboot read its own config file and decide
there whether it should parse the rest of the file.
This is step 1 towards revoking config.parse of it I/O privileges. Ideally,
all reading would be done before config.parse and config.parse would just
take text and parse it rather than being charged with the entire process.
Functionally, the latter error wouldn't necessarily hurt anything. io.write
will just error out as it's not passed a valid file handle. Still, we can do
better than that.
The functionality was correct, but our style guidelines tend to request that
we shy away from using boolean operations in place of explicit comparisons
to nil.
config.parse now takes an extra callback that is invoked on the full text of
the config file. This callback dictates where we should actually try to
parse this file or not.
For nextboot, we use this to halt parsing if we see 'nextboot_enable="NO"'.
If we don't, parse it and write 'nextboot_enable="NO" ' to it. The same
caveat as with forth still applies- writing is only supported by UFS.
The latter is good, but the former is more elegant and clear about what 'x'
is. Adopt it, preferably only using the latter kind of notation where needed
as values for tables.
I've also made some not-insignificant changes/additions to this file, to
include the added constants, ACPI changes, boot environment listing, and
some utility functions.
Graphics have a tendency to cause 80-col issues, so make an exception to our
standard indentation guidelines for these graphics. This does not hamper
readability too badly.
Two 40-column strings of spaces is trivially replaced with
string.rep(" ", 80)
luacheck pointed out an assortment of issues, ranging from non-standard
globals being created as well as unused parameters, variables, and redundant
assignments.
Using '_' as a placeholder for values unused (whether it be parameters
unused or return values unused, assuming multiple return values) feels clean
and gets the point across, so I've adopted it. It also helps flag candidates
for cleanup later in some of the lambdas I've created, giving me an easy way
to re-evaluate later if we're still not using some of these features.
Instead of the global namespace, let's attach these to the cli module. Other
users, including the "local" module, can attach functions to the cli module
at will to add other cli commands and things will still Just Work.
This distills down the candidates for functions that may be invoked via the
cli to a minimal set (boot, autoboot, arguments), rather than any function
that happens to live in the global lua namespace.
This will be the translation layer for varargs -> cmd_name, argv for cli
commands. We reserve the right to break exactly what the varargs inclulde,
but this gives us a stable way to pull the arguments out of varargs.
This module will, in the not-so-distant future, grow functionality for
reducing boilerplate in functions that implement cli commands. It will
likely also house most in-tree cli commands.
This seems to have been arbitrary; bootlock_password and password don't seem
to have any documented length restrictions, and loader(8) probably shouldn't
care about whatever GELI passphrase length restrictions might exist.
Reported by: Kalle Carlbark <kalle.carlbark+freebsd@kcbark.net>
Attempt to autoboot when we open the default menu, and only when we open the
default menu. This alleviates the need for checking menu.already_autoboot,
because we're not trying to autoboot every time we open a submenu.
I note that escaping to loader prompt and going back to the menu (by running
require('menu').run() at the loader prompt) will happily work and not
re-initiate the autoboot sequence since "Escape to loader prompt" disables
the autoboot_delay.
Instead of based it off of whether 'kernels' was specified, base it off of a
new variable: kernels_autodetect. If set to yes, we'll run the autodetection
bits and add any detected kernels to the already existing list *after* both
'kernel' and 'kernels'.
This looks a little bit differently than the forth version for the time
being, just to get off the ground- rather than a paging system, it's
implemented as a simple carousel like the kernel selector.
Reviewed by: cem
Differential Revision: https://reviews.freebsd.org/D14436
This matches forth behavior. Hitting "6" when autobooting at the welcome
menu will now take you directly to the "Boot Options" menu.
We likely have some slight optimizations we should make, like not checking
autoboot every time we open a new menu and things of this nature. Further
work will go towards this end.
Allow "name" entries to be simple strings, instead of just functions. We
know whether we support colors or not by the time any of this is setup, so
all menu names that are basically static with colors sprinkled in are good
candidates for simplification.
Also simplify "func" in many cases where it's just invoking another function
with no arguments. The downside to this simplification is that the functions
called can no longer be trivially replaced by a local module. The upside is
that it removes another layer of indirection that we likely don't need.
These can be re-evaluated later if a compelling argument is raised, on a
case-by-case basis, for replacement.
The intent here is to abstract away the name of the default menu. The
default menu is still the welcome menu, but this detail doesn't need to
matter to things outside of the menu module. You may change the default
menu, but one would need to modify a specific menu.
Provide a way for out-of-tree users of lualoader to patch into the loader
system without having to modify our distributed scripts.
Do note that we can't really offer any API compatibility guarantees at this
time due to the evolving nature of lualoader right now.
This still has some utility as local modules may add commands at the loader
prompt without relying heavily on lualoader features- this specific
functionality is less likely to change without more careful consideration.
Reviewed by: cem (earlier version)
Differential Revision: https://reviews.freebsd.org/D14439
I can't find any good reason these aren't enabled, so enable them.
The silent runs will only return false on actual parse errors, so it's ok to
be loud about those failures.
This was also a convenience convention (for me) that is not very lua-tic.
Drop it.
I've maintained some parentheses where I'd prefer them, for example,
'if x or y or (z and w) then', but these situations are far and few between.
This was previously chosen out of convenience, as we had a mixed style and
needed to be consistent. I started learning Lua on Friday, so I switched
everything over. It is not a very lua-nic convention, though, so drop it.
Excessive parenthesizing around conditionals is next on the chopping block.
Track the latest value we've set an environment variable to, and only
restore those that are unchanged from that.
This gives us some leeway to make sure we're not clobbering variables
overwritten by menu changes.
This should be functional and roughly equivalent to the Forth version.
Stop doing a loadelf() on menu exit now that we can DTRT with boot
invocations. autoboot interception will follow not long after.
core.boot and core.autoboot may both take arguments; add a helper to cleanly
append an argstring to the given loader command.
Also provide a popFrontTable() that we'll use pop the command name off of an
argv table. We don't have the table library included, and including it is
non-trivial, so we'll implement this one function that we need in lua for
the time being.
If the user's selected a kernel, we really should be trying to load that one
instead of falling back to some default kernel.
This should generally be a no-op and most desirable, unless you really
enjoyed surprises.
Carousel storage doesn't need to happen in the menu module, and indeed
storing it there introduces a circular reference between drawer and menu
that only works because of global pollution in loader.lua.
Carousel choices generally map to config entries anyways, making it as good
of place as any to store these. Move {get,set}CarouselIndex functionality
out into config so that drawer and menu may both use it. If we had more
carousel functionality, it might make sense to create a carousel module, but
this is not the case.
If we failed to execute the input line as pure lua, run the command through
parse for consistent argument parsing. Pass the parsed arguments through to
a global "cli_execute" written in Lua, which is expected to either handle it
or pass it back through to interp_builtin_cmd (via loader.command).
lua-handled cli commands will then exist as globals in whatever module they
most belong in, and invocations at the loader prompt will magically dispatch
to them if they exist.
Reviewed by: imp
Differential Revision: https://reviews.freebsd.org/D14450
We follow pretty closely the following structure of a module:
1. Copyright notice
2. Module requires
3. Module local declarations
4. Module local definitions
5. Module exports
6. return
Re-organize the one-offs (config/drawer) and denote the start of module
exports with a comment.
Declare these adjacent to the local definitions at the top of the module,
and make sure they're actually declared local to pollute global namespace a
little bit less.
This refactor makes it straightforward to add new logos for drawing and
organizes logo definitions in a logical manner.
The graphic to be drawn for each logo may again be modified outside of
drawer, but it must be done on a case-by-case basis as a modification to the
loader_logo.
As part of an effort to slowly reduce our exports overall to a set of stable
properties/functions, go ahead and reduce what drawer exposes to others.
The graphics should generally not be modified on their own, but their
position could be modified if additional grahics also need to be drawn.
Export position/shift information, but leave the actual graphic local to
the module.
The next step will be to create a 'menudef' that gets exported instead, with
each entry in the menudef table describing the graphic to be drawn along
with specific positioning information.
Pull out specialized naming behavior into drawer.menu_name_handlers. This is
currently only needed for the carousel entry, where naming is based on the
current choice and the menu item purposefully does not store this state.
The setup was designed so that every type can have a special name handler,
and the default action is to simply use the result of entry.name().
This is a bit cleaner than our former method of an if ... else chain of
handlers. Store handlers in the menu.handlers table so that they may be
added to or removed dynamically.
All handlers take the current menu and selected entry as parameters, and
their return value indicates whether the menu processor should continue or
not. An omitted return value or 'true' will indicate that we should
continue, while returning 'false' will indicate that we should exit the
current menu.
The omitted return value behavior is due to continuing the loop being the
more common situation.
Building the swapped welcome menu (first two items swapped) is kind of a
sluggish, because it requires a full (recrusive) shallow copy of the welcome
menu. Cache the result of that and re-use it later, instead of building it
everytime.
While here, don't create temporary locals just for swapping. The following
is just as good:
x, y = y, x;
Reported by: Alexander Nasonov <alnsn@yandex.ru> (swapping)
[Enter] should be moved to the single user menu item when we swap them.
Define a non-standard menu entry function "alternate_name" to use for this
purpose for ultimate flexibility if we change our minds later. When we're
booting single user, make a shallow copy of the menu that we'd normally
display and swap the items and their name functions to use alternate_name
instead. Toggling single user in the options menu and going back to the main
menu will now correctly reflect the current boot setting with the first two
menu options and "[Enter]" will always be on the right one.
This shallow copy technique has the chance of being quite slow since it's
done on every redraw, but in my testing it does not seem to make any obvious
difference.
shallowCopyTable could likely belong better in a general-purpose utility
module, but this (and the key constnats) are the only candidates we have at
the moment so we'll drop it into our core stuff for the moment and consider
re-organization at a later date.
lualoader does a pretty good job of reverting any environment changes now.
It will even wipe out boot_verbose if it's set explicitly in loader.conf(5)
and overwritten in the boot options menu.
Future work will likely change this, as explicit choices made in the menu
should probably override the new loader.conf(5). I don't suspect this will
cause much grief, though, so it is not a high priority until boot
environment support actually lands.
This will be used when boot environment support lands to make a good-faith
effort to apply any new loader.conf(5) environment settings atop the default
configuration that we started with.
If we've fetched menu.entries and it turns out it's a function, call it to
get the actual menu entries.
This will be used to swap multi-/single- user boot options if we're booting
single user by default (boot_single="YES" in loader.conf(5)). It can also be
used fairly easily for other non-standard situations.
Instead of directly listing them in menu.welcome and menu.boot_options,
store them at menu.welcome.entries and welcome.boot_options.entries.
This will come into play later when we need to re-order the welcome menu if
boot_single is specified.
Menus are actually defined as entries in the 'menu' table. These local
declarations have not been used in the history of our in-tree lua scripts,
so give them the boot.
Loading the kernel and modules can be really slow. Loading before the menu
draws and every time one changes kernel/boot environment is even more
painful.
Defer loading until we either boot, auto-boot, or escape to loader prompt.
We still need to deal with configuration changes as the boot environment
changes, but this is generally much quicker.
This commit strips all ELF loading out of config.load/config.reload so that
these are purely for configuration. config.loadelf has been created to deal
with kernel/module loads. Unloading logic has been ripped out, as we won't
need to deal with it in the menu anymore.
Discussed in part with: allanjude
r329550 introduced config.kernel_loaded. config.load() doesn't provide a
means of overriding the kernel to load, but that likely isn't necessary as
it will not be a common case. When loading the kernel, just attempt to load
the kernel previously loaded and specified in config.kernel_loaded.
If we haven't loaded a kernel yet, config.kernel_loaded will be unset/nil
and the "default"/first kernel found will be loaded. If we've loaded a
kernel, we'll try to load that same kernel again and fallback to the default
kernel if we need to.
This in also in support of upcoming boot environment support.
'nil' means the 'first kernel found in module_path', which is the same
interpretation as passing 'nil' to loadkernel.
Otherwise, it denotes the name of a kernel that we've successfully loaded.
When reloaded later, we will still need to do the full search again to
locate the actual kernel in case things have changed, so just the name is
good enough.
This is in support of upcoming boot environment support. vfs.root.mountfrom
and currdev will be changed, then we will reload configuration and attempt
to reload the currently chosen kernel unless we shouldn't.
In the worst case scenario, we have no passwords to prompt for and we end up
just clearing the screen twice before we draw the menu or proceed with boot.
In the best case scenario, we don't try drawing password prompts amidst a
bunch of kernel/module loading.
Some other points I think we need to be consistent on:
- Spacing around string concatenation (always)
- Test against 'nil' explicitly rather than relying on 'not' for things that
reasonably won't be returning a boolean. e.g. loader.getenv
Eventually this will all get formalized somewhere.
