Fix the uninstaller calculator

Don't include dependencies which were not installed before.
Fix the "Cascade dependencies" test of "solver".

Change-Id: I807503840af5c841cde89fdbbfc3f6932d4787b8
Reviewed-by: Kai Koehne <kai.koehne@theqtcompany.com>

4 files changed, 53 insertions, 79 deletions

diff --git a/src/libs/installer/packagemanagercore.cpp b/src/libs/installer/packagemanagercore.cpp
index 792323086..adf38885d 100644
--- a/src/libs/installer/packagemanagercore.cpp
+++ b/src/libs/installer/packagemanagercore.cpp
@@ -1251,11 +1251,10 @@ QList<Component*> PackageManagerCore::orderedComponentsToInstall() const
     Calculates a list of components to uninstall based on the current run mode. Auto installed
     dependencies are not yet resolved.  The aboutCalculateComponentsToUninstall() signal is emitted
     before the calculation starts, the finishedCalculateComponentsToUninstall() signal once all
-    calculations are done.
+    calculations are done. Returns always true.
 */
 bool PackageManagerCore::calculateComponentsToUninstall() const
 {
-    bool result = true;
     emit aboutCalculateComponentsToUninstall();
     if (!isUpdater()) {
         // hack to avoid removing needed dependencies
@@ -1269,10 +1268,10 @@ bool PackageManagerCore::calculateComponentsToUninstall() const
 
         d->clearUninstallerCalculator();
         d->storeCheckState();
-        result = d->uninstallerCalculator()->appendComponentsToUninstall(componentsToUninstall);
+        d->uninstallerCalculator()->appendComponentsToUninstall(componentsToUninstall);
     }
     emit finishedCalculateComponentsToUninstall();
-    return result;
+    return true;
 }
 
 /*!
diff --git a/src/libs/installer/uninstallercalculator.cpp b/src/libs/installer/uninstallercalculator.cpp
index d44eaade1..7cd24ad31 100644
--- a/src/libs/installer/uninstallercalculator.cpp
+++ b/src/libs/installer/uninstallercalculator.cpp
@@ -52,100 +52,77 @@ QSet<Component *> UninstallerCalculator::componentsToUninstall() const
     return m_componentsToUninstall;
 }
 
-bool UninstallerCalculator::appendComponentToUninstall(Component *component)
+void UninstallerCalculator::appendComponentToUninstall(Component *component)
 {
     if (!component)
-        return false;
+        return;
+
+    if (!component->isInstalled())
+        return;
 
     PackageManagerCore *core = component->packageManagerCore();
     // remove all already resolved dependees
     QSet<Component *> dependees = core->dependees(component).toSet()
             .subtract(m_componentsToUninstall);
-    if (dependees.isEmpty()) {
-        component->setCheckState(Qt::Unchecked);
-        m_componentsToUninstall.insert(component);
-        return true;
-    }
-
-    QSet<Component *> dependeesToResolve;
-    foreach (Component *dependee, dependees) {
-        if (dependee->isInstalled()) {
-            // keep them as already resolved
-            dependee->setCheckState(Qt::Unchecked);
-            m_componentsToUninstall.insert(dependee);
-            // gather possible dependees, keep them to resolve it later
-            dependeesToResolve.unite(core->dependees(dependee).toSet());
-        }
-    }
 
-    bool allResolved = true;
-    foreach (Component *dependee, dependeesToResolve)
-        allResolved &= appendComponentToUninstall(dependee);
+    foreach (Component *dependee, dependees)
+        appendComponentToUninstall(dependee);
 
-    return allResolved;
+    component->setCheckState(Qt::Unchecked);
+    m_componentsToUninstall.insert(component);
 }
 
-bool UninstallerCalculator::appendComponentsToUninstall(const QList<Component*> &components)
+void UninstallerCalculator::appendComponentsToUninstall(const QList<Component*> &components)
 {
     if (components.isEmpty()) {
         qDebug() << "components list is empty in" << Q_FUNC_INFO;
-        return true;
+        return;
     }
 
-    bool allResolved = true;
-    foreach (Component *component, components) {
-        if (component->isInstalled()) {
-            component->setCheckState(Qt::Unchecked);
-            m_componentsToUninstall.insert(component);
-            allResolved &= appendComponentToUninstall(component);
-        }
-    }
+    foreach (Component *component, components)
+        appendComponentToUninstall(component);
 
     QList<Component*> autoDependOnList;
-    if (allResolved) {
-        // All regular dependees are resolved. Now we are looking for auto depend on components.
-        foreach (Component *component, m_installedComponents) {
-            // If a components is installed and not yet scheduled for un-installation, check for auto depend.
-            if (component->isInstalled() && !m_componentsToUninstall.contains(component)) {
-                QStringList autoDependencies = component->autoDependencies();
-                if (autoDependencies.isEmpty())
-                    continue;
-
-                // This code needs to be enabled once the scripts use isInstalled, installationRequested and
-                // uninstallationRequested...
-                if (autoDependencies.first().compare(QLatin1String("script"), Qt::CaseInsensitive) == 0) {
-                    //QScriptValue valueFromScript;
-                    //try {
-                    //    valueFromScript = callScriptMethod(QLatin1String("isAutoDependOn"));
-                    //} catch (const Error &error) {
-                    //    // keep the component, should do no harm
-                    //    continue;
-                    //}
-
-                    //if (valueFromScript.isValid() && !valueFromScript.toBool())
-                    //    autoDependOnList.append(component);
-                    continue;
-                }
-
-                foreach (Component *c, m_installedComponents) {
-                    const QString replaces = c->value(scReplaces);
-                    const QStringList possibleNames = replaces.split(QInstaller::commaRegExp(),
-                        QString::SkipEmptyParts) << c->name();
-                    foreach (const QString &possibleName, possibleNames)
-                        autoDependencies.removeAll(possibleName);
-                }
-
-                // A component requested auto installation, keep it to resolve their dependencies as well.
-                if (!autoDependencies.isEmpty())
-                    autoDependOnList.append(component);
+    // All regular dependees are resolved. Now we are looking for auto depend on components.
+    foreach (Component *component, m_installedComponents) {
+        // If a components is installed and not yet scheduled for un-installation, check for auto depend.
+        if (component->isInstalled() && !m_componentsToUninstall.contains(component)) {
+            QStringList autoDependencies = component->autoDependencies();
+            if (autoDependencies.isEmpty())
+                continue;
+
+            // This code needs to be enabled once the scripts use isInstalled, installationRequested and
+            // uninstallationRequested...
+            if (autoDependencies.first().compare(QLatin1String("script"), Qt::CaseInsensitive) == 0) {
+                //QScriptValue valueFromScript;
+                //try {
+                //    valueFromScript = callScriptMethod(QLatin1String("isAutoDependOn"));
+                //} catch (const Error &error) {
+                //    // keep the component, should do no harm
+                //    continue;
+                //}
+
+                //if (valueFromScript.isValid() && !valueFromScript.toBool())
+                //    autoDependOnList.append(component);
+                continue;
+            }
+
+            foreach (Component *c, m_installedComponents) {
+                const QString replaces = c->value(scReplaces);
+                const QStringList possibleNames = replaces.split(QInstaller::commaRegExp(),
+                                                                 QString::SkipEmptyParts) << c->name();
+                foreach (const QString &possibleName, possibleNames)
+                    autoDependencies.removeAll(possibleName);
             }
+
+            // A component requested auto installation, keep it to resolve their dependencies as well.
+            if (!autoDependencies.isEmpty())
+                autoDependOnList.append(component);
         }
     }
 
     if (!autoDependOnList.isEmpty())
-        return appendComponentsToUninstall(autoDependOnList);
-    return allResolved;
+        appendComponentsToUninstall(autoDependOnList);
 }
 
-
 } // namespace QInstaller
diff --git a/src/libs/installer/uninstallercalculator.h b/src/libs/installer/uninstallercalculator.h
index d982c9231..48e666c88 100644
--- a/src/libs/installer/uninstallercalculator.h
+++ b/src/libs/installer/uninstallercalculator.h
@@ -52,11 +52,11 @@ public:
 
     QSet<Component*> componentsToUninstall() const;
 
-    bool appendComponentsToUninstall(const QList<Component*> &components);
+    void appendComponentsToUninstall(const QList<Component*> &components);
 
 private:
 
-    bool appendComponentToUninstall(Component *component);
+    void appendComponentToUninstall(Component *component);
 
     QList<Component *> m_installedComponents;
     QSet<Component *> m_componentsToUninstall;
diff --git a/tests/auto/installer/solver/tst_solver.cpp b/tests/auto/installer/solver/tst_solver.cpp
index 60fb40b1b..83f577bdf 100644
--- a/tests/auto/installer/solver/tst_solver.cpp
+++ b/tests/auto/installer/solver/tst_solver.cpp
@@ -247,9 +247,7 @@ private slots:
         calc.appendComponentsToUninstall(selectedToUninstall);
         QSet<Component *> result = calc.componentsToUninstall();
 
-        QEXPECT_FAIL("Cascade dependencies", "Will be fixed in upcomming commit", Continue);
         QCOMPARE(result.count(), expectedResult.count());
-        QEXPECT_FAIL("Cascade dependencies", "Will be fixed in upcomming commit", Continue);
         QCOMPARE(result, expectedResult);
         delete core;
     }