[Toaster] request for review + sticky/setguid/setuid bits

Lerner, Dave dave.lerner at windriver.com
Mon Mar 10 07:49:42 PDT 2014


I haven't tested the code but your changes work for me.  

-dave

> -----Original Message-----
> From: Damian, Alexandru [mailto:alexandru.damian at intel.com]
> Sent: Friday, March 07, 2014 10:51 AM
> To: Lerner, Dave
> Cc: Reyna, David; toaster at yoctoproject.org
> Subject: Re: request for review + sticky/setguid/setuid bits
> 
> I pushed out a new version, on the same branch, with permissions held as string, not as
> number.
> 
> I have the "pipe", "block" and "character" inode type of files, so I added specific
> checks for these file types in write-to-database code.
> 
> Please let me know if this works for you ?
> 
> Alex
> 
> 
> On Thu, Mar 6, 2014 at 8:00 PM, Lerner, Dave <dave.lerner at windriver.com> wrote:
> 
> 
> 	Also, I forgot to mention that if we leave the permissions as numbers, we need to
> handle 4 bits per set (per user, group, world), we need to handle sticky,setuid,setgid
> bit.  Leaving as strings as suggested in my previous email should solve that problem
> too.
> 	-dave
> 
> 	> -----Original Message-----
> 	> From: Lerner, Dave
> 	> Sent: Thursday, March 06, 2014 1:58 PM
> 	> To: Damian, Alexandru (alexandru.damian at intel.com)
> 	> Cc: Reyna, David (david.reyna at windriver.com); toaster at yoctoproject.org
> 	> Subject: FW: request for review
> 	>
> 	> Hi Alex,
> 	> See comments inline which cover 2 topics
> 	> 1) should we keep permissions as a string, since views will always display
> strings
> 	> 2) what about the other inode types: 'c', 'b', 'p', 's'
> 	> -dave
> 	>
> 	> >
> 	> >
> 	> > commit 75efb26f1d6b7d6c71921f1ecd6b235e9df2c506
> 	> > Author: Alexandru DAMIAN <alexandru.damian at intel.com>
> 	> > Date:   Thu Mar 6 16:31:40 2014 +0000
> 	> >
> 	> >     bitbake: toaster: write files-in-image  to the database
> 	> >
> 	> >     Adding code to write files-in-image data from the metadata
> 	> >     event to the database.
> 	> >
> 	> >     Signed-off-by: Alexandru DAMIAN <alexandru.damian at intel.com>
> 	> > ---
> 	> >  bitbake/lib/bb/ui/buildinfohelper.py |  135 +++++++++++++++++++++++++++++++++-
> 	> >  1 file changed, 133 insertions(+), 2 deletions(-)
> 	> >
> 	> > diff --git a/bitbake/lib/bb/ui/buildinfohelper.py
> 	> b/bitbake/lib/bb/ui/buildinfohelper.py
> 	> > index d7b526a..5060a21 100644
> 	> > --- a/bitbake/lib/bb/ui/buildinfohelper.py
> 	> > +++ b/bitbake/lib/bb/ui/buildinfohelper.py
> 	> > @@ -27,7 +27,7 @@ os.environ.setdefault("DJANGO_SETTINGS_MODULE",
> 	> > "toaster.toastermain.settings")
> 	> >  import toaster.toastermain.settings as toaster_django_settings
> 	> >  from toaster.orm.models import Build, Task, Recipe, Layer_Version, Layer,
> Target,
> 	> > LogMessage
> 	> >  from toaster.orm.models import Variable, VariableHistory
> 	> > -from toaster.orm.models import Package, Package_File, Target_Installed_Package
> 	> > +from toaster.orm.models import Package, Package_File,
> Target_Installed_Package,
> 	> > Target_File
> 	> >  from toaster.orm.models import Task_Dependency, Package_Dependency
> 	> >  from toaster.orm.models import Recipe_Dependency
> 	> >  from bb.msg import BBLogFormatter as format
> 	> > @@ -181,6 +181,133 @@ class ORMWrapper(object):
> 	> >
> 	> >          return layer_object
> 	> >
> 	> > +    def save_target_file_information(self, build_obj, target_obj, filedata):
> 	> > +        assert isinstance(build_obj, Build)
> 	> > +        assert isinstance(target_obj, Target)
> 	> > +        dirs = filedata['dirs']
> 	> > +        files = filedata['files']
> 	> > +        syms = filedata['syms']
> 	>
> 	> What about the other types of inodes?  Which of the following can't be in an
> image:
> 	> devices (character, block), fifo's, sockets?
> 	>
> 	> > +
> 	> > +        def _perm_strtono(s):
> 	> > +            if len(s) != 9:
> 	> > +                return -1
> 	> > +            p = 0
> 	> > +            if s[8] == 'x':
> 	> > +                p += 1
> 	> > +            if s[7] == 'w':
> 	> > +                p += 2
> 	> > +            if s[6] == 'r':
> 	> > +                p += 4
> 	> > +            if s[5] == 'x':
> 	> > +                p += 10
> 	> > +            if s[4] == 'w':
> 	> > +                p += 20
> 	> > +            if s[3] == 'r':
> 	> > +                p += 40
> 	> > +            if s[2] == 'x':
> 	> > +                p += 100
> 	> > +            if s[1] == 'w':
> 	> > +                p += 200
> 	> > +            if s[0] == 'r':
> 	> > +                p += 400
> 	> > +            return p
> 	>
> 	> I'm doing the opposite translation, so this seems wrong and we should keep a
> string.
> 	> Also, my perms translation handles links s[-1] = 'l', sticky /setuid/setguid
> 	> permissions.  Maybe it would be better to keep this a string.
> 	>
> 	> > +
> 	> > +
> 	> > +         # we insert directories, ordered by name depth
> 	> > +        for d in sorted(dirs, key=lambda x:len(x[-1].split("/"))):
> 	> > +            (user, group, size) = d[1:4]
> 	> > +            perm = d[0][1:]
> 	> > +            permission = _perm_strtono(perm)
> 	>
> 	> See comment above, maybe better to treat like 'user' and 'group' just leave it a
> string.
> 	>
> 	> > +            path = d[4].lstrip(".")
> 	> > +            if len(path) == 0:
> 	> > +                # we create the root directory as a special case
> 	> > +                path = "/"
> 	> > +                tf_obj = Target_File.objects.create(
> 	> > +                        target = target_obj,
> 	> > +                        path = path,
> 	> > +                        size = size,
> 	> > +                        inodetype = Target_File.ITYPE_DIRECTORY,
> 	> > +                        permission = permission,
> 	> > +                        owner = user,
> 	> > +                        group = group,
> 	> > +                        )
> 	> > +                tf_obj.directory = tf_obj
> 	> > +                tf_obj.save()
> 	> > +                continue
> 	> > +            parent_path = "/".join(path.split("/")[:len(path.split("/")) - 1])
> 	> > +            if len(parent_path) == 0:
> 	> > +                parent_path = "/"
> 	> > +            parent_obj = Target_File.objects.get(target = target_obj, path =
> 	> > parent_path, inodetype = Target_File.ITYPE_DIRECTORY)
> 	> > +            tf_obj = Target_File.objects.create(
> 	> > +                        target = target_obj,
> 	> > +                        path = path,
> 	> > +                        size = size,
> 	> > +                        inodetype = Target_File.ITYPE_DIRECTORY,
> 	> > +                        permission = permission,
> 	> > +                        owner = user,
> 	> > +                        group = group,
> 	> > +                        directory = parent_obj)
> 	> > +
> 	> > +
> 	> > +        # we insert files
> 	> > +        for d in files:
> 	> > +            (user, group, size) = d[1:4]
> 	> > +            perm = d[0][1:]
> 	> > +            permission = _perm_strtono(perm)
> 	>
> 	> Same permissions comment as above
> 	>
> 	> > +            path = d[4].lstrip(".")
> 	> > +            parent_path = "/".join(path.split("/")[:len(path.split("/")) - 1])
> 	> > +            tf_obj = Target_File.objects.create(
> 	> > +                        target = target_obj,
> 	> > +                        path = path,
> 	> > +                        size = size,
> 	> > +                        inodetype = Target_File.ITYPE_REGULAR,
> 	> > +                        permission = permission,
> 	> > +                        owner = user,
> 	> > +                        group = group)
> 	> > +            parent_obj = Target_File.objects.get(target = target_obj, path =
> 	> > parent_path, inodetype = Target_File.ITYPE_DIRECTORY)
> 	> > +            tf_obj.directory = parent_obj
> 	> > +            tf_obj.save()
> 	> > +
> 	> > +        # we insert symlinks
> 	> > +        for d in syms:
> 	> > +            (user, group, size) = d[1:4]
> 	> > +            perm = d[0][1:]
> 	> > +            permission = _perm_strtono(perm)
> 	>
> 	>
> 	> Same permissions comment as above
> 	>
> 	> > +            path = d[4].lstrip(".")
> 	> > +            filetarget_path = d[6]
> 	> > +
> 	> > +            parent_path = "/".join(path.split("/")[:len(path.split("/")) - 1])
> 	> > +            if not filetarget_path.startswith("/"):
> 	> > +                # we have a relative path, get a normalized absolute one
> 	> > +                filetarget_path = parent_path + "/" + filetarget_path
> 	> > +                fcp = filetarget_path.split("/")
> 	> > +                fcpl = []
> 	> > +                for i in fcp:
> 	> > +                    if i == "..":
> 	> > +                        fcpl.pop()
> 	> > +                    else:
> 	> > +                        fcpl.append(i)
> 	> > +                filetarget_path = "/".join(fcpl)
> 	> > +
> 	> > +            try:
> 	> > +                filetarget_obj = Target_File.objects.get(target = target_obj,
> path =
> 	> > filetarget_path)
> 	> > +            except Exception as e:
> 	> > +                # we might have an invalid link; no way to detect this. just
> set it
> 	> to
> 	> > None
> 	> > +                filetarget_obj = None
> 	> > +
> 	> > +            parent_obj = Target_File.objects.get(target = target_obj, path =
> 	> > parent_path, inodetype = Target_File.ITYPE_DIRECTORY)
> 	> > +
> 	> > +            tf_obj = Target_File.objects.create(
> 	> > +                        target = target_obj,
> 	> > +                        path = path,
> 	> > +                        size = size,
> 	> > +                        inodetype = Target_File.ITYPE_REGULAR,
> 	> > +                        permission = permission,
> 	> > +                        owner = user,
> 	> > +                        group = group,
> 	> > +                        directory = parent_obj,
> 	> > +                        sym_target = filetarget_obj)
> 	> > +
> 	> >
> 	> >      def save_target_package_information(self, build_obj, target_obj,
> packagedict,
> 	> > pkgpnmap, recipes):
> 	> >          assert isinstance(build_obj, Build)
> 	> > @@ -613,6 +740,10 @@ class BuildInfoHelper(object):
> 	> >                  pkgdata = event.data['pkgdata']
> 	> >                  imgdata = event.data['imgdata'][target.target]
> 	> >
> 	> > self.orm_wrapper.save_target_package_information(self.internal_state['build'],
> target,
> 	> > imgdata, pkgdata, self.internal_state['recipes'])
> 	> > +                filedata = event.data['filedata'][target.target]
> 	> > +
> 	> self.orm_wrapper.save_target_file_information(self.internal_state['build'],
> target,
> 	> > filedata)
> 	> > +
> 	> > +
> 	> >
> 	> >      def store_dependency_information(self, event):
> 	> >          assert '_depgraph' in vars(event)
> 	> > @@ -692,7 +823,7 @@ class BuildInfoHelper(object):
> 	> >              task_info['task_name'] = taskname
> 	> >              task_obj = self.orm_wrapper.get_update_task_object(task_info)
> 	> >              return task_obj
> 	> > -
> 	> > +
> 	>
> 	> OK.  Just trailing ws cleanup.
> 	>
> 	> >          # create tasks
> 	> >          tasks = {}
> 	> >          for taskdesc in event._depgraph['tdepends']:
> 	> >
> 	> > commit 13423eb102e394f20ddb60b86db96399478d47cc
> 	> > Author: Alexandru DAMIAN <alexandru.damian at intel.com>
> 	> > Date:   Thu Mar 6 16:29:46 2014 +0000
> 	> >
> 	> >     toaster.bbclass: read list of files in image
> 	> >
> 	> >     We read the list of files in a built image and send it
> 	> >     over with the same event for packages in image.
> 	> >
> 	> >     Signed-off-by: Alexandru DAMIAN <alexandru.damian at intel.com>
> 	> > ---
> 	> >  meta/classes/toaster.bbclass |   17 ++++++++++++++++-
> 	> >  1 file changed, 16 insertions(+), 1 deletion(-)
> 	> >
> 	> > diff --git a/meta/classes/toaster.bbclass b/meta/classes/toaster.bbclass
> 	> > index eed30d4..ddfceb5 100644
> 	> > --- a/meta/classes/toaster.bbclass
> 	> > +++ b/meta/classes/toaster.bbclass
> 	> > @@ -240,10 +240,15 @@ python toaster_buildhistory_dump() {
> 	> >      # scan the build targets for this build
> 	> >      images = {}
> 	> >      allpkgs = {}
> 	> > +    files = {}
> 	> >      for target in e._pkgs:
> 	> >          installed_img_path =
> e.data.expand(os.path.join(BUILDHISTORY_DIR_IMAGE_BASE,
> 	> > target))
> 	> >          if os.path.exists(installed_img_path):
> 	> >              images[target] = {}
> 	> > +            files[target] = {}
> 	> > +            files[target]['dirs'] = []
> 	> > +            files[target]['syms'] = []
> 	> > +            files[target]['files'] = []
> 	>
> 	> This deals with my concern about other inode types and if they can actually be
> 	> represented in installed_img_path,  fifos (pipes), sockets, block and character
> devices,
> 	> as discussed above and in last comment below.
> 	>
> 	> >              with open("%s/installed-package-sizes.txt" % installed_img_path,
> "r") as
> 	> > fin:
> 	> >                  for line in fin:
> 	> >                      line = line.rstrip(";")
> 	> > @@ -270,12 +275,22 @@ python toaster_buildhistory_dump() {
> 	> >                              images[target][dependsname] = {'size': 0,
> 'depends' : []}
> 	> >                          images[target][pname]['depends'].append((dependsname,
> 	> deptype))
> 	> >
> 	> > +            with open("%s/files-in-image.txt" % installed_img_path, "r") as
> fin:
> 	> > +                for line in fin:
> 	> > +                    lc = [ x for x in line.strip().split(" ") if len(x) > 0 ]
> 	> > +                    if lc[0].startswith("l"):
> 	> > +                        files[target]['syms'].append(lc)
> 	> > +                    elif lc[0].startswith("d"):
> 	> > +                        files[target]['dirs'].append(lc)
> 	> > +                    else:
> 	>
> 	>
> 	> Other possibilities may be startswith("c", "b", "p")?  Can you check with someone
> who
> 	> knows for sure that these can be stashed and saved in an image?
> 	>
> 	> -Dave
> 
> 
> 
> 
> 
> --
> 
> Alex Damian
> Yocto Project
> 
> SSG / OTC


More information about the toaster mailing list