Re: [PATCH] Split of normal mode (version 2)

["Yoshinori K. Okuji" <okuji@enbug.org>]

On Saturday 04 April 2009 04:49:36 Bean wrote:
> On Sat, Apr 4, 2009 at 2:40 AM, Yoshinori K. Okuji <okuji@enbug.org> wrote:
> > On Tuesday 31 March 2009 02:41:14 Bean wrote:
> >> Hi,
> >>
> >> This new patch make some changes based on the discussion of previous
> >> patch.
> >>
> >> 1, Move script engine to script/sh (sh.mod)
> >> 2, Move generic menu code to menu (menu.mod)
> >> 3, Move text menu viewer to menu/text (textmenu.mod)
> >> 4, Move misc function to lib (misc.mod)
> >> 5, Move setjmp to lib (setjmp.mod)
> >
> > I don't agree on the last two. Also, I don't like that you have just
> > removed the rescue command.
>
> Hi,
>
> The reason for a separate misc.mod is that it contains pure function
> and has no side effect, so other modules can use them freely. In the
> other hand, normal.mod does a lot of tasks in the init function, it's
> not a good place for common function. Besides, there are already
> helper function in the lib directory, like crc and hexdump, it's not
> piratical to put each one in a new module, we might just merge them
> into one helper module.

We can just put them in the normal.mod. What is wrong? Frankly, your argument 
reminds me of the old discussion about monolithic vs. micro kernels...

> The problem with setjmp is that it's platform dependent, so if we keep
> it in normal.mod, we must define it in very platform rmk file. But now
> setjmp is not used in normal.mod anymore, we could move it out, and
> thus make normal.mod platform independent.

setjmp is required for the switch between rescue mode and normal mode. Don't 
remove it. "Hey, this is easier to maintain" is not a good reason to drop an 
useful feature. If your intention is to reduce the maintenance cost, you can 
simply define a variable for common files in common.rmk and use it in each 
machine-specific rmk file.

> >> Now normal.mod only contains the reader code. To configure script
> >> engine and viewer, you should add these lines at the beginning of
> >> grub.cfg:
> >>
> >> insmod sh
> >> handler parser sh
> >> insmod textmenu
> >
> > I prefer a more sophisticated approach (note: I hate manual loading).
> >
> > For example, we can allow a config file to have a shebang, like "#!sh".
> > If not specified, GRUB can assume that "sh" is used, and load it
> > automatically. This kind of technique could even allow for using
> > different languages in one setup.
> >
> > For textmenu, I think it makes sense to have a command "textmenu". Just
> > like "boot", GRUB can execute "textmenu" implicitly if a config file
> > defines any menu entry but does not execute any menu command. This way,
> > textmenu is automatically loaded.
>
> I'm thinking about using environment variable to set handler, for
> example, we could set it like this:
>
> set parser=sh
> set menu=textmenu
> set terminal_output=gfxterm
>
> We could use variable hooks to load the modules on demand.
>
> For textmenu, we could add some test in normal.mod. After the main
> config file is parsed, if it still doesn't contain a menu viewer, we
> load textmenu automatically.

I agree that terminal_output (and so terminal_input as well) can be 
implemented as environment variables, but it is very questionable whether a 
parser or a menu interface can be considered as an environment variable. An 
important question is how you would determine the name of a module from the 
name of a parser or a menu interface.

Regards,
Okuji