Re: [PATCH v2 2/2] isar-bootstrap: simplified distro source functions

["Tolga Hoşgör" <tlghosgor@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4054 bytes --]

Hi,

The second patch is kind of an unrelated clean up that does a relatively
big change to other parts of the code (get_distro_primary_source_entry to
be exact). I believe it is better off as a separate commit that is
revertible on its own.

On Mon, Dec 17, 2018, 14:55 Claudius Heine <claudius.heine.ext@siemens.com
wrote:

> Hi Tolga,
>
> On 17/12/2018 12.45, Hosgor, Tolga (CT RDA DS EU TR MTS) wrote:
> > - distro sources are created with a generator
> > - debian-buster.list begins with a 'deb-src' to test
> > the parser works correctly
> >
> > Signed-off-by: Hosgor, Tolga (CT RDA DS EU TR MTS) <
> tolga.hosgor@siemens.com>
> > ---
> >   meta-isar/conf/distro/debian-buster.list      |  1 +
> >   .../isar-bootstrap/isar-bootstrap.inc         | 22 ++++++++-----------
> >   2 files changed, 10 insertions(+), 13 deletions(-)
> >
> > diff --git a/meta-isar/conf/distro/debian-buster.list
> b/meta-isar/conf/distro/debian-buster.list
> > index 92d20e1..7828671 100644
> > --- a/meta-isar/conf/distro/debian-buster.list
> > +++ b/meta-isar/conf/distro/debian-buster.list
> > @@ -1,3 +1,4 @@
> > +deb-src      http://ftp.de.debian.org/debian buster  main contrib
> non-free
> >   deb http://ftp.de.debian.org/debian buster  main contrib non-free
> >   deb https://debian.inf.tu-dresden.de/debian buster  main contrib
> non-free
> >   deb http://ftp.de.debian.org/debian buster-updates  main contrib
> non-free
> > diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> > index d868bb6..6dc1888 100644
> > --- a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> > +++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> > @@ -117,7 +117,7 @@ def get_aptsources_list(d, is_host=False):
> >           apt_sources_list = (d.getVar("DISTRO_APT_SOURCES", True) or
> "").split()
> >       return apt_sources_list
> >
> > -def get_distro_primary_source_entry(d, is_host=False):
> > +def generate_distro_sources(d, is_host=False):
> >       apt_sources_list = get_aptsources_list(d, is_host)
> >       for entry in apt_sources_list:
> >           entry_real = bb.parse.resolve_file(entry, d)
> > @@ -126,21 +126,17 @@ def get_distro_primary_source_entry(d,
> is_host=False):
> >                   parsed = parse_aptsources_list_line(line)
> >                   if parsed:
> >                       parsed = get_apt_source_mirror(d, parsed)
> > -                    if parsed[0] == "deb":
> > -                        return parsed[2:]
> > +                    yield parsed
> > +
> > +def get_distro_primary_source_entry(d, is_host=False):
> > +    apt_sources_list = get_aptsources_list(d, is_host)
> > +    for source in generate_distro_sources(d, is_host):
> > +        if source[0] == "deb":
> > +            return source[2:]
> >       return ["", "", ""]
> >
> >   def get_distro_have_https_source(d, is_host=False):
> > -    for entry in get_aptsources_list(d, is_host):
> > -        entry_real = bb.parse.resolve_file(entry, d)
> > -        with open(entry_real, "r") as in_fd:
> > -            for line in in_fd:
> > -                parsed = parse_aptsources_list_line(line)
> > -                if parsed:
> > -                    parsed = get_apt_source_mirror(d, parsed)
> > -                    if parsed[2].startswith("https://"):
> > -                        return True
> > -    return False
> > +    return any(source[2].startswith("https://") for source in
> generate_distro_sources(d, is_host))
>
> Maybe you should merge those changes into the first patch?
>
> Changing the same code in one patchset makes the review more difficult,
> also I don't really see a reason for this split here.
>
> Cheers,
> Claudius
>
> >
> >   def get_distro_needs_https_support(d, is_host=False):
> >       if get_distro_have_https_source(d, is_host):
> >
>
> --
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: ch@denx.de
>

[-- Attachment #2: Type: text/html, Size: 5726 bytes --]