[elbe-devel] [PATCH v2 23/28] Nuke ASCIIDocLog from packers

Torben Hohn torben.hohn at linutronix.de
Wed Jun 26 16:11:41 CEST 2019


On Fri, Jun 21, 2019 at 07:40:15PM +0200, dion at linutronix.de wrote:
> From: Olivier Dion <dion at linutronix.de>
> 
> Signed-off-by: Olivier Dion <dion at linutronix.de>

Reviewed-by: Torben Hohn <torben.hohn at linutronix.de>

fix style issue later

> ---
>  elbepack/cdroms.py              | 92 ++++++++++++-----------------------------
>  elbepack/commands/genlicence.py |  2 +-
>  elbepack/commands/mkcdrom.py    |  4 +-
>  elbepack/efilesystem.py         | 32 +++++++-------
>  elbepack/elbeproject.py         | 21 ++++------
>  elbepack/filesystem.py          |  9 ++--
>  elbepack/packers.py             | 22 +++++-----
>  7 files changed, 70 insertions(+), 112 deletions(-)
> 
> diff --git a/elbepack/cdroms.py b/elbepack/cdroms.py
> index 6f6d80a8..60265de2 100644
> --- a/elbepack/cdroms.py
> +++ b/elbepack/cdroms.py
> @@ -7,6 +7,8 @@
>  # SPDX-License-Identifier: GPL-3.0-or-later
>  
>  import os
> +import logging
> +
>  from shutil import copyfile
>  
>  from apt.package import FetchError
> @@ -16,20 +18,13 @@ from elbepack.repomanager import CdromBinRepo
>  from elbepack.repomanager import CdromInitRepo
>  from elbepack.aptpkgutils import XMLPackage
>  from elbepack.filesystem import Filesystem, hostfs
> -from elbepack.shellhelper import CommandError
> +from elbepack.shellhelper import CommandError, do
>  
>  CDROM_SIZE = 640 * 1000 * 1000
>  
>  
> -def mk_source_cdrom(
> -        rfs,
> -        arch,
> -        codename,
> -        init_codename,
> -        target,
> -        log,
> -        cdrom_size=CDROM_SIZE,
> -        xml=None):
> +def mk_source_cdrom(rfs, arch, codename, init_codename, target,
> +                    cdrom_size=CDROM_SIZE, xml=None):
>  
>      # pylint: disable=too-many-arguments
>      # pylint: disable=too-many-locals
> @@ -66,22 +61,14 @@ def mk_source_cdrom(
>          # Do not include forbidden packages in src cdrom
>          if pkg.name in forbiddenPackages:
>              continue
> +        pkg_id = "%s-%s" % (pkg.name, pkg.installed_version)
>          try:
>              dsc = cache.download_source(pkg.name, '/var/cache/elbe/sources')
>              repo.includedsc(dsc, force=True)
>          except ValueError:
> -            log.printo(
> -                "No sources for Package " +
> -                pkg.name +
> -                "-" +
> -                pkg.installed_version)
> +            logging.error("No sources for package '%s'" % pkg_id)
>          except FetchError:
> -            log.printo(
> -                "Source for Package " +
> -                pkg.name +
> -                "-" +
> -                pkg.installed_version +
> -                " could not be downloaded")
> +            logging.error("Source for package '%s' could not be downloaded" % pkg_id)
>  
>      # elbe fetch_initvm_pkgs has downloaded all sources to
>      # /var/cache/elbe/sources
> @@ -103,15 +90,8 @@ def mk_source_cdrom(
>  
>      return repo.buildiso(os.path.join(target, "src-cdrom.iso"))
>  
> -def mk_binary_cdrom(
> -        rfs,
> -        arch,
> -        codename,
> -        init_codename,
> -        xml,
> -        target,
> -        log,
> -        cdrom_size=CDROM_SIZE):
> +def mk_binary_cdrom(rfs, arch, codename, init_codename, xml, target,
> +                    cdrom_size=CDROM_SIZE):
>      # pylint: disable=too-many-arguments
>      # pylint: disable=too-many-locals
>      # pylint: disable=too-many-branches
> @@ -132,16 +112,17 @@ def mk_binary_cdrom(
>      # just copy it. the repo __init__() afterwards will
>      # not touch the repo config, nor generate a new key.
>      try:
> -        log.do('cp -av /var/cache/elbe/initvm-bin-repo "%s"' % repo_path)
> +        do('cp -av /var/cache/elbe/initvm-bin-repo "%s"' % repo_path)
>      except CommandError:
>          # When /var/cache/elbe/initvm-bin-repo has not been created
>          # (because the initvm install was an old version or somthing,
>          #  log an error, and continue with an empty directory.
> -        log.printo('ERROR: /var/cache/elbe/initvm-bin-repo does not exist')
> -        log.printo('       The generated CDROM will not contain initvm pkgs')
> -        log.printo('       This happened because the initvm was probably')
> -        log.printo('       generated with --skip-build-bin')
> -        log.do('mkdir -p "%s"' % repo_path)
> +        logging.exception("/var/cache/elbe/initvm-bin-repo does not exist\n"
> +                          "The generated CDROM will not contain initvm pkgs\n"
> +                          "This happened because the initvm was probably\n"
> +                          "generated with --skip-build-bin")
> +
> +        do('mkdir -p "%s"' % repo_path)
>  
>      repo = CdromInitRepo(init_codename, repo_path, cdrom_size, mirror)
>  
> @@ -152,57 +133,36 @@ def mk_binary_cdrom(
>          cache = get_rpcaptcache(rfs, arch)
>          for p in xml.node("debootstrappkgs"):
>              pkg = XMLPackage(p, arch)
> +            pkg_id = "%s-%s" % (pkg.name, pkg.installed_version)
>              try:
>                  deb = cache.download_binary(pkg.name,
>                                              '/var/cache/elbe/binaries/main',
>                                              pkg.installed_version)
>                  target_repo.includedeb(deb, 'main')
>              except ValueError:
> -                log.printo(
> -                    "No Package " +
> -                    pkg.name +
> -                    "-" +
> -                    pkg.installed_version)
> +                logging.error("No package '%s'" % pkg_id)
>              except FetchError:
> -                log.printo(
> -                    "Package " +
> -                    pkg.name +
> -                    "-" +
> -                    pkg.installed_version +
> -                    " could not be downloaded")
> +                logging.error("Package '%s' could not be downloaded" % pkg_id)
>              except TypeError:
> -                log.printo(
> -                    "Package " +
> -                    pkg.name +
> -                    "-" +
> -                    pkg.installed_version +
> -                    " missing name or version")
> +                logging.error("Package '%s' missing name or version" % pkg_id)
>  
>      cache = get_rpcaptcache(rfs, arch)
>      pkglist = cache.get_installed_pkgs()
>      for pkg in pkglist:
> +        pkg_id = "%s-%s" % (pkg.name, pkg.installed_version)
>          try:
>              deb = cache.download_binary(pkg.name,
>                                          '/var/cache/elbe/binaries/added',
>                                          pkg.installed_version)
>              target_repo.includedeb(deb, 'added', pkg.name, True)
>          except KeyError as ke:
> -            log.printo(str(ke))
> +            logging.error(str(ke))
>          except ValueError:
> -            log.printo("No Package " + pkg.name + "-" + pkg.installed_version)
> +            logging.error("No package %s" % pkg_id)
>          except FetchError:
> -            log.printo("Package " +
> -                       pkg.name +
> -                       "-" +
> -                       str(pkg.installed_version) +
> -                       " could not be downloaded")
> +            logging.error("Package %s could not be downloaded" % pkg_id)
>          except TypeError:
> -            log.printo(
> -                "Package " +
> -                pkg.name +
> -                "-" +
> -                pkg.installed_version +
> -                " missing name or version")
> +            logging.error("Package %s missing name or version")
>  
>      target_repo.finalize()
>  
> diff --git a/elbepack/commands/genlicence.py b/elbepack/commands/genlicence.py
> index 63798d60..14e49258 100644
> --- a/elbepack/commands/genlicence.py
> +++ b/elbepack/commands/genlicence.py
> @@ -39,5 +39,5 @@ def run_command(argv):
>          else:
>              f = io.open('licence.txt', "w+", encoding='utf-8')
>  
> -        rfs.write_licenses(f, log, opt.xml)
> +        rfs.write_licenses(f, opt.xml)
>          f.close()
> diff --git a/elbepack/commands/mkcdrom.py b/elbepack/commands/mkcdrom.py
> index d1c0d467..a8e9afad 100644
> --- a/elbepack/commands/mkcdrom.py
> +++ b/elbepack/commands/mkcdrom.py
> @@ -61,6 +61,7 @@ def run_command(argv):
>          oparser.print_help()
>          sys.exit(20)
>  
> +
>      with elbe_logging(opt.log):
>  
>          if not opt.rfs_only:
> @@ -91,7 +92,7 @@ def run_command(argv):
>          if opt.source:
>              with rfs:
>                  generated_files += mk_source_cdrom(rfs, arch, codename,
> -                                                   init_codename, builddir, log,
> +                                                   init_codename, builddir,
>                                                     opt.cdrom_size)
>  
>          if opt.binary:
> @@ -102,7 +103,6 @@ def run_command(argv):
>                                                     init_codename,
>                                                     xml,
>                                                     builddir,
> -                                                   log,
>                                                     opt.cdrom_size)
>  
>          print("")
> diff --git a/elbepack/efilesystem.py b/elbepack/efilesystem.py
> index 3d2e3d6d..d10fc6ab 100644
> --- a/elbepack/efilesystem.py
> +++ b/elbepack/efilesystem.py
> @@ -12,14 +12,15 @@ import shutil
>  import subprocess
>  import io
>  import stat
> +import logging
>  
> -from elbepack.asciidoclog import CommandError
>  from elbepack.filesystem import Filesystem
>  from elbepack.version import elbe_version
>  from elbepack.hdimg import do_hdimg
>  from elbepack.fstab import fstabentry
>  from elbepack.licencexml import copyright_xml
>  from elbepack.packers import default_packer
> +from elbepack.shellhelper import CommandError, do, chroot, get_command_out
>  
>  
>  def copy_filelist(src, filelist, dst):
> @@ -41,7 +42,7 @@ def copy_filelist(src, filelist, dst):
>              shutil.copystat(src.fname(f), dst.fname(f))
>  
>  
> -def extract_target(src, xml, dst, log, cache):
> +def extract_target(src, xml, dst, cache):
>  
>      # pylint: disable=too-many-locals
>      # pylint: disable=too-many-branches
> @@ -102,19 +103,19 @@ def extract_target(src, xml, dst, log, cache):
>              for item in pkglist:
>                  f.write("%s  install\n" % item)
>  
> -        host_arch = log.get_command_out("dpkg --print-architecture").strip()
> +        host_arch = get_command_out("dpkg --print-architecture").strip()
>          if xml.is_cross(host_arch):
>              ui = "/usr/share/elbe/qemu-elbe/" + str(xml.defs["userinterpr"])
>              if not os.path.exists(ui):
>                  ui = "/usr/bin/" + str(xml.defs["userinterpr"])
> -            log.do('cp %s %s' % (ui, dst.fname("usr/bin")))
> +            do('cp %s %s' % (ui, dst.fname("usr/bin")))
>  
> -        log.chroot(dst.path, "/usr/bin/dpkg --clear-selections")
> -        log.chroot(
> +        chroot(dst.path, "/usr/bin/dpkg --clear-selections")
> +        chroot(
>              dst.path,
>              "/usr/bin/dpkg --set-selections < %s " %
>              dst.fname(psel))
> -        log.chroot(dst.path, "/usr/bin/dpkg --purge -a")
> +        chroot(dst.path, "/usr/bin/dpkg --purge -a")
>  
>  
>  class ElbeFilesystem(Filesystem):
> @@ -136,15 +137,15 @@ class ElbeFilesystem(Filesystem):
>          xml.xml.write(elbe_base)
>          self.chmod("etc/elbe_base.xml", stat.S_IREAD)
>  
> -    def write_licenses(self, f, log, xml_fname=None):
> +    def write_licenses(self, f, xml_fname=None):
>          licence_xml = copyright_xml()
>          for d in self.listdir("usr/share/doc/", skiplinks=True):
>              try:
>                  with io.open(os.path.join(d, "copyright"), "rb") as lic:
>                      lic_text = lic.read()
>              except IOError as e:
> -                log.printo("Error while processing license file %s: '%s'" %
> -                           (os.path.join(d, "copyright"), e.strerror))
> +                logging.error("Error while processing license file %s: '%s'" %
> +                              (os.path.join(d, "copyright"), e.strerror))
>                  lic_text = "Error while processing license file %s: '%s'" % (
>                      os.path.join(d, "copyright"), e.strerror)
>  
> @@ -285,9 +286,8 @@ class ChRootFilesystem(ElbeFilesystem):
>  
>  
>  class TargetFs(ChRootFilesystem):
> -    def __init__(self, path, log, xml, clean=True):
> +    def __init__(self, path, xml, clean=True):
>          ChRootFilesystem.__init__(self, path, xml.defs["userinterpr"], clean)
> -        self.log = log
>          self.xml = xml
>          self.images = []
>          self.image_packers = {}
> @@ -329,7 +329,7 @@ class TargetFs(ChRootFilesystem):
>                      fname=targz_name,
>                      sdir=self.fname('')
>                  )
> -                self.log.do(cmd % args)
> +                do(cmd % args)
>                  # only append filename if creating tarball was successful
>                  self.images.append(targz_name)
>              except CommandError:
> @@ -341,7 +341,7 @@ class TargetFs(ChRootFilesystem):
>              cpio_name = self.xml.text("target/package/cpio/name")
>              os.chdir(self.fname(''))
>              try:
> -                self.log.do(
> +                do(
>                      "find . -print | cpio -ov -H newc >%s" %
>                      os.path.join(
>                          targetdir, cpio_name))
> @@ -356,7 +356,7 @@ class TargetFs(ChRootFilesystem):
>              sfs_name = self.xml.text("target/package/squashfs/name")
>              os.chdir(self.fname(''))
>              try:
> -                self.log.do(
> +                do(
>                      "mksquashfs %s %s/%s -noappend -no-progress" %
>                      (self.fname(''), targetdir, sfs_name))
>                  # only append filename if creating mksquashfs was successful
> @@ -368,7 +368,7 @@ class TargetFs(ChRootFilesystem):
>      def pack_images(self, builddir):
>          for img, packer in self.image_packers.items():
>              self.images.remove(img)
> -            packed = packer.pack_file(self.log, builddir, img)
> +            packed = packer.pack_file(builddir, img)
>              if packed:
>                  self.images.append(packed)
>  
> diff --git a/elbepack/elbeproject.py b/elbepack/elbeproject.py
> index 0269a638..30fe59cd 100644
> --- a/elbepack/elbeproject.py
> +++ b/elbepack/elbeproject.py
> @@ -202,8 +202,8 @@ class ElbeProject (object):
>  
>          # Create TargetFs instance, if the target directory exists
>          if os.path.exists(self.targetpath) and self.buildenv:
> -            self.targetfs = TargetFs(self.targetpath, self.log,
> -                                     self.buildenv.xml, clean=False)
> +            self.targetfs = TargetFs(self.targetpath, self.buildenv.xml,
> +                                     clean=False)
>          else:
>              self.targetfs = None
>  
> @@ -462,7 +462,6 @@ class ElbeProject (object):
>                                                      init_codename,
>                                                      self.xml,
>                                                      self.builddir,
> -                                                    self.log,
>                                                      cdrom_size=cdrom_size)
>              if build_sources:
>                  elog.h1("Source CD %s" % sysrootstr)
> @@ -472,7 +471,6 @@ class ElbeProject (object):
>                                                          self.codename,
>                                                          init_codename,
>                                                          self.builddir,
> -                                                        self.log,
>                                                          cdrom_size=cdrom_size,
>                                                          xml=self.xml)
>                  except SystemError as e:
> @@ -538,11 +536,11 @@ class ElbeProject (object):
>  
>          # Extract target FS. We always create a new instance here with
>          # clean=true, because we want a pristine directory.
> -        self.targetfs = TargetFs(self.targetpath, self.log,
> -                                 self.buildenv.xml, clean=True)
> +        self.targetfs = TargetFs(self.targetpath, self.buildenv.xml,
> +                                 clean=True)
>          os.chdir(self.buildenv.rfs.fname(''))
>          extract_target(self.buildenv.rfs, self.xml, self.targetfs,
> -                       self.log, self.get_rpcaptcache())
> +                       self.get_rpcaptcache())
>  
>          # The validation file is created using check_full_pkgs() and
>          # elbe_report(), both opening the file in append mode. So if an
> @@ -598,9 +596,8 @@ class ElbeProject (object):
>                      "licence.txt"),
>                  "w+",
>                  encoding='utf-8')
> -            self.buildenv.rfs.write_licenses(
> -                f, self.log, os.path.join(
> -                    self.builddir, "licence.xml"))
> +            self.buildenv.rfs.write_licenses(f,
> +                                             os.path.join(self.builddir, "licence.xml"))
>          except Exception:
>              self.log.printo("error during generating licence.txt/xml")
>              self.log.printo(sys.exc_info()[0])
> @@ -867,8 +864,8 @@ class ElbeProject (object):
>          # We use the old content of the directory if no rebuild is done, so
>          # don't clean it (yet).
>          if os.path.exists(self.targetpath):
> -            self.targetfs = TargetFs(self.targetpath, self.log,
> -                                     self.xml, clean=False)
> +            self.targetfs = TargetFs(self.targetpath, self.xml,
> +                                     clean=False)
>          else:
>              self.targetfs = None
>  
> diff --git a/elbepack/filesystem.py b/elbepack/filesystem.py
> index cc7d08f5..47bd8f88 100644
> --- a/elbepack/filesystem.py
> +++ b/elbepack/filesystem.py
> @@ -13,6 +13,8 @@ from glob import glob
>  from tempfile import mkdtemp
>  from string import digits
>  
> +from elbepack.shellhelper import do
> +
>  def size_to_int(size):
>      if size[-1] in digits:
>          return int(size)
> @@ -263,20 +265,19 @@ class TmpdirFilesystem (Filesystem):
>  
>  
>  class ImgMountFilesystem(Filesystem):
> -    def __init__(self, mntpoint, dev, log):
> +    def __init__(self, mntpoint, dev):
>          Filesystem.__init__(self, mntpoint)
>  
>          self.dev = dev
> -        self.log = log
>  
>      def __enter__(self):
>          cmd = 'mount "%s" "%s"' % (self.dev, self.path)
> -        self.log.do(cmd)
> +        do(cmd)
>  
>          return self
>  
>      def __exit__(self, typ, value, traceback):
> -        self.log.do('umount "%s"' % self.path)
> +        do('umount "%s"' % self.path)
>  
>  
>  hostfs = Filesystem('/')
> diff --git a/elbepack/packers.py b/elbepack/packers.py
> index 38bf95b8..894f94c1 100644
> --- a/elbepack/packers.py
> +++ b/elbepack/packers.py
> @@ -4,18 +4,18 @@
>  # SPDX-License-Identifier: GPL-3.0-or-later
>  
>  import os
> -from elbepack.shellhelper import CommandError
> +from elbepack.shellhelper import CommandError, do
>  
>  
>  class Packer(object):
>      # pylint: disable=too-few-public-methods
> -    def pack_file(self, _log, _builddir, _fname):
> +    def pack_file(self, _builddir, _fname):
>          raise NotImplementedError('abstract method called')
>  
>  
>  class NoPacker(Packer):
>      # pylint: disable=too-few-public-methods
> -    def pack_file(self, _log, _builddir, fname):
> +    def pack_file(self, _builddir, fname):
>          return fname
>  
>  
> @@ -26,10 +26,10 @@ class InPlacePacker(Packer):
>          self.cmd = cmd
>          self.suffix = suffix
>  
> -    def pack_file(self, log, builddir, fname):
> +    def pack_file(self, builddir, fname):
>          try:
>              fpath = os.path.join(builddir, fname)
> -            log.do('%s "%s"' % (self.cmd, fpath))
> +            do('%s "%s"' % (self.cmd, fpath))
>          except CommandError:
>              # in case of an error, we just return None
>              # which means, that the orig file does not
> @@ -45,17 +45,17 @@ class TarArchiver(Packer):
>          self.flag = flag
>          self.suffix = suffix
>  
> -    def pack_file(self, log, builddir, fname):
> +    def pack_file(self, builddir, fname):
>          try:
>              fpath = os.path.join(builddir, fname)
>              dirname = os.path.dirname(fpath)
>              basename = os.path.basename(fpath)
>              archname = fpath + self.suffix
> -            log.do('tar cv%sf "%s" --sparse -C "%s" "%s"' % (self.flag,
> -                                                             archname,
> -                                                             dirname,
> -                                                             basename))
> -            log.do('rm -f "%s"' % fpath)
> +            do('tar cv%sf "%s" --sparse -C "%s" "%s"' % (self.flag,
> +                                                         archname,
> +                                                         dirname,
> +                                                         basename))
> +            do('rm -f "%s"' % fpath)
>          except CommandError:
>              # in case of an error, we just return None
>              # which means, that the orig file does not
> -- 
> 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



More information about the elbe-devel mailing list