From mboxrd@z Thu Jan 1 00:00:00 1970 X-GM-THRID: 6635926855683670016 X-Received: by 2002:a02:95ae:: with SMTP id b43mr12040625jai.4.1545070315911; Mon, 17 Dec 2018 10:11:55 -0800 (PST) X-BeenThere: isar-users@googlegroups.com Received: by 2002:a24:695:: with SMTP id 143ls16391itv.0.gmail; Mon, 17 Dec 2018 10:11:55 -0800 (PST) X-Google-Smtp-Source: AFSGD/WhCdWCg16hbO1gjrNq09LnQAiLXxHaQH6n58WgtZxX72MJ12wW3oIAg6sA2dzCvAqPrZGQ X-Received: by 2002:a24:5f49:: with SMTP id r70mr128947itb.22.1545070315536; Mon, 17 Dec 2018 10:11:55 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1545070315; cv=none; d=google.com; s=arc-20160816; b=MLKfosEQaV8RXmNsrhvHKDjwFlNR5cLYB2BHeP64ErAQt+omxRd7beXYRym2o5TH9r M2TMkolSl5F+jfa26G6/G7qqL87H0NvYjjXTjdhClCoWCeEAKa46IU8qK1H1AiPByy+P HxmIFXybq+LB1HMhNjhSRWIc52G0FKYBooVc2K538w2uo9mIRsQObF/igd+Obw79GqrA /P3eJCsqJrC4dPbE6AY04XUGHGfTgvlURrPgsYtOzwiZwcOLLiLb3U68gYInAhz1AKUD qzRtRsU0YYArvj6vb6JxhRpzZu+AV81BPIbvq1mYA+uS3moKD++C7GKJNPtaxqyyMrmj uhtg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:dkim-signature; bh=b0ePY2zAw6vxfqptbnNaQGUP5Ek/crKnj0ckiy4qTjc=; b=Iow6y+a1hwnO+kt2RMLl3L1anISgkU3ECrfePm/Jqj/EcXcAFo6Jz1gxtGO9uBBXh3 YNlFo3I41onRrz/wqH49GIm1FsX/7fWljHFkiTfnslM67+7ZpirUvJzawDVY1IUbWE/e TTZ88tBZqtb9xG1ebbb/nfzn3AaNLm/KmnkqkH3kbQEh8MHzqiMjaY/KWBCWlDOaijxm Oyz6GYFli+E+J+FC6/Jg+EFBIndFoKSTEzNnMojY7mq8HAbF/ERQoOyzY66JYJzNkclc SD753gKQ4l5nd2v76I9gX8RHjhWeWTK+WV1Hv4zhw9L3vz5DVI+/yghG9acMZ8XSKvFe G1Xw== ARC-Authentication-Results: i=1; gmr-mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=hPv6hVud; spf=pass (google.com: domain of fasdfasdas@gmail.com designates 2607:f8b0:4864:20::d36 as permitted sender) smtp.mailfrom=fasdfasdas@gmail.com; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from mail-io1-xd36.google.com (mail-io1-xd36.google.com. [2607:f8b0:4864:20::d36]) by gmr-mx.google.com with ESMTPS id f13si808335itf.2.2018.12.17.10.11.55 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 17 Dec 2018 10:11:55 -0800 (PST) Received-SPF: pass (google.com: domain of fasdfasdas@gmail.com designates 2607:f8b0:4864:20::d36 as permitted sender) client-ip=2607:f8b0:4864:20::d36; Authentication-Results: gmr-mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=hPv6hVud; spf=pass (google.com: domain of fasdfasdas@gmail.com designates 2607:f8b0:4864:20::d36 as permitted sender) smtp.mailfrom=fasdfasdas@gmail.com; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: by mail-io1-xd36.google.com with SMTP id o13so7222333iom.4 for ; Mon, 17 Dec 2018 10:11:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=b0ePY2zAw6vxfqptbnNaQGUP5Ek/crKnj0ckiy4qTjc=; b=hPv6hVudoHgI8aqAxWDaCbUhkIyk77q2bLZtDUjfQYeRF5nahxMw3lDVFQpZW20ZsD XxU2S9Sb9VED4opp3Ag9/63/X9P8UiX6EI7tSYAUFncGohUGNrq8Y8HXeLru/+2yqeQ4 XRS12NNjg1j3SeJ+xHCPakJdOxUb9WgqDTVTleA23PxlaE4GLQrAjlGnXP+QGR6B840j CtTUT0PxZSJw6z0DNscriyW1zioZMu4goAttLq1HWM3hQJLwAaSxmG3L5cW6kapOup8K 9q4L5k5M63u8ZUW+lxbdJ+LwDPpdHjavO3/EE/HCsm3aw+pxkgK9pgpRXTIJk8H/HBme JoUA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=b0ePY2zAw6vxfqptbnNaQGUP5Ek/crKnj0ckiy4qTjc=; b=EcQSbdYzA6TYWakgMK3j29d6pzKtBHQ9LObHZ1GJkDuvITIwZHvXYhH0Nyt6A+lTr/ ZK64iPAQ2sKG8l7v0aZGpR8I76jizm/6wVo04SuKRcbZMpIBc6o1CKO9C0yMPS2a8q3R JbRPPcxwTWPBJ7yOdJUBhjKEp7PHfUtJ4eGKfiVo97/cSaBAM8w410hbcq/CW5X/fo0F MdIq6RtSAVtOI/rMbbmN+vN73RbbPTFKI0ZJAKE69+NaOgt2L9ULOeyr7xTG60xF2Htw RMXqB/I6agKPcA6G8SbutDTXcVd4cE6xFnO0AHB8irLuoq4px3XEfKsrZ0A1siu5v2Tc yS1Q== X-Gm-Message-State: AA+aEWZT86c+Ad7QEJNPb7vH/i3iPC3KaSgxWnfE5GYhHXwjoIw2H/bN vJ3dTnhYZXl6IloK7E3/MwaOD2NLfkaXBvGevl4= X-Received: by 2002:a5d:9b12:: with SMTP id y18mr5103839ion.243.1545070315079; Mon, 17 Dec 2018 10:11:55 -0800 (PST) MIME-Version: 1.0 References: <20181217114518.17995-1-tolga.hosgor@siemens.com> <20181217114518.17995-3-tolga.hosgor@siemens.com> <91c39b5a-f581-b309-7626-f24149da0473@siemens.com> In-Reply-To: <91c39b5a-f581-b309-7626-f24149da0473@siemens.com> From: =?UTF-8?B?VG9sZ2EgSG/Fn2fDtnI=?= Date: Mon, 17 Dec 2018 21:11:42 +0300 Message-ID: Subject: Re: [PATCH v2 2/2] isar-bootstrap: simplified distro source functions To: Claudius Heine Cc: =?UTF-8?B?VG9sZ2EgSG/Fn2fDtnI=?= , isar-users@googlegroups.com Content-Type: multipart/alternative; boundary="000000000000488e0d057d3bb79e" X-TUID: 837x+bHdM9h2 --000000000000488e0d057d3bb79e Content-Type: text/plain; charset="UTF-8" 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 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 > --000000000000488e0d057d3bb79e Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi,

The= second patch is kind of an unrelated clean up that does a relatively big c= hange to other parts of the code (ge= t_distro_primary_source_entry= to be exact). I believe it is better off as a separate commit that is reve= rtible 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.hos= gor@siemens.com>
> ---
>=C2=A0 =C2=A0meta-isar/conf/distro/debian-buster.list=C2=A0 =C2=A0 =C2= =A0 |=C2=A0 1 +
>=C2=A0 =C2=A0.../isar-bootstrap/isar-bootstrap.inc=C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0| 22 ++++++++-----------
>=C2=A0 =C2=A02 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=C2=A0 =C2=A0 =C2=A0 http://ftp.de.debian.org= /debian buster=C2=A0 main contrib non-free
>=C2=A0 =C2=A0deb http://ftp.de.debian.org/debian b= uster=C2=A0 main contrib non-free
>=C2=A0 =C2=A0deb https://debian.inf.tu-dresden= .de/debian buster=C2=A0 main contrib non-free
>=C2=A0 =C2=A0deb http://ftp.de.debian.org/debian b= uster-updates=C2=A0 main contrib non-free
> diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc b/met= a/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=3DFalse):
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0apt_sources_list =3D (d.getVar= ("DISTRO_APT_SOURCES", True) or "").split()
>=C2=A0 =C2=A0 =C2=A0 =C2=A0return apt_sources_list
>=C2=A0 =C2=A0
> -def get_distro_primary_source_entry(d, is_host=3DFalse):
> +def generate_distro_sources(d, is_host=3DFalse):
>=C2=A0 =C2=A0 =C2=A0 =C2=A0apt_sources_list =3D get_aptsources_list(d, = is_host)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0for entry in apt_sources_list:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0entry_real =3D bb.parse.resolv= e_file(entry, d)
> @@ -126,21 +126,17 @@ def get_distro_primary_source_entry(d, is_host= =3DFalse):
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0pa= rsed =3D parse_aptsources_list_line(line)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if= parsed:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0parsed =3D get_apt_source_mirror(d, parsed)
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= if parsed[0] =3D=3D "deb":
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 return parsed[2:]
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= yield parsed
> +
> +def get_distro_primary_source_entry(d, is_host=3DFalse):
> +=C2=A0 =C2=A0 apt_sources_list =3D get_aptsources_list(d, is_host) > +=C2=A0 =C2=A0 for source in generate_distro_sources(d, is_host):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if source[0] =3D=3D "deb":
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return source[2:]
>=C2=A0 =C2=A0 =C2=A0 =C2=A0return ["", "", "&q= uot;]
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0def get_distro_have_https_source(d, is_host=3DFalse):
> -=C2=A0 =C2=A0 for entry in get_aptsources_list(d, is_host):
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 entry_real =3D bb.parse.resolve_file(entr= y, d)
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 with open(entry_real, "r") as i= n_fd:
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 for line in in_fd:
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 parsed =3D pa= rse_aptsources_list_line(line)
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if parsed: > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= parsed =3D get_apt_source_mirror(d, parsed)
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= if parsed[2].startswith("https://"):
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 return True
> -=C2=A0 =C2=A0 return False
> +=C2=A0 =C2=A0 return any(source[2].startswith("https://") f= or 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

>=C2=A0 =C2=A0
>=C2=A0 =C2=A0def get_distro_needs_https_support(d, is_host=3DFalse): >=C2=A0 =C2=A0 =C2=A0 =C2=A0if get_distro_have_https_source(d, is_host):=
>

--
DENX Software Engineering GmbH,=C2=A0 =C2=A0 =C2=A0 Managing Director: Wolf= gang 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
--000000000000488e0d057d3bb79e--