[PATCH] wic: Generate startup.nsh for EFI cases if none found

[PATCH] wic: Generate startup.nsh for EFI cases if none found

[Tom Rini]

In the case of non-wic images there is logic today to generate a
startup.nsh file that will be executed by EFI to run the loader that the
image contains.  In the WIC case is currently depends on that file being
generated elsewhere and placed in DEPLOY_DIR_IMAGE and only used if
present there.  This extends the logic we have today in wic to save the
name of the loader that's being placed and if no startup.nsh is provided
already generate the default kind that grub-efi/systemd-boot.bbclass
generate.

Cc: Ed Bartosh <ed.bartosh@linux.intel.com>
Cc: Christopher Larson <chris_larson@mentor.com>
Signed-off-by: Tom Rini <trini@konsulko.com>
---
 scripts/lib/wic/plugins/source/bootimg-efi.py | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/scripts/lib/wic/plugins/source/bootimg-efi.py b/scripts/lib/wic/plugins/source/bootimg-efi.py
index 4c4f36a32f56..f4643fc233b2 100644
--- a/scripts/lib/wic/plugins/source/bootimg-efi.py
+++ b/scripts/lib/wic/plugins/source/bootimg-efi.py
@@ -204,13 +204,15 @@ class BootimgEFIPlugin(SourcePlugin):
                 shutil.copyfile("%s/hdd/boot/EFI/BOOT/grub.cfg" % cr_workdir,
                                 "%s/grub.cfg" % cr_workdir)
                 for mod in [x for x in os.listdir(kernel_dir) if x.startswith("grub-efi-")]:
-                    cp_cmd = "cp %s/%s %s/EFI/BOOT/%s" % (kernel_dir, mod, hdddir, mod[9:])
+                    loader = mod[9:]
+                    cp_cmd = "cp %s/%s %s/EFI/BOOT/%s" % (kernel_dir, mod, hdddir, loader)
                     exec_cmd(cp_cmd, True)
                 shutil.move("%s/grub.cfg" % cr_workdir,
                             "%s/hdd/boot/EFI/BOOT/grub.cfg" % cr_workdir)
             elif source_params['loader'] == 'systemd-boot':
                 for mod in [x for x in os.listdir(kernel_dir) if x.startswith("systemd-")]:
-                    cp_cmd = "cp %s/%s %s/EFI/BOOT/%s" % (kernel_dir, mod, hdddir, mod[8:])
+                    loader = mod[8:]
+                    cp_cmd = "cp %s/%s %s/EFI/BOOT/%s" % (kernel_dir, mod, hdddir, loader)
                     exec_cmd(cp_cmd, True)
             else:
                 raise WicError("unrecognized bootimg-efi loader: %s" %
@@ -222,6 +224,10 @@ class BootimgEFIPlugin(SourcePlugin):
         if os.path.exists(startup):
             cp_cmd = "cp %s %s/" % (startup, hdddir)
             exec_cmd(cp_cmd, True)
+        else:
+            cfg = open("%s/startup.nsh" % hdddir, "w")
+            cfg.write("fs0:EFI\\BOOT\\%s\n" % loader)
+            cfg.close()
 
         du_cmd = "du -bks %s" % hdddir
         out = exec_cmd(du_cmd)
-- 
1.9.1

Re: [PATCH] wic: Generate startup.nsh for EFI cases if none found

[Ed Bartosh]

On Wed, Sep 20, 2017 at 12:03:27PM -0400, Tom Rini wrote:
> In the case of non-wic images there is logic today to generate a
> startup.nsh file that will be executed by EFI to run the loader that the
> image contains.  In the WIC case is currently depends on that file being
> generated elsewhere and placed in DEPLOY_DIR_IMAGE and only used if
> present there.

What's wrong with this approach?

I'd be happy to make wic to do only partitioning and assembling the
image and avoiding to modify image content as much as possible.
That would make wic design much more clear and let us to remove
a lot of duplication between wic and meta/classes code.

Regarding boot partition content, I think preparing it from bootfs
directory the same way as it's done for root partition is the way to go.
You can find more details about it here:
https://bugzilla.yoctoproject.org/show_bug.cgi?id=10073

>  This extends the logic we have today in wic to save the
> name of the loader that's being placed and if no startup.nsh is provided
> already generate the default kind that grub-efi/systemd-boot.bbclass
> generate.
> 
> Cc: Ed Bartosh <ed.bartosh@linux.intel.com>
> Cc: Christopher Larson <chris_larson@mentor.com>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
>  scripts/lib/wic/plugins/source/bootimg-efi.py | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/lib/wic/plugins/source/bootimg-efi.py b/scripts/lib/wic/plugins/source/bootimg-efi.py
> index 4c4f36a32f56..f4643fc233b2 100644
> --- a/scripts/lib/wic/plugins/source/bootimg-efi.py
> +++ b/scripts/lib/wic/plugins/source/bootimg-efi.py
> @@ -204,13 +204,15 @@ class BootimgEFIPlugin(SourcePlugin):
>                  shutil.copyfile("%s/hdd/boot/EFI/BOOT/grub.cfg" % cr_workdir,
>                                  "%s/grub.cfg" % cr_workdir)
>                  for mod in [x for x in os.listdir(kernel_dir) if x.startswith("grub-efi-")]:
> -                    cp_cmd = "cp %s/%s %s/EFI/BOOT/%s" % (kernel_dir, mod, hdddir, mod[9:])
> +                    loader = mod[9:]
> +                    cp_cmd = "cp %s/%s %s/EFI/BOOT/%s" % (kernel_dir, mod, hdddir, loader)
>                      exec_cmd(cp_cmd, True)
>                  shutil.move("%s/grub.cfg" % cr_workdir,
>                              "%s/hdd/boot/EFI/BOOT/grub.cfg" % cr_workdir)
>              elif source_params['loader'] == 'systemd-boot':
>                  for mod in [x for x in os.listdir(kernel_dir) if x.startswith("systemd-")]:
> -                    cp_cmd = "cp %s/%s %s/EFI/BOOT/%s" % (kernel_dir, mod, hdddir, mod[8:])
> +                    loader = mod[8:]
> +                    cp_cmd = "cp %s/%s %s/EFI/BOOT/%s" % (kernel_dir, mod, hdddir, loader)
>                      exec_cmd(cp_cmd, True)
>              else:
>                  raise WicError("unrecognized bootimg-efi loader: %s" %
> @@ -222,6 +224,10 @@ class BootimgEFIPlugin(SourcePlugin):
>          if os.path.exists(startup):
>              cp_cmd = "cp %s %s/" % (startup, hdddir)
>              exec_cmd(cp_cmd, True)
> +        else:
> +            cfg = open("%s/startup.nsh" % hdddir, "w")
> +            cfg.write("fs0:EFI\\BOOT\\%s\n" % loader)
> +            cfg.close()
>  
>          du_cmd = "du -bks %s" % hdddir
>          out = exec_cmd(du_cmd)
> -- 
> 1.9.1
> 

-- 
--
Regards,
Ed

Re: [PATCH] wic: Generate startup.nsh for EFI cases if none found

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1871 bytes --]

On Thu, Sep 28, 2017 at 06:47:07PM +0300, Ed Bartosh wrote:
> On Wed, Sep 20, 2017 at 12:03:27PM -0400, Tom Rini wrote:
> > In the case of non-wic images there is logic today to generate a
> > startup.nsh file that will be executed by EFI to run the loader that the
> > image contains.  In the WIC case is currently depends on that file being
> > generated elsewhere and placed in DEPLOY_DIR_IMAGE and only used if
> > present there.
> 
> What's wrong with this approach?

No one ever provides a startup.nsh and everyone that wants one creates
the same one line trivial example.  The end result is that no WIC images
are Just Bootable on UEFI systems unless you first go and spell that out
as the desired booting device.  This isn't an awesome workflow which is
why the non-WIC cases make the required startup.nsh :)

> I'd be happy to make wic to do only partitioning and assembling the
> image and avoiding to modify image content as much as possible.
> That would make wic design much more clear and let us to remove
> a lot of duplication between wic and meta/classes code.
> 
> Regarding boot partition content, I think preparing it from bootfs
> directory the same way as it's done for root partition is the way to go.
> You can find more details about it here:
> https://bugzilla.yoctoproject.org/show_bug.cgi?id=10073

I don't conceptually see a problem with going that route.  But today WIC
images aren't nearly as useful as they could be, with a rather tiny
change.

My patch is also a regression-fix, I believe, in that at some point in
the past, when Christopher's patch went in, things were laid out such
that startup.nsh was often/always generated by another class and placed
where WIC would find it and copy it in.  At some point that was
broken/changed, and no one noticed / was interested enough to fix it.

-- 
Tom

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] wic: Generate startup.nsh for EFI cases if none found

[Otavio Salvador]

Hello Tom,

On Thu, Sep 28, 2017 at 2:44 PM, Tom Rini <trini@konsulko.com> wrote:
> On Thu, Sep 28, 2017 at 06:47:07PM +0300, Ed Bartosh wrote:
>> On Wed, Sep 20, 2017 at 12:03:27PM -0400, Tom Rini wrote:
>> > In the case of non-wic images there is logic today to generate a
>> > startup.nsh file that will be executed by EFI to run the loader that the
>> > image contains.  In the WIC case is currently depends on that file being
>> > generated elsewhere and placed in DEPLOY_DIR_IMAGE and only used if
>> > present there.
>>
>> What's wrong with this approach?
>
> No one ever provides a startup.nsh and everyone that wants one creates
> the same one line trivial example.  The end result is that no WIC images
> are Just Bootable on UEFI systems unless you first go and spell that out
> as the desired booting device.  This isn't an awesome workflow which is
> why the non-WIC cases make the required startup.nsh :)

I think it could be done as we did for u-boot-extlinux support.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Re: [PATCH] wic: Generate startup.nsh for EFI cases if none found

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1178 bytes --]

On Thu, Sep 28, 2017 at 02:46:23PM -0300, Otavio Salvador wrote:
> Hello Tom,
> 
> On Thu, Sep 28, 2017 at 2:44 PM, Tom Rini <trini@konsulko.com> wrote:
> > On Thu, Sep 28, 2017 at 06:47:07PM +0300, Ed Bartosh wrote:
> >> On Wed, Sep 20, 2017 at 12:03:27PM -0400, Tom Rini wrote:
> >> > In the case of non-wic images there is logic today to generate a
> >> > startup.nsh file that will be executed by EFI to run the loader that the
> >> > image contains.  In the WIC case is currently depends on that file being
> >> > generated elsewhere and placed in DEPLOY_DIR_IMAGE and only used if
> >> > present there.
> >>
> >> What's wrong with this approach?
> >
> > No one ever provides a startup.nsh and everyone that wants one creates
> > the same one line trivial example.  The end result is that no WIC images
> > are Just Bootable on UEFI systems unless you first go and spell that out
> > as the desired booting device.  This isn't an awesome workflow which is
> > why the non-WIC cases make the required startup.nsh :)
> 
> I think it could be done as we did for u-boot-extlinux support.

That's complete overkill for a static one line file.

-- 
Tom

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] wic: Generate startup.nsh for EFI cases if none found

[Otavio Salvador]

On Thu, Sep 28, 2017 at 2:49 PM, Tom Rini <trini@konsulko.com> wrote:
> On Thu, Sep 28, 2017 at 02:46:23PM -0300, Otavio Salvador wrote:
>> Hello Tom,
>>
>> On Thu, Sep 28, 2017 at 2:44 PM, Tom Rini <trini@konsulko.com> wrote:
>> > On Thu, Sep 28, 2017 at 06:47:07PM +0300, Ed Bartosh wrote:
>> >> On Wed, Sep 20, 2017 at 12:03:27PM -0400, Tom Rini wrote:
>> >> > In the case of non-wic images there is logic today to generate a
>> >> > startup.nsh file that will be executed by EFI to run the loader that the
>> >> > image contains.  In the WIC case is currently depends on that file being
>> >> > generated elsewhere and placed in DEPLOY_DIR_IMAGE and only used if
>> >> > present there.
>> >>
>> >> What's wrong with this approach?
>> >
>> > No one ever provides a startup.nsh and everyone that wants one creates
>> > the same one line trivial example.  The end result is that no WIC images
>> > are Just Bootable on UEFI systems unless you first go and spell that out
>> > as the desired booting device.  This isn't an awesome workflow which is
>> > why the non-WIC cases make the required startup.nsh :)
>>
>> I think it could be done as we did for u-boot-extlinux support.
>
> That's complete overkill for a static one line file.

No, it is not. It allows for reproducable builds.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Re: [PATCH] wic: Generate startup.nsh for EFI cases if none found

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1625 bytes --]

On Thu, Sep 28, 2017 at 02:57:04PM -0300, Otavio Salvador wrote:
> On Thu, Sep 28, 2017 at 2:49 PM, Tom Rini <trini@konsulko.com> wrote:
> > On Thu, Sep 28, 2017 at 02:46:23PM -0300, Otavio Salvador wrote:
> >> Hello Tom,
> >>
> >> On Thu, Sep 28, 2017 at 2:44 PM, Tom Rini <trini@konsulko.com> wrote:
> >> > On Thu, Sep 28, 2017 at 06:47:07PM +0300, Ed Bartosh wrote:
> >> >> On Wed, Sep 20, 2017 at 12:03:27PM -0400, Tom Rini wrote:
> >> >> > In the case of non-wic images there is logic today to generate a
> >> >> > startup.nsh file that will be executed by EFI to run the loader that the
> >> >> > image contains.  In the WIC case is currently depends on that file being
> >> >> > generated elsewhere and placed in DEPLOY_DIR_IMAGE and only used if
> >> >> > present there.
> >> >>
> >> >> What's wrong with this approach?
> >> >
> >> > No one ever provides a startup.nsh and everyone that wants one creates
> >> > the same one line trivial example.  The end result is that no WIC images
> >> > are Just Bootable on UEFI systems unless you first go and spell that out
> >> > as the desired booting device.  This isn't an awesome workflow which is
> >> > why the non-WIC cases make the required startup.nsh :)
> >>
> >> I think it could be done as we did for u-boot-extlinux support.
> >
> > That's complete overkill for a static one line file.
> 
> No, it is not. It allows for reproducable builds.

Either solution would allow for reproducible builds.  Perhaps it would
be clearer if the patch just always generated that content instead like
the systemd and grub2 classes do?

-- 
Tom

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] wic: Generate startup.nsh for EFI cases if none found

[Otavio Salvador]

On Thu, Sep 28, 2017 at 2:59 PM, Tom Rini <trini@konsulko.com> wrote:
> On Thu, Sep 28, 2017 at 02:57:04PM -0300, Otavio Salvador wrote:
>> On Thu, Sep 28, 2017 at 2:49 PM, Tom Rini <trini@konsulko.com> wrote:
>> > On Thu, Sep 28, 2017 at 02:46:23PM -0300, Otavio Salvador wrote:
>> >> Hello Tom,
>> >>
>> >> On Thu, Sep 28, 2017 at 2:44 PM, Tom Rini <trini@konsulko.com> wrote:
>> >> > On Thu, Sep 28, 2017 at 06:47:07PM +0300, Ed Bartosh wrote:
>> >> >> On Wed, Sep 20, 2017 at 12:03:27PM -0400, Tom Rini wrote:
>> >> >> > In the case of non-wic images there is logic today to generate a
>> >> >> > startup.nsh file that will be executed by EFI to run the loader that the
>> >> >> > image contains.  In the WIC case is currently depends on that file being
>> >> >> > generated elsewhere and placed in DEPLOY_DIR_IMAGE and only used if
>> >> >> > present there.
>> >> >>
>> >> >> What's wrong with this approach?
>> >> >
>> >> > No one ever provides a startup.nsh and everyone that wants one creates
>> >> > the same one line trivial example.  The end result is that no WIC images
>> >> > are Just Bootable on UEFI systems unless you first go and spell that out
>> >> > as the desired booting device.  This isn't an awesome workflow which is
>> >> > why the non-WIC cases make the required startup.nsh :)
>> >>
>> >> I think it could be done as we did for u-boot-extlinux support.
>> >
>> > That's complete overkill for a static one line file.
>>
>> No, it is not. It allows for reproducable builds.
>
> Either solution would allow for reproducible builds.  Perhaps it would
> be clearer if the patch just always generated that content instead like
> the systemd and grub2 classes do?

No; if the file is generated by wic it is external of the build and
thus can change without the image change.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Re: [PATCH] wic: Generate startup.nsh for EFI cases if none found

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2040 bytes --]

On Thu, Sep 28, 2017 at 03:01:13PM -0300, Otavio Salvador wrote:
> On Thu, Sep 28, 2017 at 2:59 PM, Tom Rini <trini@konsulko.com> wrote:
> > On Thu, Sep 28, 2017 at 02:57:04PM -0300, Otavio Salvador wrote:
> >> On Thu, Sep 28, 2017 at 2:49 PM, Tom Rini <trini@konsulko.com> wrote:
> >> > On Thu, Sep 28, 2017 at 02:46:23PM -0300, Otavio Salvador wrote:
> >> >> Hello Tom,
> >> >>
> >> >> On Thu, Sep 28, 2017 at 2:44 PM, Tom Rini <trini@konsulko.com> wrote:
> >> >> > On Thu, Sep 28, 2017 at 06:47:07PM +0300, Ed Bartosh wrote:
> >> >> >> On Wed, Sep 20, 2017 at 12:03:27PM -0400, Tom Rini wrote:
> >> >> >> > In the case of non-wic images there is logic today to generate a
> >> >> >> > startup.nsh file that will be executed by EFI to run the loader that the
> >> >> >> > image contains.  In the WIC case is currently depends on that file being
> >> >> >> > generated elsewhere and placed in DEPLOY_DIR_IMAGE and only used if
> >> >> >> > present there.
> >> >> >>
> >> >> >> What's wrong with this approach?
> >> >> >
> >> >> > No one ever provides a startup.nsh and everyone that wants one creates
> >> >> > the same one line trivial example.  The end result is that no WIC images
> >> >> > are Just Bootable on UEFI systems unless you first go and spell that out
> >> >> > as the desired booting device.  This isn't an awesome workflow which is
> >> >> > why the non-WIC cases make the required startup.nsh :)
> >> >>
> >> >> I think it could be done as we did for u-boot-extlinux support.
> >> >
> >> > That's complete overkill for a static one line file.
> >>
> >> No, it is not. It allows for reproducable builds.
> >
> > Either solution would allow for reproducible builds.  Perhaps it would
> > be clearer if the patch just always generated that content instead like
> > the systemd and grub2 classes do?
> 
> No; if the file is generated by wic it is external of the build and
> thus can change without the image change.

WIC isn't external to the build, it's part of the build.

-- 
Tom

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] wic: Generate startup.nsh for EFI cases if none found

[Ed Bartosh]

On Thu, Sep 28, 2017 at 01:44:29PM -0400, Tom Rini wrote:
> On Thu, Sep 28, 2017 at 06:47:07PM +0300, Ed Bartosh wrote:
> > On Wed, Sep 20, 2017 at 12:03:27PM -0400, Tom Rini wrote:
> > > In the case of non-wic images there is logic today to generate a
> > > startup.nsh file that will be executed by EFI to run the loader that the
> > > image contains.  In the WIC case is currently depends on that file being
> > > generated elsewhere and placed in DEPLOY_DIR_IMAGE and only used if
> > > present there.
> > 
> > What's wrong with this approach?
> 
> No one ever provides a startup.nsh and everyone that wants one creates
> the same one line trivial example.  The end result is that no WIC images
> are Just Bootable on UEFI systems unless you first go and spell that out
> as the desired booting device.  This isn't an awesome workflow which is
> why the non-WIC cases make the required startup.nsh :)

Would it be better if EFI providers create this file?

I still believe that wic should't hack the filesystem content unless
it's really unavoidable. So far I know only one exception: updating
/etc/fstab. And even that is not always needed (see --no-update-fstab
patchset for further details.)

> 
> > I'd be happy to make wic to do only partitioning and assembling the
> > image and avoiding to modify image content as much as possible.
> > That would make wic design much more clear and let us to remove
> > a lot of duplication between wic and meta/classes code.
> > 
> > Regarding boot partition content, I think preparing it from bootfs
> > directory the same way as it's done for root partition is the way to go.
> > You can find more details about it here:
> > https://bugzilla.yoctoproject.org/show_bug.cgi?id=10073
> 
> I don't conceptually see a problem with going that route.  But today WIC
> images aren't nearly as useful as they could be, with a rather tiny
> change.
> 

If we agree that wic should avoid modifying content then the obvious way
to solve this is to provide required content (startup.nsh in this case)
either by EFI related recipes or core classes.

> My patch is also a regression-fix, I believe, in that at some point in
> the past, when Christopher's patch went in, things were laid out such
> that startup.nsh was often/always generated by another class and placed
> where WIC would find it and copy it in.  At some point that was
> broken/changed, and no one noticed / was interested enough to fix it.
>

If this functionality is covered by wic test suite this wouldn't
happen.


--
Regards,
Ed

Re: [PATCH] wic: Generate startup.nsh for EFI cases if none found

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3396 bytes --]

On Fri, Sep 29, 2017 at 02:27:57PM +0300, Ed Bartosh wrote:
> On Thu, Sep 28, 2017 at 01:44:29PM -0400, Tom Rini wrote:
> > On Thu, Sep 28, 2017 at 06:47:07PM +0300, Ed Bartosh wrote:
> > > On Wed, Sep 20, 2017 at 12:03:27PM -0400, Tom Rini wrote:
> > > > In the case of non-wic images there is logic today to generate a
> > > > startup.nsh file that will be executed by EFI to run the loader that the
> > > > image contains.  In the WIC case is currently depends on that file being
> > > > generated elsewhere and placed in DEPLOY_DIR_IMAGE and only used if
> > > > present there.
> > > 
> > > What's wrong with this approach?
> > 
> > No one ever provides a startup.nsh and everyone that wants one creates
> > the same one line trivial example.  The end result is that no WIC images
> > are Just Bootable on UEFI systems unless you first go and spell that out
> > as the desired booting device.  This isn't an awesome workflow which is
> > why the non-WIC cases make the required startup.nsh :)
> 
> Would it be better if EFI providers create this file?
> 
> I still believe that wic should't hack the filesystem content unless
> it's really unavoidable. So far I know only one exception: updating
> /etc/fstab. And even that is not always needed (see --no-update-fstab
> patchset for further details.)

Well, it depends on your view of who is supposed to do what.  Today, in
wic BootimgEFIPlugin mirrors the efi_populate() function of
systemd-boot/grub-efi.bbclass.  That's where startup.nsh is made because
it needs to know the name of the EFI application (also technically the
path, but EFI\BOOT\ is spec mandatated I believe).  So we can't
easily make the deploy functions create startup.nsh without duplicating
logic from the bbclasss.

> > > I'd be happy to make wic to do only partitioning and assembling the
> > > image and avoiding to modify image content as much as possible.
> > > That would make wic design much more clear and let us to remove
> > > a lot of duplication between wic and meta/classes code.
> > > 
> > > Regarding boot partition content, I think preparing it from bootfs
> > > directory the same way as it's done for root partition is the way to go.
> > > You can find more details about it here:
> > > https://bugzilla.yoctoproject.org/show_bug.cgi?id=10073
> > 
> > I don't conceptually see a problem with going that route.  But today WIC
> > images aren't nearly as useful as they could be, with a rather tiny
> > change.
> 
> If we agree that wic should avoid modifying content then the obvious way
> to solve this is to provide required content (startup.nsh in this case)
> either by EFI related recipes or core classes.

Maybe I have to change my mind after thinking harder :)  Where's the
logic that creates the boot partition now?

> > My patch is also a regression-fix, I believe, in that at some point in
> > the past, when Christopher's patch went in, things were laid out such
> > that startup.nsh was often/always generated by another class and placed
> > where WIC would find it and copy it in.  At some point that was
> > broken/changed, and no one noticed / was interested enough to fix it.
> 
> If this functionality is covered by wic test suite this wouldn't
> happen.

Once we agree on what the fix looks like, I'll see if I can figure out
how to add in another test. :)

-- 
Tom

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] wic: Generate startup.nsh for EFI cases if none found

[Ed Bartosh]

On Fri, Sep 29, 2017 at 08:35:42AM -0400, Tom Rini wrote:
> On Fri, Sep 29, 2017 at 02:27:57PM +0300, Ed Bartosh wrote:
> > On Thu, Sep 28, 2017 at 01:44:29PM -0400, Tom Rini wrote:
> > > On Thu, Sep 28, 2017 at 06:47:07PM +0300, Ed Bartosh wrote:
> > > > On Wed, Sep 20, 2017 at 12:03:27PM -0400, Tom Rini wrote:
> > > > > In the case of non-wic images there is logic today to generate a
> > > > > startup.nsh file that will be executed by EFI to run the loader that the
> > > > > image contains.  In the WIC case is currently depends on that file being
> > > > > generated elsewhere and placed in DEPLOY_DIR_IMAGE and only used if
> > > > > present there.
> > > > 
> > > > What's wrong with this approach?
> > > 
> > > No one ever provides a startup.nsh and everyone that wants one creates
> > > the same one line trivial example.  The end result is that no WIC images
> > > are Just Bootable on UEFI systems unless you first go and spell that out
> > > as the desired booting device.  This isn't an awesome workflow which is
> > > why the non-WIC cases make the required startup.nsh :)
> > 
> > Would it be better if EFI providers create this file?
> > 
> > I still believe that wic should't hack the filesystem content unless
> > it's really unavoidable. So far I know only one exception: updating
> > /etc/fstab. And even that is not always needed (see --no-update-fstab
> > patchset for further details.)
> 
> Well, it depends on your view of who is supposed to do what.  Today, in
> wic BootimgEFIPlugin mirrors the efi_populate() function of
> systemd-boot/grub-efi.bbclass.
I'd call this unnecessary duplication. This content should be prepared
by EFI provider class or recipe and taken by wic as is, without
modification.

I did some work on this: http://lists.openembedded.org/pipermail/openembedded-core/2017-May/136656.html
If this or similar approach is accepted wic wouldn't need most of boot
plugings. Boot partition can be prepared out of bootfs directory using
rootfs plugin.

>  That's where startup.nsh is made because
> it needs to know the name of the EFI application (also technically the
> path, but EFI\BOOT\ is spec mandatated I believe).  So we can't
> easily make the deploy functions create startup.nsh without duplicating
> logic from the bbclasss.
> 
> > > > I'd be happy to make wic to do only partitioning and assembling the
> > > > image and avoiding to modify image content as much as possible.
> > > > That would make wic design much more clear and let us to remove
> > > > a lot of duplication between wic and meta/classes code.
> > > > 
> > > > Regarding boot partition content, I think preparing it from bootfs
> > > > directory the same way as it's done for root partition is the way to go.
> > > > You can find more details about it here:
> > > > https://bugzilla.yoctoproject.org/show_bug.cgi?id=10073
> > > 
> > > I don't conceptually see a problem with going that route.  But today WIC
> > > images aren't nearly as useful as they could be, with a rather tiny
> > > change.
> > 
> > If we agree that wic should avoid modifying content then the obvious way
> > to solve this is to provide required content (startup.nsh in this case)
> > either by EFI related recipes or core classes.
> 
> Maybe I have to change my mind after thinking harder :)  Where's the
> logic that creates the boot partition now?
>
Now it's in several places: in meta/classes, efi recipes and wic. I
think wic should be removed from this list. It's not a wic job to prepare boot content.

> > > My patch is also a regression-fix, I believe, in that at some point in
> > > the past, when Christopher's patch went in, things were laid out such
> > > that startup.nsh was often/always generated by another class and placed
> > > where WIC would find it and copy it in.  At some point that was
> > > broken/changed, and no one noticed / was interested enough to fix it.
> > 
> > If this functionality is covered by wic test suite this wouldn't
> > happen.
> 
> Once we agree on what the fix looks like, I'll see if I can figure out
> how to add in another test. :)


--
Regards,
Ed

Re: [PATCH] wic: Generate startup.nsh for EFI cases if none found

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3942 bytes --]

On Fri, Sep 29, 2017 at 04:44:54PM +0300, Ed Bartosh wrote:
> On Fri, Sep 29, 2017 at 08:35:42AM -0400, Tom Rini wrote:
> > On Fri, Sep 29, 2017 at 02:27:57PM +0300, Ed Bartosh wrote:
> > > On Thu, Sep 28, 2017 at 01:44:29PM -0400, Tom Rini wrote:
> > > > On Thu, Sep 28, 2017 at 06:47:07PM +0300, Ed Bartosh wrote:
> > > > > On Wed, Sep 20, 2017 at 12:03:27PM -0400, Tom Rini wrote:
> > > > > > In the case of non-wic images there is logic today to generate a
> > > > > > startup.nsh file that will be executed by EFI to run the loader that the
> > > > > > image contains.  In the WIC case is currently depends on that file being
> > > > > > generated elsewhere and placed in DEPLOY_DIR_IMAGE and only used if
> > > > > > present there.
> > > > > 
> > > > > What's wrong with this approach?
> > > > 
> > > > No one ever provides a startup.nsh and everyone that wants one creates
> > > > the same one line trivial example.  The end result is that no WIC images
> > > > are Just Bootable on UEFI systems unless you first go and spell that out
> > > > as the desired booting device.  This isn't an awesome workflow which is
> > > > why the non-WIC cases make the required startup.nsh :)
> > > 
> > > Would it be better if EFI providers create this file?
> > > 
> > > I still believe that wic should't hack the filesystem content unless
> > > it's really unavoidable. So far I know only one exception: updating
> > > /etc/fstab. And even that is not always needed (see --no-update-fstab
> > > patchset for further details.)
> > 
> > Well, it depends on your view of who is supposed to do what.  Today, in
> > wic BootimgEFIPlugin mirrors the efi_populate() function of
> > systemd-boot/grub-efi.bbclass.
> I'd call this unnecessary duplication. This content should be prepared
> by EFI provider class or recipe and taken by wic as is, without
> modification.
> 
> I did some work on this: http://lists.openembedded.org/pipermail/openembedded-core/2017-May/136656.html
> If this or similar approach is accepted wic wouldn't need most of boot
> plugings. Boot partition can be prepared out of bootfs directory using
> rootfs plugin.
> 
> >  That's where startup.nsh is made because
> > it needs to know the name of the EFI application (also technically the
> > path, but EFI\BOOT\ is spec mandatated I believe).  So we can't
> > easily make the deploy functions create startup.nsh without duplicating
> > logic from the bbclasss.
> > 
> > > > > I'd be happy to make wic to do only partitioning and assembling the
> > > > > image and avoiding to modify image content as much as possible.
> > > > > That would make wic design much more clear and let us to remove
> > > > > a lot of duplication between wic and meta/classes code.
> > > > > 
> > > > > Regarding boot partition content, I think preparing it from bootfs
> > > > > directory the same way as it's done for root partition is the way to go.
> > > > > You can find more details about it here:
> > > > > https://bugzilla.yoctoproject.org/show_bug.cgi?id=10073
> > > > 
> > > > I don't conceptually see a problem with going that route.  But today WIC
> > > > images aren't nearly as useful as they could be, with a rather tiny
> > > > change.
> > > 
> > > If we agree that wic should avoid modifying content then the obvious way
> > > to solve this is to provide required content (startup.nsh in this case)
> > > either by EFI related recipes or core classes.
> > 
> > Maybe I have to change my mind after thinking harder :)  Where's the
> > logic that creates the boot partition now?
> >
> Now it's in several places: in meta/classes, efi recipes and wic. I
> think wic should be removed from this list. It's not a wic job to prepare boot content.

OK.  I still argue this is a regression from previous, but I'm also
willing to set this patch aside in favour of the larger rework that's in
progress, thanks.

-- 
Tom

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]