From 9686c85a469c193438fb457bb41f4c0eb3e22188 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Mon, 4 Mar 2019 17:29:57 +0100 Subject: [PATCH] Qmake: Improve the build dir location warning The infamous "build dir is not at the same level at source dir" warning for qmake projects has gone through a number of iterations in Qt Creator, having been added, removed and re-added, always to the criticism of some users. Our new approach is as follows: - The warning appears at the widgets where the build directory is set, both in the target setup page and the build config widget. - The warning also appears in the issues pane, but only if the build failed. - The user can disable the warning altogether in a newly introduced qmake settings page. - This option is disabled by default on Unix, because to my knowledge all failure reports have been for Windows hosts. This should finally please everybody. Fixes: QTCREATORBUG-16945 Change-Id: I638be1f15e8c260a5d72047d6850a3a0f685cf03 Reviewed-by: Joerg Bornemann --- .../projectexplorer/abstractprocessstep.h | 2 +- .../qmakebuildconfiguration.cpp | 36 +++-- .../qmakebuildconfiguration.h | 4 + .../qmakeprojectmanager/qmakemakestep.cpp | 15 ++ .../qmakeprojectmanager/qmakemakestep.h | 4 +- .../qmakeprojectconfigwidget.cpp | 11 ++ .../qmakeprojectmanager.pro | 2 + .../qmakeprojectmanager.qbs | 1 + .../qmakeprojectmanagerplugin.cpp | 3 + .../qmakeprojectmanager/qmakesettings.cpp | 141 ++++++++++++++++++ .../qmakeprojectmanager/qmakesettings.h | 76 ++++++++++ 11 files changed, 280 insertions(+), 15 deletions(-) create mode 100644 src/plugins/qmakeprojectmanager/qmakesettings.cpp create mode 100644 src/plugins/qmakeprojectmanager/qmakesettings.h diff --git a/src/plugins/projectexplorer/abstractprocessstep.h b/src/plugins/projectexplorer/abstractprocessstep.h index 5d76bf7f4e6..769010c7afa 100644 --- a/src/plugins/projectexplorer/abstractprocessstep.h +++ b/src/plugins/projectexplorer/abstractprocessstep.h @@ -57,6 +57,7 @@ protected: ~AbstractProcessStep() override; bool init() override; void doRun() override; + virtual void finish(bool success); virtual void processStarted(); virtual void processFinished(int exitCode, QProcess::ExitStatus status); @@ -68,7 +69,6 @@ protected: void doCancel() override; private: - virtual void finish(bool success); void processReadyReadStdOutput(); void processReadyReadStdError(); diff --git a/src/plugins/qmakeprojectmanager/qmakebuildconfiguration.cpp b/src/plugins/qmakeprojectmanager/qmakebuildconfiguration.cpp index a2a187194ca..0b37c741804 100644 --- a/src/plugins/qmakeprojectmanager/qmakebuildconfiguration.cpp +++ b/src/plugins/qmakeprojectmanager/qmakebuildconfiguration.cpp @@ -31,6 +31,7 @@ #include "qmakeprojectconfigwidget.h" #include "qmakeprojectmanagerconstants.h" #include "qmakenodes.h" +#include "qmakesettings.h" #include "qmakestep.h" #include "qmakemakestep.h" #include "makefileparse.h" @@ -293,6 +294,23 @@ void QmakeBuildConfiguration::emitProFileEvaluateNeeded() static_cast(p)->scheduleAsyncUpdate(); } +QString QmakeBuildConfiguration::unalignedBuildDirWarning() +{ + return tr("The build directory should be at the same level as the source directory."); +} + +bool QmakeBuildConfiguration::isBuildDirAtSafeLocation(const QString &sourceDir, + const QString &buildDir) +{ + return buildDir.count('/') == sourceDir.count('/'); +} + +bool QmakeBuildConfiguration::isBuildDirAtSafeLocation() const +{ + return isBuildDirAtSafeLocation(project()->projectDirectory().toString(), + buildDirectory().toString()); +} + void QmakeBuildConfiguration::emitQMakeBuildConfigurationChanged() { emit qmakeBuildConfigurationChanged(); @@ -571,19 +589,11 @@ QmakeBuildConfigurationFactory::QmakeBuildConfigurationFactory() QList issues; if (version) issues << version->reportIssues(projectPath, buildDir); - - QString tmpBuildDir = QDir(buildDir).absolutePath(); - const QChar slash = QLatin1Char('/'); - if (!tmpBuildDir.endsWith(slash)) - tmpBuildDir.append(slash); - QString sourcePath = QFileInfo(projectPath).absolutePath(); - if (!sourcePath.endsWith(slash)) - sourcePath.append(slash); - if (tmpBuildDir.count(slash) != sourcePath.count(slash)) { - const QString msg = QCoreApplication::translate("QmakeProjectManager::QtVersion", - "The build directory needs to be at the same level as the source directory."); - - issues.append(Task(Task::Warning, msg, Utils::FileName(), -1, + if (QmakeSettings::warnAgainstUnalignedBuildDir() + && !QmakeBuildConfiguration::isBuildDirAtSafeLocation( + QDir(projectPath).absolutePath(), QDir(buildDir).absolutePath())) { + issues.append(Task(Task::Warning, QmakeBuildConfiguration::unalignedBuildDirWarning(), + Utils::FileName(), -1, ProjectExplorer::Constants::TASK_CATEGORY_BUILDSYSTEM)); } return issues; diff --git a/src/plugins/qmakeprojectmanager/qmakebuildconfiguration.h b/src/plugins/qmakeprojectmanager/qmakebuildconfiguration.h index 480226e1191..833f280a147 100644 --- a/src/plugins/qmakeprojectmanager/qmakebuildconfiguration.h +++ b/src/plugins/qmakeprojectmanager/qmakebuildconfiguration.h @@ -101,6 +101,10 @@ public: void emitProFileEvaluateNeeded(); + static QString unalignedBuildDirWarning(); + static bool isBuildDirAtSafeLocation(const QString &sourceDir, const QString &buildDir); + bool isBuildDirAtSafeLocation() const; + signals: /// emitted for setQMakeBuildConfig, not emitted for Qt version changes, even /// if those change the qmakebuildconfig diff --git a/src/plugins/qmakeprojectmanager/qmakemakestep.cpp b/src/plugins/qmakeprojectmanager/qmakemakestep.cpp index 86bd85359aa..77351d4ef9f 100644 --- a/src/plugins/qmakeprojectmanager/qmakemakestep.cpp +++ b/src/plugins/qmakeprojectmanager/qmakemakestep.cpp @@ -30,6 +30,7 @@ #include "qmakenodes.h" #include "qmakebuildconfiguration.h" #include "qmakeprojectmanagerconstants.h" +#include "qmakesettings.h" #include #include @@ -38,6 +39,7 @@ #include #include #include +#include #include #include #include @@ -168,6 +170,7 @@ bool QmakeMakeStep::init() // it has a low priority. m_scriptTarget = (static_cast(bc->target()->project())->rootProjectNode()->projectType() == ProjectType::ScriptTemplate); + m_unalignedBuildDir = !bc->isBuildDirAtSafeLocation(); return AbstractProcessStep::init(); } @@ -190,6 +193,18 @@ void QmakeMakeStep::doRun() AbstractProcessStep::doRun(); } +void QmakeMakeStep::finish(bool success) +{ + if (!success && !isCanceled() && m_unalignedBuildDir + && QmakeSettings::warnAgainstUnalignedBuildDir()) { + const QString msg = tr("The build directory is not at the same level as the source " + "directory, which could be the reason for the build failure."); + emit addTask(Task(Task::Warning, msg, Utils::FileName(), -1, + ProjectExplorer::Constants::TASK_CATEGORY_BUILDSYSTEM)); + } + MakeStep::finish(success); +} + /// // QmakeMakeStepFactory /// diff --git a/src/plugins/qmakeprojectmanager/qmakemakestep.h b/src/plugins/qmakeprojectmanager/qmakemakestep.h index 0a1cbe91989..b8142feff6e 100644 --- a/src/plugins/qmakeprojectmanager/qmakemakestep.h +++ b/src/plugins/qmakeprojectmanager/qmakemakestep.h @@ -54,12 +54,14 @@ public: QmakeBuildConfiguration *qmakeBuildConfiguration() const; +private: + void finish(bool success) override; bool init() override; void doRun() override; -private: bool m_scriptTarget = false; QString m_makeFileToCheck; + bool m_unalignedBuildDir; }; } // QmakeProjectManager diff --git a/src/plugins/qmakeprojectmanager/qmakeprojectconfigwidget.cpp b/src/plugins/qmakeprojectmanager/qmakeprojectconfigwidget.cpp index 17dc6d578bf..9657d6edc73 100644 --- a/src/plugins/qmakeprojectmanager/qmakeprojectconfigwidget.cpp +++ b/src/plugins/qmakeprojectmanager/qmakeprojectconfigwidget.cpp @@ -28,6 +28,7 @@ #include "qmakeproject.h" #include "qmakebuildconfiguration.h" #include "qmakenodes.h" +#include "qmakesettings.h" #include "ui_qmakeprojectconfigwidget.h" #include @@ -111,6 +112,8 @@ QmakeProjectConfigWidget::QmakeProjectConfigWidget(QmakeBuildConfiguration *bc) this, &QmakeProjectConfigWidget::updateProblemLabel); connect(project, &Project::parsingFinished, this, &QmakeProjectConfigWidget::updateProblemLabel); + connect(&QmakeSettings::instance(), &QmakeSettings::settingsChanged, + this, &QmakeProjectConfigWidget::updateProblemLabel); connect(bc->target(), &Target::kitChanged, this, &QmakeProjectConfigWidget::updateProblemLabel); @@ -250,6 +253,11 @@ void QmakeProjectConfigWidget::updateProblemLabel() } } + const bool unalignedBuildDir = QmakeSettings::warnAgainstUnalignedBuildDir() + && !m_buildConfiguration->isBuildDirAtSafeLocation(); + if (unalignedBuildDir) + allGood = false; + if (allGood) { QString buildDirectory = m_buildConfiguration->target()->project()->projectDirectory().toString(); if (m_buildConfiguration->isShadowBuild()) @@ -293,6 +301,9 @@ void QmakeProjectConfigWidget::updateProblemLabel() .arg(errorString) .arg(m_buildConfiguration->buildDirectory().toUserOutput())); return; + } else if (unalignedBuildDir) { + setProblemLabel(m_buildConfiguration->unalignedBuildDirWarning()); + return; } setProblemLabel(QString()); diff --git a/src/plugins/qmakeprojectmanager/qmakeprojectmanager.pro b/src/plugins/qmakeprojectmanager/qmakeprojectmanager.pro index b08bc4e87c6..5d907a0e955 100644 --- a/src/plugins/qmakeprojectmanager/qmakeprojectmanager.pro +++ b/src/plugins/qmakeprojectmanager/qmakeprojectmanager.pro @@ -12,6 +12,7 @@ HEADERS += \ qmakeprojectmanagerplugin.h \ qmakeprojectmanager.h \ qmakeproject.h \ + qmakesettings.h \ qmakenodes.h \ qmakenodetreebuilder.h \ profileeditor.h \ @@ -52,6 +53,7 @@ SOURCES += \ qmakeprojectmanager.cpp \ qmakeproject.cpp \ qmakenodes.cpp \ + qmakesettings.cpp \ qmakenodetreebuilder.cpp \ profileeditor.cpp \ profilehighlighter.cpp \ diff --git a/src/plugins/qmakeprojectmanager/qmakeprojectmanager.qbs b/src/plugins/qmakeprojectmanager/qmakeprojectmanager.qbs index 21670d086fb..b32dc907a5a 100644 --- a/src/plugins/qmakeprojectmanager/qmakeprojectmanager.qbs +++ b/src/plugins/qmakeprojectmanager/qmakeprojectmanager.qbs @@ -41,6 +41,7 @@ Project { "qmakeparser.cpp", "qmakeparser.h", "qmakeparsernodes.cpp", "qmakeparsernodes.h", "qmakeprojectimporter.cpp", "qmakeprojectimporter.h", + "qmakesettings.cpp", "qmakesettings.h", "qmakestep.cpp", "qmakestep.h", "qmakestep.ui", "qmakebuildconfiguration.cpp", "qmakebuildconfiguration.h", "qmakenodes.cpp", "qmakenodes.h", diff --git a/src/plugins/qmakeprojectmanager/qmakeprojectmanagerplugin.cpp b/src/plugins/qmakeprojectmanager/qmakeprojectmanagerplugin.cpp index feeb03f2b9e..20f6667992d 100644 --- a/src/plugins/qmakeprojectmanager/qmakeprojectmanagerplugin.cpp +++ b/src/plugins/qmakeprojectmanager/qmakeprojectmanagerplugin.cpp @@ -28,6 +28,7 @@ #include "profileeditor.h" #include "qmakeprojectmanager.h" #include "qmakenodes.h" +#include "qmakesettings.h" #include "qmakestep.h" #include "qmakemakestep.h" #include "qmakebuildconfiguration.h" @@ -102,6 +103,8 @@ public: ProFileEditorFactory profileEditorFactory; + QmakeSettingsPage settingsPage; + ExternalQtEditor *m_designerEditor{ExternalQtEditor::createDesignerEditor()}; ExternalQtEditor *m_linguistEditor{ExternalQtEditor::createLinguistEditor()}; diff --git a/src/plugins/qmakeprojectmanager/qmakesettings.cpp b/src/plugins/qmakeprojectmanager/qmakesettings.cpp new file mode 100644 index 00000000000..e5cd77e64d3 --- /dev/null +++ b/src/plugins/qmakeprojectmanager/qmakesettings.cpp @@ -0,0 +1,141 @@ +/**************************************************************************** +** +** Copyright (C) 2019 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of Qt Creator. +** +** 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. +** +****************************************************************************/ + +#include "qmakesettings.h" + +#include +#include +#include + +#include +#include +#include + +namespace QmakeProjectManager { +namespace Internal { + +const char BUILD_DIR_WARNING_KEY[] = "QmakeProjectManager/WarnAgainstUnalignedBuildDir"; + +static bool operator==(const QmakeSettingsData &s1, const QmakeSettingsData &s2) +{ + return s1.warnAgainstUnalignedBuildDir == s2.warnAgainstUnalignedBuildDir; +} +static bool operator!=(const QmakeSettingsData &s1, const QmakeSettingsData &s2) +{ + return !(s1 == s2); +} + +bool QmakeSettings::warnAgainstUnalignedBuildDir() +{ + return instance().m_settings.warnAgainstUnalignedBuildDir; +} + +QmakeSettings &QmakeSettings::instance() +{ + static QmakeSettings theSettings; + return theSettings; +} + +void QmakeSettings::setSettingsData(const QmakeSettingsData &settings) +{ + if (instance().m_settings != settings) { + instance().m_settings = settings; + instance().storeSettings(); + emit instance().settingsChanged(); + } +} + +QmakeSettings::QmakeSettings() +{ + loadSettings(); +} + +void QmakeSettings::loadSettings() +{ + m_settings.warnAgainstUnalignedBuildDir = Core::ICore::settings()->value( + BUILD_DIR_WARNING_KEY, Utils::HostOsInfo::isWindowsHost()).toBool(); +} + +void QmakeSettings::storeSettings() const +{ + Core::ICore::settings()->setValue(BUILD_DIR_WARNING_KEY, warnAgainstUnalignedBuildDir()); +} + +class QmakeSettingsPage::SettingsWidget : public QWidget +{ + Q_DECLARE_TR_FUNCTIONS(QmakeProjectManager::Internal::QmakeSettingsPage) +public: + SettingsWidget() + { + m_warnAgainstUnalignedBuildDirCheckbox.setText(tr("Warn if a project's source and " + "build directories are not at the same level")); + m_warnAgainstUnalignedBuildDirCheckbox.setToolTip(tr("Qmake has subtle bugs that " + "can trigger if source and build directory are not at the same level.")); + m_warnAgainstUnalignedBuildDirCheckbox.setChecked( + QmakeSettings::warnAgainstUnalignedBuildDir()); + const auto layout = new QVBoxLayout(this); + layout->addWidget(&m_warnAgainstUnalignedBuildDirCheckbox); + layout->addStretch(1); + } + + void apply() + { + QmakeSettingsData settings; + settings.warnAgainstUnalignedBuildDir = m_warnAgainstUnalignedBuildDirCheckbox.isChecked(); + QmakeSettings::setSettingsData(settings); + } + +private: + QCheckBox m_warnAgainstUnalignedBuildDirCheckbox; +}; + +QmakeSettingsPage::QmakeSettingsPage() +{ + setId("K.QmakeProjectManager.QmakeSettings"); + setDisplayName(tr("Qmake")); + setCategory(ProjectExplorer::Constants::BUILD_AND_RUN_SETTINGS_CATEGORY); +} + +QWidget *QmakeSettingsPage::widget() +{ + if (!m_widget) + m_widget = new SettingsWidget; + return m_widget; + +} + +void QmakeSettingsPage::apply() +{ + if (m_widget) + m_widget->apply(); +} + +void QmakeSettingsPage::finish() +{ + delete m_widget; +} + +} // namespace Internal +} // namespace QmakeProjectManager diff --git a/src/plugins/qmakeprojectmanager/qmakesettings.h b/src/plugins/qmakeprojectmanager/qmakesettings.h new file mode 100644 index 00000000000..30f9d4a34d8 --- /dev/null +++ b/src/plugins/qmakeprojectmanager/qmakesettings.h @@ -0,0 +1,76 @@ +/**************************************************************************** +** +** Copyright (C) 2019 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of Qt Creator. +** +** 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. +** +****************************************************************************/ + +#pragma once + +#include + +#include +#include + +namespace QmakeProjectManager { +namespace Internal { + +class QmakeSettingsData { +public: + bool warnAgainstUnalignedBuildDir = false; +}; + +class QmakeSettings : public QObject +{ + Q_OBJECT +public: + static QmakeSettings &instance(); + static bool warnAgainstUnalignedBuildDir(); + static void setSettingsData(const QmakeSettingsData &settings); + +signals: + void settingsChanged(); + +private: + QmakeSettings(); + void loadSettings(); + void storeSettings() const; + + QmakeSettingsData m_settings; +}; + +class QmakeSettingsPage : public Core::IOptionsPage +{ + Q_OBJECT +public: + QmakeSettingsPage(); + +private: + QWidget *widget() override; + void apply() override; + void finish() override; + + class SettingsWidget; + QPointer m_widget; +}; + +} // namespace Internal +} // namespace QmakeProjectManager