Re: [PATCH] Fix for bug 12539

[Michael Tremer <michael.tremer@ipfire.org>]

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

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

> 
> - 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