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

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

[Daniel Cordero]

From: Daniel Cordero <catalyst@0xdc.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.
 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",
             "unpack",
             "config_profile_link",
             "setup_confdir",
@@ -51,7 +50,6 @@ class embedded(StageBase):
             "setup_environment",
             "build_kernel",
             "build_packages",
-            "bootloader",
             "root_overlay",
             "fsscript",
             "unmerge",
-- 
2.26.2

[gentoo-catalyst] [PATCH 2/5] stagebase: allow specfiles to define install_mask

[Daniel Cordero]

From: Daniel Cordero <catalyst@0xdc.io>

There is some code that uses this option - set_install_mask() - but it was not
added to the list of valid specfile options.

The only mention of use is in the embedded target, but it may also be useful
for other stages.
---
install_mask allows the use of wildcard glob patterns, where */rm
doesn't.

 catalyst/base/stagebase.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py
index 651bf4e4..3e8f074e 100644
--- a/catalyst/base/stagebase.py
+++ b/catalyst/base/stagebase.py
@@ -54,6 +54,7 @@ class StageBase(TargetBase, ClearBase, GenBase):
             "fcflags",
             "fflags",
             "hostuse",
+            "install_mask",
             "kerncache_path",
             "ldflags",
             "makeopts",
-- 
2.26.2

Re: [gentoo-catalyst] [PATCH 2/5] stagebase: allow specfiles to define install_mask

[Matt Turner]

On Thu, May 21, 2020 at 10:34 AM Daniel Cordero <gentoo.catalyst@xxoo.ws> wrote:
>
> From: Daniel Cordero <catalyst@0xdc.io>
>
> There is some code that uses this option - set_install_mask() - but it was not
> added to the list of valid specfile options.
>
> The only mention of use is in the embedded target, but it may also be useful
> for other stages.
> ---
> install_mask allows the use of wildcard glob patterns, where */rm
> doesn't.
>
>  catalyst/base/stagebase.py | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py
> index 651bf4e4..3e8f074e 100644
> --- a/catalyst/base/stagebase.py
> +++ b/catalyst/base/stagebase.py
> @@ -54,6 +54,7 @@ class StageBase(TargetBase, ClearBase, GenBase):
>              "fcflags",
>              "fflags",
>              "hostuse",
> +            "install_mask",
>              "kerncache_path",
>              "ldflags",
>              "makeopts",
> --
> 2.26.2

Nice. I'll add a bit of documentation to the man page when I commit this.

[gentoo-catalyst] [PATCH 3/5] embedded/rm: use destpath to clean mergeroot

[Daniel Cordero]

From: Daniel Cordero <catalyst@0xdc.io>

When using remove() with a capture subpath (e.g. the embedded target), specifying
files in rm removes the files from the seed stage, not the captured stage.

Use destpath as a base for removals. When not using a subpath, destpath
equals chroot_path.

Also unhardcode log prefix to match the target.
---
 catalyst/base/stagebase.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py
index 3e8f074e..ed9bb697 100644
--- a/catalyst/base/stagebase.py
+++ b/catalyst/base/stagebase.py
@@ -1171,8 +1171,8 @@ class StageBase(TargetBase, ClearBase, GenBase):
                 for x in self.settings[self.settings["spec_prefix"] + "/rm"]:
                     # We're going to shell out for all these cleaning
                     # operations, so we get easy glob handling.
-                    log.notice('livecd: removing %s', x)
-                    clear_path(self.settings["chroot_path"] + x)
+                    log.notice('%s: removing %s', self.settings["spec_prefix"], x)
+                    clear_path(self.settings["destpath"] + x)
                 try:
                     if os.path.exists(self.settings["controller_file"]):
                         cmd([self.settings['controller_file'], 'clean'],
-- 
2.26.2

Re: [gentoo-catalyst] [PATCH 3/5] embedded/rm: use destpath to clean mergeroot

[Matt Turner]

On Thu, May 21, 2020 at 10:34 AM Daniel Cordero <gentoo.catalyst@xxoo.ws> wrote:
>
> From: Daniel Cordero <catalyst@0xdc.io>
>
> When using remove() with a capture subpath (e.g. the embedded target), specifying
> files in rm removes the files from the seed stage, not the captured stage.

Hah, nice.

> Use destpath as a base for removals. When not using a subpath, destpath
> equals chroot_path.
>
> Also unhardcode log prefix to match the target.
> ---
>  catalyst/base/stagebase.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py
> index 3e8f074e..ed9bb697 100644
> --- a/catalyst/base/stagebase.py
> +++ b/catalyst/base/stagebase.py
> @@ -1171,8 +1171,8 @@ class StageBase(TargetBase, ClearBase, GenBase):
>                  for x in self.settings[self.settings["spec_prefix"] + "/rm"]:
>                      # We're going to shell out for all these cleaning
>                      # operations, so we get easy glob handling.
> -                    log.notice('livecd: removing %s', x)
> -                    clear_path(self.settings["chroot_path"] + x)
> +                    log.notice('%s: removing %s', self.settings["spec_prefix"], x)
> +                    clear_path(self.settings["destpath"] + x)
>                  try:
>                      if os.path.exists(self.settings["controller_file"]):
>                          cmd([self.settings['controller_file'], 'clean'],

Looks good. Thanks!

[gentoo-catalyst] [PATCH 4/5] embedded/root_overlay: copy root overlay into destpath

[Daniel Cordero]

From: Daniel Cordero <catalyst@0xdc.io>

When using a merge subpath (e.g. the embedded target's /tmp/mergeroot),
root_overlay() copies the overlay into the seed stage, rather than the subpath.

Copy the files into destpath. When not using a subpath, destpath equals
chroot_path (so other stages are not affected).
---
This is unpacked after the build. I don't see any possible reason why the
embedded target would need customisations after this point, and it
didn't match the behaviour of e.g. stage4/root_overlay.

 catalyst/base/stagebase.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py
index ed9bb697..67c2d3e3 100644
--- a/catalyst/base/stagebase.py
+++ b/catalyst/base/stagebase.py
@@ -841,7 +841,7 @@ class StageBase(TargetBase, ClearBase, GenBase):
                                    "/root_overlay"]:
                 if os.path.exists(x):
                     log.info('Copying root_overlay: %s', x)
-                    cmd(['rsync', '-a', x + '/', self.settings['chroot_path']],
+                    cmd(['rsync', '-a', x + '/', self.settings['destpath']],
                         env=self.env)
 
     def bind(self):
-- 
2.26.2

Re: [gentoo-catalyst] [PATCH 4/5] embedded/root_overlay: copy root overlay into destpath

[Matt Turner]

On Thu, May 21, 2020 at 10:34 AM Daniel Cordero <gentoo.catalyst@xxoo.ws> wrote:
>
> From: Daniel Cordero <catalyst@0xdc.io>
>
> When using a merge subpath (e.g. the embedded target's /tmp/mergeroot),
> root_overlay() copies the overlay into the seed stage, rather than the subpath.
>
> Copy the files into destpath. When not using a subpath, destpath equals
> chroot_path (so other stages are not affected).
> ---
> This is unpacked after the build. I don't see any possible reason why the
> embedded target would need customisations after this point, and it
> didn't match the behaviour of e.g. stage4/root_overlay.
>
>  catalyst/base/stagebase.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py
> index ed9bb697..67c2d3e3 100644
> --- a/catalyst/base/stagebase.py
> +++ b/catalyst/base/stagebase.py
> @@ -841,7 +841,7 @@ class StageBase(TargetBase, ClearBase, GenBase):
>                                     "/root_overlay"]:
>                  if os.path.exists(x):
>                      log.info('Copying root_overlay: %s', x)
> -                    cmd(['rsync', '-a', x + '/', self.settings['chroot_path']],
> +                    cmd(['rsync', '-a', x + '/', self.settings['destpath']],
>                          env=self.env)
>
>      def bind(self):
> --
> 2.26.2

Makes sense to me. Thanks!

[gentoo-catalyst] [PATCH 5/5] Allow embedded target to unpack a root_overlay

[Daniel Cordero]

From: Daniel Cordero <catalyst@0xdc.io>

This was broken due to the previous commit.
---
 catalyst/targets/embedded.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/catalyst/targets/embedded.py b/catalyst/targets/embedded.py
index 1b4ad9d6..99739512 100644
--- a/catalyst/targets/embedded.py
+++ b/catalyst/targets/embedded.py
@@ -31,6 +31,7 @@ class embedded(StageBase):
         "embedded/mergeroot",
         "embedded/packages",
         "embedded/rm",
+        "embedded/root_overlay",
         "embedded/runscript",
         "embedded/unmerge",
         "embedded/use",
-- 
2.26.2

Re: [gentoo-catalyst] [PATCH 5/5] Allow embedded target to unpack a root_overlay

[Matt Turner]

On Thu, May 21, 2020 at 10:34 AM Daniel Cordero <gentoo.catalyst@xxoo.ws> wrote:
>
> From: Daniel Cordero <catalyst@0xdc.io>
>
> This was broken due to the previous commit.

I'm confused. The previous commit didn't break anything that this
patch fixes, as far as I can tell. This patch just enables
root_overlay for the embedded target, right?

If so, I think the commit is right, but that we should remove the
incorrect sentence from the commit message.

Re: [gentoo-catalyst] [PATCH 5/5] Allow embedded target to unpack a root_overlay

[Daniel Cordero]

On Thu, May 21, 2020 at 01:52:55PM -0700, Matt Turner wrote:
> On Thu, May 21, 2020 at 10:34 AM Daniel Cordero <gentoo.catalyst@xxoo.ws> wrote:
> >
> > From: Daniel Cordero <catalyst@0xdc.io>
> >
> > This was broken due to the previous commit.
> 
> I'm confused. The previous commit didn't break anything that this
> patch fixes, as far as I can tell. This patch just enables
> root_overlay for the embedded target, right?
> 
> If so, I think the commit is right, but that we should remove the
> incorrect sentence from the commit message.
> 

Whoops, I misspoke. It was broken 'until' the previous commit.

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

[Matt Turner]

On Thu, May 21, 2020 at 10:34 AM Daniel Cordero <gentoo.catalyst@xxoo.ws> wrote:
>
> From: Daniel Cordero <catalyst@0xdc.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.

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

[Brian Dolbec]

On Thu, 21 May 2020 13:41:18 -0700
Matt Turner <mattst88@gentoo.org> wrote:

> On Thu, May 21, 2020 at 10:34 AM Daniel Cordero
> <gentoo.catalyst@xxoo.ws> wrote:
> >
> > From: Daniel Cordero <catalyst@0xdc.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.

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@gentoo.org> wrote:
>
> On Thu, 21 May 2020 13:41:18 -0700
> Matt Turner <mattst88@gentoo.org> wrote:
>
> > On Thu, May 21, 2020 at 10:34 AM Daniel Cordero
> > <gentoo.catalyst@xxoo.ws> wrote:
> > >
> > > From: Daniel Cordero <catalyst@0xdc.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.

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

[Daniel Cordero]

On Thu, May 21, 2020 at 01:41:18PM -0700, Matt Turner wrote:
> So, you must use the embedded target. Could you tell me how you use
> it, for what device, etc?

I produce mini-systemd tarballs to run under systemd-nspawn (or other
container systems). Sometimes, those tarballs can be used as
installation tarballs on bare metal systems, but in those cases
I need to bring my own kernel and bootloader.

systemd does require dynamic library dependencies so I modified (patches
probably will not sent upstream for inclusion) the embedded target to
pull in dependencies. The embedded target usually leaves dependency
resolution to the user (with embedded/packages).


> 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.

So the "controller.sh bootloader" is executed, but it kills the build in
the create_bootloader() function:

22 May 2020 03:58:27 UTC: NOTICE  : --- Running action sequence: bootloader
NOTICE:catalyst:--- Running action sequence: bootloader
touch: cannot touch '/substrate/next/builds/amd64/minimal/20200521/embedded-amd64-minimal-20200521/livecd': No such file or directory
Unable to find grub-mkstandalone
22 May 2020 03:58:27 UTC: ERROR   : CatalystError: cmd(['/substrate/next/catalyst/targets/embedded/controller.sh', 'bootloader', '/substrate/next/builds/amd64/minimal/20200521/embedded-amd64-minimal-20200521']) exited 1
ERROR:catalyst:CatalystError: cmd(['/substrate/next/catalyst/targets/embedded/controller.sh', 'bootloader', '/substrate/next/builds/amd64/minimal/20200521/embedded-amd64-minimal-20200521']) exited 1

I don't have grub in the seed stage (it _is_ optional), and don't make livecd images (I
don't know why it's trying to touch the builds/ path; maybe it wants tmp/...)


> 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.

Personally, I just need the tarball of the mergeroot, so I don't need the
bootloader action. In lieu of this particular change, maybe some if
statements to see if bootloader-setup.sh can run successfully may be
needed.