Hello,
> On 10 Dec 2020, at 15:35, Bernhard Bitsch <Bernhard.Bitsch(a)gmx.de> wrote:
>
> Hi,
>
>> Gesendet: Donnerstag, 10. Dezember 2020 um 14:29 Uhr
>> Von: "Michael Tremer" <michael.tremer(a)ipfire.org>
>> An: "Bernhard Bitsch" <Bernhard.Bitsch(a)gmx.de>
>> Cc: "Adolf Belka" <ahb.ipfire(a)gmail.com>, development(a)lists.ipfire.org
>> Betreff: Re: [PATCH] Fix for bug 12539
>>
>> Hi,
>>
>>> On 10 Dec 2020, at 14:13, Bernhard Bitsch <Bernhard.Bitsch(a)gmx.de> wrote:
>>>
>>> Hello,
>>>
>>>> Gesendet: Donnerstag, 10. Dezember 2020 um 13:48 Uhr
>>>> Von: "Michael Tremer" <michael.tremer(a)ipfire.org>
>>>> An: "Adolf Belka" <ahb.ipfire(a)gmail.com>
>>>> Cc: development(a)lists.ipfire.org
>>>> Betreff: Re: [PATCH] Fix for bug 12539
>>>>
>>>> Hello,
>>>>
>>>> Great work. This looks like how it should be.
>>>>
>>>>> On 7 Dec 2020, at 15:01, Adolf Belka <ahb.ipfire(a)gmail.com> wrote:
>>>>>
>>>>> 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.
>>>>>
>>>>> Tested in my ipfire test bed system and only results in one cups
>>>>> entry.
>>>>> Signed-off-by: Adolf Belka <ahb.ipfire(a)gmail.com>
>>>>> ---
>>>>> html/cgi-bin/services.cgi | 9 +++++----
>>>>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>>>>
>>>>> 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=0; # Used to count the outputlines to make different bgcolor
>>>>>
>>>>> # Generate list of installed addon pak's
>>>>> - my @pak = `find /opt/pakfire/db/installed/meta-* 2>/dev/null | cut -d"-" -f2`;
>>>>> + opendir (DIR, "/opt/pakfire/db/installed") || die "Cannot opendir /opt/pakfire/db/installed/: $!";
>>>>> + my @pak = sort readdir DIR;
>>>>> foreach (@pak){
>>>>> chomp($_);
>>>>> + next unless (m/^meta-/);
>>>>> + s/^meta-//;
>>>>
>>>> Although this is the least intuitive thing to do. I have no idea who designed Perl. I hope they are proud of all the chaos they have created :)
>>>>
>>>> -Michael
>>>
>>> Can't see where the solution is 'least intuitive'. As far as I can see from the patch, now the directory of installed addons is read with Perl functions ( not in a extra forked process ). The definition of filenames is used consequently ( 'meta-<addon name>', possible services are stored in /etc/init.d/<addon name> ).
>>
>> To be clear, I am complaining about the Perl language design instead of the code. In my view something like
>>
>> name = name.replace(“meta-“, “”)
>>
>> is a thousand times easier to read.
>>
>> Best,
>> -Michael
>>
>
> Okay, that makes it clearer.
> Perl has its roots in pattern matching. So the base functions of matching and replacing are defined with minimal overhead in writing.
> Maybe the designer(s) of Perl had in mind the description of COBOL as "speaking english with your computer". This was in the beginnings of computing, which did not experience all of us. ;)
> With this single example you are right. Your formulation is much more readable ( besides the fact that name is a string object ). But if you have many such snippets in your code, the Perl way makes a shorter and thus more understandable program.
I disagree. Why would the usage of something that is hard to comprehend be better when it is being used more often? Getting used to the wrong pattern doesn’t make it right.
My main complaint about this is, that it is not clear what is being manipulated. It is better to use the variable name and then there is no confusion about 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 expression 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 easier to understand for reviewers what is supposed to go on.
Best,
-Michael
>
> Best,
> Bernhard
>
>>>
>>> - Bernhard
>>>
>>>
>>>>
>>>>>
>>>>> # Check which of the paks are services
>>>>> - my @svc = `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 system
>>>>> #
>>>>> - chomp($_);
>>>>> if ( $_ eq 'squid' ) {
>>>>> next;
>>>>> }
>>>>> --
>>>>> 2.29.2