[OE-core] [PATCH 1/2] wic: Fix permissions when using exclude or include path

[Ricardo Ribalda Delgado]

Hi Paul

On Thu, Mar 5, 2020 at 10:32 AM Paul Barker <pbarker at konsulko.com> wrote:
>
> On Wed, 4 Mar 2020 11:02:47 +0100
> Ricardo Ribalda Delgado <ricardo at ribalda.com> wrote:
>
> > Hi Paul
> >
> > On Wed, Mar 4, 2020 at 10:53 AM Paul Barker <pbarker at konsulko.com> wrote:
> > >
> > > On Wed,  4 Mar 2020 09:34:37 +0100
> > > Ricardo Ribalda Delgado <ricardo at ribalda.com> wrote:
> > >
> > > > When parameters include_path or exclude_path are passed to the rootfs
> > > > plugin, it will copy the partition content into a folder and make all
> > > > the modifications there.
> > > >
> > > > This is done using copyhardlinktree(), which does not take into
> > > > consideration the content of the pseudo folder, which contains the
> > > > information about the right permissions and ownership of the folders.
> > >
> > > How are you running wic here? In the do_image_wic task it's executed under
> > > pseudo so all this is handled already. Executing wic outside of bitbake may
> > > need some more testing here.
> >
> > I am running wic outside bitbake. But even if it is run under bitbake,
> > it should also fail. The pseudo directory needs to be present on the
> > target image
>
> If you're running wic outside of bitbake, is there any guarantee that pseudo
> is available?

Yes, the same guarantee that the ext3_tools are available. So I
believe we are safe here. Actually in my docker pseudo is not
installed and when I invoke with wic, everything is fine.

>
> >
> > >
> > > >
> > > > This results in a rootfs owned by the user that is running the wic
> > > > command (usually UID 1000), which makes some rootfs unbootable.
> > > >
> > > > To fix this we copy the content of the pseudo folders to the new folder
> > > > and modify the pseudo database using the "pseudo -B" command.
> > > >
> > > > Signed-off-by: Ricardo Ribalda Delgado <ricardo at ribalda.com>
> > > > ---
> > > >  scripts/lib/wic/plugins/source/rootfs.py | 22 +++++++++++++++++++---
> > > >  1 file changed, 19 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
> > > > index 705aeb5563..40419a64b3 100644
> > > > --- a/scripts/lib/wic/plugins/source/rootfs.py
> > > > +++ b/scripts/lib/wic/plugins/source/rootfs.py
> > > > @@ -16,11 +16,11 @@ import os
> > > >  import shutil
> > > >  import sys
> > > >
> > > > -from oe.path import copyhardlinktree
> > > > +from oe.path import copyhardlinktree, copytree
> > > >
> > > >  from wic import WicError
> > > >  from wic.pluginbase import SourcePlugin
> > > > -from wic.misc import get_bitbake_var
> > > > +from wic.misc import get_bitbake_var, exec_native_cmd
> > > >
> > > >  logger = logging.getLogger('wic')
> > > >
> > > > @@ -44,6 +44,15 @@ class RootfsPlugin(SourcePlugin):
> > > >
> > > >          return os.path.realpath(image_rootfs_dir)
> > > >
> > > > +    @staticmethod
> > > > +    def __get_pseudo(native_sysroot, rootfs):
> > > > +        pseudo = "export PSEUDO_PREFIX=%s/usr;" % native_sysroot
> > > > +        pseudo += "export PSEUDO_LOCALSTATEDIR=%s;" % os.path.join(rootfs, "../pseudo")
> > > > +        pseudo += "export PSEUDO_PASSWD=%s;" % rootfs
> > > > +        pseudo += "export PSEUDO_NOSYMLINKEXP=1;"
> > > > +        pseudo += "%s " % get_bitbake_var("FAKEROOTCMD")
> > > > +        return pseudo
> > > > +
> > > >      @classmethod
> > > >      def do_prepare_partition(cls, part, source_params, cr, cr_workdir,
> > > >                               oe_builddir, bootimg_dir, kernel_dir,
> > > > @@ -78,9 +87,16 @@ class RootfsPlugin(SourcePlugin):
> > > >
> > > >              if os.path.lexists(new_rootfs):
> > > >                  shutil.rmtree(os.path.join(new_rootfs))
> > > > -
> > > >              copyhardlinktree(part.rootfs_dir, new_rootfs)
> > > >
> > > > +            if os.path.lexists(os.path.join(new_rootfs, "../pseudo")):
> > > > +                shutil.rmtree(os.path.join(new_rootfs, "../pseudo"))
> > > > +            copytree(os.path.join(part.rootfs_dir, "../pseudo"),
> > > > +                     os.path.join(new_rootfs, "../pseudo"))
> > >
> > > I don't like stepping up the directory tree like this. We should be more
> > > explicit with the paths.
> >
> > You are thinking on:
> > os.path.dirname(directory)
>
> No, I'm wondering why we're taking a step up the directory tree from
> `part.rootfs_dir`. I can point that at any path using the `--rootfs-dir`
> argument and there's no guarantee that ../pseudo exists or is relevant to the
> path I gave to `--rootfs-dir`.

Because we are asuming that the rootfs is being generated with
OE/yocto, and there is where the pseudo folder lives.
It is the same asumption part.prepare_rootfs() is taking.

>
> >
> > >
> > > > +            pseudo_cmd = "%s -B -m %s -M %s" % (cls.__get_pseudo(native_sysroot,new_rootfs),
> > > > +                                                part.rootfs_dir, new_rootfs)
> > > > +            exec_native_cmd(pseudo_cmd, native_sysroot)
> > > > +
> > > >              for path in part.include_path or []:
> > > >                  copyhardlinktree(path, new_rootfs)
> > >
> > >                    ^^^^^^^^^^^^^^^^
> > >
> > > If this is the right approach I imagine you would also need to fix things up
> > > with pseudo after the copyhardlinktree call above.
> >
> > I do not think it is needed. include_path does not contain its own
> > pseudo directory
>
> That's not necessarily true. The include_path could have been built by Yocto
> using pseudo.

Then you need to provide where the pseudo folder is. Eg

--include_path folder/A/B/C/D/file

Where is the pseudo database? in folder/A/B/C/D/pseudo,
folder/A/B/C/pseudo , folder/A/B/C/pseudo /...



>
> I can see that there is an issue using these arguments to wic outside of
> bitbake but I'm not sure this is the correct solution.
>
> --
> Paul Barker
> Konsulko Group



-- 
Ricardo Ribalda