CMake: Only allow one target in cmake build step

CMake will silently ignore all but the last target (or fail to build
anything). This allows to simplify our code.

Change-Id: Ieee3931aca0788307107e2021d507073ef42a21f
Task-number: QTCREATORBUG-15928
Reviewed-by: Tim Jenssen <tim.jenssen@theqtcompany.com>

src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp

@@ -409,14 +409,14 @@ ProjectExplorer::BuildConfiguration *CMakeBuildConfigurationFactory::create(Proj
 
     auto cleanStep = new CMakeBuildStep(cleanSteps);
     cleanSteps->insertStep(0, cleanStep);
-    cleanStep->setBuildTarget(CMakeBuildStep::cleanTarget(), true);
+    cleanStep->setBuildTarget(CMakeBuildStep::cleanTarget());
 
     bc->setBuildDirectory(copy.buildDirectory);
     bc->setCMakeConfiguration(copy.configuration);
 
     // Default to all
-    if (project->hasBuildTarget(QLatin1String("all")))
+    if (project->hasBuildTarget(CMakeBuildStep::allTarget()))
-        buildStep->setBuildTarget(QLatin1String("all"), true);
+        buildStep->setBuildTarget(CMakeBuildStep::allTarget());
 
     return bc;
 }

src/plugins/cmakeprojectmanager/cmakebuildstep.cpp

@@ -82,9 +82,8 @@ CMakeBuildStep::CMakeBuildStep(BuildStepList *bsl, Core::Id id) : AbstractProces
 
 CMakeBuildStep::CMakeBuildStep(BuildStepList *bsl, CMakeBuildStep *bs) :
     AbstractProcessStep(bsl, bs),
-    m_buildTargets(bs->m_buildTargets),
+    m_buildTarget(bs->m_buildTarget),
-    m_toolArguments(bs->m_toolArguments),
+    m_toolArguments(bs->m_toolArguments)
-    m_addRunConfigurationArgument(bs->m_addRunConfigurationArgument)
 {
     ctor(bsl);
 }
@@ -125,31 +124,34 @@ CMakeRunConfiguration *CMakeBuildStep::targetsActiveRunConfiguration() const
 
 void CMakeBuildStep::handleBuildTargetChanges()
 {
-    const QStringList filteredTargets
+    if (static_cast<CMakeProject *>(project())->buildTargetTitles().contains(m_buildTarget))
-            = Utils::filtered(static_cast<CMakeProject *>(project())->buildTargetTitles(),
+        setBuildTarget(m_buildTarget);
-                              [this](const QString &s) { return m_buildTargets.contains(s); });
+    else
-    setBuildTargets(filteredTargets);
+        setBuildTarget(CMakeBuildStep::allTarget());
     emit buildTargetsChanged();
 }
 
 QVariantMap CMakeBuildStep::toMap() const
 {
     QVariantMap map(AbstractProcessStep::toMap());
-    map.insert(QLatin1String(BUILD_TARGETS_KEY), m_buildTargets);
+    // Use QStringList for compatibility with old files
+    map.insert(QLatin1String(BUILD_TARGETS_KEY), QStringList(m_buildTarget));
     map.insert(QLatin1String(TOOL_ARGUMENTS_KEY), m_toolArguments);
-    map.insert(QLatin1String(ADD_RUNCONFIGURATION_ARGUMENT_KEY), m_addRunConfigurationArgument);
     return map;
 }
 
 bool CMakeBuildStep::fromMap(const QVariantMap &map)
 {
     if (map.value(QLatin1String(CLEAN_KEY), false).toBool()) {
-        m_buildTargets = QStringList(CMakeBuildStep::cleanTarget());
+        m_buildTarget = CMakeBuildStep::cleanTarget();
     } else {
-        m_buildTargets = map.value(QLatin1String(BUILD_TARGETS_KEY)).toStringList();
+        const QStringList targetList = map.value(QLatin1String(BUILD_TARGETS_KEY)).toStringList();
+        if (!targetList.isEmpty())
+            m_buildTarget = targetList.last();
         m_toolArguments = map.value(QLatin1String(TOOL_ARGUMENTS_KEY)).toString();
     }
-    m_addRunConfigurationArgument = map.value(QLatin1String(ADD_RUNCONFIGURATION_ARGUMENT_KEY), false).toBool();
+    if (map.value(QLatin1String(ADD_RUNCONFIGURATION_ARGUMENT_KEY), false).toBool())
+        m_buildTarget = QLatin1String(ADD_RUNCONFIGURATION_TEXT);
 
     return BuildStep::fromMap(map);
 }
@@ -178,7 +180,7 @@ bool CMakeBuildStep::init(QList<const BuildStep *> &earlierSteps)
     }
 
     CMakeRunConfiguration *rc = targetsActiveRunConfiguration();
-    if (m_addRunConfigurationArgument && (!rc || rc->title().isEmpty())) {
+    if ((m_buildTarget == QLatin1String(ADD_RUNCONFIGURATION_TEXT)) && (!rc || rc->title().isEmpty())) {
         emit addTask(Task(Task::Error,
                           QCoreApplication::translate("ProjectExplorer::Task",
                                     "You asked to build the current Run Configuration's build target only, "
@@ -196,7 +198,7 @@ bool CMakeBuildStep::init(QList<const BuildStep *> &earlierSteps)
 
     QString arguments = allArguments(rc);
 
-    setIgnoreReturnValue(m_buildTargets.contains(CMakeBuildStep::cleanTarget()));
+    setIgnoreReturnValue(m_buildTarget == CMakeBuildStep::cleanTarget());
 
     ProcessParameters *pp = processParameters();
     pp->setMacroExpander(bc->macroExpander());
@@ -292,44 +294,27 @@ void CMakeBuildStep::stdOutput(const QString &line)
         AbstractProcessStep::stdOutput(line);
 }
 
-QStringList CMakeBuildStep::buildTargets() const
+QString CMakeBuildStep::buildTarget() const
 {
-    return m_buildTargets;
+    return m_buildTarget;
 }
 
 bool CMakeBuildStep::buildsBuildTarget(const QString &target) const
 {
-    if (target == tr(ADD_RUNCONFIGURATION_TEXT))
+    return target == m_buildTarget;
-        return addRunConfigurationArgument();
-    else
-        return m_buildTargets.contains(target);
 }
 
-void CMakeBuildStep::setBuildTarget(const QString &buildTarget, bool on)
+void CMakeBuildStep::setBuildTarget(const QString &buildTarget)
 {
-    if (buildTarget == tr(ADD_RUNCONFIGURATION_TEXT)) {
+    if (m_buildTarget == buildTarget)
-        setAddRunConfigurationArgument(on);
+        return;
-    } else {
+    m_buildTarget = buildTarget;
-        QStringList old = m_buildTargets;
+    emit targetToBuildChanged();
-        if (on && !old.contains(buildTarget))
-            old << buildTarget;
-        else if (!on && old.contains(buildTarget))
-            old.removeOne(buildTarget);
-        setBuildTargets(old);
-    }
-}
-
-void CMakeBuildStep::setBuildTargets(const QStringList &targets)
-{
-    if (targets != m_buildTargets) {
-        m_buildTargets = targets;
-        emit targetsToBuildChanged();
-    }
 }
 
 void CMakeBuildStep::clearBuildTargets()
 {
-    m_buildTargets.clear();
+    m_buildTarget.clear();
 }
 
 QString CMakeBuildStep::toolArguments() const
@@ -349,18 +334,20 @@ QString CMakeBuildStep::allArguments(const CMakeRunConfiguration *rc) const
     Utils::QtcProcess::addArg(&arguments, QLatin1String("--build"));
     Utils::QtcProcess::addArg(&arguments, QLatin1String("."));
 
-    if (m_addRunConfigurationArgument) {
+    QString target;
-        Utils::QtcProcess::addArg(&arguments, QLatin1String("--target"));
+
+    if (m_buildTarget == QLatin1String(ADD_RUNCONFIGURATION_TEXT)) {
         if (rc)
-            Utils::QtcProcess::addArg(&arguments, rc->title());
+            target = rc->title();
         else
-            Utils::QtcProcess::addArg(&arguments, QLatin1String("<i>&lt;") + tr(ADD_RUNCONFIGURATION_TEXT) + QLatin1String("&gt;</i>"));
+            target = QLatin1String("<i>&lt;") + tr(ADD_RUNCONFIGURATION_TEXT) + QLatin1String("&gt;</i>");
-    }
+    } else {
-    foreach (const QString &t, m_buildTargets) {
+        target = m_buildTarget;
-        Utils::QtcProcess::addArg(&arguments, QLatin1String("--target"));
-        Utils::QtcProcess::addArg(&arguments, t);
     }
 
+    Utils::QtcProcess::addArg(&arguments, QLatin1String("--target"));
+    Utils::QtcProcess::addArg(&arguments, target);
+
     if (!m_toolArguments.isEmpty()) {
         Utils::QtcProcess::addArg(&arguments, QLatin1String("--"));
         arguments += QLatin1Char(' ') + m_toolArguments;
@@ -369,16 +356,6 @@ QString CMakeBuildStep::allArguments(const CMakeRunConfiguration *rc) const
     return arguments;
 }
 
-bool CMakeBuildStep::addRunConfigurationArgument() const
-{
-    return m_addRunConfigurationArgument;
-}
-
-void CMakeBuildStep::setAddRunConfigurationArgument(bool add)
-{
-    m_addRunConfigurationArgument = add;
-}
-
 QString CMakeBuildStep::cmakeCommand() const
 {
     CMakeTool *tool = CMakeKitInformation::cmakeTool(target()->kit());
@@ -390,6 +367,11 @@ QString CMakeBuildStep::cleanTarget()
     return QLatin1String("clean");
 }
 
+QString CMakeBuildStep::allTarget()
+{
+    return QLatin1String("all");
+}
+
 //
 // CMakeBuildStepConfigWidget
 //
@@ -407,7 +389,6 @@ CMakeBuildStepConfigWidget::CMakeBuildStepConfigWidget(CMakeBuildStep *buildStep
     fl->addRow(tr("Tool arguments:"), m_toolArguments);
     m_toolArguments->setText(m_buildStep->toolArguments());
 
-
     m_buildTargetsList->setFrameStyle(QFrame::NoFrame);
     m_buildTargetsList->setMinimumHeight(200);
 
@@ -420,22 +401,7 @@ CMakeBuildStepConfigWidget::CMakeBuildStepConfigWidget(CMakeBuildStep *buildStep
 
     fl->addRow(tr("Targets:"), frame);
 
-    auto itemAddRunConfigurationArgument = new QListWidgetItem(tr(ADD_RUNCONFIGURATION_TEXT), m_buildTargetsList);
+    buildTargetsChanged();
-    itemAddRunConfigurationArgument->setFlags(itemAddRunConfigurationArgument->flags() | Qt::ItemIsUserCheckable);
-    itemAddRunConfigurationArgument->setCheckState(m_buildStep->addRunConfigurationArgument() ? Qt::Checked : Qt::Unchecked);
-    QFont f;
-    f.setItalic(true);
-    itemAddRunConfigurationArgument->setFont(f);
-
-    CMakeProject *pro = static_cast<CMakeProject *>(m_buildStep->project());
-    QStringList targetList = pro->buildTargetTitles();
-    targetList.sort();
-    foreach (const QString &buildTarget, targetList) {
-        auto item = new QListWidgetItem(buildTarget, m_buildTargetsList);
-        item->setFlags(item->flags() | Qt::ItemIsUserCheckable);
-        item->setCheckState(m_buildStep->buildsBuildTarget(item->text()) ? Qt::Checked : Qt::Unchecked);
-    }
-
     updateDetails();
 
     connect(m_toolArguments, &QLineEdit::textEdited, this, &CMakeBuildStepConfigWidget::toolArgumentsEdited);
@@ -444,8 +410,9 @@ CMakeBuildStepConfigWidget::CMakeBuildStepConfigWidget(CMakeBuildStep *buildStep
             this, &CMakeBuildStepConfigWidget::updateDetails);
 
     connect(m_buildStep, &CMakeBuildStep::buildTargetsChanged, this, &CMakeBuildStepConfigWidget::buildTargetsChanged);
-    connect(m_buildStep, &CMakeBuildStep::targetsToBuildChanged, this, &CMakeBuildStepConfigWidget::selectedBuildTargetsChanged);
+    connect(m_buildStep, &CMakeBuildStep::targetToBuildChanged, this, &CMakeBuildStepConfigWidget::selectedBuildTargetsChanged);
-    connect(pro, &CMakeProject::environmentChanged, this, &CMakeBuildStepConfigWidget::updateDetails);
+    connect(static_cast<CMakeProject *>(m_buildStep->project()), &CMakeProject::environmentChanged,
+            this, &CMakeBuildStepConfigWidget::updateDetails);
 }
 
 void CMakeBuildStepConfigWidget::toolArgumentsEdited()
@@ -456,7 +423,9 @@ void CMakeBuildStepConfigWidget::toolArgumentsEdited()
 
 void CMakeBuildStepConfigWidget::itemChanged(QListWidgetItem *item)
 {
-    m_buildStep->setBuildTarget(item->text(), item->checkState() & Qt::Checked);
+    const QString target =
+            (item->checkState() == Qt::Checked) ? item->data(Qt::UserRole).toString() : CMakeBuildStep::allTarget();
+    m_buildStep->setBuildTarget(target);
     updateDetails();
 }
 
@@ -467,30 +436,44 @@ QString CMakeBuildStepConfigWidget::displayName() const
 
 void CMakeBuildStepConfigWidget::buildTargetsChanged()
 {
-    disconnect(m_buildTargetsList, &QListWidget::itemChanged, this, &CMakeBuildStepConfigWidget::itemChanged);
+    const bool wasBlocked = m_buildTargetsList->blockSignals(true);
-
-    auto *addRunConfigurationArgumentItem = m_buildTargetsList->takeItem(0);
     m_buildTargetsList->clear();
-    m_buildTargetsList->insertItem(0, addRunConfigurationArgumentItem);
 
-    CMakeProject *pro = static_cast<CMakeProject *>(m_buildStep->target()->project());
+    auto item = new QListWidgetItem(tr(ADD_RUNCONFIGURATION_TEXT), m_buildTargetsList);
-    foreach (const QString& buildTarget, pro->buildTargetTitles()) {
+
+    item->setData(Qt::UserRole, QString::fromLatin1(ADD_RUNCONFIGURATION_TEXT));
+    QFont f;
+    f.setItalic(true);
+    item->setFont(f);
+
+    CMakeProject *pro = static_cast<CMakeProject *>(m_buildStep->project());
+    QStringList targetList = pro->buildTargetTitles();
+    targetList.sort();
+
+    foreach (const QString &buildTarget, targetList) {
         auto item = new QListWidgetItem(buildTarget, m_buildTargetsList);
-        item->setFlags(item->flags() | Qt::ItemIsUserCheckable);
+        item->setData(Qt::UserRole, buildTarget);
-        item->setCheckState(m_buildStep->buildsBuildTarget(item->text()) ? Qt::Checked : Qt::Unchecked);
     }
-    connect(m_buildTargetsList, &QListWidget::itemChanged, this, &CMakeBuildStepConfigWidget::itemChanged);
+
+    for (int i = 0; i < m_buildTargetsList->count(); ++i) {
+        QListWidgetItem *item = m_buildTargetsList->item(i);
+        item->setFlags(item->flags() | Qt::ItemIsUserCheckable);
+        item->setCheckState(m_buildStep->buildsBuildTarget(item->data(Qt::UserRole).toString())
+                            ? Qt::Checked : Qt::Unchecked);
+    }
+    m_buildTargetsList->blockSignals(wasBlocked);
     updateSummary();
 }
 
 void CMakeBuildStepConfigWidget::selectedBuildTargetsChanged()
 {
-    disconnect(m_buildTargetsList, &QListWidget::itemChanged, this, &CMakeBuildStepConfigWidget::itemChanged);
+    const bool wasBlocked = m_buildTargetsList->blockSignals(true);
     for (int y = 0; y < m_buildTargetsList->count(); ++y) {
         QListWidgetItem *item = m_buildTargetsList->item(y);
-        item->setCheckState(m_buildStep->buildsBuildTarget(item->text()) ? Qt::Checked : Qt::Unchecked);
+        item->setCheckState(m_buildStep->buildsBuildTarget(item->data(Qt::UserRole).toString())
+                            ? Qt::Checked : Qt::Unchecked);
     }
-    connect(m_buildTargetsList, &QListWidget::itemChanged, this, &CMakeBuildStepConfigWidget::itemChanged);
+    m_buildTargetsList->blockSignals(wasBlocked);
     updateSummary();
 }
 
@@ -541,7 +524,7 @@ BuildStep *CMakeBuildStepFactory::create(BuildStepList *parent, Core::Id id)
         return 0;
     auto step = new CMakeBuildStep(parent);
     if (parent->id() == ProjectExplorer::Constants::BUILDSTEPS_CLEAN)
-        step->setBuildTarget(CMakeBuildStep::cleanTarget(), true);
+        step->setBuildTarget(CMakeBuildStep::cleanTarget());
     return step;
 }
 

src/plugins/cmakeprojectmanager/cmakebuildstep.h

@@ -61,10 +61,9 @@ public:
     ProjectExplorer::BuildStepConfigWidget *createConfigWidget() override;
     bool immutable() const override;
 
-    QStringList buildTargets() const;
+    QString buildTarget() const;
     bool buildsBuildTarget(const QString &target) const;
-    void setBuildTarget(const QString &target, bool on);
+    void setBuildTarget(const QString &target);
-    void setBuildTargets(const QStringList &targets);
     void clearBuildTargets();
 
     QString toolArguments() const;
@@ -72,18 +71,16 @@ public:
 
     QString allArguments(const CMakeRunConfiguration *rc) const;
 
-    bool addRunConfigurationArgument() const;
-    void setAddRunConfigurationArgument(bool add);
-
     QString cmakeCommand() const;
 
     QVariantMap toMap() const override;
 
     static QString cleanTarget();
+    static QString allTarget();
 
 signals:
     void cmakeCommandChanged();
-    void targetsToBuildChanged();
+    void targetToBuildChanged();
     void buildTargetsChanged();
 
 protected:
@@ -112,9 +109,8 @@ private:
     QRegExp m_percentProgress;
     QRegExp m_ninjaProgress;
     QString m_ninjaProgressString;
-    QStringList m_buildTargets;
+    QString m_buildTarget;
     QString m_toolArguments;
-    bool m_addRunConfigurationArgument = false;
     bool m_useNinja = false;
 };
 

src/plugins/cmakeprojectmanager/cmakelocatorfilter.cpp

@@ -111,13 +111,13 @@ void CMakeLocatorFilter::accept(Core::LocatorFilterEntry selection) const
         return;
 
     // Change the make step to build only the given target
-    QStringList oldTargets = buildStep->buildTargets();
+    QString oldTarget = buildStep->buildTarget();
     buildStep->clearBuildTargets();
-    buildStep->setBuildTarget(selection.displayName, true);
+    buildStep->setBuildTarget(selection.displayName);
 
     // Build
     ProjectExplorerPlugin::buildProject(cmakeProject);
-    buildStep->setBuildTargets(oldTargets);
+    buildStep->setBuildTarget(oldTarget);
 }
 
 void CMakeLocatorFilter::refresh(QFutureInterface<void> &future)