Re: [gentoo-catalyst] [PATCH 1/5] embedded: remove actions that are broken by default

[Matt Turner]

On Thu, May 21, 2020 at 3:08 PM Brian Dolbec <dolsen@g.o> wrote:
>
> On Thu, 21 May 2020 13:41:18 -0700
> Matt Turner <mattst88@g.o> wrote:
>
> > On Thu, May 21, 2020 at 10:34 AM Daniel Cordero
> > <gentoo.catalyst@××××.ws> wrote:
> > >
> > > From: Daniel Cordero <catalyst@××××.io>
> > >
> > > dir_setup() doesn't exist, bootloader() exists but requires specific
> > > tools to be installed in the seed stage and doesn't check that they
> > > are, causing the build to fail.
> > > ---
> > > If I have misconstrued the purpose of bootloader, then documentation
> > > needs to be written.
> >
> > I don't actually see any documentation about
> > >  catalyst/targets/embedded.py | 2 --
> > >  1 file changed, 2 deletions(-)
> > >
> > > diff --git a/catalyst/targets/embedded.py
> > > b/catalyst/targets/embedded.py index aa23f5b3..1b4ad9d6 100644
> > > --- a/catalyst/targets/embedded.py
> > > +++ b/catalyst/targets/embedded.py
> > > @@ -41,7 +41,6 @@ class embedded(StageBase):
> > >
> > >      def set_action_sequence(self):
> > >          self.settings['action_sequence'] = [
> > > -            "dir_setup",
> >
> > Nice. This function was removed in 2005, so the embedded target has
> > been broken ever since. That certainly answers my question as to
> > whether anyone uses it.
> >
> > Fixes: 1dafb5fa06d2 (Add locking support. ...)
> >
> > So, you must use the embedded target. Could you tell me how you use
> > it, for what device, etc?
> >
> > >              "unpack",
> > >              "config_profile_link",
> > >              "setup_confdir",
> > > @@ -51,7 +50,6 @@ class embedded(StageBase):
> > >              "setup_environment",
> > >              "build_kernel",
> > >              "build_packages",
> > > -            "bootloader",
> >
> > It's not obvious to me what specific tools this requires to be
> > installed in the seed stage. Presumably you're referring to this?
> >
> > cmd([self.settings['controller_file'], 'bootloader',
> > self.settings['target_path'].rstrip('/')]
> >
> > which eventually runs bootloader-setup.sh.
> >
> > I'd assume it's generally sensible to install a bootloader in the
> > embedded target, and I don't think we have mechanism for customizing
> > the action_sequence, so if we remove bootloader from the list then I
> > think the python bootloader() function is just dead code, isn't it? I
> > doubt that's the end result we want.
> >
>
>
> No, if you notice, this is removing the bootloader action sequence only
> for the embedded target.  Each target subclasses stagebase, so can
> override what is defined in stagebase as needed.  Customizing the
> action_sequence is something that pretty much all the targets do.

Sorry I misspoke. Yes, bootloader() is called for livecd_stage2 and
stage4 as well, so removing it from embedded's action sequence would
not make the function dead.

I have doubts that we actually want to not run bootloader() for the
embedded target though.