McuSupport: Change Kit Aspects validation check CMake variables

Before QUL 2.0, path for tools used during compilation were passed as
environment variables. Starting with 2.0, they are passed as CMake
configuration arguments. So, in order to validate a kit, the
dependencies aspect now needs to validate the CMake configuration
aspects. As 1.9 will no longer be supported, starting with this version
of Qt Creator, the environment checking code is removed.

Change-Id: I24991063dcdf9b455bd8f20226756dc2b3637f6d
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
Reviewed-by: Alessandro Portale <alessandro.portale@qt.io>

src/plugins/mcusupport/mcukitinformation.cpp

@@ -24,8 +24,12 @@
 ****************************************************************************/
 
 #include "mcukitinformation.h"
+#include "mcusupportcmakemapper.h"
 
+#include <cmakeprojectmanager/cmakekitinformation.h>
 #include <utils/qtcassert.h>
+#include <utils/algorithm.h>
+#include <utils/filepath.h>
 
 using namespace ProjectExplorer;
 
@@ -57,31 +61,31 @@ McuDependenciesKitAspect::McuDependenciesKitAspect()
     setPriority(28500);
 }
 
-Tasks McuDependenciesKitAspect::validate(const Kit *k) const
+Tasks McuDependenciesKitAspect::validate(const Kit *kit) const
 {
     Tasks result;
-    QTC_ASSERT(k, return result);
+    QTC_ASSERT(kit, return result);
 
-    const QVariant checkFormat = k->value(McuDependenciesKitAspect::id());
+    // check dependencies are defined properly for this kit
-    if (!checkFormat.isNull() && !checkFormat.canConvert(QVariant::List))
+    const QVariant checkFormat = kit->value(McuDependenciesKitAspect::id());
+    if (!checkFormat.isValid() || checkFormat.isNull())
+        return result;
+    if (!checkFormat.canConvert(QVariant::List))
         return {BuildSystemTask(Task::Error, tr("The MCU dependencies setting value is invalid."))};
 
-    const QVariant envStringList = k->value(EnvironmentKitAspect::id());
+    // check paths defined in cmake variables for given dependencies exist
-    if (!envStringList.isNull() && !envStringList.canConvert(QVariant::List))
+    const auto cMakeEntries = Utils::NameValueDictionary(configuration(kit));
-        return {BuildSystemTask(Task::Error, tr("The environment setting value is invalid."))};
+    for (const auto &dependency: dependencies(kit)) {
-
+        auto givenPath = Utils::FilePath::fromString(cMakeEntries.value(dependency.name));
-    const auto environment = Utils::NameValueDictionary(envStringList.toStringList());
+        if (givenPath.isEmpty()) {
-    for (const auto &dependency : dependencies(k)) {
+            result << BuildSystemTask(Task::Warning, tr("CMake variable %1 not defined.").arg(
-        if (!environment.hasKey(dependency.name)) {
+                                          dependency.name));
-            result << BuildSystemTask(Task::Warning,
-                                      tr("Environment variable %1 not defined.")
-                                          .arg(dependency.name));
         } else {
-            const auto path = Utils::FilePath::fromUserInput(environment.value(dependency.name)
+            const auto detectionPath = givenPath.resolvePath(dependency.value);
-                                                             + "/" + dependency.value);
+            if (!detectionPath.exists()) {
-            if (!path.exists()) {
+                result << BuildSystemTask(Task::Warning, tr("CMake variable %1: path %2 does not exist.").arg(
-                result << BuildSystemTask(Task::Warning,
+                                              dependency.name,
-                                          tr("%1 not found.").arg(path.toUserOutput()));
+                                              detectionPath.toUserOutput()));
             }
         }
     }
@@ -89,40 +93,40 @@ Tasks McuDependenciesKitAspect::validate(const Kit *k) const
     return result;
 }
 
-void McuDependenciesKitAspect::fix(Kit *k)
+void McuDependenciesKitAspect::fix(Kit *kit)
 {
-    QTC_ASSERT(k, return );
+    QTC_ASSERT(kit, return);
 
-    const QVariant variant = k->value(McuDependenciesKitAspect::id());
+    const QVariant variant = kit->value(McuDependenciesKitAspect::id());
     if (!variant.isNull() && !variant.canConvert(QVariant::List)) {
-        qWarning("Kit \"%s\" has a wrong mcu dependencies value set.", qPrintable(k->displayName()));
+        qWarning("Kit \"%s\" has a wrong mcu dependencies value set.", qPrintable(kit->displayName()));
-        setDependencies(k, Utils::NameValueItems());
+        setDependencies(kit, Utils::NameValueItems());
     }
 }
 
-KitAspectWidget *McuDependenciesKitAspect::createConfigWidget(Kit *k) const
+KitAspectWidget *McuDependenciesKitAspect::createConfigWidget(Kit *kit) const
 {
-    QTC_ASSERT(k, return nullptr);
+    QTC_ASSERT(kit, return nullptr);
-    return new McuDependenciesKitAspectWidget(k, this);
+    return new McuDependenciesKitAspectWidget(kit, this);
 }
 
-KitAspect::ItemList McuDependenciesKitAspect::toUserOutput(const Kit *k) const
+KitAspect::ItemList McuDependenciesKitAspect::toUserOutput(const Kit *kit) const
 {
-    Q_UNUSED(k)
+    Q_UNUSED(kit)
 
     return {};
 }
 
 Utils::Id McuDependenciesKitAspect::id()
 {
-    return "PE.Profile.McuDependencies";
+    return "PE.Profile.McuCMakeDependencies";
 }
 
-Utils::NameValueItems McuDependenciesKitAspect::dependencies(const Kit *k)
+Utils::NameValueItems McuDependenciesKitAspect::dependencies(const Kit *kit)
 {
-    if (k)
+    if (kit)
         return Utils::NameValueItem::fromStringList(
-            k->value(McuDependenciesKitAspect::id()).toStringList());
+            kit->value(McuDependenciesKitAspect::id()).toStringList());
     return Utils::NameValueItems();
 }
 
@@ -133,5 +137,14 @@ void McuDependenciesKitAspect::setDependencies(Kit *k, const Utils::NameValueIte
                     Utils::NameValueItem::toStringList(dependencies));
 }
 
+Utils::NameValuePairs McuDependenciesKitAspect::configuration(const Kit *kit)
+{
+    using namespace CMakeProjectManager;
+    const auto config = CMakeConfigurationKitAspect::configuration(kit).toList();
+    return Utils::transform(config, [](const CMakeConfigItem &it) {
+        return Utils::NameValuePair(QString::fromUtf8(it.key), QString::fromUtf8(it.value));
+    });
+}
+
 } // namespace Internal
 } // namespace McuSupport

src/plugins/mcusupport/mcukitinformation.h

@@ -37,16 +37,17 @@ class McuDependenciesKitAspect final : public ProjectExplorer::KitAspect
 public:
     McuDependenciesKitAspect();
 
-    ProjectExplorer::Tasks validate(const ProjectExplorer::Kit *k) const override;
+    ProjectExplorer::Tasks validate(const ProjectExplorer::Kit *kit) const override;
-    void fix(ProjectExplorer::Kit *k) override;
+    void fix(ProjectExplorer::Kit *kit) override;
 
-    ProjectExplorer::KitAspectWidget *createConfigWidget(ProjectExplorer::Kit *k) const override;
+    ProjectExplorer::KitAspectWidget *createConfigWidget(ProjectExplorer::Kit *kit) const override;
 
-    ItemList toUserOutput(const ProjectExplorer::Kit *k) const override;
+    ItemList toUserOutput(const ProjectExplorer::Kit *kit) const override;
 
     static Utils::Id id();
-    static Utils::NameValueItems dependencies(const ProjectExplorer::Kit *k);
+    static Utils::NameValueItems dependencies(const ProjectExplorer::Kit *kit);
-    static void setDependencies(ProjectExplorer::Kit *k, const Utils::NameValueItems &dependencies);
+    static void setDependencies(ProjectExplorer::Kit *kit, const Utils::NameValueItems &dependencies);
+    static Utils::NameValuePairs configuration(const ProjectExplorer::Kit *kit);
 };
 
 } // namespace Internal

src/plugins/mcusupport/mcukitmanager.cpp

@@ -32,6 +32,7 @@
 #include "mcutarget.h"
 #include "mcusupportplugin.h"
 #include "mcusupportsdk.h"
+#include "mcusupportcmakemapper.h"
 
 #include <cmakeprojectmanager/cmakekitinformation.h>
 #include <cmakeprojectmanager/cmaketoolmanager.h>
@@ -57,6 +58,26 @@ namespace McuKitManager {
 
 static const int KIT_VERSION = 9; // Bumps up whenever details in Kit creation change
 
+static bool expectsCmakeVars(const McuTarget *mcuTarget)
+{
+    return mcuTarget->qulVersion() >= QVersionNumber{2, 0};
+}
+
+void remapQul2xCmakeVars(Kit *kit, const EnvironmentItems &envItems)
+{
+    const auto cmakeVars = mapEnvVarsToQul2xCmakeVars(envItems);
+    const auto cmakeVarNames = Utils::transform(cmakeVars, &CMakeConfigItem::key);
+
+    // First filter out all Qul2.x CMake vars
+    auto config = Utils::filtered(CMakeConfigurationKitAspect::configuration(kit),
+                                  [&](const auto &configItem) {
+                                      return !cmakeVarNames.contains(configItem.key);
+                                  });
+    // Then append them with new values
+    config.append(cmakeVars);
+    CMakeConfigurationKitAspect::setConfiguration(kit, config);
+}
+
 static void setKitToolchains(Kit *k, const McuToolChainPackage *tcPackage)
 {
     switch (tcPackage->toolchainType()) {
@@ -85,7 +106,6 @@ static void setKitToolchains(Kit *k, const McuToolChainPackage *tcPackage)
     }
 }
 
-
 static void setKitProperties(const QString &kitName,
                              Kit *k,
                              const McuTarget *mcuTarget,
@@ -120,6 +140,67 @@ static void setKitProperties(const QString &kitName,
 }
 
 
+static void setKitEnvironment(Kit *k,
+                                          const McuTarget *mcuTarget,
+                                          const McuAbstractPackage *qtForMCUsSdkPackage)
+{
+    EnvironmentItems changes;
+    QStringList pathAdditions;
+
+    // The Desktop version depends on the Qt shared libs in Qul_DIR/bin.
+    // If CMake's fileApi is avaialble, we can rely on the "Add library search path to PATH"
+    // feature of the run configuration. Otherwise, we just prepend the path, here.
+    if (mcuTarget->toolChainPackage()->isDesktopToolchain()
+        && !CMakeProjectManager::CMakeToolManager::defaultCMakeTool()->hasFileApi())
+        pathAdditions.append(qtForMCUsSdkPackage->path().pathAppended("bin").toUserOutput());
+
+    auto processPackage = [&pathAdditions, &changes](const McuAbstractPackage *package) {
+        if (package->isAddToSystemPath())
+            pathAdditions.append(package->path().toUserOutput());
+        if (!package->environmentVariableName().isEmpty())
+            changes.append({package->environmentVariableName(), package->path().toUserOutput()});
+    };
+    for (auto package : mcuTarget->packages())
+        processPackage(package);
+    processPackage(qtForMCUsSdkPackage);
+
+    if (McuSupportOptions::kitsNeedQtVersion())
+        changes.append({QLatin1String("LD_LIBRARY_PATH"), "%{Qt:QT_INSTALL_LIBS}"});
+
+    // Hack, this problem should be solved in lower layer
+    if (expectsCmakeVars(mcuTarget)) {
+        remapQul2xCmakeVars(k, changes);
+    }
+
+    EnvironmentKitAspect::setEnvironmentChanges(k, changes);
+}
+
+void updateKitEnvironment(Kit *k, const McuTarget *mcuTarget)
+{
+    EnvironmentItems changes = EnvironmentKitAspect::environmentChanges(k);
+    for (auto package : mcuTarget->packages()) {
+        const QString varName = package->environmentVariableName();
+        if (!varName.isEmpty() && package->isValidStatus()) {
+            const int index = Utils::indexOf(changes, [varName](const EnvironmentItem &item) {
+                return item.name == varName;
+            });
+            const EnvironmentItem item = {package->environmentVariableName(),
+                                          package->path().toUserOutput()};
+            if (index != -1)
+                changes.replace(index, item);
+            else
+                changes.append(item);
+        }
+    }
+
+    // Hack, this problem should be solved in lower layer
+    if (expectsCmakeVars(mcuTarget)) {
+        remapQul2xCmakeVars(k, changes);
+    }
+
+    EnvironmentKitAspect::setEnvironmentChanges(k, changes);
+}
+
 static void setKitDebugger(Kit *k, const McuToolChainPackage *tcPackage)
 {
     if (tcPackage->isDesktopToolchain()) {
@@ -166,8 +247,9 @@ static void setKitDependencies(Kit *k,
     NameValueItems dependencies;
 
     auto processPackage = [&dependencies](const McuAbstractPackage *package) {
-        if (!package->environmentVariableName().isEmpty())
+        const auto cmakeVariableName = mapEnvVarToQul2xCmakeVar(package->environmentVariableName());
-            dependencies.append({package->environmentVariableName(),
+        if (!cmakeVariableName.isEmpty())
+            dependencies.append({cmakeVariableName,
                                  package->detectionPath().toUserOutput()});
     };
     for (auto package : mcuTarget->packages())
@@ -298,12 +380,12 @@ QList<Kit *> upgradeableKits(const McuTarget *mcuTarget,
 QList<Kit *> kitsWithMismatchedDependencies(const McuTarget *mcuTarget)
 {
     return Utils::filtered(existingKits(mcuTarget), [mcuTarget](Kit *kit) {
-        const auto environment = Utils::NameValueDictionary(
+        const auto entries = Utils::NameValueDictionary(McuDependenciesKitAspect::configuration(kit));
-            Utils::NameValueItem::toStringList(EnvironmentKitAspect::environmentChanges(kit)));
         return Utils::anyOf(mcuTarget->packages(),
-                            [&environment](const McuAbstractPackage *package) {
+                            [&entries](const McuAbstractPackage *package) {
-                                return !package->environmentVariableName().isEmpty()
+                                const QString cmakeVariableName = mapEnvVarToQul2xCmakeVar(package->environmentVariableName());
-                                       && environment.value(package->environmentVariableName())
+                                return !cmakeVariableName.isEmpty()
+                                       && entries.value(cmakeVariableName)
                                               != package->path().toUserOutput();
                             });
     });
@@ -332,7 +414,7 @@ Kit *newKit(const McuTarget *mcuTarget, const McuAbstractPackage *qtForMCUsSdk)
         setKitDevice(k, mcuTarget);
         setKitToolchains(k, mcuTarget->toolChainPackage());
         setKitDebugger(k, mcuTarget->toolChainPackage());
-        McuSupportOptions::setKitEnvironment(k, mcuTarget, qtForMCUsSdk);
+        setKitEnvironment(k, mcuTarget, qtForMCUsSdk);
         setKitDependencies(k, mcuTarget, qtForMCUsSdk);
         setKitCMakeOptions(k, mcuTarget, qtForMCUsSdk->path());
         setKitQtVersionOptions(k);
@@ -479,7 +561,7 @@ void upgradeKitInPlace(ProjectExplorer::Kit *kit,
                                           const McuAbstractPackage *qtForMCUsSdk)
 {
     setKitProperties(kitName(mcuTarget), kit, mcuTarget, qtForMCUsSdk->path());
-    McuSupportOptions::setKitEnvironment(kit, mcuTarget, qtForMCUsSdk);
+    setKitEnvironment(kit, mcuTarget, qtForMCUsSdk);
     setKitDependencies(kit, mcuTarget, qtForMCUsSdk);
 }
 
@@ -493,7 +575,8 @@ void fixKitsDependencies()
     for (const auto &target : qAsConst(repo.mcuTargets)) {
         if (target->isValid()) {
             for (auto *kit : kitsWithMismatchedDependencies(target)) {
-                McuSupportOptions::updateKitEnvironment(kit, target);
+                // ToDo: should not be environment, but cmake vars
+                updateKitEnvironment(kit, target);
             }
         }
     }

src/plugins/mcusupport/mcukitmanager.h

@@ -71,6 +71,7 @@ namespace McuKitManager
     // Fixing kits:
     void fixKitsDependencies();
     void fixExistingKits();
+    void updateKitEnvironment(ProjectExplorer::Kit *k, const McuTarget *mcuTarget);
 
     // Outdated kits:
     QList<ProjectExplorer::Kit *> outdatedKits();

src/plugins/mcusupport/mcusupportcmakemapper.cpp

@@ -58,6 +58,7 @@ static const QHash<QString, QString> &envVarToCMakeVarMapping()
         {"JLINK_PATH", "JLINK_PATH"},
         {"CYPRESS_AUTO_FLASH_UTILITY_DIR", "INFINEON_AUTO_FLASH_UTILITY_DIR"},
         {"EK_RA6M3G_E2_PROJECT_PATH", "EK_RA6M3G_E2_PROJECT_PATH"},
+        {"Qul_DIR", "Qul_ROOT"},
     };
     return mapping;
 }
@@ -77,3 +78,8 @@ QList<CMakeProjectManager::CMakeConfigItem> McuSupport::Internal::mapEnvVarsToQu
         return !item.key.isEmpty();
     });
 }
+
+QString McuSupport::Internal::mapEnvVarToQul2xCmakeVar(const QString &envVar)
+{
+    return envVarToCMakeVarMapping().value(envVar, QString());
+}

src/plugins/mcusupport/mcusupportcmakemapper.h

@@ -31,5 +31,6 @@ namespace McuSupport {
 namespace Internal {
 QList<CMakeProjectManager::CMakeConfigItem> mapEnvVarsToQul2xCmakeVars(
     const Utils::EnvironmentItems &envVars);
+QString mapEnvVarToQul2xCmakeVar(const QString &envVar);
 }
 } // namespace McuSupport

src/plugins/mcusupport/mcusupportoptions.cpp

@@ -29,7 +29,6 @@
 #include "mcutarget.h"
 #include "mcukitmanager.h"
 #include "mcukitinformation.h"
-#include "mcusupportcmakemapper.h"
 #include "mcusupportconstants.h"
 #include "mcusupportsdk.h"
 #include "mcusupportplugin.h"
@@ -149,87 +148,6 @@ FilePath McuSupportOptions::qulDirFromSettings()
                                         {});
 }
 
-void McuSupportOptions::remapQul2xCmakeVars(Kit *kit, const EnvironmentItems &envItems)
-{
-    const auto cmakeVars = mapEnvVarsToQul2xCmakeVars(envItems);
-    const auto cmakeVarNames = Utils::transform(cmakeVars, &CMakeConfigItem::key);
-
-    // First filter out all Qul2.x CMake vars
-    auto config = Utils::filtered(CMakeConfigurationKitAspect::configuration(kit),
-                                  [&](const auto &configItem) {
-                                      return !cmakeVarNames.contains(configItem.key);
-                                  });
-    // Then append them with new values
-    config.append(cmakeVars);
-    CMakeConfigurationKitAspect::setConfiguration(kit, config);
-}
-
-static bool expectsCmakeVars(const McuTarget *mcuTarget)
-{
-    return mcuTarget->qulVersion() >= QVersionNumber{2, 0};
-}
-
-void McuSupportOptions::setKitEnvironment(Kit *k,
-                                          const McuTarget *mcuTarget,
-                                          const McuAbstractPackage *qtForMCUsSdkPackage)
-{
-    EnvironmentItems changes;
-    QStringList pathAdditions;
-
-    // The Desktop version depends on the Qt shared libs in Qul_DIR/bin.
-    // If CMake's fileApi is avaialble, we can rely on the "Add library search path to PATH"
-    // feature of the run configuration. Otherwise, we just prepend the path, here.
-    if (mcuTarget->toolChainPackage()->isDesktopToolchain()
-        && !CMakeProjectManager::CMakeToolManager::defaultCMakeTool()->hasFileApi())
-        pathAdditions.append(qtForMCUsSdkPackage->path().pathAppended("bin").toUserOutput());
-
-    auto processPackage = [&pathAdditions, &changes](const McuAbstractPackage *package) {
-        if (package->isAddToSystemPath())
-            pathAdditions.append(package->path().toUserOutput());
-        if (!package->environmentVariableName().isEmpty())
-            changes.append({package->environmentVariableName(), package->path().toUserOutput()});
-    };
-    for (auto package : mcuTarget->packages())
-        processPackage(package);
-    processPackage(qtForMCUsSdkPackage);
-
-    if (McuSupportOptions::kitsNeedQtVersion())
-        changes.append({QLatin1String("LD_LIBRARY_PATH"), "%{Qt:QT_INSTALL_LIBS}"});
-
-    // Hack, this problem should be solved in lower layer
-    if (expectsCmakeVars(mcuTarget)) {
-        McuSupportOptions::remapQul2xCmakeVars(k, changes);
-    }
-
-    EnvironmentKitAspect::setEnvironmentChanges(k, changes);
-}
-
-void McuSupportOptions::updateKitEnvironment(Kit *k, const McuTarget *mcuTarget)
-{
-    EnvironmentItems changes = EnvironmentKitAspect::environmentChanges(k);
-    for (auto package : mcuTarget->packages()) {
-        const QString varName = package->environmentVariableName();
-        if (!varName.isEmpty() && package->isValidStatus()) {
-            const int index = Utils::indexOf(changes, [varName](const EnvironmentItem &item) {
-                return item.name == varName;
-            });
-            const EnvironmentItem item = {package->environmentVariableName(),
-                                          package->path().toUserOutput()};
-            if (index != -1)
-                changes.replace(index, item);
-            else
-                changes.append(item);
-        }
-    }
-
-    // Hack, this problem should be solved in lower layer
-    if (expectsCmakeVars(mcuTarget)) {
-        remapQul2xCmakeVars(k, changes);
-    }
-
-    EnvironmentKitAspect::setEnvironmentChanges(k, changes);
-}
-
 McuKitManager::UpgradeOption McuSupportOptions::askForKitUpgrades()
 {
     QMessageBox upgradePopup(Core::ICore::dialogParent());

src/plugins/mcusupport/mcusupportoptions.h

@@ -74,11 +74,6 @@ public:
     McuSdkRepository sdkRepository;
 
     void setQulDir(const Utils::FilePath &dir);
-    static void setKitEnvironment(ProjectExplorer::Kit *,
-                                  const McuTarget *,
-                                  const McuAbstractPackage *);
-    static void updateKitEnvironment(ProjectExplorer::Kit *, const McuTarget *);
-    static void remapQul2xCmakeVars(ProjectExplorer::Kit *, const Utils::EnvironmentItems &);
     static Utils::FilePath qulDirFromSettings();
     static McuKitManager::UpgradeOption askForKitUpgrades();
 

src/plugins/mcusupport/test/unittest.cpp

@@ -82,7 +82,7 @@ void McuSupportTest::test_addNewKit()
 
 void McuSupportTest::test_addFreeRtosCmakeVarToKit()
 {
-    McuSupportOptions::updateKitEnvironment(&kit, &mcuTarget);
+    McuKitManager::updateKitEnvironment(&kit, &mcuTarget);
 
     QVERIFY(kit.hasValue(EnvironmentKitAspect::id()));
     QVERIFY(kit.isValid());