Add support dash (-) symbol in component name

Add colon (:) symbol as alternative separator beetwen
component name and version to use dash (-) symbol in
component name.

For names with dash (-) symbol use colon (:) symbol
as separator in dependencies between name and version,
even if you do not specify a version.

Requirement example: package-with-dash:>=1.2.3, stable:1.x,
demo:v1.0-rc1, backward-compatibility:.

Also add test 'tst_ComponentIdentifier' for check new feature.

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

11 files changed, 334 insertions, 26 deletions

diff --git a/src/libs/installer/downloadarchivesjob.cpp b/src/libs/installer/downloadarchivesjob.cpp
index 8fd8a40a9..80c094ae0 100644
--- a/src/libs/installer/downloadarchivesjob.cpp
+++ b/src/libs/installer/downloadarchivesjob.cpp
@@ -269,7 +269,7 @@ KDUpdater::FileDownloader *DownloadArchivesJob::setupDownloader(const QString &s
 {
     KDUpdater::FileDownloader *downloader = 0;
     const QFileInfo fi = QFileInfo(m_archivesToDownload.first().first);
-    const Component *const component = m_core->componentByName(QFileInfo(fi.path()).fileName());
+    const Component *const component = m_core->componentByName(PackageManagerCore::checkableName(QFileInfo(fi.path()).fileName()));
     if (component) {
         QString fullQueryString;
         if (!queryString.isEmpty())
diff --git a/src/libs/installer/globals.cpp b/src/libs/installer/globals.cpp
index 5f3009905..c059b6630 100644
--- a/src/libs/installer/globals.cpp
+++ b/src/libs/installer/globals.cpp
@@ -50,7 +50,7 @@ QStringList loggingCategories()
     return categories;
 }
 
-Q_GLOBAL_STATIC_WITH_ARGS(QRegExp, staticCommaRegExp, (QLatin1String("\\b(,|, )\\b")));
+Q_GLOBAL_STATIC_WITH_ARGS(QRegExp, staticCommaRegExp, (QLatin1String("(, |,)")));
 QRegExp commaRegExp()
 {
     return *staticCommaRegExp();
diff --git a/src/libs/installer/installercalculator.cpp b/src/libs/installer/installercalculator.cpp
index 064dbafa5..d44450eb2 100644
--- a/src/libs/installer/installercalculator.cpp
+++ b/src/libs/installer/installercalculator.cpp
@@ -173,13 +173,15 @@ bool InstallerCalculator::appendComponentToInstall(Component *component, const Q
         }
         //Check if component requires higher version than what might be already installed
         bool isUpdateRequired = false;
-        if (dependencyComponentName.contains(QChar::fromLatin1('-')) &&
+        QString requiredName;
+        QString requiredVersion;
+        PackageManagerCore::parseNameAndVersion(dependencyComponentName, &requiredName, &requiredVersion);
+        if (!requiredVersion.isEmpty() &&
                 !dependencyComponent->value(scInstalledVersion).isEmpty()) {
             QRegExp compEx(QLatin1String("([<=>]+)(.*)"));
             const QString installedVersion = compEx.exactMatch(dependencyComponent->value(scInstalledVersion)) ?
                 compEx.cap(2) : dependencyComponent->value(scInstalledVersion);
 
-            QString requiredVersion = dependencyComponentName.section(QLatin1Char('-'), 1);
             requiredVersion = compEx.exactMatch(requiredVersion) ? compEx.cap(2) : requiredVersion;
 
             if (KDUpdater::compareVersion(requiredVersion, installedVersion) >= 1 ) {
diff --git a/src/libs/installer/packagemanagercore.cpp b/src/libs/installer/packagemanagercore.cpp
index 7b6842720..58f7a8145 100644
--- a/src/libs/installer/packagemanagercore.cpp
+++ b/src/libs/installer/packagemanagercore.cpp
@@ -727,7 +727,7 @@ void PackageManagerCore::rollBackInstallation()
 
             const QString componentName = operation->value(QLatin1String("component")).toString();
             if (!componentName.isEmpty()) {
-                Component *component = componentByName(componentName);
+                Component *component = componentByName(checkableName(componentName));
                 if (!component)
                     component = d->componentsToReplace().value(componentName).second;
                 if (component) {
@@ -1552,12 +1552,9 @@ Component *PackageManagerCore::componentByName(const QString &name, const QList<
         return 0;
 
     QString fixedVersion;
-    QString fixedName = name;
-    if (name.contains(QChar::fromLatin1('-'))) {
-        // the last part is considered to be the version, then
-        fixedVersion = name.section(QLatin1Char('-'), 1);
-        fixedName = name.section(QLatin1Char('-'), 0, 0);
-    }
+    QString fixedName;
+
+    parseNameAndVersion(name, &fixedName, &fixedVersion);
 
     foreach (Component *component, components) {
         if (componentMatches(component, fixedName, fixedVersion))
@@ -1752,14 +1749,13 @@ QList<Component*> PackageManagerCore::dependees(const Component *_component) con
     if (availableComponents.isEmpty())
         return QList<Component *>();
 
-    const QLatin1Char dash('-');
     QList<Component *> dependees;
+    QString name;
+    QString version;
     foreach (Component *component, availableComponents) {
         const QStringList &dependencies = component->dependencies();
         foreach (const QString &dependency, dependencies) {
-            // the last part is considered to be the version then
-            const QString name = dependency.contains(dash) ? dependency.section(dash, 0, 0) : dependency;
-            const QString version = dependency.contains(dash) ? dependency.section(dash, 1) : QString();
+            parseNameAndVersion(dependency, &name, &version);
             if (componentMatches(_component, name, version))
                 dependees.append(component);
         }
@@ -2946,3 +2942,52 @@ void PackageManagerCore::addFilesForDelayedDeletion(const QStringList &files)
 {
     d->m_filesForDelayedDeletion.append(files);
 }
+
+QString PackageManagerCore::checkableName(const QString &name)
+{
+    // to ensure backward compatibility, fix component name with dash (-) symbol
+    if (!name.contains(QLatin1Char(':')))
+        if (name.contains(QLatin1Char('-')))
+            return name + QLatin1Char(':');
+
+    return name;
+}
+
+void PackageManagerCore::parseNameAndVersion(const QString &requirement, QString *name, QString *version)
+{
+    if (requirement.isEmpty()) {
+        if (name)
+            name->clear();
+        if (version)
+            version->clear();
+        return;
+    }
+
+    int pos = requirement.indexOf(QLatin1Char(':'));
+    // to ensure backward compatibility, check dash (-) symbol too
+    if (pos == -1)
+        pos = requirement.indexOf(QLatin1Char('-'));
+    if (pos != -1) {
+        if (name)
+            *name = requirement.left(pos);
+        if (version)
+            *version = requirement.mid(pos + 1);
+    } else {
+        if (name)
+            *name = requirement;
+        if (version)
+            version->clear();
+    }
+}
+
+QStringList PackageManagerCore::parseNames(const QStringList &requirements)
+{
+    QString name;
+    QString version;
+    QStringList names;
+    foreach (const QString &requirement, requirements) {
+        parseNameAndVersion(requirement, &name, &version);
+        names.append(name);
+    }
+    return names;
+}
diff --git a/src/libs/installer/packagemanagercore.h b/src/libs/installer/packagemanagercore.h
index 2a235bd44..10aad54d1 100644
--- a/src/libs/installer/packagemanagercore.h
+++ b/src/libs/installer/packagemanagercore.h
@@ -265,6 +265,10 @@ public:
     QStringList filesForDelayedDeletion() const;
     void addFilesForDelayedDeletion(const QStringList &files);
 
+    static QString checkableName(const QString &name);
+    static void parseNameAndVersion(const QString &requirement, QString *name, QString *version);
+    static QStringList parseNames(const QStringList &requirements);
+
 public Q_SLOTS:
     bool runInstaller();
     bool runUninstaller();
diff --git a/src/libs/installer/packagemanagercore_p.cpp b/src/libs/installer/packagemanagercore_p.cpp
index fb802ef8b..ef288c1f2 100644
--- a/src/libs/installer/packagemanagercore_p.cpp
+++ b/src/libs/installer/packagemanagercore_p.cpp
@@ -529,7 +529,7 @@ UninstallerCalculator *PackageManagerCorePrivate::uninstallerCalculator() const
 
         QList<Component*> installedComponents;
         foreach (const QString &name, pmcp->localInstalledPackages().keys()) {
-            if (Component *component = m_core->componentByName(name)) {
+            if (Component *component = m_core->componentByName(PackageManagerCore::checkableName(name))) {
                 if (!component->uninstallationRequested())
                     installedComponents.append(component);
             }
@@ -1651,7 +1651,7 @@ bool PackageManagerCorePrivate::runPackageUpdater()
             const QString &name = operation->value(QLatin1String("component")).toString();
             Component *component = componentsByName.value(name, 0);
             if (!component)
-                component = m_core->componentByName(name);
+                component = m_core->componentByName(PackageManagerCore::checkableName(name));
             if (component)
                 componentsByName.insert(name, component);
 
@@ -2089,7 +2089,7 @@ void PackageManagerCorePrivate::runUndoOperations(const OperationList &undoOpera
                     else if (button == QMessageBox::Ignore)
                         ignoreError = true;
                 }
-                Component *component = m_core->componentByName(componentName);
+                Component *component = m_core->componentByName(PackageManagerCore::checkableName(componentName));
                 if (!component)
                     component = componentsToReplace().value(componentName).second;
                 if (component) {
@@ -2389,11 +2389,10 @@ OperationList PackageManagerCorePrivate::sortOperationsBasedOnComponentDependenc
             componentOperationHash[componentName].append(operation);
     }
 
-    const QRegExp dash(QLatin1String("-.*"));
     Graph<QString> componentGraph;  // create the complete component graph
     foreach (const Component* node, m_core->components(PackageManagerCore::ComponentType::All)) {
         componentGraph.addNode(node->name());
-        componentGraph.addEdges(node->name(), node->dependencies().replaceInStrings(dash, QString()));
+        componentGraph.addEdges(node->name(), m_core->parseNames(node->dependencies()));
     }
 
     const QStringList resolvedComponents = componentGraph.sort();
diff --git a/src/libs/installer/uninstallercalculator.cpp b/src/libs/installer/uninstallercalculator.cpp
index 2109cc8b4..9e669c8cf 100644
--- a/src/libs/installer/uninstallercalculator.cpp
+++ b/src/libs/installer/uninstallercalculator.cpp
@@ -75,7 +75,7 @@ void UninstallerCalculator::appendComponentsToUninstall(const QList<Component*>
     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();
+            QStringList autoDependencies = PackageManagerCore::parseNames(component->autoDependencies());
             if (autoDependencies.isEmpty())
                 continue;
 
diff --git a/tests/auto/installer/componentidentifier/componentidentifier.pro b/tests/auto/installer/componentidentifier/componentidentifier.pro
new file mode 100644
index 000000000..dade1f025
--- /dev/null
+++ b/tests/auto/installer/componentidentifier/componentidentifier.pro
@@ -0,0 +1,6 @@
+include(../../qttest.pri)
+
+QT -= gui
+
+SOURCES += tst_componentidentifier.cpp
+
diff --git a/tests/auto/installer/componentidentifier/tst_componentidentifier.cpp b/tests/auto/installer/componentidentifier/tst_componentidentifier.cpp
new file mode 100644
index 000000000..d7aa6cac3
--- /dev/null
+++ b/tests/auto/installer/componentidentifier/tst_componentidentifier.cpp
@@ -0,0 +1,252 @@
+/**************************************************************************
+**
+** Copyright (C) 2017 Konstantin Podsvirov <konstantin@podsvirov.pro>
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the Qt Installer Framework.
+**
+** $QT_BEGIN_LICENSE:GPL-EXCEPT$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+**************************************************************************/
+
+#include <component.h>
+#include <packagemanagercore.h>
+
+#include <QTest>
+
+using namespace QInstaller;
+
+class NamedComponent : public Component
+{
+public:
+    NamedComponent(PackageManagerCore *core, const QString &name)
+        : Component(core)
+    {
+        setValue(scName, name);
+        setValue(scVersion, QLatin1String("1.0.0"));
+    }
+
+    NamedComponent(PackageManagerCore *core, const QString &name, const QString &version)
+        : Component(core)
+    {
+        setValue(scName, name);
+        setValue(scVersion, version);
+    }
+
+};
+
+class tst_ComponentIdentifier : public QObject
+{
+    Q_OBJECT
+
+private slots:
+    void testPackageManagerCoreSetterGetter_data();
+    void testPackageManagerCoreSetterGetter();
+
+    void testComponentDependencies();
+};
+
+void tst_ComponentIdentifier::testPackageManagerCoreSetterGetter_data()
+{
+    const char *tag = 0;
+
+    QTest::addColumn<PackageManagerCore *>("core");
+    QTest::addColumn<QStringList>("requirements");
+    QTest::addColumn<QList<Component *> >("expectedResult");
+
+    PackageManagerCore *core = 0;
+    QStringList req; // requirements;
+    QList<Component *> res; // expectedResult;
+
+    tag = "Alphabetic name";
+    core = 0; req.clear(); res.clear();
+    {
+        core = new PackageManagerCore();
+        core->setPackageManager();
+
+        NamedComponent *componentRoot = new NamedComponent(core, QLatin1String("root"));
+        core->appendRootComponent(componentRoot);
+
+        req << QLatin1String("root");
+        res << componentRoot;
+    }
+    QTest::newRow(tag) << core << req << res;
+
+    tag = "Alphabetic name, dash (-) and numbered version";
+    core = 0; req.clear(); res.clear();
+    {
+        core = new PackageManagerCore();
+        core->setPackageManager();
+
+        NamedComponent *componentRoot = new NamedComponent(core,
+                                                           QLatin1String("root"),
+                                                           QLatin1String("1.0.1"));
+        core->appendRootComponent(componentRoot);
+
+        req << QLatin1String("root-1.0.1");
+        res << componentRoot;
+
+        req << QLatin1String("root->1");
+        res << componentRoot;
+
+        req << QLatin1String("root-<2");
+        res << componentRoot;
+
+        req << QLatin1String("root-");
+        res << componentRoot;
+    }
+    QTest::newRow(tag) << core << req << res;
+
+    tag = "Alphabetic name, colon (:) and numbered version";
+    core = 0; req.clear(); res.clear();
+    {
+        core = new PackageManagerCore();
+        core->setPackageManager();
+
+        NamedComponent *componentRoot = new NamedComponent(core,
+                                                           QLatin1String("root"),
+                                                           QLatin1String("1.0.1"));
+        core->appendRootComponent(componentRoot);
+
+        req << QLatin1String("root:1.0.1");
+        res << componentRoot;
+
+        req << QLatin1String("root:>1");
+        res << componentRoot;
+
+        req << QLatin1String("root:<2");
+        res << componentRoot;
+
+        req << QLatin1String("root:");
+        res << componentRoot;
+    }
+    QTest::newRow(tag) << core << req << res;
+
+    tag = "Kebab-case name, dash (-) and numbered version";
+    core = 0; req.clear(); res.clear();
+    {
+        core = new PackageManagerCore();
+        core->setPackageManager();
+
+        NamedComponent *componentSdk = new NamedComponent(core,
+                                                          QLatin1String("org.qt-project.sdk.qt"),
+                                                          QLatin1String("4.5"));
+        NamedComponent *componentQt = new NamedComponent(core,
+                                                         QLatin1String("org.qt"),
+                                                         QLatin1String("project.sdk.qt->=4.5"));
+        componentQt->appendComponent(componentSdk);
+        core->appendRootComponent(componentQt);
+
+        // this example has been present for many years in
+        // the PackageManagerCore::componentByName() method documentation,
+        // but it does not work as expected
+        // in this case, name is "org.qt" and version is "project.sdk.qt->=4.5"
+        req << QLatin1String("org.qt-project.sdk.qt->=4.5");
+        res << componentQt; // and you expected componentSdk?
+    }
+    QTest::newRow(tag) << core << req << res;
+
+    tag = "Kebab-case name, colon (:) and numbered version";
+    core = 0; req.clear(); res.clear();
+    {
+        core = new PackageManagerCore();
+        core->setPackageManager();
+
+        NamedComponent *componentSdk = new NamedComponent(core,
+                                                           QLatin1String("org.qt-project.sdk.qt"),
+                                                           QLatin1String("4.5"));
+        core->appendRootComponent(componentSdk);
+
+
+        req << QLatin1String("org.qt-project.sdk.qt:>=4.5");
+        res << componentSdk; // work as expected
+
+        req << QLatin1String("org.qt-project.sdk.qt");
+        res << 0;
+
+        // we should fix names with dash (-) while support dash (-) as separator
+        req << PackageManagerCore::checkableName(QLatin1String("org.qt-project.sdk.qt"));
+        res << componentSdk;
+    }
+    QTest::newRow(tag) << core << req << res;
+}
+
+void tst_ComponentIdentifier::testPackageManagerCoreSetterGetter()
+{
+    QFETCH(PackageManagerCore *, core);
+    QFETCH(QStringList, requirements);
+    QFETCH(QList<Component *>, expectedResult);
+
+    QCOMPARE(requirements.count(), expectedResult.count());
+    for (int i = 0; i < requirements.count(); ++i) {
+        QCOMPARE(core->componentByName(requirements.at(i)), expectedResult.at(i));
+    }
+
+    delete core;
+}
+
+void tst_ComponentIdentifier::testComponentDependencies()
+{
+    PackageManagerCore *core = new PackageManagerCore();
+    core->setPackageManager();
+
+    Component *componentA = new NamedComponent(core, "A");
+    Component *componentB = new NamedComponent(core, "B");
+    Component *componentC = new NamedComponent(core, "component-C");
+    Component *componentD = new NamedComponent(core, "D-backward-compatibility");
+    Component *componentE = new NamedComponent(core, "E");
+
+    componentA->addDependency("B-1.0.0");
+    componentA->addDependency("component-C:1.0.0");
+    componentA->addDependency("D-backward-compatibility:");
+
+    componentE->addAutoDependOn("B-1.0.0");
+    componentE->addAutoDependOn("component-C:1.0.0");
+    componentE->addAutoDependOn("D-backward-compatibility:");
+
+    core->appendRootComponent(componentA);
+    core->appendRootComponent(componentB);
+    core->appendRootComponent(componentC);
+    core->appendRootComponent(componentD);
+    core->appendRootComponent(componentE);
+
+    QList<Component*> expectedResult = QList<Component*>()
+            << componentB << componentC << componentD;
+
+    QList<Component*> dependencies;
+    foreach (const QString &identifier, componentA->dependencies()) {
+        dependencies.append(core->componentByName(identifier));
+    }
+
+    QCOMPARE(expectedResult, dependencies);
+
+    QList<Component*> autoDependOn;
+    foreach (const QString &identifier, componentE->autoDependencies()) {
+        autoDependOn.append(core->componentByName(identifier));
+    }
+
+    QCOMPARE(expectedResult, autoDependOn);
+
+    delete core;
+}
+
+QTEST_MAIN(tst_ComponentIdentifier)
+
+#include "tst_componentidentifier.moc"
diff --git a/tests/auto/installer/installer.pro b/tests/auto/installer/installer.pro
index 01a26da47..db94a23cb 100644
--- a/tests/auto/installer/installer.pro
+++ b/tests/auto/installer/installer.pro
@@ -4,6 +4,7 @@ SUBDIRS += \
     settings \
     repository \
     compareversion\
+    componentidentifier \
     componentmodel \
     fakestopprocessforupdateoperation \
     messageboxhandler \
diff --git a/tools/common/repositorygen.cpp b/tools/common/repositorygen.cpp
index 908020ea4..295dea4d0 100644
--- a/tools/common/repositorygen.cpp
+++ b/tools/common/repositorygen.cpp
@@ -424,11 +424,10 @@ PackageInfoVector QInstallerTools::createListOfPackages(const QStringList &packa
         qDebug() << "Found subdirectory" << it->fileName();
         // because the filter is QDir::Dirs - filename means the name of the subdirectory
         if (it->fileName().contains(QLatin1Char('-'))) {
-            if (ignoreInvalidPackages)
-                continue;
-            throw QInstaller::Error(QString::fromLatin1("Component \"%1\" must not contain '-'. This is not "
-                "allowed, because dashes are used as the separator between the component name and the "
-                "version number internally.").arg(QDir::toNativeSeparators(it->fileName())));
+            qDebug("When using the component \"%s\" as a dependency, "
+                "to ensure backward compatibility, you must add a colon symbol at the end, "
+                "even if you do not specify a version.",
+                qUtf8Printable(it->fileName()));
         }
 
         QFile file(QString::fromLatin1("%1/meta/package.xml").arg(it->filePath()));