[elbe-devel] [PATCH v3 2/6] Add optional mirror for host architecture

[Köry Maincent]

Hello Olivier,

On 15/06/2020 15:17, Olivier Dion wrote:
> On Mon, 15 Jun 2020, Kory Maincent <kory.maincent at bootlin.com> wrote:
>> If the primary mirror does not contain the packages for both target and host
>> architecture the build_host_sysroot process fails.
>>
>> This patch adds support for describing in the XML configuration an additional
>> optional mirror containing packages for the host architecture, like:
>> <mirror>
>> <host>http://archive.ubuntu.com/ubuntu</host>
>> </mirror>
>>
>> It also adds the architecture type in the sources.list file to avoid issues
>> with apt-get update.
>>
>> Signed-off-by: Kory Maincent <kory.maincent at bootlin.com>
>> ---
>>   elbepack/elbexml.py | 54 +++++++++++++++++++++++++++++----------------
>>   elbepack/rfs.py     | 24 ++++++++++++--------
>>   schema/dbsfed.xsd   |  7 ++++++
>>   3 files changed, 57 insertions(+), 28 deletions(-)
>>
>> diff --git a/elbepack/elbexml.py b/elbepack/elbexml.py
>> index fa81583..7329e4c 100644
>> --- a/elbepack/elbexml.py
>> +++ b/elbepack/elbexml.py
>> @@ -155,31 +160,42 @@ 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 = []
>> +        options = []
>> +
>> +        options.append(noauth)
>> +        options.append("arch=%s" % arch)
>>   
>> -        mirror = ""
>>           if self.prj.has("mirror/primary_host"):
>> -            mirror += "deb " + noauth + self.get_primary_mirror(None)
>> -            mirror += " " + self.prj.text("suite") + " main\n"
>> +            pmirror = self.get_primary_mirror(None, hostsdk=hostsdk)
>> +            mirror.append("deb [%s] %s %s main" % (' '.join(options),
>> +                               pmirror, self.prj.text("suite")))
> 
> Please align this line correctly like so:
> ----------------------------------------------------------------------
> mirror.append("deb [%s] %s %s main" % (' '.join(options), pmirror,
>                                         self.prj.text("suite")))
> ----------------------------------------------------------------------
> 
> 
>>   
>>               if build_sources:
>> -                mirror += "deb-src " + noauth + self.get_primary_mirror(None)
>> -                mirror += " " + self.prj.text("suite") + " main\n"
>> +                mirror.append("deb-src [%s] %s %s main" % (' '.join(options),
>> +                                  pmirror, self.prj.text("suite")))
> 
> Idem
> 
>>   
>> -            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.append("deb [%s] %s" % (' '.join(options),
>> +                                          url.text("binary").strip()))
> 
> Idem
> 
>>                       if url.has("source"):
>> -                        mirror += "deb-src " + noauth + \
>> -                            url.text("source").strip() + "\n"
>> +                        mirror.append("deb-src [%s] %s" % (' '.join(options),
>> +                                          url.text("source").strip()))
> 
> Idem
> 
>>           if self.prj.has("mirror/cdrom"):
>> -            mirror += "deb copy:///cdrom/targetrepo %s main added\n" % (
>> -                self.prj.text("suite"))
>> +            mirror.append("deb copy:///cdrom/targetrepo %s main added" % (
>> +                self.prj.text("suite")))
> 
> Never do that please (ending a line with an open parenthesis).  Elbe
> source code is clustered with these.  It's very bad for grepping.  Feel
> free to start the formatting on a newline like this:
> ----------------------------------------------------------------------
> mirror.append("deb copy:///cdrom/targetrepo %s main added" %
>                (self.prj.text("suite")))
> ----------------------------------------------------------------------
> or do the formatting in a temporary variable before appending to the
> list if you think that it's more greppable.
> 
> Since I'm not familiar with Ubuntu stuff, I don't feel confortable with
> giving you review for this patch.  But I think that things are in order,
> just fix the syntax please.

Ok, I will fix this.
Thanks for your reviews.

> 
> I'm CC some Debian wizards that can give you review for that :-)
> 

Regards,

-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com