Re: [PATCH] pakfire.cgi: Bring back old logic for log displaying

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

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

Hello,

> On 15 Nov 2021, at 20:23, Stefan Schantl <stefan.schantl(a)ipfire.org> wrote:
> 
> Trying to get rid of the system backpipe check if a pakfire is running
> does not work very well. It simply makes the code more complex and
> only introduced some new problems.
> 
> This commit switches back to the old logic which worked well in the
> past.
> 
> Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
> ---
> html/cgi-bin/pakfire.cgi | 57 +++++++---------------------------------
> 1 file changed, 10 insertions(+), 47 deletions(-)
> 
> diff --git a/html/cgi-bin/pakfire.cgi b/html/cgi-bin/pakfire.cgi
> index f2381f031..4d6eee284 100644
> --- a/html/cgi-bin/pakfire.cgi
> +++ b/html/cgi-bin/pakfire.cgi
> @@ -44,8 +44,6 @@ $cgiparams{'VALID'} = '';
> $cgiparams{'INSPAKS'} = '';
> $cgiparams{'DELPAKS'} = '';
> 
> -my $page_lock;
> -
> sub refreshpage{&Header::openbox( 'Waiting', 1, "<meta http-equiv='refresh' content='1;'>" );print "<center><img src='/images/clock.gif' alt='' /><br/><font color='red'>$Lang::tr{'pagerefresh'}</font></center>";&Header::closebox();}
> 
> &Header::getcgihash(\%cgiparams);
> @@ -59,9 +57,8 @@ sub refreshpage{&Header::openbox( 'Waiting', 1, "<meta http-equiv='refresh' cont
> if (($cgiparams{'ACTION'} eq 'install') && (! -e $Pakfire::lockfile)) {
> 	my @pkgs = split(/\|/, $cgiparams{'INSPAKS'});
> 	if ("$cgiparams{'FORCE'}" eq "on") {
> -		# Lock the page.
> -		$page_lock = "1";
> 		&General::system_background("/usr/local/bin/pakfire", "install", "--non-interactive", "--no-colors", @pkgs);
> +		sleep(1);
> 	} else {
> 		&Header::openbox("100%", "center", $Lang::tr{'request'});
> 		my @output = &General::system_output("/usr/local/bin/pakfire", "resolvedeps", "--no-colors", @pkgs);
> @@ -98,9 +95,8 @@ END
> } elsif (($cgiparams{'ACTION'} eq 'remove') && (! -e $Pakfire::lockfile)) {
> 	my @pkgs = split(/\|/, $cgiparams{'DELPAKS'});
> 	if ("$cgiparams{'FORCE'}" eq "on") {
> -		# Lock the page.
> -		$page_lock = "1";
> 		&General::system_background("/usr/local/bin/pakfire", "remove", "--non-interactive", "--no-colors", @pkgs);
> +		sleep(1);
> 	} else {
> 		&Header::openbox("100%", "center", $Lang::tr{'request'});
> 		my @output = &General::system_output("/usr/local/bin/pakfire", "resolvedeps", "--no-colors", @pkgs);
> @@ -136,13 +132,11 @@ END
> 	}
> 
> } elsif (($cgiparams{'ACTION'} eq 'update') && (! -e $Pakfire::lockfile)) {
> -	# Set variable to lock the page.
> -	$page_lock = "1";
> 	&General::system_background("/usr/local/bin/pakfire", "update", "--force", "--no-colors");
> +	sleep(1);
> } elsif (($cgiparams{'ACTION'} eq 'upgrade') && (!-e $Pakfire::lockfile)) {
> -	# Lock the page.
> -	$page_lock = "1";
> 	&General::system_background("/usr/local/bin/pakfire", "upgrade", "-y", "--no-colors");
> +	sleep(1);
> } elsif ($cgiparams{'ACTION'} eq "$Lang::tr{'save'}") {
> 	$pakfiresettings{"TREE"} = $cgiparams{"TREE"};
> 
> @@ -156,6 +150,7 @@ END
> 
> 		# Update lists
> 		&General::system_background("/usr/local/bin/pakfire", "update", "--force", "--no-colors");
> +		sleep(1);
> 	}
> }
> 
> @@ -177,44 +172,12 @@ if ($errormessage) {
> 	&Header::closebox();
> }
> 
> -# Check if a page lock is required.
> -if ($page_lock) {
> -	&Header::openbox('Waiting', 1, ,);
> -		print <<END;
> -		<table>
> -			<tr>
> -				<td>
> -					<img src='/images/indicator.gif' alt='$Lang::tr{'active'}' title='$Lang::tr{'active'}'>
> -				</td>
> -
> -				<td>
> -					$Lang::tr{'pakfire working'}
> -				</td>
> -			</tr>
> -		</table>
> -END
> -	&Header::closebox();
> -
> -	# Infinite loop to lock the page until pakfire lockfile is present.
> -	while($page_lock) {
> -		unless (-e $Pakfire::lockfile) {
> -			sleep(1);
> -		} else {
> -			# Release page lock.
> -			undef($page_lock);
> -
> -			# Break loop.
> -			last;
> -		}
> -	}
> -
> -	# Perform page reload.
> -	print "<meta http-equiv='refresh' content='1;'>\n";

There should indeed be no semi-colon here.

> -	exit;
> -}
> +# Check if pakfire is already running.
> +#
> +# The system backpipe command is safe, because no user input is computed.
> +my $pid = `pidof pakfire`;
> 
> -# Check if pakfire is already running. In this case a lockfile is present.
> -if (-e $Pakfire::lockfile) {
> +if ($pid) {
> 	&Header::openbox( 'Waiting', 1, "<meta http-equiv='refresh' content='10;'>" );

Likewise.

And yes, it should be in the header, too. However, since the entire file does not have any functions, it is impossible to go back to this.

Since this is already a very ugly solution (but one that works), I would say we leave it like this and hope for the best. We have bigger fish to fry.

-Michael

> 	print <<END;
> 	<table>
> -- 
> 2.30.2
>