Hi, The problem we try to solve with --set=VARNAME in ls. ===================================================== In the GNU Boot project (a free software distribution that releases free software boot firmware images), we provide images with (a deblobbed) Coreboot and GRUB (run as a Coreboot payload). We use GRUB mainly to find other configuration files like syslinux.cfg (to boot on external medias) or grub.cfg (to boot on the (usually GNU/Linux) distribution installed to the hard disk / SSD).
We also provide images with a SeaBIOS Coreboot payload instead, but we plan to make the images with GRUB become the preffered way of booting because in practice it works very well with the Coreboot Framebuffer, and with it we only lack a way to reliabily list the devices being present in order to be able to also find grub.cfg config files inside filesystems present on LVM logical volumes as well. The alternative to using GRUB as a Coreboot payload is to use SeaBIOS instead but that doesn't work well because when SeaBIOS loads the (usually GNU/Linux) distribution's GRUB, it results in a black screen unless the users tweak the /etc/default/grub configuration to use the 'console' output instead of the default gfxterm, and we also want less technical users to be able to easily use computers with GNU Boot. This issue is probably due to SeaVGABIOS that probably doesn't fully implement the VGA standard, so my guess is that fixing this is more work than adding --set=VARNAME to the 'ls' command. Our current GRUB configuration file is in our git repository[1] and it hardcodes devices like ahciX,Y and then tries to find the grub.cfg with (a limited) number of X,Y combination. [1]https://git.savannah.gnu.org/cgit/gnuboot.git/tree/resources/grub/config/grub.cfg Questions about the implementation ================================== The patch set that follows is far from optimal: * The 'commands/ls: add --set=VARNAME.' patch only implements --set=VARNAME for 'ls' without other arguments, and it returns an error otherwise. I'm not sure if it's the right solution but in another hand implementing --set=VARNAME for all the ls command would make the patch too big given how the implementation is done (more on that later). * The patches adding --set=VARNAME 'commands/ls: add --set=VARNAME.' changes is not very intrusive but the later patch 'commands/ls: support --set for files/directories.' shows the broader issue very clearly: all the prints are duplicated with some 'if (varname) { ... }' construct. Since here my goal is only to add '--set=VARNAME' for 'ls' without arguments, what would be the best way to proceed? Would a patch that doesn't cover all the 'ls' arguments be acceptable? If not, I guess that the way to go would be to rework a bit the printing as with the current way, there is too much duplication of code and it also makes the code harder to follow which in turn makes maintenance of this code harder. In this case what kind of API would be acceptable? Should we introduce some functions that have an argument that can select where to print? If so would something similar to fprintf be ok? It could be used like that 'grub_xfprintf( varname ? stdout : varname, "%s\n", "Hello world");' and make the code more redable than with the 'commands/ls: support --set for files/directories.' patch. Denis 'GNUtoo' Carikli (4): Add grub_env_append function. Add command to append to existing environment variables. commands/ls: add --set=VARNAME. commands/ls: support --set for files/directories. grub-core/commands/ls.c | 249 ++++++++++++++++++++++++++++++++++----- grub-core/kern/corecmd.c | 25 ++++ grub-core/kern/env.c | 38 ++++++ include/grub/env.h | 1 + 4 files changed, 282 insertions(+), 31 deletions(-) -- 2.45.1 _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
