[elbe-devel] [PATCH v2 3/4] cdroms: Use SrcManager to download packages

[Torben Hohn]

On Mon, May 04, 2020 at 01:42:02PM -0400, Olivier Dion wrote:
> Signed-off-by: Olivier Dion <dion at linutronix.de>
> ---
>  elbepack/cdroms.py | 38 ++++++++++++++++++++++----------------
>  1 file changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/elbepack/cdroms.py b/elbepack/cdroms.py
> index ed8730fc..70582b12 100644
> --- a/elbepack/cdroms.py
> +++ b/elbepack/cdroms.py
> @@ -52,7 +52,7 @@ class SrcManager(object):
>          self.forbid.add(name)
>  
>  def mk_source_cdrom(rfs, arch, codename, init_codename, target,
> -                    cdrom_size=CDROM_SIZE, xml=None):
> +                    cdrom_size=CDROM_SIZE, xml=None, others={}):
>  
>      # pylint: disable=too-many-arguments
>      # pylint: disable=too-many-locals
> @@ -73,30 +73,36 @@ def mk_source_cdrom(rfs, arch, codename, init_codename, target,
>  
>      cache = get_rpcaptcache(rfs, arch)
>      cache.update()
> -    pkglist = cache.get_installed_pkgs()
>  
> -    forbiddenPackages = []
> +    man = SrcManager()
> +
>      if xml is not None and xml.has('target/pkg-list'):
>          for i in xml.node('target/pkg-list'):
>              try:
>                  if i.tag == 'pkg' and i.et.attrib['on_src_cd'] == 'False':
> -                    forbiddenPackages.append(i.text('.').strip())
> -
> +                    man.forbid_pkg(i.text('.').strip())
>              except KeyError:
>                  pass
>  
> -    for pkg in pkglist:
> -        # Do not include forbidden packages in src cdrom
> -        if pkg.name in forbiddenPackages:
> -            continue
> -        pkg_id = "%s-%s" % (pkg.name, pkg.installed_version)
> +    # Do main component
> +    for pkg in cache.get_installed_pkgs():
> +            man.add_pkg(pkg.name, pkg.installed_version, "main")

What is missing is the differentiation between "added" packages and the
"main" packages.

the binary cdrom contains "added" and "main" component.

This is because debootstrap only supports a single repo.
And then afterwards the other package sources are installed via an
update.

We replicate this, when installing from cdrom.


i am also not really happy with the srcmanager. i mean ... you 
have components, and cache.get_installed_pkgs(), then you put them
both in to the manager, just to iterate over them and d/l the sources.




> +
> +    # Do other components
> +    for component in others:
> +        for name, version in others[component]:
> +            man.add_pkg(name, version, component)
> +
> +    for pkg, version, component in man:
> +        logging.info('Downloading "%s-%s" for %s', pkg, version, component)
>          try:
> -            dsc = cache.download_source(pkg.name, '/var/cache/elbe/sources')
> -            repo.includedsc(dsc, force=True)
> -        except ValueError:
> -            logging.error("No sources for package '%s'", pkg_id)
> -        except FetchError:
> -            logging.error("Source for package '%s' could not be downloaded", pkg_id)
> +            dsc = cache.download_source(pkg,
> +                                        '/var/cache/elbe/sources',
> +                                        version=version)
> +            repo.includedsc(dsc, component=component, force=True)

why dont you just isolate the body of this loop into a separate
method/function and call that instead of man.add_pkg ?

a local function might make sense here... to keep the number
of parameters low.


> +        except Exception as E:
> +            logging.exception(E)

why do you remove this Error handling.
logging an exception is nice for programmers. But
Users are happier, when there is a clear Error message.

Also ValueError is not as good, as The Package Source does not exist
in any of the debian repositories you have added. Did you forget to
add a <src> line in <url-list> ??

pylint complains about catching general Exception for a reason.
There are some occurences of that. But we really want to keep these
scarce. They are mainly to catch any Exception and marshal it through
the soap interface, or rpc aptcache interface.


> +
>  
>      # elbe fetch_initvm_pkgs has downloaded all sources to
>      # /var/cache/elbe/sources
> -- 
> 2.26.2
> 
> 
> _______________________________________________
> 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