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

[Manuel Traut]

OK. I ACK all your arguments.

On 14:59 Tue 04 Dec     , Torben Hohn wrote:
> 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