Do not block installation when there are conflicting component names

- Components with conflicting original identifiers are not registered.
- Components with conflicting tree names are registered with the
  original name, if installer is configured to allow unstable
  components. Otherwise they are not registered.
- Unaffected components can be installed as usual.

Task-number: QTIFW-2444
Change-Id: Ic1eaf3878f974185bc68202930134e5199dc0398
Reviewed-by: Katja Marttila <katja.marttila@qt.io>

7 files changed, 166 insertions, 57 deletions

diff --git a/src/libs/installer/component.cpp b/src/libs/installer/component.cpp
index 21daa795c..21cb39be1 100644
--- a/src/libs/installer/component.cpp
+++ b/src/libs/installer/component.cpp
@@ -87,6 +87,8 @@ static const QLatin1String scUnstable("Unstable");
             Component script has errors or loading fails.
     \value  MissingDependency
             Component has dependencies to missing components.
+    \value  InvalidTreeName
+            Component has an invalid tree name.
 */
 
 /*!
@@ -425,6 +427,16 @@ QString Component::value(const QString &key, const QString &defaultValue) const
 }
 
 /*!
+    Removes all the values that have the \a key from the variables set for this component.
+    Returns the number of values removed which is 1 if the key exists in the variables,
+    and 0 otherwise.
+*/
+int Component::removeValue(const QString &key)
+{
+    return d->m_vars.remove(key);
+}
+
+/*!
     Sets the value of the variable with \a key to \a value.
 
     \sa {component::setValue}{component.setValue}
diff --git a/src/libs/installer/component.h b/src/libs/installer/component.h
index d042bfe6a..d8588b2ca 100644
--- a/src/libs/installer/component.h
+++ b/src/libs/installer/component.h
@@ -72,7 +72,8 @@ public:
         DepencyToUnstable = 0,
         ShaMismatch,
         ScriptLoadingFailed,
-        MissingDependency
+        MissingDependency,
+        InvalidTreeName
     };
     Q_ENUM(UnstableError)
 
@@ -106,6 +107,7 @@ public:
     QHash<QString, QString> variables() const;
     Q_INVOKABLE void setValue(const QString &key, const QString &value);
     Q_INVOKABLE QString value(const QString &key, const QString &defaultValue = QString()) const;
+    int removeValue(const QString &key);
 
     QStringList archives() const;
     PackageManagerCore *packageManagerCore() const;
diff --git a/src/libs/installer/packagemanagercore.cpp b/src/libs/installer/packagemanagercore.cpp
index 8524a559d..58896de98 100644
--- a/src/libs/installer/packagemanagercore.cpp
+++ b/src/libs/installer/packagemanagercore.cpp
@@ -1417,22 +1417,56 @@ bool PackageManagerCore::fetchLocalPackagesTree()
     d->clearAllComponentLists();
     QHash<QString, QInstaller::Component*> components;
 
-    const QStringList &keys = installedPackages.keys();
-    foreach (const QString &key, keys) {
-        QScopedPointer<QInstaller::Component> component(new QInstaller::Component(this));
-        component->loadDataFromPackage(installedPackages.value(key));
-        const QString &name = component->treeName();
-        if (components.contains(name)) {
-            d->setStatus(Failure, tr("Cannot register component! Component with identifier %1 "
-                                     "already exists.").arg(name));
-            return false;
+    std::function<void(QList<LocalPackage> *, bool)> loadLocalPackages;
+    loadLocalPackages = [&](QList<LocalPackage> *treeNamePackages, bool firstRun) {
+        foreach (auto &package, (firstRun ? installedPackages.values() : *treeNamePackages)) {
+            if (firstRun && !package.treeName.isEmpty()) {
+                // Package has a tree name, leave for later
+                treeNamePackages->append(package);
+                continue;
+            }
+
+            QScopedPointer<QInstaller::Component> component(new QInstaller::Component(this));
+            component->loadDataFromPackage(package);
+            QString name = component->treeName();
+            if (components.contains(name)) {
+                qCritical() << "Cannot register component" << component->name() << "with name" << name
+                            << "! Component with identifier" << name << "already exists.";
+                // Conflicting original name, skip.
+                if (component->value(scTreeName).isEmpty())
+                    continue;
+
+                // Conflicting tree name, check if we can add with original name.
+                name = component->name();
+                if (!settings().allowUnstableComponents() || components.contains(name))
+                    continue;
+
+                qCDebug(lcInstallerInstallLog)
+                    << "Registering component with the original indetifier:" << name;
+                component->removeValue(scTreeName);
+                const QString errorString = QLatin1String("Tree name conflicts with an existing indentifier");
+                d->m_pendingUnstableComponents.insert(component->name(),
+                    QPair<Component::UnstableError, QString>(Component::InvalidTreeName, errorString));
+            }
+            components.insert(name, component.take());
         }
-        components.insert(name, component.take());
+        // Second pass with leftover packages
+        if (firstRun)
+            loadLocalPackages(treeNamePackages, false);
+    };
+
+    {
+        // Loading package data is performed in two steps: first, components without
+        // - and then components with tree names. This is to ensure components with tree
+        // names do not replace other components when registering fails to a name conflict.
+        QList<LocalPackage> treeNamePackagesTmp;
+        loadLocalPackages(&treeNamePackagesTmp, true);
     }
 
     if (!d->buildComponentTree(components, false))
         return false;
 
+    d->commitPendingUnstableComponents();
     updateDisplayVersions(scDisplayVersion);
 
     emit finishAllComponentsReset(d->m_rootComponents);
@@ -3657,22 +3691,38 @@ bool PackageManagerCore::updateComponentData(struct Data &data, Component *compo
     try {
         // Check if we already added the component to the available components list.
         // Component treenames and names must be unique.
-        QString name = data.package->data(scTreeName).toString();
-        if (name.isEmpty())
-            name = data.package->data(scName).toString();
+        const QString packageName = data.package->data(scName).toString();
+        const QString packageTreeName = data.package->data(scTreeName).toString();
+
+        QString name = packageTreeName.isEmpty() ? packageName : packageTreeName;
         if (data.components->contains(name)) {
-            d->setStatus(Failure, tr("Cannot register component! Component with identifier %1 "
-                                     "already exists.").arg(name));
-            return false;
+            qCritical() << "Cannot register component" << packageName << "with name" << name
+                        << "! Component with identifier" << name << "already exists.";
+            // Conflicting original name, skip.
+            if (packageTreeName.isEmpty())
+                return false;
+
+            // Conflicting tree name, check if we can add with original name.
+            if (!settings().allowUnstableComponents() || data.components->contains(packageName))
+                return false;
+
+            qCDebug(lcInstallerInstallLog)
+                << "Registering component with the original indetifier:" << packageName;
+
+            component->removeValue(scTreeName);
+            const QString errorString = QLatin1String("Tree name conflicts with an existing indentifier");
+            d->m_pendingUnstableComponents.insert(component->name(),
+                QPair<Component::UnstableError, QString>(Component::InvalidTreeName, errorString));
         }
-        name = data.package->data(scName).toString();
+        name = packageName;
         if (settings().allowUnstableComponents()) {
             // Check if there are sha checksum mismatch. Component will still show in install tree
             // but is unselectable.
             foreach (const QString packageName, d->m_metadataJob.shaMismatchPackages()) {
                 if (packageName == component->name()) {
-                    QString errorString = QLatin1String("SHA mismatch detected for component ") + packageName;
-                    component->setUnstable(Component::UnstableError::ShaMismatch, errorString);
+                    const QString errorString = QLatin1String("SHA mismatch detected for component ") + packageName;
+                    d->m_pendingUnstableComponents.insert(component->name(),
+                        QPair<Component::UnstableError, QString>(Component::ShaMismatch, errorString));
                 }
             }
         }
@@ -3786,28 +3836,47 @@ bool PackageManagerCore::fetchAllPackages(const PackagesList &remotes, const Loc
     data.installedPackages = &locals;
 
     QMap<QString, QString> treeNameComponents;
-    foreach (Package *const package, remotes) {
-        if (d->statusCanceledOrFailed())
-            return false;
 
-        if (!ProductKeyCheck::instance()->isValidPackage(package->data(scName).toString()))
-            continue;
+    std::function<bool(PackagesList *, bool)> loadRemotePackages;
+    loadRemotePackages = [&](PackagesList *treeNamePackages, bool firstRun) -> bool {
+        foreach (Package *const package, (firstRun ? remotes : *treeNamePackages)) {
+            if (d->statusCanceledOrFailed())
+                return false;
 
-        QScopedPointer<QInstaller::Component> component(new QInstaller::Component(this));
-        data.package = package;
-        component->loadDataFromPackage(*package);
-        if (updateComponentData(data, component.data())) {
-            // Create a list where is name and treename. Repo can contain a package with
-            // a different treename of component which is already installed. We don't want
-            // to move already installed local packages.
-            const QString treeName = component->value(scTreeName);
-            if (!treeName.isEmpty())
-                treeNameComponents.insert(component->name(), treeName);
-            QString name = component->treeName();
-            components.insert(name, component.take());
-        } else {
-            return false;
+            if (!ProductKeyCheck::instance()->isValidPackage(package->data(scName).toString()))
+                continue;
+
+            if (firstRun && !package->data(scTreeName).toString().isEmpty()) {
+                // Package has a tree name, leave for later
+                treeNamePackages->append(package);
+                continue;
+            }
+
+            QScopedPointer<QInstaller::Component> component(new QInstaller::Component(this));
+            data.package = package;
+            component->loadDataFromPackage(*package);
+            if (updateComponentData(data, component.data())) {
+                // Create a list where is name and treename. Repo can contain a package with
+                // a different treename of component which is already installed. We don't want
+                // to move already installed local packages.
+                const QString treeName = component->value(scTreeName);
+                if (!treeName.isEmpty())
+                    treeNameComponents.insert(component->name(), treeName);
+                QString name = component->treeName();
+                components.insert(name, component.take());
+            }
         }
+        // Second pass with leftover packages
+        return firstRun ? loadRemotePackages(treeNamePackages, false) : true;
+    };
+
+    {
+        // Loading remote package data is performed in two steps: first, components without
+        // - and then components with tree names. This is to ensure components with tree
+        // names do not replace other components when registering fails to a name conflict.
+        PackagesList treeNamePackagesTmp;
+        if (!loadRemotePackages(&treeNamePackagesTmp, true))
+            return false;
     }
 
     foreach (const QString &key, locals.keys()) {
@@ -3841,6 +3910,8 @@ bool PackageManagerCore::fetchAllPackages(const PackagesList &remotes, const Loc
     if (!d->buildComponentTree(components, true))
         return false;
 
+    d->commitPendingUnstableComponents();
+
     emit finishAllComponentsReset(d->m_rootComponents);
     return true;
 }
@@ -3916,8 +3987,6 @@ bool PackageManagerCore::fetchUpdaterPackages(const PackagesList &remotes, const
 
             // this is not a dependency, it is a real update
             components.insert(name, d->m_updaterComponentsDeps.takeLast());
-        } else {
-            return false;
         }
     }
 
diff --git a/src/libs/installer/packagemanagercore_p.cpp b/src/libs/installer/packagemanagercore_p.cpp
index 51c044356..058418f8e 100644
--- a/src/libs/installer/packagemanagercore_p.cpp
+++ b/src/libs/installer/packagemanagercore_p.cpp
@@ -31,7 +31,6 @@
 #include "binarycontent.h"
 #include "binaryformatenginehandler.h"
 #include "binarylayout.h"
-#include "component.h"
 #include "scriptengine.h"
 #include "componentmodel.h"
 #include "errors.h"
@@ -2873,4 +2872,25 @@ bool PackageManagerCorePrivate::packageNeedsUpdate(const LocalPackage &localPack
     return updateNeeded;
 }
 
+void PackageManagerCorePrivate::commitPendingUnstableComponents()
+{
+    if (m_pendingUnstableComponents.isEmpty())
+        return;
+
+    for (auto &componentName : m_pendingUnstableComponents.keys()) {
+        Component *const component = m_core->componentByName(componentName);
+        if (!component) {
+            qCWarning(lcInstallerInstallLog) << "Failure while marking component "
+                "unstable. No such component exists:" << componentName;
+            continue;
+        }
+
+        const QPair<Component::UnstableError, QString> unstableError
+            = m_pendingUnstableComponents.value(componentName);
+
+        component->setUnstable(unstableError.first, unstableError.second);
+    }
+    m_pendingUnstableComponents.clear();
+}
+
 } // namespace QInstaller
diff --git a/src/libs/installer/packagemanagercore_p.h b/src/libs/installer/packagemanagercore_p.h
index 65f0e43eb..73c378f43 100644
--- a/src/libs/installer/packagemanagercore_p.h
+++ b/src/libs/installer/packagemanagercore_p.h
@@ -35,6 +35,7 @@
 #include "packagemanagerproxyfactory.h"
 #include "packagesource.h"
 #include "qinstallerglobal.h"
+#include "component.h"
 
 #include "sysinfo.h"
 #include "updatefinder.h"
@@ -52,7 +53,6 @@ using namespace KDUpdater;
 namespace QInstaller {
 
 struct BinaryLayout;
-class Component;
 class ScriptEngine;
 class ComponentModel;
 class TempDirDeleter;
@@ -254,6 +254,7 @@ private:
     bool askUserAcceptLicense(const QString &name, const QString &content) const;
     bool askUserConfirmCommand() const;
     bool packageNeedsUpdate(const LocalPackage &localPackage, const Package *update) const;
+    void commitPendingUnstableComponents();
 
 private:
     PackageManagerCore *m_core;
@@ -274,6 +275,8 @@ private:
     QHash<QString, QPair<Component*, Component*> > m_componentsToReplaceAllMode;
     QHash<QString, QPair<Component*, Component*> > m_componentsToReplaceUpdaterMode;
 
+    QHash<QString, QPair<Component::UnstableError, QString>> m_pendingUnstableComponents;
+
     InstallerCalculator *m_installerCalculator;
     UninstallerCalculator *m_uninstallerCalculator;
 
diff --git a/tests/auto/installer/treename/data/invalid_repository/Updates.xml b/tests/auto/installer/treename/data/invalid_repository/Updates.xml
index a9542eb0d..282e3a43f 100644
--- a/tests/auto/installer/treename/data/invalid_repository/Updates.xml
+++ b/tests/auto/installer/treename/data/invalid_repository/Updates.xml
@@ -9,10 +9,6 @@
   <Version>1.0.0</Version>
   <ReleaseDate>2014-08-25</ReleaseDate>
   <SortingPriority>80</SortingPriority>
-  <Dependencies>componentB.sub2</Dependencies>
-  <UpdateFile CompressedSize="275" UncompressedSize="101" OS="Any"/>
-  <DownloadableArchives>content.7z</DownloadableArchives>
-  <SHA1>570dec768b1f266c66656f015e772f0e6e41b73d</SHA1>
  </PackageUpdate>
  <PackageUpdate>
   <Name>componentA.sub1</Name>
@@ -21,9 +17,6 @@
   <Version>1.0.0</Version>
   <ReleaseDate>2014-08-25</ReleaseDate>
   <SortingPriority>80</SortingPriority>
-  <UpdateFile CompressedSize="283" UncompressedSize="101" OS="Any"/>
-  <DownloadableArchives>content.7z</DownloadableArchives>
-  <SHA1>da5819910a7f7c95eb61a49543e273fd6e2e9aae</SHA1>
  </PackageUpdate>
  <PackageUpdate>
   <Name>componentB</Name>
@@ -32,9 +25,6 @@
   <Version>1.0.0</Version>
   <ReleaseDate>2014-08-25</ReleaseDate>
   <SortingPriority>40</SortingPriority>
-  <UpdateFile CompressedSize="275" UncompressedSize="101" OS="Any"/>
-  <DownloadableArchives>content.7z</DownloadableArchives>
-  <SHA1>72eee5304ff866e024b477d7b2432df8f2428483</SHA1>
   <TreeName>componentA.sub1</TreeName>
  </PackageUpdate>
 </Updates>
diff --git a/tests/auto/installer/treename/tst_treename.cpp b/tests/auto/installer/treename/tst_treename.cpp
index b7a511ffb..58d005848 100644
--- a/tests/auto/installer/treename/tst_treename.cpp
+++ b/tests/auto/installer/treename/tst_treename.cpp
@@ -44,7 +44,8 @@ private slots:
     void moveToSubItem();
     void dependencyToMovedItem();
     void autodependOnMovedItem();
-    void moveToExistingItem();
+    void moveToExistingItemAllowUnstableComponents();
+    void moveToExistingItemNoUnstableComponents();
 
     void init();
     void cleanup();
@@ -111,12 +112,24 @@ void tst_TreeName::autodependOnMovedItem()
             << "componentASub2.txt" << "componentD.txt");
 }
 
-void tst_TreeName::moveToExistingItem()
+void tst_TreeName::moveToExistingItemAllowUnstableComponents()
 {
     QScopedPointer<PackageManagerCore> core(PackageManager::getPackageManagerWithInit
             (m_installDir, ":///data/invalid_repository"));
-    QCOMPARE(PackageManagerCore::Failure, core->installSelectedComponentsSilently(QStringList() << "componentA"));
-    QCOMPARE(core->error(), "Cannot register component! Component with identifier componentA.sub1 already exists.");
+    core->settings().setAllowUnstableComponents(true);
+
+    QCOMPARE(PackageManagerCore::Success, core->installSelectedComponentsSilently(QStringList() << "componentA"));
+    QVERIFY(core->componentByName("componentB")->isUnstable());
+}
+
+void tst_TreeName::moveToExistingItemNoUnstableComponents()
+{
+    QScopedPointer<PackageManagerCore> core(PackageManager::getPackageManagerWithInit
+            (m_installDir, ":///data/invalid_repository"));
+    core->settings().setAllowUnstableComponents(false);
+
+    QCOMPARE(PackageManagerCore::Success, core->installSelectedComponentsSilently(QStringList() << "componentA"));
+    QVERIFY(!core->componentByName("componentB"));
 }
 
 void tst_TreeName::init()