[RFC][PATCH v1 0/4] How to add --set=VARNAME to the ls command?

[Denis 'GNUtoo' Carikli]

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