[elbe-devel] [PATCH 1/3] elbeproject: split build_cdroms() out of build()

[John Ogness]

Hi Manuel,

It would have been helpful if this patch was split into 2 patches. The
first one pulling the build cdrom code into the new build_cdroms() (but
without adding any new functionality), then a second patch adding the
new functionality. It makes it much easier to review.

Comments inline...

On 2018-07-12, Manuel Traut <manut at linutronix.de> wrote:
> this allows building cdroms at different stages in the future.
> e.g. after building the sysroot, to include the dev packages
> on the cdroms.
>
> Signed-off-by: Manuel Traut <manut at linutronix.de>
> ---
>  elbepack/elbeproject.py | 69 +++++++++++++++++++++++++----------------
>  1 file changed, 43 insertions(+), 26 deletions(-)
>
> diff --git a/elbepack/elbeproject.py b/elbepack/elbeproject.py
> index 6a8090ff..a50b47c2 100644
> --- a/elbepack/elbeproject.py
> +++ b/elbepack/elbeproject.py
> @@ -349,6 +349,48 @@ class ElbeProject (object):
>          # pdebuild_build(-1) means use all cpus
>          self.pdebuild_build(cpuset=-1)
>  
> +
> +    def build_cdroms(self, build_bin=True, build_sources=False, cdrom_size=None):
> +        self.repo_images = []
> +
> +        elog = ASCIIDocLog(self.validationpath, True)
> +        elog.h1("Source CD")

Since this function builds binary and source CD's, I would expect this
new log entry to be added for each of the CD's. I inserted the lines
where I think it is more appropriate.

> +
> +        env = None
> +        if os.path.exists(self.sysrootpath):
> +            env = BuildEnv(self.xml, self.log, self.sysrootpath,
> +                    build_sources=build_sources, clean=False)
> +        else:
> +            env = BuildEnv(self.xml, self.log, self.chrootpath,
> +                    build_sources=build_sources, clean=False)
> +
> +        with env:
> +            init_codename = self.xml.get_initvm_codename()
> +
> +
> +            if build_bin:

                   elog.h1("Binary CD")

> +                self.repo_images += mk_binary_cdrom(env.rfs,
> +                                                    self.arch,
> +                                                    self.codename,
> +                                                    init_codename,
> +                                                    self.xml,
> +                                                    self.builddir,
> +                                                    self.log,
> +                                                    cdrom_size=cdrom_size)
> +            if build_sources:

                   if os.path.exists(self.sysrootpath):
                       elog.h1("Source CD (including sysroot packages)")
                   else:
                       elog.h1("Source CD")

> +                try:
> +                    self.repo_images += mk_source_cdrom(env.rfs,
> +                                                        self.arch,
> +                                                        self.codename,
> +                                                        init_codename,
> +                                                        self.builddir,
> +                                                        self.log,
> +                                                        cdrom_size=cdrom_size,
> +                                                        xml=self.xml)
> +                except SystemError as e:
> +                    # e.g. no deb-src urls specified
> +                    elog.printo(str(e))
> +
>      def build(self, skip_debootstrap=False, build_bin=False,
>                build_sources=False, cdrom_size=None, debug=False,
>                skip_pkglist=False, skip_pbuild=False):
> @@ -504,32 +546,7 @@ class ElbeProject (object):
>              grub_fw_type = ""
>          self.targetfs.part_target(self.builddir, grub_version, grub_fw_type)
>  
> -        # Build cdrom images
> -        self.repo_images = []
> -        with self.buildenv:
> -            init_codename = self.xml.get_initvm_codename()
> -            if build_bin:
> -                self.repo_images += mk_binary_cdrom(self.buildenv.rfs,
> -                                                    self.arch,
> -                                                    self.codename,
> -                                                    init_codename,
> -                                                    self.xml,
> -                                                    self.builddir,
> -                                                    self.log,
> -                                                    cdrom_size=cdrom_size)
> -            if build_sources:
> -                try:
> -                    self.repo_images += mk_source_cdrom(self.buildenv.rfs,
> -                                                        self.arch,
> -                                                        self.codename,
> -                                                        init_codename,
> -                                                        self.builddir,
> -                                                        self.log,
> -                                                        cdrom_size=cdrom_size,
> -                                                        xml=self.xml)
> -                except SystemError as e:
> -                    # e.g. no deb-src urls specified
> -                    self.log.printo(str(e))
> +        self.build_cdroms(build_bin, build_sources, cdrom_size)
>  
>          if self.postbuild_file:
>              self.log.h2("postbuild script:")