[elbe-devel] [PATCH v2 2/4] Add optional mirror for host architecture

[Olivier Dion]

On Fri, 12 Jun 2020, Kory Maincent <kory.maincent at bootlin.com> wrote:
> diff --git a/elbepack/elbexml.py b/elbepack/elbexml.py
> index fa81583..f0e3307 100644
> --- a/elbepack/elbexml.py
> +++ b/elbepack/elbexml.py
> @@ -132,13 +134,16 @@ class ElbeXML(object):
>  
>          return mirror.replace("LOCALMACHINE", "10.0.2.2")
>  
> -    def get_primary_mirror(self, cdrompath, initvm=True):
> +    def get_primary_mirror(self, cdrompath, initvm=True, hostsdk=False):
>          if self.prj.has("mirror/primary_host"):
>              m = self.prj.node("mirror")
>  
> -            mirror = m.text("primary_proto") + "://"
> -            mirror += m.text("primary_host") + "/"
> -            mirror += m.text("primary_path")
> +            if hostsdk and self.prj.has("mirror/host"):
> +                mirror = m.text("host")
> +            else:
> +                mirror = m.text("primary_proto") + "://"
> +                mirror += m.text("primary_host") + "/"
> +                mirror += m.text("primary_path")
>  
>          elif self.prj.has("mirror/cdrom") and cdrompath:
>              mirror = "file://%s" % cdrompath
> @@ -146,7 +151,7 @@ class ElbeXML(object):
>          return replace_localmachine(mirror, initvm)
>  
>      # XXX: maybe add cdrom path param ?
> -    def create_apt_sources_list(self, build_sources=False, initvm=True):
> +    def create_apt_sources_list(self, build_sources=False, initvm=True, hostsdk=False):
>          if self.prj is None:
>              return "# No Project"
>  
> @@ -155,24 +160,31 @@ class ElbeXML(object):
>  
>          noauth = ""
>          if self.prj.has("noauth"):
> -            noauth = "[trusted=yes] "
> +            noauth = "trusted=yes "
> +
> +        if hostsdk:
> +            arch = get_command_out("dpkg --print-architecture").strip().decode()
> +        else:
> +            arch = self.text("project/buildimage/arch", key="arch")
>  
>          mirror = ""
> +        option = "[" + noauth + " arch=" + arch +"] "
>          if self.prj.has("mirror/primary_host"):
> -            mirror += "deb " + noauth + self.get_primary_mirror(None)
> +            mirror += "deb " + option + self.get_primary_mirror(None, hostsdk=hostsdk)
>              mirror += " " + self.prj.text("suite") + " main\n"
>  
> +
>              if build_sources:
> -                mirror += "deb-src " + noauth + self.get_primary_mirror(None)
> +                mirror += "deb-src " + option + self.get_primary_mirror(None, hostsdk=hostsdk)
>                  mirror += " " + self.prj.text("suite") + " main\n"
>  
> -            if self.prj.has("mirror/url-list"):
> +            if self.prj.has("mirror/url-list") and not hostsdk:
>                  for url in self.prj.node("mirror/url-list"):
>                      if url.has("binary"):
> -                        mirror += "deb " + noauth + \
> -                                   url.text("binary").strip() + "\n"
> +                        mirror += "deb " + option + \
> +                            url.text("binary").strip() + "\n"
>                      if url.has("source"):
> -                        mirror += "deb-src " + noauth + \
> +                        mirror += "deb-src " + option + \
>                              url.text("source").strip() + "\n"
>  
For these parts, I would tend to accumulate thing in a list for mirror
instead of self-concatenating.  I would also format the strings instead
of concatenating them.  Something like:
----------------------------------------------------------------------
options = []
mirror  = []

options.append(noauth)
options.append("arch=%s" % arch)

if self.prj.has("mirror/primary_host"):
   pmirror     = self.get_primary_mirror(None)
   pmirror_sdk = self.get_primary_mirror(None, hostsdk=hostsdk)
   
   mirror.append("deb %s %s" % (noauth, pmirror))
   mirror.append("deb [%s] %s" % (' '.join(options), pmirror_sdk))
   ...
return replace_localmachine('\n'.join(mirror), initvm)
----------------------------------------------------------------------
I find it easier to understand and less error prone to missing spaces or
newlines in string.  This code has not been touched for a long time, but
it would benifit from that I think.

> diff --git a/elbepack/rfs.py b/elbepack/rfs.py
> index 6967d28..85c3d4b 100644
> --- a/elbepack/rfs.py
> +++ b/elbepack/rfs.py
> @@ -96,7 +96,14 @@ class BuildEnv (object):
>              self.fresh_debootstrap = False
>              self.need_dumpdebootstrap = False
>  
> -        self.initialize_dirs(build_sources=build_sources)
> +        host_arch = get_command_out("dpkg --print-architecture").strip().decode()
> +
> +        if arch == host_arch:
> +            hostsdk = True
> +        else:
> +            hostsdk = False
host_sdk = arch == host_arch

> @@ -161,8 +168,18 @@ class BuildEnv (object):
> +        if arch == host_arch:
> +            hostsdk = True
> +        else:
> +            hostsdk = False
host_sdk = arch == host_arch

-- 
Olivier Dion
Linutronix GmbH | Bahnhofstrasse 3 | D-88690 Uhldingen-Mühlhofen