Kernels are reported as successfully installed when they aren't

[es20490446e]

HOW TO REPRODUCE:

1. Disconnect from the Internet.
2. Open the application "Kernel".
3. On any non installed kernel, click on the "install" button. Follow the screens.

RESULT: Eventually you reach to a screen that says that all changes were made successfully, but if you click on the link "show details" you actually can see that the installation failed.

[Huluti]

@Kirek @philmmanjaro I'm not very comfortable with C ++ but I've the impression that there are some inconsistencies in the ActionDialog.cpp file : Lines 146-149 :

if ( job->error() == 0 )
    jobDone( true, tr ( "Changes failed, click on 'Show Details' for more information" ) );
else
    jobDone( false, tr ( "Changes failed, click on 'Show Details' for more information" ) );

Why does two states (true, false) have the same message ?

Lines 179-194 :

if (success) {
  m_jobSuccesful = true;
  if (message != NULL)
    m_messageLabel-> setText(tr("Changes were made successfully"));
  else
    m_messageLabel->setText(message);
} else {
  m_jobSuccesful = false;
  if (message != NULL)
    m_messageLabel->setText(tr("Changes failed, click on 'Show Details' for more information"));
  else
    m_messageLabel->setText(message);
}

Why verify if message is not NULL and then not use it ?

I don't know if the problem is related with these code blocks but ...

[es20490446e]

Bugs I see:

1. "if (message != NULL)" should be "if (message == NULL)".
2. The variable "success" is true in cases where the command is failing.

Root cause: needs unit testing.

[vfjpl]

For me job->error() is returning 0 no matter what…

[es20490446e]

Where's "error()" declared?

[vfjpl]

https://api.kde.org/frameworks/kcoreaddons/html/classKJob.html#ae0ac2567b61681f4811d128825fbcd0b ?

[es20490446e]

In C++ conditionals aren't integers, but they have their own type.

So the line: if ( job->error() == 0 )

Should be: if (job->error())

[vfjpl]

No, I don't think so.( I have also tested this right now). if(job->error()) is the same as if(job->error() == true) or if(job->error() == 1). 0 is the same as false and 1 is the same as true. Also if you are returning error code it can be different than simply true or false so you have to have ability to differentiate. Also the documentation states that this function return int. int KJob::error()const (link above)

I think that however packets are installed in the job, that how they are installed simply return 0 regardless if they are installed or not. Or another thing is that the thing inside job fails but the job itself finishes successfully so the error code is zero.

I have already fixed the rest of the code(you can look at it there https://github.com/vfjpl/manjaro-settings-manager/tree/fix_for_125) but now the problem is with error() returning 0.

Also I was not able to get the error for failed authorization. (please test)When I click cancel when it ask me about password then the message "Changes failed, click on 'Show Details' for more information" appear and the error code is different that 0. But when it fails to install because of no internet connection then the error code is 0.

[es20490446e]

With the Internet disconnected, if in the Terminal I enter: sudo pacman --sync --noconfirm cheese; echo $?

The return code is 1, with the same error messages than in the Kernel application. It is 0 however only if the package is already available in the system to install, but not already installed.

This discards the bug being in Pacman.

[es20490446e]

So the problematic line is:

auto job = qobject_cast<KAuth::ExecuteJob*>( kjob );

[vfjpl]

I did this but I still have the same problem. connect( kAuthJob, &KAuth::ExecuteJob::result, [this] ( KJob *kjob ) { if ( kjob->error() == 0 ) jobDone( true, tr ( "Changes were made successfully" ) ); else jobDone( false, tr ( "Changes failed, click on 'Show Details' for more information" ) ); } );

I think I found it. https://github.com/manjaro/manjaro-settings-manager/blob/master/src/modules/kernel/KernelAuthHelper.cpp returning code from pacman is somewhat shaky.

[es20490446e]

I can't test it right now, but as I read the qobject cast isn't even necessary?

connect (kAuthJob, &KAuth::ExecuteJob::result, [this] (KJob *job) {
    if (job->error())  jobDone (false, tr ("Failed"));
    else               jobDone (true, tr ("Success"));
});

[vfjpl]

Please test v0.5.4-8 Please do extensive testing and write everything you observe.

[es20490446e]

Added to my todo list.

[es20490446e]

I can't do it myself right now because my Internet connection during holiday is minimal. Only after Christmas.

[vfjpl]

@es20490446e Any news?

[es20490446e]

Reading the above merge pull request, the fix would be available in next upgrade.

[vfjpl]

@es20490446e Fix for this was available in v0.5.4-8 by this https://github.com/manjaro/packages-extra/commit/182b68d3e1592b4871483e818a10a3ea0cd43a07#diff-282c1dba5e2fbf7c997798b6fde80a3e. Merge was done when no bug reports showed up. :)

[vfjpl]

@es20490446e what about your testing? Is it fixed for you? :)

[es20490446e]

Yes.