[elbe-devel] [PATCH v2 3/3] Unpack the archives depending on volume attribute

Torben Hohn torben.hohn at linutronix.de
Tue Sep 10 13:26:00 CEST 2019


On Tue, Sep 10, 2019 at 11:20:31AM +0200, Akash Satamkar wrote:
> The volume attribute can be used to specify the volume number
> in which the archive is added.
> 
> When volume attribute is not specified, the archive is added
> to each volume.
> 
> Signed-off-by: Akash Satamkar <akash at linutronix.de>
> ---
>  elbepack/archivedir.py  | 44 +++++++++++++++++++++++++++--------------
>  elbepack/cdroms.py      | 15 ++++++++++++++
>  elbepack/repomanager.py | 12 +++++++++--
>  3 files changed, 54 insertions(+), 17 deletions(-)
> 
> diff --git a/elbepack/archivedir.py b/elbepack/archivedir.py
> index 10155228..6e7da2f8 100644
> --- a/elbepack/archivedir.py
> +++ b/elbepack/archivedir.py
> @@ -17,8 +17,9 @@ from base64 import standard_b64encode
>  from bz2 import compress as bz2compress
>  from subprocess import CalledProcessError
>  
> -from elbepack.treeutils import etree
> +from elbepack.treeutils import etree, elem
>  from elbepack.shellhelper import system
> +from elbepack.filesystem import TmpdirFilesystem
>  
>  class ArchivedirError(Exception):
>      pass
> @@ -90,32 +91,45 @@ def get_and_append_method(url):
>          'file': get_and_append_local,
>      }.get(urlparse(url).scheme, get_and_append_unknown)
>  
> -def _combinearchivedir(xml):
> +def _combinearchivedir(xml, xpath):
>      elbexml = etree(None)
>      elbexml.et = xml
>  
> -    archive = '.combinedarchive.tar'
> -    for archivedir in xml.iterfind("archivedir"):
> +    tmp = TmpdirFilesystem()
> +    for archivedir in xml.iterfind(xpath):
>          try:
>              archiveurl = urljoin(archivedir.base, archivedir.text)
>              keep = elbexml.check_boolean(archivedir, "keep-attributes")
> +            parent = archivedir.getparent()
> +            volume_attr = archivedir.get('volume')
> +            if volume_attr:
> +                arch = parent.get("archive[@volume='%s']" % volume_attr)
> +                if not arch:
> +                    arch = elem(parent).append("archive")
> +                    arch.et.set("volume", volume_attr)
> +            else:
> +                volume_attr = ''
> +                arch = elem(parent).ensure_child("archive")
> +                if arch.et.get('volume'):
> +                    arch = elem(parent).append("archive")

here is the comment from my v1 review...

----------------------------------------------------------------------------
this is overly complicated.                                                                                                   
                                                                                                                              
if volume was mandatory and "all" for all cdroms, the first block                                                             
of the if statement would cover all cases. and the confusing usage of                                                         
ensure_child could be dropped.                                                                                                
                                                                                                                              
however, care must be taken for the old archive code, which does not                                                          
accept the volume attribute.                                                                                                  
But a bool parameter 'use_volume' could take care of that.        
----------------------------------------------------------------------------


> +
>              get_and_append = get_and_append_method(archiveurl)
> -            get_and_append(archiveurl, archive, keep)
> -            archivedir.getparent().remove(archivedir)
> +
> +            archname = tmp.fname('archive%s.tar.bz2' % volume_attr)
> +            get_and_append(archiveurl, archname, keep)
> +            arch.set_text(enbase(archname, True))
> +
> +            parent.remove(archivedir)
> +
>          except (CalledProcessError, OSError):
>              msg = "Failure while processing \"" + archivedir.text + "\":\n"
>              msg += str(sys.exc_info()[1])
>              raise ArchivedirError(msg)
>  
> -    arch = elbexml.ensure_child("archive")
> -    arch.set_text(enbase(archive, True))
> -
> -    os.remove(archive)
> -
> -    return xml
> -
>  def combinearchivedir(xml):
> -    if xml.find("archivedir") is None:
> +    if xml.find("//archivedir") is None:
>          return xml
>  
> -    return _combinearchivedir(xml)
> +    _combinearchivedir(xml, "archivedir")
> +    _combinearchivedir(xml, "src-cdrom/archivedir")
> +    return xml
> diff --git a/elbepack/cdroms.py b/elbepack/cdroms.py
> index d2c1b2b0..cbeea6a4 100644
> --- a/elbepack/cdroms.py
> +++ b/elbepack/cdroms.py
> @@ -20,6 +20,8 @@ from elbepack.aptpkgutils import XMLPackage
>  from elbepack.filesystem import Filesystem, hostfs
>  from elbepack.shellhelper import CommandError, do
>  from elbepack.isooptions import get_iso_options
> +from elbepack.treeutils import etree, elem
> +from elbepack.shellhelper import system

i dont see that system is used anywhere, why is that imported ?

>  
>  CDROM_SIZE = 640 * 1000 * 1000
>  
> @@ -91,6 +93,19 @@ def mk_source_cdrom(rfs, arch, codename, init_codename, target,
>  
>      if xml is not None:
>          options = get_iso_options(xml)
> +
> +        for arch in xml.node('src-cdrom').all('archive'):
> +            volume_attr = arch.et.get('volume')
> +            if volume_attr:
> +                volume_list = [int(v) for v in volume_attr.split(",")]
> +            else:
> +                volume_list = repo.volume_indexes()
> +            for volume_number in volume_list:
> +                if (volume_number <= 8):

why shall the number of volumes be limited to 8 ?

> +                    with xml.archive_tmpfile(arch) as fp:
> +                        do('tar xvfj "%s" -h -C "%s"' % (fp.name, repo.get_volume_fs(volume_number).path))
> +                else:
> +                    logging.warning("Invalid volume_no. Please try between 0...8")
>      else:
>          options = ""

this is unpacking of archive.
should be in a different patch from the preprocessor changes.

>  
> diff --git a/elbepack/repomanager.py b/elbepack/repomanager.py
> index f8ec8a23..1a4eab66 100644
> --- a/elbepack/repomanager.py
> +++ b/elbepack/repomanager.py
> @@ -98,7 +98,13 @@ class RepoBase(object):
>  
>      def get_volume_fs(self, volume):
>          if self.maxsize:
> -            volname = os.path.join(self.vol_path, "vol%02d" % volume)
> +            if volume >= 0:
> +                volume_no = volume
> +            else:
> +                volume_no = self.volume_count + 1 + volume
> +            # negative numbers represent the volumes counted from last
> +            # (-1: last, -2: second last, ...)
> +            volname = os.path.join(self.vol_path, "vol%02d" % volume_no)
>              return Filesystem(volname)
>  
>          return Filesystem(self.vol_path)
> @@ -289,7 +295,7 @@ class RepoBase(object):
>                 (options, fname, new_path))
>              files.append(fname)
>          else:
> -            for i in range(self.volume_count + 1):
> +            for i in self.volume_indexes():
>                  volfs = self.get_volume_fs(i)
>                  newname = fname + ("%02d" % i)
>                  do("genisoimage %s -o %s -J -joliet-long -R %s" %
> @@ -298,6 +304,8 @@ class RepoBase(object):
>  
>          return files
>  
> +    def volume_indexes(self):
> +        return range(self.volume_count + 1)

this could be a property please decorate with @property

>  
>  class UpdateRepo(RepoBase):
>      def __init__(self, xml, path):
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> 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