Use base make step for autotools projects

Unify generic and autotools make steps, by moving the better maintained
logic from the generic make step to the base make step.

Add fallback code for restoring settings from previous autotools make
steps, since a key was named slightly different.

The autotools make step was behaving a little bit better when there is
no C++ toolchain set in the kit, but on the other hand would just take
the make command from a random existing toolchain, which can be wrong
too. Anyhow, this must be fixed in a follow-up patch for all make steps.

Change-Id: I47af7d327feb9336790d30c8b9a4968c25b25db3
Reviewed-by: Tobias Hunger <tobias.hunger@qt.io>

8 files changed, 100 insertions, 317 deletions

diff --git a/src/plugins/autotoolsprojectmanager/autotoolsbuildconfiguration.cpp b/src/plugins/autotoolsprojectmanager/autotoolsbuildconfiguration.cpp
index d930c380d7..c4f90b83c1 100644
--- a/src/plugins/autotoolsprojectmanager/autotoolsbuildconfiguration.cpp
+++ b/src/plugins/autotoolsprojectmanager/autotoolsbuildconfiguration.cpp
@@ -89,14 +89,12 @@ void AutotoolsBuildConfiguration::initialize(const BuildInfo *info)
             configureStep, &ConfigureStep::notifyBuildDirectoryChanged);
 
     // make
-    MakeStep *makeStep = new MakeStep(buildSteps);
+    MakeStep *makeStep = new MakeStep(buildSteps, "all");
     buildSteps->insertStep(2, makeStep);
-    makeStep->setBuildTarget(QLatin1String("all"),  /*on =*/ true);
 
     // ### Build Steps Clean ###
     BuildStepList *cleanSteps = stepList(BUILDSTEPS_CLEAN);
-    MakeStep *cleanMakeStep = new MakeStep(cleanSteps);
-    cleanMakeStep->setAdditionalArguments("clean");
+    MakeStep *cleanMakeStep = new MakeStep(cleanSteps, "clean");
     cleanMakeStep->setClean(true);
     cleanSteps->insertStep(0, cleanMakeStep);
 }
diff --git a/src/plugins/autotoolsprojectmanager/makestep.cpp b/src/plugins/autotoolsprojectmanager/makestep.cpp
index 26b3a39b5c..eebef955a2 100644
--- a/src/plugins/autotoolsprojectmanager/makestep.cpp
+++ b/src/plugins/autotoolsprojectmanager/makestep.cpp
@@ -31,216 +31,40 @@
 #include "autotoolsbuildconfiguration.h"
 
 #include <projectexplorer/buildsteplist.h>
-#include <projectexplorer/target.h>
-#include <projectexplorer/toolchain.h>
-#include <projectexplorer/gnumakeparser.h>
-#include <projectexplorer/kitinformation.h>
-#include <projectexplorer/projectexplorer.h>
 #include <projectexplorer/projectexplorerconstants.h>
-#include <qtsupport/qtkitinformation.h>
-#include <qtsupport/qtparser.h>
-#include <utils/qtcprocess.h>
-
-#include <QVariantMap>
-#include <QLineEdit>
-#include <QFormLayout>
 
 using namespace AutotoolsProjectManager;
 using namespace AutotoolsProjectManager::Internal;
 using namespace AutotoolsProjectManager::Constants;
-using namespace ProjectExplorer;
-using namespace ProjectExplorer::Constants;
 
 const char MAKE_STEP_ID[] = "AutotoolsProjectManager.MakeStep";
-const char CLEAN_KEY[]  = "AutotoolsProjectManager.MakeStep.Clean";
-const char BUILD_TARGETS_KEY[] = "AutotoolsProjectManager.MakeStep.BuildTargets";
-const char MAKE_STEP_ADDITIONAL_ARGUMENTS_KEY[] = "AutotoolsProjectManager.MakeStep.AdditionalArguments";
-
 
 // MakeStepFactory
 
 MakeStepFactory::MakeStepFactory()
 {
+    struct Step : public MakeStep
+    {
+        Step(ProjectExplorer::BuildStepList *bsl) : MakeStep(bsl)
+        {
+            if (bsl->id() == ProjectExplorer::Constants::BUILDSTEPS_CLEAN) {
+                setBuildTarget("clean", true);
+                setClean(true);
+            } else {
+                setBuildTarget("all", true);
+            }
+        }
+    };
+
     setObjectName("Autotools::MakeStepFactory");
-    registerStep<MakeStep>(MAKE_STEP_ID);
-    setDisplayName(tr("Make", "Display name for AutotoolsProjectManager::MakeStep id."));
+    registerStep<Step>(MAKE_STEP_ID);
+    setDisplayName(ProjectExplorer::MakeStep::defaultDisplayName());
     setSupportedProjectType(AUTOTOOLS_PROJECT_ID);
 }
 
 // MakeStep
 
-MakeStep::MakeStep(BuildStepList *bsl) : AbstractProcessStep(bsl, MAKE_STEP_ID)
-{
-    setDefaultDisplayName(tr("Make"));
-}
-
-void MakeStep::setClean(bool clean)
-{
-    m_clean = clean;
-}
-
-bool MakeStep::init(QList<const BuildStep *> &earlierSteps)
-{
-    BuildConfiguration *bc = buildConfiguration();
-    if (!bc)
-        emit addTask(Task::buildConfigurationMissingTask());
-
-    QList<ToolChain *> tcList = ToolChainKitInformation::toolChains(target()->kit());
-    if (tcList.isEmpty())
-        emit addTask(Task::compilerMissingTask());
-
-    if (tcList.isEmpty() || !bc) {
-        emitFaultyConfigurationMessage();
-        return false;
-    }
-
-    QString arguments = Utils::QtcProcess::joinArgs(m_buildTargets);
-    Utils::QtcProcess::addArgs(&arguments, additionalArguments());
-
-    setIgnoreReturnValue(m_clean);
-
-    ProcessParameters *pp = processParameters();
-    pp->setMacroExpander(bc->macroExpander());
-    Utils::Environment env = bc->environment();
-    Utils::Environment::setupEnglishOutput(&env);
-    pp->setEnvironment(env);
-    pp->setWorkingDirectory(bc->buildDirectory().toString());
-    pp->setCommand(tcList.at(0)->makeCommand(bc->environment()));
-    pp->setArguments(arguments);
-    pp->resolveAll();
-
-    setOutputParser(new GnuMakeParser());
-    IOutputParser *parser = target()->kit()->createOutputParser();
-    if (parser)
-        appendOutputParser(parser);
-    outputParser()->setWorkingDirectory(pp->effectiveWorkingDirectory());
-
-    return AbstractProcessStep::init(earlierSteps);
-}
-
-void MakeStep::run(QFutureInterface<bool> &interface)
-{
-    AbstractProcessStep::run(interface);
-}
-
-BuildStepConfigWidget *MakeStep::createConfigWidget()
-{
-    return new MakeStepConfigWidget(this);
-}
-
-bool MakeStep::immutable() const
-{
-    return false;
-}
-
-void MakeStep::setBuildTarget(const QString &target, bool on)
-{
-    QStringList old = m_buildTargets;
-    if (on && !old.contains(target))
-         old << target;
-    else if (!on && old.contains(target))
-        old.removeOne(target);
-
-    m_buildTargets = old;
-}
-
-void MakeStep::setAdditionalArguments(const QString &list)
+MakeStep::MakeStep(ProjectExplorer::BuildStepList *bsl, const QString &buildTarget)
+    : ProjectExplorer::MakeStep(bsl, MAKE_STEP_ID, buildTarget, {"all", "clean"})
 {
-    if (list == m_additionalArguments)
-        return;
-
-    m_additionalArguments = list;
-
-    emit additionalArgumentsChanged(list);
-}
-
-QString MakeStep::additionalArguments() const
-{
-    return m_additionalArguments;
-}
-
-QVariantMap MakeStep::toMap() const
-{
-    QVariantMap map = AbstractProcessStep::toMap();
-
-    map.insert(BUILD_TARGETS_KEY, m_buildTargets);
-    map.insert(MAKE_STEP_ADDITIONAL_ARGUMENTS_KEY, m_additionalArguments);
-    map.insert(CLEAN_KEY, m_clean);
-    return map;
-}
-
-bool MakeStep::fromMap(const QVariantMap &map)
-{
-    m_buildTargets = map.value(BUILD_TARGETS_KEY).toStringList();
-    m_additionalArguments = map.value(MAKE_STEP_ADDITIONAL_ARGUMENTS_KEY).toString();
-    m_clean = map.value(CLEAN_KEY).toBool();
-
-    return BuildStep::fromMap(map);
-}
-
-///////////////////////////////
-// MakeStepConfigWidget class
-///////////////////////////////
-MakeStepConfigWidget::MakeStepConfigWidget(MakeStep *makeStep) :
-    m_makeStep(makeStep),
-    m_summaryText(),
-    m_additionalArguments(0)
-{
-    QFormLayout *fl = new QFormLayout(this);
-    fl->setMargin(0);
-    fl->setFieldGrowthPolicy(QFormLayout::ExpandingFieldsGrow);
-    setLayout(fl);
-
-    m_additionalArguments = new QLineEdit(this);
-    fl->addRow(tr("Arguments:"), m_additionalArguments);
-    m_additionalArguments->setText(m_makeStep->additionalArguments());
-
-    updateDetails();
-
-    connect(m_additionalArguments, &QLineEdit::textChanged,
-            makeStep, &MakeStep::setAdditionalArguments);
-    connect(makeStep, &MakeStep::additionalArgumentsChanged,
-            this, &MakeStepConfigWidget::updateDetails);
-    m_makeStep->project()->subscribeSignal(&BuildConfiguration::environmentChanged, this, [this]() {
-        if (static_cast<BuildConfiguration *>(sender())->isActive())
-            updateDetails();
-    });
-    connect(makeStep->project(), &Project::activeProjectConfigurationChanged,
-            this, [this](ProjectConfiguration *pc) {
-        if (pc && pc->isActive())
-            updateDetails();
-    });
-}
-
-QString MakeStepConfigWidget::displayName() const
-{
-    return tr("Make", "AutotoolsProjectManager::MakeStepConfigWidget display name.");
-}
-
-QString MakeStepConfigWidget::summaryText() const
-{
-    return m_summaryText;
-}
-
-void MakeStepConfigWidget::updateDetails()
-{
-    BuildConfiguration *bc = m_makeStep->buildConfiguration();
-    QList<ToolChain *> tcList = ToolChainKitInformation::toolChains(m_makeStep->target()->kit());
-
-    if (!tcList.isEmpty()) {
-        QString arguments = Utils::QtcProcess::joinArgs(m_makeStep->m_buildTargets);
-        Utils::QtcProcess::addArgs(&arguments, m_makeStep->additionalArguments());
-
-        ProcessParameters param;
-        param.setMacroExpander(m_makeStep->macroExpander());
-        param.setEnvironment(bc->environment());
-        param.setWorkingDirectory(bc->buildDirectory().toString());
-        param.setCommand(tcList.at(0)->makeCommand(bc->environment()));
-        param.setArguments(arguments);
-        m_summaryText = param.summary(displayName());
-    } else {
-        m_summaryText = "<b>" + ToolChainKitInformation::msgNoToolChainInTarget()  + "</b>";
-    }
-
-    emit updateSummary();
 }
diff --git a/src/plugins/autotoolsprojectmanager/makestep.h b/src/plugins/autotoolsprojectmanager/makestep.h
index 1ded149366..cbc5496529 100644
--- a/src/plugins/autotoolsprojectmanager/makestep.h
+++ b/src/plugins/autotoolsprojectmanager/makestep.h
@@ -27,27 +27,14 @@
 
 #pragma once
 
-#include <projectexplorer/abstractprocessstep.h>
-#include <projectexplorer/task.h>
-
-QT_BEGIN_NAMESPACE
-class QLineEdit;
-QT_END_NAMESPACE
+#include <projectexplorer/makestep.h>
 
 namespace AutotoolsProjectManager {
 namespace Internal {
 
-class AutotoolsProject;
-class MakeStep;
-
 ///////////////////////////
 // MakeStepFactory class
 ///////////////////////////
-/**
- * @brief Implementation of the ProjectExplorer::IBuildStepFactory interface.
- *
- * The factory is used to create instances of MakeStep.
- */
 class MakeStepFactory : public ProjectExplorer::BuildStepFactory
 {
     Q_OBJECT
@@ -59,71 +46,12 @@ public:
 /////////////////////
 // MakeStep class
 /////////////////////
-/**
- * @brief Implementation of the ProjectExplorer::AbstractProcessStep interface.
- *
- * A make step can be configured by selecting the "Projects" button of Qt Creator
- * (in the left hand side menu) and under "Build Settings".
- *
- * It is possible for the user to specify custom arguments. The corresponding
- * configuration widget is created by MakeStep::createConfigWidget and is
- * represented by an instance of the class MakeStepConfigWidget.
- */
-class MakeStep : public ProjectExplorer::AbstractProcessStep
-{
-    Q_OBJECT
-    friend class MakeStepFactory;
-    friend class MakeStepConfigWidget;
-
-public:
-    explicit MakeStep(ProjectExplorer::BuildStepList *bsl);
-
-    bool init(QList<const BuildStep *> &earlierSteps) override;
-    void run(QFutureInterface<bool> &interface) override;
-    ProjectExplorer::BuildStepConfigWidget *createConfigWidget() override;
-    void setClean(bool clean);
-    bool immutable() const override;
-    void setBuildTarget(const QString &target, bool on);
-    QString additionalArguments() const;
-    QVariantMap toMap() const override;
-
-    void setAdditionalArguments(const QString &list);
-
-signals:
-    void additionalArgumentsChanged(const QString &);
-
-private:
-    bool fromMap(const QVariantMap &map) override;
-
-    QStringList m_buildTargets;
-    QString m_additionalArguments;
-    bool m_clean = false;
-};
-
-///////////////////////////////
-// MakeStepConfigWidget class
-///////////////////////////////
-/**
- * @brief Implementation of the ProjectExplorer::BuildStepConfigWidget interface.
- *
- * Allows to configure a make step in the GUI.
- */
-class MakeStepConfigWidget : public ProjectExplorer::BuildStepConfigWidget
+class MakeStep : public ProjectExplorer::MakeStep
 {
     Q_OBJECT
 
 public:
-    MakeStepConfigWidget(MakeStep *makeStep);
-
-    QString displayName() const override;
-    QString summaryText() const override;
-
-private:
-    void updateDetails();
-
-    MakeStep *m_makeStep;
-    QString m_summaryText;
-    QLineEdit *m_additionalArguments;
+    explicit MakeStep(ProjectExplorer::BuildStepList *bsl, const QString &buildTarget = QString());
 };
 
 } // namespace Internal
diff --git a/src/plugins/genericprojectmanager/genericmakestep.cpp b/src/plugins/genericprojectmanager/genericmakestep.cpp
index 62d7c1265e..9753140156 100644
--- a/src/plugins/genericprojectmanager/genericmakestep.cpp
+++ b/src/plugins/genericprojectmanager/genericmakestep.cpp
@@ -26,11 +26,7 @@
 #include "genericmakestep.h"
 #include "genericprojectconstants.h"
 
-#include <projectexplorer/buildconfiguration.h>
-#include <projectexplorer/gnumakeparser.h>
-#include <projectexplorer/kitinformation.h>
 #include <projectexplorer/projectexplorerconstants.h>
-#include <projectexplorer/target.h>
 
 using namespace ProjectExplorer;
 
@@ -44,45 +40,6 @@ GenericMakeStep::GenericMakeStep(BuildStepList *parent, const QString &buildTarg
 {
 }
 
-bool GenericMakeStep::init(QList<const BuildStep *> &earlierSteps)
-{
-    BuildConfiguration *bc = buildConfiguration();
-    if (!bc)
-        emit addTask(Task::buildConfigurationMissingTask());
-
-    ToolChain *tc = ToolChainKitInformation::toolChain(target()->kit(), ProjectExplorer::Constants::CXX_LANGUAGE_ID);
-    if (!tc)
-        emit addTask(Task::compilerMissingTask());
-
-    if (!bc || !tc) {
-        emitFaultyConfigurationMessage();
-        return false;
-    }
-
-    ProcessParameters *pp = processParameters();
-    pp->setMacroExpander(bc->macroExpander());
-    pp->setWorkingDirectory(bc->buildDirectory().toString());
-    Utils::Environment env = bc->environment();
-    Utils::Environment::setupEnglishOutput(&env);
-    pp->setEnvironment(env);
-    pp->setCommand(effectiveMakeCommand());
-    pp->setArguments(allArguments());
-    pp->resolveAll();
-
-    // If we are cleaning, then make can fail with an error code, but that doesn't mean
-    // we should stop the clean queue
-    // That is mostly so that rebuild works on an already clean project
-    setIgnoreReturnValue(isClean());
-
-    setOutputParser(new GnuMakeParser());
-    IOutputParser *parser = target()->kit()->createOutputParser();
-    if (parser)
-        appendOutputParser(parser);
-    outputParser()->setWorkingDirectory(pp->effectiveWorkingDirectory());
-
-    return AbstractProcessStep::init(earlierSteps);
-}
-
 //
 // GenericMakeAllStepFactory
 //
diff --git a/src/plugins/genericprojectmanager/genericmakestep.h b/src/plugins/genericprojectmanager/genericmakestep.h
index ce0f0546a1..6fe6bc4798 100644
--- a/src/plugins/genericprojectmanager/genericmakestep.h
+++ b/src/plugins/genericprojectmanager/genericmakestep.h
@@ -38,8 +38,6 @@ class GenericMakeStep : public ProjectExplorer::MakeStep
 
 public:
     explicit GenericMakeStep(ProjectExplorer::BuildStepList *parent, const QString &buildTarget = {});
-
-    bool init(QList<const BuildStep *> &earlierSteps) override;
 };
 
 class GenericMakeAllStepFactory : public ProjectExplorer::BuildStepFactory
diff --git a/src/plugins/projectexplorer/makestep.cpp b/src/plugins/projectexplorer/makestep.cpp
index 82b36f0127..f1419b501b 100644
--- a/src/plugins/projectexplorer/makestep.cpp
+++ b/src/plugins/projectexplorer/makestep.cpp
@@ -27,6 +27,7 @@
 #include "ui_makestep.h"
 
 #include "buildconfiguration.h"
+#include "gnumakeparser.h"
 #include "kitinformation.h"
 #include "project.h"
 #include "projectexplorer.h"
@@ -59,6 +60,45 @@ MakeStep::MakeStep(BuildStepList *parent,
         setBuildTarget(buildTarget, true);
 }
 
+bool MakeStep::init(QList<const BuildStep *> &earlierSteps)
+{
+    BuildConfiguration *bc = buildConfiguration();
+    if (!bc)
+        emit addTask(Task::buildConfigurationMissingTask());
+
+    ToolChain *tc = ToolChainKitInformation::toolChain(target()->kit(), ProjectExplorer::Constants::CXX_LANGUAGE_ID);
+    if (!tc)
+        emit addTask(Task::compilerMissingTask());
+
+    if (!bc || !tc) {
+        emitFaultyConfigurationMessage();
+        return false;
+    }
+
+    ProcessParameters *pp = processParameters();
+    pp->setMacroExpander(bc->macroExpander());
+    pp->setWorkingDirectory(bc->buildDirectory().toString());
+    Utils::Environment env = bc->environment();
+    Utils::Environment::setupEnglishOutput(&env);
+    pp->setEnvironment(env);
+    pp->setCommand(effectiveMakeCommand());
+    pp->setArguments(allArguments());
+    pp->resolveAll();
+
+    // If we are cleaning, then make can fail with an error code, but that doesn't mean
+    // we should stop the clean queue
+    // That is mostly so that rebuild works on an already clean project
+    setIgnoreReturnValue(isClean());
+
+    setOutputParser(new GnuMakeParser());
+    IOutputParser *parser = target()->kit()->createOutputParser();
+    if (parser)
+        appendOutputParser(parser);
+    outputParser()->setWorkingDirectory(pp->effectiveWorkingDirectory());
+
+    return AbstractProcessStep::init(earlierSteps);
+}
+
 void MakeStep::setClean(bool clean)
 {
     m_clean = clean;
diff --git a/src/plugins/projectexplorer/makestep.h b/src/plugins/projectexplorer/makestep.h
index df87923e04..126ca68809 100644
--- a/src/plugins/projectexplorer/makestep.h
+++ b/src/plugins/projectexplorer/makestep.h
@@ -46,6 +46,7 @@ public:
                       const QString &buildTarget = QString(),
                       const QStringList &availableTargets = {});
 
+    bool init(QList<const BuildStep *> &earlierSteps) override;
     ProjectExplorer::BuildStepConfigWidget *createConfigWidget() override;
     bool immutable() const override;
     bool buildsTarget(const QString &target) const;
diff --git a/src/plugins/projectexplorer/userfileaccessor.cpp b/src/plugins/projectexplorer/userfileaccessor.cpp
index 17ab86214e..085e2cf3a8 100644
--- a/src/plugins/projectexplorer/userfileaccessor.cpp
+++ b/src/plugins/projectexplorer/userfileaccessor.cpp
@@ -263,6 +263,18 @@ private:
     QVariantList m_sticky;
 };
 
+// Version 18 renames "AutotoolsProjectManager.MakeStep.AdditionalArguments" to
+// "AutotoolsProjectManager.MakeStep.MakeArguments" to account for
+// sharing the MakeStep implementation
+class UserFileVersion18Upgrader : public VersionUpgrader
+{
+public:
+    UserFileVersion18Upgrader() : VersionUpgrader(18, "4.8-pre1") { }
+    QVariantMap upgrade(const QVariantMap &map) final;
+
+    static QVariant process(const QVariant &entry);
+};
+
 } // namespace
 
 //
@@ -476,6 +488,7 @@ UserFileAccessor::UserFileAccessor(Project *project) :
     addVersionUpgrader(std::make_unique<UserFileVersion15Upgrader>());
     addVersionUpgrader(std::make_unique<UserFileVersion16Upgrader>());
     addVersionUpgrader(std::make_unique<UserFileVersion17Upgrader>());
+    addVersionUpgrader(std::make_unique<UserFileVersion18Upgrader>());
 }
 
 Project *UserFileAccessor::project() const
@@ -2188,6 +2201,30 @@ QVariant UserFileVersion17Upgrader::process(const QVariant &entry)
     }
 }
 
+QVariantMap UserFileVersion18Upgrader::upgrade(const QVariantMap &map)
+{
+    return process(map).toMap();
+}
+
+QVariant UserFileVersion18Upgrader::process(const QVariant &entry)
+{
+    switch (entry.type()) {
+    case QVariant::List:
+        return Utils::transform(entry.toList(), &UserFileVersion18Upgrader::process);
+    case QVariant::Map:
+        return Utils::transform<QMap<QString, QVariant>>(
+            entry.toMap().toStdMap(), [](const std::pair<const QString, QVariant> &item) {
+                const QString key = (item.first
+                                             == "AutotoolsProjectManager.MakeStep.AdditionalArguments"
+                                         ? QString("AutotoolsProjectManager.MakeStep.MakeArguments")
+                                         : item.first);
+                return qMakePair(key, UserFileVersion18Upgrader::process(item.second));
+            });
+    default:
+        return entry;
+    }
+}
+
 #if defined(WITH_TESTS)
 
 #include <QTest>