From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Tremer To: development@lists.ipfire.org Subject: Re: [PATCH] Fix for bug 12539 Date: Thu, 10 Dec 2020 15:48:50 +0100 Message-ID: <54087019-8A43-4FB1-8811-8C1C8B4B2D86@ipfire.org> In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7608883937071429249==" List-Id: --===============7608883937071429249== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hello, > On 10 Dec 2020, at 15:35, Bernhard Bitsch wrote: >=20 > Hi, >=20 >> Gesendet: Donnerstag, 10. Dezember 2020 um 14:29 Uhr >> Von: "Michael Tremer" >> An: "Bernhard Bitsch" >> Cc: "Adolf Belka" , development(a)lists.ipfire.org >> Betreff: Re: [PATCH] Fix for bug 12539 >>=20 >> Hi, >>=20 >>> On 10 Dec 2020, at 14:13, Bernhard Bitsch wrot= e: >>>=20 >>> Hello, >>>=20 >>>> Gesendet: Donnerstag, 10. Dezember 2020 um 13:48 Uhr >>>> Von: "Michael Tremer" >>>> An: "Adolf Belka" >>>> Cc: development(a)lists.ipfire.org >>>> Betreff: Re: [PATCH] Fix for bug 12539 >>>>=20 >>>> Hello, >>>>=20 >>>> Great work. This looks like how it should be. >>>>=20 >>>>> On 7 Dec 2020, at 15:01, Adolf Belka wrote: >>>>>=20 >>>>> The installer recognises cups and cups-filters both as cups and puts >>>>> two instances of cups in the add-on services table. >>>>> Based on input from Michael Tremer this patch replaces the command >>>>> returning the second element between hyphens with one that takes >>>>> what comes after "meta-" using Perl code rather than a shell command. >>>>> The second find command was changed as per Michael's suggestion. >>>>>=20 >>>>> Tested in my ipfire test bed system and only results in one cups >>>>> entry. >>>>> Signed-off-by: Adolf Belka >>>>> --- >>>>> html/cgi-bin/services.cgi | 9 +++++---- >>>>> 1 file changed, 5 insertions(+), 4 deletions(-) >>>>>=20 >>>>> diff --git a/html/cgi-bin/services.cgi b/html/cgi-bin/services.cgi >>>>> index 26ab4f314..36954ba70 100644 >>>>> --- a/html/cgi-bin/services.cgi >>>>> +++ b/html/cgi-bin/services.cgi >>>>> @@ -161,19 +161,20 @@ END >>>>> my $lines=3D0; # Used to count the outputlines to make different bgcol= or >>>>>=20 >>>>> # Generate list of installed addon pak's >>>>> - my @pak =3D `find /opt/pakfire/db/installed/meta-* 2>/dev/null | cut = -d"-" -f2`; >>>>> + opendir (DIR, "/opt/pakfire/db/installed") || die "Cannot opendir /op= t/pakfire/db/installed/: $!"; >>>>> + my @pak =3D sort readdir DIR; >>>>> foreach (@pak){ >>>>> chomp($_); >>>>> + next unless (m/^meta-/); >>>>> + s/^meta-//; >>>>=20 >>>> Although this is the least intuitive thing to do. I have no idea who des= igned Perl. I hope they are proud of all the chaos they have created :) >>>>=20 >>>> -Michael >>>=20 >>> Can't see where the solution is 'least intuitive'. As far as I can see fr= om the patch, now the directory of installed addons is read with Perl functio= ns ( not in a extra forked process ). The definition of filenames is used con= sequently ( 'meta-', possible services are stored in /etc/init.d/= ). >>=20 >> To be clear, I am complaining about the Perl language design instead of th= e code. In my view something like >>=20 >> name =3D name.replace(=E2=80=9Cmeta-=E2=80=9C, =E2=80=9C=E2=80=9D) >>=20 >> is a thousand times easier to read. >>=20 >> Best, >> -Michael >>=20 >=20 > Okay, that makes it clearer.=20 > Perl has its roots in pattern matching. So the base functions of matching a= nd replacing are defined with minimal overhead in writing. > Maybe the designer(s) of Perl had in mind the description of COBOL as "spea= king english with your computer". This was in the beginnings of computing, wh= ich did not experience all of us. ;) > With this single example you are right. Your formulation is much more reada= ble ( besides the fact that name is a string object ). But if you have many s= uch snippets in your code, the Perl way makes a shorter and thus more underst= andable program. I disagree. Why would the usage of something that is hard to comprehend be be= tter when it is being used more often? Getting used to the wrong pattern does= n=E2=80=99t make it right. My main complaint about this is, that it is not clear what is being manipulat= ed. It is better to use the variable name and then there is no confusion abou= t it. The same is true for loads of other Perl-isms like shift, chomp, chop, = etc. Shorter code does not necessarily mean that it executes faster. Probably the = opposite is true in this case, because you would have to compile a regular ex= pression first and the Python version would simply perform a string search. The point is, that code should be obvious. It avoids bugs and it makes it eas= ier to understand for reviewers what is supposed to go on. Best, -Michael >=20 > Best, > Bernhard >=20 >>>=20 >>> - Bernhard >>>=20 >>>=20 >>>>=20 >>>>>=20 >>>>> # Check which of the paks are services >>>>> - my @svc =3D `find /etc/init.d/$_ 2>/dev/null | cut -d"/" -f4`; >>>>> - foreach (@svc){ >>>>> + if (-e "/etc/init.d/$_") { >>>>> # blacklist some packages >>>>> # >>>>> # alsa has trouble with the volume saving and was not really stopped >>>>> # mdadm should not stopped with webif because this could crash the s= ystem >>>>> # >>>>> - chomp($_); >>>>> if ( $_ eq 'squid' ) { >>>>> next; >>>>> } >>>>> -- >>>>> 2.29.2 --===============7608883937071429249==--