Fix bug when installer is not installing required components

Components can be requested to install by several components using
dependency. Components should have a reference count to inform how many
components are requesting the install, so that the component can be
removed from install when nobody has requested it, or user has not
explicitly selected it for install.

Dependency reference count was not calculated correctly, user selection
was ignored if component was already added as dependency. Also if
component which is added as dependency is also autodependency to same
component should not increase the reference count, as the component is
uninstalled when the reference count is zero. Having both autodependency
and dependency would never cause the reference count to be zero.

Task-number: QTIFW-2708
Change-Id: I398bc892f6626d76c872402d8ab55cb8115112ae
Reviewed-by: Arttu Tarkiainen <arttu.tarkiainen@qt.io>

2 files changed, 73 insertions, 44 deletions

diff --git a/src/libs/installer/installercalculator.cpp b/src/libs/installer/installercalculator.cpp
index 2ef5d3b74..e321872ee 100644
--- a/src/libs/installer/installercalculator.cpp
+++ b/src/libs/installer/installercalculator.cpp
@@ -102,10 +102,16 @@ bool InstallerCalculator::appendComponentsToInstall(const QList<Component *> &co
                     qPrintable(errorMessage));
                 return false;
             }
-            if (!revertFromInstall) {
-              // We can end up here if component is already added as dependency and
-              // user explicitly selects it to install
-              continue;
+            if (!revertFromInstall && !modelReset) {
+                // We can end up here if component is already added as dependency and
+                // user explicitly selects it to install. Increase the references to
+                // know when the component should be removed from install
+                const QStringList dependenciesList = component->currentDependencies();
+                QSet<QString> allDependencies(dependenciesList.begin(), dependenciesList.end());
+
+                foreach (const QString &dependencyComponentName, allDependencies)
+                    calculateComponentDependencyReferences(dependencyComponentName, component);
+                continue;
             }
         }
 
@@ -178,7 +184,7 @@ void InstallerCalculator::realAppendToInstallComponents(Component *component, co
 bool InstallerCalculator::appendComponentToInstall(Component *component, const QString &version, bool revertFromInstall)
 {
     const QStringList dependenciesList = component->currentDependencies();
-    QSet<QString> allDependencies(dependenciesList.begin(), dependenciesList.end());
+    const QSet<QString> allDependencies(dependenciesList.begin(), dependenciesList.end());
     QString requiredDependencyVersion = version;
     foreach (const QString &dependencyComponentName, allDependencies) {
         // PackageManagerCore::componentByName returns 0 if dependencyComponentName contains a
@@ -220,52 +226,53 @@ bool InstallerCalculator::appendComponentToInstall(Component *component, const Q
         }
 
         // Component can be requested for install by several component (as a dependency).
-        // Keep the reference count to a component, so we know when component needs to be
-        // removed from install.
-        if (!revertFromInstall && m_toInstallComponentIds.contains(dependencyComponentName)) {
-            QStringList value = m_referenceCount.value(dependencyComponentName, QStringList());
-            if (!value.contains(component->name()))
-                value << component->name();
-            m_referenceCount.insert(dependencyComponentName, value);
-        }
+        // Keep the reference count to a dependency component, so we know when component
+        // needs to be removed from install. Do not increase the reference count when
+        // the dependency is also autodependency as the component is removed anyway only
+        // when there are no references to the dependency.
         if (revertFromInstall) {
             if (m_toInstallComponentIds.contains(dependencyComponentName)
                     && m_referenceCount.contains(dependencyComponentName)) {
-                QStringList value = m_referenceCount.value(dependencyComponentName);
-                if (value.contains(component->name()))
-                    value.removeOne(component->name());
-                if (value.isEmpty())
-                    m_referenceCount.remove(dependencyComponentName);
-                else
-                    m_referenceCount.insert(dependencyComponentName, value);
+                if (!component->autoDependencies().contains(dependencyComponentName)) {
+                    QStringList value = m_referenceCount.value(dependencyComponentName);
+                    if (value.contains(component->name())) {
+                        value.removeOne(component->name());
+                        if (value.isEmpty())
+                            m_referenceCount.remove(dependencyComponentName);
+                        else
+                            m_referenceCount.insert(dependencyComponentName, value);
+                    }
+                }
             }
-            insertInstallReason(dependencyComponent, InstallerCalculator::Dependent, component->name(), true);
+            insertInstallReason(dependencyComponent, InstallerCalculator::Dependent,
+                component->name(), true);
             if (!appendComponentToInstall(dependencyComponent, requiredDependencyVersion, revertFromInstall))
                 return false;
             m_visitedComponents.remove(component);
-        }
-
-        //Check dependencies only if
-        //- Dependency is not installed or update requested, nor newer version of dependency component required
-        //- And dependency component is not already added for install
-        //- And component is not already added for install, then dependencies are already resolved
-        if (!revertFromInstall && ((!dependencyComponent->isInstalled() || dependencyComponent->updateRequested())
-                || isUpdateRequired) && (!m_toInstallComponentIds.contains(dependencyComponent->name())
-                && !m_toInstallComponentIds.contains(component->name()))) {
-            if (m_visitedComponents.value(component).contains(dependencyComponent)) {
-                const QString errorMessage = recursionError(component);
-                qCWarning(QInstaller::lcInstallerInstallLog).noquote() << errorMessage;
-                m_componentsToInstallError = errorMessage;
-                Q_ASSERT_X(!m_visitedComponents.value(component).contains(dependencyComponent),
-                    Q_FUNC_INFO, qPrintable(errorMessage));
-                return false;
+        } else {
+            //Check dependencies only if
+            //- Dependency is not installed or update requested, nor newer version of dependency component required
+            //- And dependency component is not already added for install
+            //- And component is not already added for install, then dependencies are already resolved
+            if (((!dependencyComponent->isInstalled() || dependencyComponent->updateRequested())
+                    || isUpdateRequired) && (!m_toInstallComponentIds.contains(dependencyComponent->name())
+                    && !m_toInstallComponentIds.contains(component->name()))) {
+                if (m_visitedComponents.value(component).contains(dependencyComponent)) {
+                    const QString errorMessage = recursionError(component);
+                    qCWarning(QInstaller::lcInstallerInstallLog).noquote() << errorMessage;
+                    m_componentsToInstallError = errorMessage;
+                    Q_ASSERT_X(!m_visitedComponents.value(component).contains(dependencyComponent),
+                        Q_FUNC_INFO, qPrintable(errorMessage));
+                    return false;
+                }
+                m_visitedComponents[component].insert(dependencyComponent);
             }
-            m_visitedComponents[component].insert(dependencyComponent);
-
-            QStringList value = m_referenceCount.value(dependencyComponentName, QStringList());
-            if (!value.contains(component->name()))
+            if (!component->autoDependencies().contains(dependencyComponentName)) {
+                QStringList value = m_referenceCount.value(dependencyComponentName, QStringList());
                 value << component->name();
-            m_referenceCount.insert(dependencyComponentName, value);
+                m_referenceCount.insert(dependencyComponentName, value);
+            }
+
             insertInstallReason(dependencyComponent, InstallerCalculator::Dependent, component->name());
             if (!appendComponentToInstall(dependencyComponent, requiredDependencyVersion, revertFromInstall))
                 return false;
@@ -295,7 +302,8 @@ QSet<Component *> InstallerCalculator::autodependencyComponents(const bool rever
     // dependency components based on that list.
     QSet<Component *> foundAutoDependOnList;
     for (const Component *component : qAsConst(m_componentsForAutodepencencyCheck)) {
-        if (!m_autoDependencyComponentHash.contains(component->name()))
+        if (!m_autoDependencyComponentHash.contains(component->name())
+                || (!revertFromInstall && m_core->isUpdater() && !component->updateRequested()))
             continue;
         for (const QString& autoDependency : m_autoDependencyComponentHash.value(component->name())) {
             // If a components is already installed or is scheduled for installation, no need to check
@@ -316,7 +324,9 @@ QSet<Component *> InstallerCalculator::autodependencyComponents(const bool rever
                     foundAutoDependOnList.insert(autoDependComponent);
                     insertInstallReason(autoDependComponent, InstallerCalculator::Automatic);
                 }
-            } else if (revertFromInstall && m_toInstallComponentIds.contains(autoDependComponent->name())) {
+            } else if (revertFromInstall
+                       && m_toInstallComponentIds.contains(autoDependComponent->name())
+                       && !m_toInstallComponentIds.contains(component->name())) {
                 foundAutoDependOnList.insert(autoDependComponent);
             }
         }
@@ -325,4 +335,22 @@ QSet<Component *> InstallerCalculator::autodependencyComponents(const bool rever
     return foundAutoDependOnList;
 }
 
+void InstallerCalculator::calculateComponentDependencyReferences(const QString dependencyComponentName, const Component *component)
+{
+    Component *dependencyComponent = m_core->componentByName(dependencyComponentName);
+    if (!dependencyComponent || component->autoDependencies().contains(dependencyComponentName))
+        return;
+    QStringList value = m_referenceCount.value(dependencyComponentName, QStringList());
+    value << component->name();
+    m_referenceCount.insert(dependencyComponentName, value);
+
+    const QStringList dependenciesList = dependencyComponent->currentDependencies();
+    QSet<QString> allDependencies(dependenciesList.begin(), dependenciesList.end());
+    for (const QString &depComponentName : allDependencies) {
+        Component *dependencyComponent =
+            PackageManagerCore::componentByName(depComponentName, m_allComponents);
+        calculateComponentDependencyReferences(depComponentName, dependencyComponent);
+    }
+}
+
 } // namespace QInstaller
diff --git a/src/libs/installer/installercalculator.h b/src/libs/installer/installercalculator.h
index 21b3775eb..d952a1e8c 100644
--- a/src/libs/installer/installercalculator.h
+++ b/src/libs/installer/installercalculator.h
@@ -71,6 +71,7 @@ private:
     bool appendComponentToInstall(Component *component, const QString &version, const bool revertFromInstall);
     QString recursionError(const Component *component) const;
     QSet<Component *> autodependencyComponents(const bool revertFromInstall);
+    void calculateComponentDependencyReferences(const QString dependencyComponentName, const Component *component);
 
 private:
     PackageManagerCore *m_core;