From mboxrd@z Thu Jan  1 00:00:00 1970
X-GM-THRID: 6750962531146137600
Date: Thu, 24 Oct 2019 02:34:30 -0700 (PDT)
From: chombourger@gmail.com
To: isar-users <isar-users@googlegroups.com>
Message-Id: <97aebfdb-669f-4bb3-abd8-b5b97e34e979@googlegroups.com>
In-Reply-To: <20191023203604.4fnnttjymfypiktg@yssyq.m.ilbers.de>
References: <20191023114227.31308-1-henning.schild@siemens.com>
 <20191023114227.31308-3-henning.schild@siemens.com>
 <20191023203604.4fnnttjymfypiktg@yssyq.m.ilbers.de>
Subject: Re: [PATCHv2 2/9] base-apt: use the "basename" command instead of
 pattern substitution
MIME-Version: 1.0
Content-Type: multipart/mixed; 
	boundary="----=_Part_3360_548758477.1571909670473"
X-Google-Token: EKbgxe0FBWbf7JQdrbU0
X-Google-IP: 139.181.48.2
X-TUID: We4WwOXqXFqi

------=_Part_3360_548758477.1571909670473
Content-Type: multipart/alternative; 
	boundary="----=_Part_3361_816133696.1571909670473"

------=_Part_3361_816133696.1571909670473
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit



On Wednesday, October 23, 2019 at 10:36:12 PM UTC+2, Baurzhan Ismagulov 
wrote:
>
> On Wed, Oct 23, 2019 at 01:42:20PM +0200, Henning Schild wrote: 
> > Cosmetic and not functional. Let us not reinvet the wheel and possibly 
> > make mistakes. 
> ... 
> > --- a/meta/classes/base-apt-helper.bbclass 
> > +++ b/meta/classes/base-apt-helper.bbclass 
> > @@ -26,7 +26,7 @@ populate_base_apt() { 
> >          # robust than querying reprepro by name. 
> >   
> >          # Check if this package is taken from Isar-apt, if so - ingore 
> it. 
> > -        base_name=${package##*/} 
> > +        base_name=$( basename "${package}") 
>
> I'm not in favor of this one. The suggested replacement spawns a subshell 
> and 
> executes a binary, which is much more expensive than shell's builtin 
> operation, 
> and that is executed in a loop. The current code is correct. The variable 
> name 
> explains what it does. 
>

there is a typo in the explanation by the way (ingore => ignore)
the parameter expansion appears to work OK with dash and bash
(we always need to be careful with bashisms, we seem to be OK here)


> With kind regards, 
> Baurzhan. 
>

------=_Part_3361_816133696.1571909670473
Content-Type: text/html; charset=utf-8
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><br><br>On Wednesday, October 23, 2019 at 10:36:12 PM UTC+=
2, Baurzhan Ismagulov wrote:<blockquote class=3D"gmail_quote" style=3D"marg=
in: 0;margin-left: 0.8ex;border-left: 1px #ccc solid;padding-left: 1ex;">On=
 Wed, Oct 23, 2019 at 01:42:20PM +0200, Henning Schild wrote:
<br>&gt; Cosmetic and not functional. Let us not reinvet the wheel and poss=
ibly
<br>&gt; make mistakes.
<br>...
<br>&gt; --- a/meta/classes/base-apt-<wbr>helper.bbclass
<br>&gt; +++ b/meta/classes/base-apt-<wbr>helper.bbclass
<br>&gt; @@ -26,7 +26,7 @@ populate_base_apt() {
<br>&gt; =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0# robust than querying reprepro =
by name.
<br>&gt; =C2=A0
<br>&gt; =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0# Check if this package is taken=
 from Isar-apt, if so - ingore it.
<br>&gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0base_name=3D${package##*/}
<br>&gt; + =C2=A0 =C2=A0 =C2=A0 =C2=A0base_name=3D$( basename &quot;${packa=
ge}&quot;)
<br>
<br>I&#39;m not in favor of this one. The suggested replacement spawns a su=
bshell and
<br>executes a binary, which is much more expensive than shell&#39;s builti=
n operation,
<br>and that is executed in a loop. The current code is correct. The variab=
le name
<br>explains what it does.
<br></blockquote><div><br></div><div>there is a typo in the explanation by =
the way (ingore =3D&gt; ignore)</div><div>the parameter expansion appears t=
o work OK with dash and bash</div><div>(we always need to be careful with b=
ashisms, we seem to be OK here)</div><div><br></div><blockquote class=3D"gm=
ail_quote" style=3D"margin: 0;margin-left: 0.8ex;border-left: 1px #ccc soli=
d;padding-left: 1ex;">
<br>With kind regards,
<br>Baurzhan.
<br></blockquote></div>
------=_Part_3361_816133696.1571909670473--

------=_Part_3360_548758477.1571909670473--