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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
Once we've successfully loaded a kernel, we add its directory to
module_path. If we switch kernels with the kernel selector, we again prepend
the kernel directory to the current module_path and end up with multiple
kernel paths, potentially with mismatched kernel/modules, added to
module_path.
Fix it by caching module_path at load() time and using the cached version
whenever we load a new kernel.
These are the style points that I'd like to try and maintain in our lua
scripts:
- Parentheses around conditionals
- Trailing semicolons, except on block terminators
- s:method(...) instead of string.method(s, ...) where applicable
There's likely more, but that'll get hammered out as we continue.
This is the procedure that config.loadkernel tries to go through, but
reloading kernel config didn't use this function. Amend config.loadkernel to
take an optional other_kernel.
While here, be a little more verbose ("Trying to load kernel") so that it's
easy to follow where we've gone wrong.
1.) Instead of string.function(s, ...), use s:function(...)
2.) Don't try to concatenate `res`, it was just tested to be nil
3.) Note that "Loading configuration" is configured modules, and be a little
more precise in mentioning what failed ("loading of one or more modules")
An empty module_path to start with isn't ideal, but if all modules are
contained within a kernel directory (which is what we just tested) then it
isn't strictly an error. Don't assume that module_path has a value already.
When we fail to load the kernel, printing the result (which is guaranteed to
be nil) is not intended; print the name of the kernel.
These are the .lua files from from Pedro Souza's 2014 Summer of Code
project. Rui Paulo, Pedro Arthur and Wojciech A. Koszek also
contributed.
Obtained from: https://wiki.freebsd.org/SummerOfCode2014/LuaLoader
Sponsored by: Google Summer of Code
Improve the SoC lua menu code to bring it in line with forth
menu functionality
Submitted by: Zakary Nafziger
Sponsored by: FreeBSD Foundation
Use loader.setenv and loader.unsetenv instead of loader.perform
Convert from include("/boot/foo.lua") to foo = require("foo");
to bring in line with latest lua module conventions.
Enforce a uniform style for the new .lua files:
o hard tab indenation for 8 spaces
o don't have if foo then bar; else bas; end on one line
MFC After: 1 month
Relnotes: yes
Differential Review: https://reviews.freebsd.org/D14295
