pkgconfig: Merge packages and broken packages

This is required for the ongoing patch that merges the
dependencies on the pkgconfig level - a valid package
may change it's type to broken if it has unsatisfied
dependencies.

Change-Id: I21e6a214d8524fb95e6b837604ae6b7f32360d4f
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

13 files changed, 117 insertions, 45 deletions

diff --git a/share/qbs/module-providers/qbspkgconfig.qbs b/share/qbs/module-providers/qbspkgconfig.qbs
index ad6d64027..923355e57 100644
--- a/share/qbs/module-providers/qbspkgconfig.qbs
+++ b/share/qbs/module-providers/qbspkgconfig.qbs
@@ -161,19 +161,18 @@ ModuleProvider {
         }
 
         var pkgConfig = new PkgConfig(options);
-        var brokenPackages = pkgConfig.brokenPackages();
-        if (brokenPackages.length !== 0) {
-            console.warn("Failed to load some pkg-config packages:");
-            for (var i = 0; i < brokenPackages.length; ++i) {
-                console.warn("    " + brokenPackages[i].filePath
-                    + ": " + brokenPackages[i].errorText);
-            }
-        }
+
+        var brokenPackages = [];
         var packages = pkgConfig.packages();
         for (var packageName in packages) {
+            var pkg = packages[packageName];
+            if (pkg.isBroken) {
+                brokenPackages.push(pkg);
+                continue;
+            }
             var moduleName = getModuleName(packageName);
-            var moduleInfo = getModuleInfo(packages[packageName], staticMode);
-            var deps = getModuleDependencies(packages[packageName], staticMode);
+            var moduleInfo = getModuleInfo(pkg, staticMode);
+            var deps = getModuleDependencies(pkg, staticMode);
 
             var moduleDir = FileInfo.joinPaths(outputDir, moduleName);
             File.makePath(moduleDir);
@@ -211,6 +210,15 @@ ModuleProvider {
             module.writeLine("}");
             module.close();
         }
+
+        if (brokenPackages.length !== 0) {
+            console.warn("Failed to load some pkg-config packages:");
+            for (var i = 0; i < brokenPackages.length; ++i) {
+                console.warn("    " + brokenPackages[i].filePath
+                    + ": " + brokenPackages[i].errorText);
+            }
+        }
+
         return "";
     }
 }
diff --git a/src/lib/corelib/jsextensions/pkgconfigjs.cpp b/src/lib/corelib/jsextensions/pkgconfigjs.cpp
index 49058818e..f4b7b08d2 100644
--- a/src/lib/corelib/jsextensions/pkgconfigjs.cpp
+++ b/src/lib/corelib/jsextensions/pkgconfigjs.cpp
@@ -65,6 +65,7 @@ template<typename C, typename F> QVariantList convert(const C &c, F &&f)
 QVariantMap packageToMap(const PcPackage &package)
 {
     QVariantMap result;
+    result[QStringLiteral("isBroken")] = false;
     result[QStringLiteral("filePath")] = QString::fromStdString(package.filePath);
     result[QStringLiteral("baseFileName")] = QString::fromStdString(package.baseFileName);
     result[QStringLiteral("name")] = QString::fromStdString(package.name);
@@ -133,11 +134,24 @@ QVariantMap packageToMap(const PcPackage &package)
 QVariantMap brokenPackageToMap(const PcBrokenPackage &package)
 {
     QVariantMap result;
+    result[QStringLiteral("isBroken")] = true;
     result[QStringLiteral("filePath")] = QString::fromStdString(package.filePath);
+    result[QStringLiteral("baseFileName")] = QString::fromStdString(package.baseFileName);
     result[QStringLiteral("errorText")] = QString::fromStdString(package.errorText);
     return result;
 }
 
+QVariantMap packageVariantToMap(const PcPackageVariant &package)
+{
+    return package.visit([](const auto &value) {
+        using T = std::decay_t<decltype(value)>;
+        if constexpr (std::is_same_v<T, PcPackage>)
+            return packageToMap(value);
+        else
+            return brokenPackageToMap(value);
+    });
+}
+
 PcPackage::VariablesMap envToVariablesMap(const QProcessEnvironment &env)
 {
     PcPackage::VariablesMap result;
@@ -195,11 +209,10 @@ PkgConfigJs::PkgConfigJs(
         convertOptions(static_cast<ScriptEngine *>(engine)->environment(), options)))
 {
     Q_UNUSED(context);
-    for (const auto &package : m_pkgConfig->packages())
-        m_packages.insert(QString::fromStdString(package.baseFileName), packageToMap(package));
-
-    for (const auto &package : m_pkgConfig->brokenPackages())
-        m_brokenPackages.push_back(brokenPackageToMap(package));
+    for (const auto &package : m_pkgConfig->packages()) {
+        m_packages.insert(
+                QString::fromStdString(package.getBaseFileName()), packageVariantToMap(package));
+    }
 }
 
 PkgConfig::Options PkgConfigJs::convertOptions(const QProcessEnvironment &env, const QVariantMap &map)
diff --git a/src/lib/corelib/jsextensions/pkgconfigjs.h b/src/lib/corelib/jsextensions/pkgconfigjs.h
index b66b9d7d3..82181c8c8 100644
--- a/src/lib/corelib/jsextensions/pkgconfigjs.h
+++ b/src/lib/corelib/jsextensions/pkgconfigjs.h
@@ -91,7 +91,6 @@ public:
             QScriptContext *context, QScriptEngine *engine, const QVariantMap &options = {});
 
     Q_INVOKABLE QVariantMap packages() const { return m_packages; }
-    Q_INVOKABLE QVariantList brokenPackages() const { return m_brokenPackages; }
 
     // also used in tests
     static PkgConfig::Options convertOptions(const QProcessEnvironment &env, const QVariantMap &map);
@@ -99,7 +98,6 @@ public:
 private:
     std::unique_ptr<PkgConfig> m_pkgConfig;
     QVariantMap m_packages;
-    QVariantList m_brokenPackages;
 };
 
 } // namespace Internal
diff --git a/src/lib/pkgconfig/CMakeLists.txt b/src/lib/pkgconfig/CMakeLists.txt
index 551e2bfef..e64d934c8 100644
--- a/src/lib/pkgconfig/CMakeLists.txt
+++ b/src/lib/pkgconfig/CMakeLists.txt
@@ -27,6 +27,6 @@ add_qbs_library(qbspkgconfig
         "HAS_STD_FILESYSTEM=${HAS_STD_FILESYSTEM}"
     PUBLIC_DEFINES
         "QBS_PC_WITH_QT_SUPPORT=1"
-    PUBLIC_DEPENDS Qt${QT_VERSION_MAJOR}::Core ${QBS_PKGCONFIG_PUBLIC_DEPENDS}
+    PUBLIC_DEPENDS Qt${QT_VERSION_MAJOR}::Core ${QBS_PKGCONFIG_PUBLIC_DEPENDS} qbsvariant
     SOURCES ${SOURCES}
 )
diff --git a/src/lib/pkgconfig/pcpackage.h b/src/lib/pkgconfig/pcpackage.h
index df6905185..9edd56c60 100644
--- a/src/lib/pkgconfig/pcpackage.h
+++ b/src/lib/pkgconfig/pcpackage.h
@@ -40,12 +40,15 @@
 #ifndef PC_PACKAGE_H
 #define PC_PACKAGE_H
 
+#include <variant.h>
+
 #include <map>
 #include <optional>
 #include <stdexcept>
 #include <string>
 #include <unordered_map>
 #include <unordered_set>
+#include <utility>
 #include <vector>
 
 namespace qbs {
@@ -123,9 +126,45 @@ class PcBrokenPackage
 {
 public:
     std::string filePath;
+    std::string baseFileName;
     std::string errorText;
 };
 
+class PcPackageVariant: public Variant::variant<PcPackage, PcBrokenPackage>
+{
+public:
+    using Base = Variant::variant<PcPackage, PcBrokenPackage>;
+    using Base::Base;
+
+    bool isValid() const noexcept { return index() == 0; }
+    bool isBroken() const noexcept { return index() == 1; }
+
+    const PcPackage &asPackage() const { return Variant::get<PcPackage>(*this); }
+    PcPackage &asPackage() { return Variant::get<PcPackage>(*this); }
+
+    const PcBrokenPackage &asBrokenPackage() const { return Variant::get<PcBrokenPackage>(*this); }
+    PcBrokenPackage &asBrokenPackage() { return Variant::get<PcBrokenPackage>(*this); }
+
+    template<typename F>
+    decltype(auto) visit(F &&f) const
+    {
+         return Variant::visit(std::forward<F>(f), static_cast<const Base &>(*this));
+    }
+
+    template<typename F>
+    decltype(auto) visit(F &&f)
+    {
+        return Variant::visit(std::forward<F>(f), static_cast<Base &>(*this));
+    }
+
+    const std::string &getBaseFileName() const
+    {
+        return visit([](auto &&value) noexcept -> const std::string & {
+            return value.baseFileName;
+        });
+    }
+};
+
 class PcException: public std::runtime_error
 {
 public:
diff --git a/src/lib/pkgconfig/pcparser.cpp b/src/lib/pkgconfig/pcparser.cpp
index 6f58896af..b3ba57c3d 100644
--- a/src/lib/pkgconfig/pcparser.cpp
+++ b/src/lib/pkgconfig/pcparser.cpp
@@ -407,7 +407,8 @@ PcParser::PcParser(const PkgConfig &pkgConfig)
 
 }
 
-PcPackage PcParser::parsePackageFile(const std::string &path)
+PcPackageVariant PcParser::parsePackageFile(const std::string &path)
+try
 {
     PcPackage package;
 
@@ -434,6 +435,8 @@ PcPackage PcParser::parsePackageFile(const std::string &path)
     while (readOneLine(file, line))
         parseLine(package, line);
     return package;
+} catch(const PcException &ex) {
+    return PcBrokenPackage{path, baseName(path), ex.what()};
 }
 
 std::string PcParser::trimAndSubstitute(const PcPackage &pkg, std::string_view str) const
diff --git a/src/lib/pkgconfig/pcparser.h b/src/lib/pkgconfig/pcparser.h
index 8443629a6..ffdf86aaa 100644
--- a/src/lib/pkgconfig/pcparser.h
+++ b/src/lib/pkgconfig/pcparser.h
@@ -51,7 +51,7 @@ class PcParser
 public:
     explicit PcParser(const PkgConfig &pkgConfig);
 
-    PcPackage parsePackageFile(const std::string &path);
+    PcPackageVariant parsePackageFile(const std::string &path);
 
 private:
     std::string trimAndSubstitute(const PcPackage &pkg, std::string_view str) const;
diff --git a/src/lib/pkgconfig/pkgconfig.cpp b/src/lib/pkgconfig/pkgconfig.cpp
index 6c1179eae..2b9c1edea 100644
--- a/src/lib/pkgconfig/pkgconfig.cpp
+++ b/src/lib/pkgconfig/pkgconfig.cpp
@@ -141,19 +141,27 @@ PkgConfig::PkgConfig(Options options)
         m_options.globalVariables["pc_sysrootdir"] = m_options.sysroot;
     m_options.globalVariables["pc_top_builddir"] = m_options.topBuildDir;
 
-    std::tie(m_packages, m_brokenPackages) = findPackages();
+    m_packages = findPackages();
 }
 
-const PcPackage &PkgConfig::getPackage(std::string_view baseFileName) const
+const PcPackageVariant &PkgConfig::getPackage(std::string_view baseFileName) const
 {
     // heterogeneous comparator so we can search the package using string_view
-    const auto lessThan = [](const PcPackage &package, const std::string_view &name)
+    const auto lessThan = [](const PcPackageVariant &package, const std::string_view &name)
     {
-        return package.baseFileName < name;
+        return package.visit([name](auto &&value) noexcept {
+            return value.baseFileName < name;
+        });
+    };
+
+    const auto testPackage = [baseFileName](const PcPackageVariant &package) {
+        return package.visit([baseFileName](auto &&value) noexcept {
+            return baseFileName != value.baseFileName;
+        });
     };
 
     const auto it = std::lower_bound(m_packages.begin(), m_packages.end(), baseFileName, lessThan);
-    if (it == m_packages.end() || baseFileName != it->baseFileName)
+    if (it == m_packages.end() || testPackage(*it))
         raizeUnknownPackageException(baseFileName);
     return *it;
 }
@@ -229,10 +237,9 @@ std::vector<std::string> getPcFilePaths(const std::vector<std::string> &searchPa
 }
 #endif
 
-std::pair<PkgConfig::Packages, PkgConfig::BrokenPackages> PkgConfig::findPackages() const
+PkgConfig::Packages PkgConfig::findPackages() const
 {
     Packages result;
-    BrokenPackages brokenResult;
     PcParser parser(*this);
 
     const auto systemLibraryPaths = !m_options.allowSystemLibraryPaths ?
@@ -248,26 +255,27 @@ std::pair<PkgConfig::Packages, PkgConfig::BrokenPackages> PkgConfig::findPackage
                 continue;
         }
 
-        try {
-            result.emplace_back(
-                    parser.parsePackageFile(pcFilePath)
+        auto pkg = parser.parsePackageFile(pcFilePath);
+        pkg.visit([&](auto &value) {
+            using T = std::decay_t<decltype(value)>;
+            if constexpr (std::is_same_v<T, PcPackage>) { // NOLINT
+                value = std::move(value)
                         // Weird, but pkg-config removes libs first and only then appends
                         // sysroot. Looks like sysroot has to be used with
                         // allowSystemLibraryPaths: true
                         .removeSystemLibraryPaths(systemLibraryPaths)
-                        .prependSysroot(m_options.sysroot));
-        } catch (const PcException &ex) {
-            // not sure if it's OK to use exceptions for handling errors like
-            brokenResult.push_back(PcBrokenPackage{pcFilePath, ex.what()});
-        }
+                        .prependSysroot(m_options.sysroot);
+            }
+        });
+        result.emplace_back(std::move(pkg));
     }
 
-    const auto lessThanPackage = [](const PcPackage &lhs, const PcPackage &rhs)
+    const auto lessThanPackage = [](const PcPackageVariant &lhs, const PcPackageVariant &rhs)
     {
-        return lhs.baseFileName < rhs.baseFileName;
+        return lhs.getBaseFileName() < rhs.getBaseFileName();
     };
     std::sort(result.begin(), result.end(), lessThanPackage);
-    return {result, brokenResult};
+    return result;
 }
 
 } // namespace qbs
diff --git a/src/lib/pkgconfig/pkgconfig.h b/src/lib/pkgconfig/pkgconfig.h
index 17b5ea9fa..d66d58985 100644
--- a/src/lib/pkgconfig/pkgconfig.h
+++ b/src/lib/pkgconfig/pkgconfig.h
@@ -60,27 +60,24 @@ public:
         VariablesMap systemVariables;
     };
 
-    using Packages = std::vector<PcPackage>;
-    using BrokenPackages = std::vector<PcBrokenPackage>;
+    using Packages = std::vector<PcPackageVariant>;
 
     explicit PkgConfig();
     explicit PkgConfig(Options options);
 
     const Options &options() const { return m_options; }
     const Packages &packages() const { return m_packages; }
-    const BrokenPackages &brokenPackages() const { return m_brokenPackages; }
-    const PcPackage &getPackage(std::string_view baseFileName) const;
+    const PcPackageVariant &getPackage(std::string_view baseFileName) const;
 
     std::string_view packageGetVariable(const PcPackage &pkg, std::string_view var) const;
 
 private:
-    std::pair<Packages, BrokenPackages> findPackages() const;
+    Packages findPackages() const;
 
 private:
     Options m_options;
 
     Packages m_packages;
-    BrokenPackages m_brokenPackages;
 };
 
 } // namespace qbs
diff --git a/src/lib/pkgconfig/pkgconfig.pro b/src/lib/pkgconfig/pkgconfig.pro
index 14b9b2c5d..dcabf0ba1 100644
--- a/src/lib/pkgconfig/pkgconfig.pro
+++ b/src/lib/pkgconfig/pkgconfig.pro
@@ -1,5 +1,6 @@
 TARGET = qbspkgconfig
 include(../staticlibrary.pri)
+include(../../shared/variant/variant.pri)
 
 DEFINES += \
     PKG_CONFIG_PC_PATH=\\\"/usr/lib/pkgconfig:/usr/share/pkgconfig\\\" \
diff --git a/src/lib/pkgconfig/pkgconfig.qbs b/src/lib/pkgconfig/pkgconfig.qbs
index e6894429e..0be5065f5 100644
--- a/src/lib/pkgconfig/pkgconfig.qbs
+++ b/src/lib/pkgconfig/pkgconfig.qbs
@@ -4,6 +4,7 @@ import qbs.Utilities
 QbsStaticLibrary {
     Depends { name: "cpp" }
     Depends { name: "qbsbuildconfig" }
+    Depends { name: "qbsvariant" }
 
     property stringList pcPaths: {
         var result = [];
@@ -56,6 +57,7 @@ QbsStaticLibrary {
 
     Export {
         Depends { name: "cpp" }
+        Depends { name: "qbsvariant" }
         cpp.defines: exportingProduct.publicDefines
         cpp.staticLibraries: {
             if (qbs.toolchainType === "gcc" && cpp.compilerVersionMajor === 7)
diff --git a/src/lib/pkgconfig/use_pkgconfig.pri b/src/lib/pkgconfig/use_pkgconfig.pri
index d6fd2205c..e0e485e46 100644
--- a/src/lib/pkgconfig/use_pkgconfig.pri
+++ b/src/lib/pkgconfig/use_pkgconfig.pri
@@ -1,4 +1,5 @@
 include(../../library_dirname.pri)
+include(../../shared/variant/variant.pri)
 
 isEmpty(QBSLIBDIR) {
     QBSLIBDIR = $${OUT_PWD}/../../../$${QBS_LIBRARY_DIRNAME}
diff --git a/tests/auto/pkgconfig/tst_pkgconfig.cpp b/tests/auto/pkgconfig/tst_pkgconfig.cpp
index 53dc696ed..96a70d199 100644
--- a/tests/auto/pkgconfig/tst_pkgconfig.cpp
+++ b/tests/auto/pkgconfig/tst_pkgconfig.cpp
@@ -74,7 +74,9 @@ void TestPkgConfig::pkgConfig()
     const auto json = QJsonDocument::fromJson(jsonFile.readAll(), &error).toVariant().toMap();
     QCOMPARE(error.error, QJsonParseError::NoError);
 
-    const auto &package = pkgConfig.getPackage(fileName.toStdString());
+    const auto &packageOr = pkgConfig.getPackage(fileName.toStdString());
+    QVERIFY(packageOr.isValid());
+    const auto &package = packageOr.asPackage();
     QCOMPARE(QString::fromStdString(package.baseFileName), fileName);
     QCOMPARE(QString::fromStdString(package.name), json.value("Name").toString());
     QCOMPARE(QString::fromStdString(package.description), json.value("Description").toString());