[elbe-devel] [PATCH 4/9] hdimg efilesystem: compress images later in the process

[Torben Hohn]

On Fri, Nov 30, 2018 at 03:00:17PM +0100, Manuel Traut wrote:
> On 17:28 Wed 28 Nov     , Torben Hohn wrote:
> > Images are compressed directly in do_hdimg(). This is before
> > <project-finetuning> is executed.
> > 
> > To make it possible to mount an image in <project-finetuning> the
> > compression step needs to move to a step that is executed later.
> > While at it, prepare for different compressors.
> > 
> > Signed-off-by: Torben Hohn <torben.hohn at linutronix.de>
> > ---
> >  elbepack/efilesystem.py | 14 ++++++++++++++
> >  elbepack/elbeproject.py |  2 ++
> >  elbepack/hdimg.py       | 13 +++----------
> >  3 files changed, 19 insertions(+), 10 deletions(-)
> > 
> > diff --git a/elbepack/efilesystem.py b/elbepack/efilesystem.py
> > index e6a8c2ab..dd75cdd1 100644
> > --- a/elbepack/efilesystem.py
> > +++ b/elbepack/efilesystem.py
> > @@ -289,6 +289,7 @@ class TargetFs(ChRootFilesystem):
> >          self.log = log
> >          self.xml = xml
> >          self.images = []
> > +        self.image_packers = {}
> >  
> >      def write_fstab(self, xml):
> >          if not self.exists("etc"):
> > @@ -313,6 +314,9 @@ class TargetFs(ChRootFilesystem):
> >              grub_version,
> >              grub_fw_type)
> >  
> > +        for i in self.images:
> > +            self.image_packers[i] = ('gzip -f', '.gz')
> > +
> >          if self.xml.has("target/package/tar"):
> >              targz_name = self.xml.text("target/package/tar/name")
> >              try:
> > @@ -362,6 +366,16 @@ class TargetFs(ChRootFilesystem):
> >                  # error was logged; continue
> >                  pass
> >  
> > +    def pack_images(self, builddir):
> > +        for img, (packer, ending) in self.image_packers.items():
> > +            self.images.remove(img)
> 
> This might be a nfsroot.tar.gz that shouldn't be recompressed.

no ! nfsroot.tar.gz is added to self.images right
after self.image_packers is built out of self.images

So nfsroot.tar.gz is not marked to get packed.

> IMHO we should build just a tar in part_target.

that would be a bit dumb. why should we store the uncompressed
file, when we could compress it on the fly.

we can already add options to the tar commandline at that place.
maybe we should remove the z ... but i dont think, that should
be part of this PQ. And this should not be done in the currentl release
cycle, it think. 


> Other case where behaviour will be different than before is cpio.
> But cpio.gz doesn't hurt.

no. its not different than before.

> 
> > +            try:
> > +                self.log.do('%s "%s"' % (packer, os.path.join(builddir, img)))
> > +                # only add packed image when no exception is thrown
> > +                self.images.append(img + ending)
> > +            except CommandError:
> > +                pass
> 
> Should we add the unpacked image in case of error?

I hate this behaviour, to paper over errors. and continue.
its bad enough, that we just continue without an image, and not fail.

Anyways...
this could yield gigabytes of zeros, because the attribute of being
sparse is lost.

> 
> >  class BuildImgFs(ChRootFilesystem):
> >      def __init__(self, path, interpreter):
> > diff --git a/elbepack/elbeproject.py b/elbepack/elbeproject.py
> > index cc8922e4..1f004b77 100644
> > --- a/elbepack/elbeproject.py
> > +++ b/elbepack/elbeproject.py
> > @@ -587,6 +587,8 @@ class ElbeProject (object):
> >                            self.targetfs,
> >                            self.builddir)
> >  
> > +        self.targetfs.pack_images(self.builddir)
> > +
> >          os.system('cat "%s"' % self.validationpath)
> >  
> >      def pdebuild_init(self):
> > diff --git a/elbepack/hdimg.py b/elbepack/hdimg.py
> > index 843c5f85..771c9a35 100644
> > --- a/elbepack/hdimg.py
> > +++ b/elbepack/hdimg.py
> > @@ -664,13 +664,6 @@ def do_hdimg(outf, xml, target, rfs, grub_version, grub_fw_type=None):
> >          if (i.tag == "msdoshd") or (i.tag == "gpthd"):
> >              add_binary_blob(outf, i, target)
> >  
> > -    gz_img_files = []
> > -    for i in set(img_files):
> > -        try:
> > -            outf.do('gzip -f "%s"' % os.path.join(target, i))
> > -            # only add gz to gz_img_files, if no exception is thrown
> > -            gz_img_files.append(i + ".gz")
> > -        except CommandError:
> > -            pass
> > -
> > -    return gz_img_files
> > +    # use set() to remove duplicates, but
> > +    # return a list
> > +    return list(set(img_files))
> > -- 
> > 2.11.0
> > 
> > 
> > _______________________________________________
> > elbe-devel mailing list
> > elbe-devel at linutronix.de
> > https://lists.linutronix.de/mailman/listinfo/elbe-devel

-- 
Torben Hohn
Linutronix GmbH | Bahnhofstrasse 3 | D-88690 Uhldingen-Mühlhofen
Phone: +49 7556 25 999 18; Fax.: +49 7556 25 999 99

Hinweise zum Datenschutz finden Sie hier (Informations on data privacy 
can be found here): https://linutronix.de/kontakt/Datenschutz.php

Linutronix GmbH | Firmensitz (Registered Office): Uhldingen-Mühlhofen | 
Registergericht (Registration Court): Amtsgericht Freiburg i.Br., HRB700 
806 | Geschäftsführer (Managing Directors): Heinz Egger, Thomas Gleixner
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.linutronix.de/pipermail/elbe-devel/attachments/20181204/64174e43/attachment.sig>