QbsProjectManager: Reparse project before building.

Otherwise, if "save before build" is enabled and the user presses Ctrl+B
with unsaved changes to a project file, these would get ignored due to
the delay (and also if the delay were not there, because the file system
watchers trigger later than the "Build" action).
If there are no actual changes to any build file, the overhead caused by
this operation consists of loading (and possibly storing) the build
graph file.

Task-number: QBS-596
Change-Id: I1f837cc0fcdc77a249b423834f4b6711f5c0bc87
Reviewed-by: Joerg Bornemann <joerg.bornemann@digia.com>

src/plugins/qbsprojectmanager/qbsbuildstep.cpp

@@ -67,7 +67,7 @@ namespace Internal {
 
 QbsBuildStep::QbsBuildStep(ProjectExplorer::BuildStepList *bsl) :
     ProjectExplorer::BuildStep(bsl, Core::Id(Constants::QBS_BUILDSTEP_ID)),
-    m_job(0), m_parser(0)
+    m_job(0), m_parser(0), m_parsingProject(false)
 {
     setDisplayName(tr("Qbs Build"));
     setQbsConfiguration(QVariantMap());
@@ -76,7 +76,7 @@ QbsBuildStep::QbsBuildStep(ProjectExplorer::BuildStepList *bsl) :
 
 QbsBuildStep::QbsBuildStep(ProjectExplorer::BuildStepList *bsl, const QbsBuildStep *other) :
     ProjectExplorer::BuildStep(bsl, Core::Id(Constants::QBS_BUILDSTEP_ID)),
-    m_qbsBuildOptions(other->m_qbsBuildOptions),  m_job(0), m_parser(0)
+    m_qbsBuildOptions(other->m_qbsBuildOptions),  m_job(0), m_parser(0), m_parsingProject(false)
 {
     setQbsConfiguration(other->qbsConfiguration());
 }
@@ -125,30 +125,9 @@ void QbsBuildStep::run(QFutureInterface<bool> &fi)
 {
     m_fi = &fi;
 
-    QbsProject *pro = static_cast<QbsProject *>(project());
+    // We need a pre-build parsing step in order not to lose project file changes done
-    qbs::BuildOptions options(m_qbsBuildOptions);
+    // right before building (but before the delay has elapsed).
-    options.setChangedFiles(m_changedFiles);
+    parseProject();
-    options.setFilesToConsider(m_changedFiles);
-    options.setActiveFileTags(m_activeFileTags);
-
-    m_job = pro->build(options, m_products);
-
-    if (!m_job) {
-        m_fi->reportResult(false);
-        return;
-    }
-
-    m_progressBase = 0;
-
-    connect(m_job, SIGNAL(finished(bool,qbs::AbstractJob*)), this, SLOT(buildingDone(bool)));
-    connect(m_job, SIGNAL(taskStarted(QString,int,qbs::AbstractJob*)),
-            this, SLOT(handleTaskStarted(QString,int)));
-    connect(m_job, SIGNAL(taskProgress(int,qbs::AbstractJob*)),
-            this, SLOT(handleProgress(int)));
-    connect(m_job, SIGNAL(reportCommandDescription(QString,QString)),
-            this, SLOT(handleCommandDescriptionReport(QString,QString)));
-    connect(m_job, SIGNAL(reportProcessResult(qbs::ProcessResult)),
-            this, SLOT(handleProcessResultReport(qbs::ProcessResult)));
 }
 
 ProjectExplorer::BuildStepConfigWidget *QbsBuildStep::createConfigWidget()
@@ -163,7 +142,9 @@ bool QbsBuildStep::runInGuiThread() const
 
 void QbsBuildStep::cancel()
 {
-    if (m_job)
+    if (m_parsingProject)
+        qbsProject()->cancelParsing();
+    else if (m_job)
         m_job->cancel();
 }
 
@@ -249,19 +230,24 @@ void QbsBuildStep::buildingDone(bool success)
 
     // The reparsing, if it is necessary, has to be done before finished() is emitted, as
     // otherwise a potential additional build step could conflict with the parsing step.
-    if (pro->parsingScheduled()) {
+    if (pro->parsingScheduled())
-        connect(pro, SIGNAL(projectParsingDone(bool)), this, SLOT(reparsingDone()));
+        parseProject();
-        pro->parseCurrentBuildConfiguration(true);
+    else
-    } else {
         finish();
-    }
 }
 
-void QbsBuildStep::reparsingDone()
+void QbsBuildStep::reparsingDone(bool success)
 {
-    disconnect(static_cast<QbsProject *>(project()), SIGNAL(projectParsingDone(bool)),
+    disconnect(qbsProject(), SIGNAL(projectParsingDone(bool)), this, SLOT(reparsingDone(bool)));
-               this, SLOT(reparsingDone()));
+    m_parsingProject = false;
+    if (m_job) { // This was a scheduled reparsing after building.
         finish();
+    } else if (!success) {
+        m_lastWasSuccess = false;
+        finish();
+    } else {
+        build();
+    }
 }
 
 void QbsBuildStep::handleTaskStarted(const QString &desciption, int max)
@@ -375,17 +361,59 @@ void QbsBuildStep::setMaxJobs(int jobcount)
     emit qbsBuildOptionsChanged();
 }
 
+void QbsBuildStep::parseProject()
+{
+    m_parsingProject = true;
+    connect(qbsProject(), SIGNAL(projectParsingDone(bool)), SLOT(reparsingDone(bool)));
+    qbsProject()->parseCurrentBuildConfiguration(true);
+}
+
+void QbsBuildStep::build()
+{
+    qbs::BuildOptions options(m_qbsBuildOptions);
+    options.setChangedFiles(m_changedFiles);
+    options.setFilesToConsider(m_changedFiles);
+    options.setActiveFileTags(m_activeFileTags);
+
+    m_job = qbsProject()->build(options, m_products);
+
+    if (!m_job) {
+        m_fi->reportResult(false);
+        return;
+    }
+
+    m_progressBase = 0;
+
+    connect(m_job, SIGNAL(finished(bool,qbs::AbstractJob*)), this, SLOT(buildingDone(bool)));
+    connect(m_job, SIGNAL(taskStarted(QString,int,qbs::AbstractJob*)),
+            this, SLOT(handleTaskStarted(QString,int)));
+    connect(m_job, SIGNAL(taskProgress(int,qbs::AbstractJob*)),
+            this, SLOT(handleProgress(int)));
+    connect(m_job, SIGNAL(reportCommandDescription(QString,QString)),
+            this, SLOT(handleCommandDescriptionReport(QString,QString)));
+    connect(m_job, SIGNAL(reportProcessResult(qbs::ProcessResult)),
+            this, SLOT(handleProcessResultReport(qbs::ProcessResult)));
+
+}
+
 void QbsBuildStep::finish()
 {
     QTC_ASSERT(m_fi, return);
     m_fi->reportResult(m_lastWasSuccess);
     m_fi = 0; // do not delete, it is not ours
+    if (m_job) {
         m_job->deleteLater();
         m_job = 0;
+    }
 
     emit finished();
 }
 
+QbsProject *QbsBuildStep::qbsProject() const
+{
+    return static_cast<QbsProject *>(project());
+}
+
 // --------------------------------------------------------------------
 // QbsBuildStepConfigWidget:
 // --------------------------------------------------------------------

src/plugins/qbsprojectmanager/qbsbuildstep.h

@@ -39,6 +39,7 @@
 
 namespace QbsProjectManager {
 namespace Internal {
+class QbsProject;
 
 class QbsBuildStepConfigWidget;
 
@@ -80,7 +81,7 @@ signals:
 
 private slots:
     void buildingDone(bool success);
-    void reparsingDone();
+    void reparsingDone(bool success);
     void handleTaskStarted(const QString &desciption, int max);
     void handleProgress(int value);
     void handleCommandDescriptionReport(const QString &highlight, const QString &message);
@@ -98,8 +99,12 @@ private:
     void setCheckTimestamps(bool ts);
     void setMaxJobs(int jobcount);
 
+    void parseProject();
+    void build();
     void finish();
 
+    QbsProject *qbsProject() const;
+
     QVariantMap m_qbsConfiguration;
     qbs::BuildOptions m_qbsBuildOptions;
 
@@ -113,6 +118,7 @@ private:
     int m_progressBase;
     bool m_lastWasSuccess;
     ProjectExplorer::IOutputParser *m_parser;
+    bool m_parsingProject;
 
     friend class QbsBuildStepConfigWidget;
 };

src/plugins/qbsprojectmanager/qbsproject.cpp

@@ -102,7 +102,7 @@ QbsProject::QbsProject(QbsManager *manager, const QString &fileName) :
     m_qbsUpdateFutureInterface(0),
     m_forceParsing(false),
     m_parsingScheduled(false),
-    m_cancelingParsing(false),
+    m_cancelStatus(CancelStatusNone),
     m_currentBc(0)
 {
     m_parsingDelay.setInterval(1000); // delay parsing by 1s.
@@ -276,9 +276,11 @@ void QbsProject::handleQbsParsingDone(bool success)
 {
     QTC_ASSERT(m_qbsProjectParser, return);
 
-    // If this parse operation was canceled, start a new one right away, ignoring the old result.
+    const CancelStatus cancelStatus = m_cancelStatus;
-    if (m_cancelingParsing) {
+    m_cancelStatus = CancelStatusNone;
-        m_cancelingParsing = false;
+
+    // Start a new one parse operation right away, ignoring the old result.
+    if (cancelStatus == CancelStatusCancelingForReparse) {
         m_qbsProjectParser->deleteLater();
         m_qbsProjectParser = 0;
         parseCurrentBuildConfiguration(m_forceParsing);
@@ -340,9 +342,10 @@ void QbsProject::startParsing()
 {
     // Qbs does update the build graph during the build. So we cannot
     // start to parse while a build is running or we will lose information.
-    // Just return since the qbsbuildstep will trigger a reparse after the build.
+    if (ProjectExplorer::BuildManager::isBuilding(this)) {
-    if (ProjectExplorer::BuildManager::isBuilding(this))
+        scheduleParsing();
         return;
+    }
 
     parseCurrentBuildConfiguration(false);
 }
@@ -381,9 +384,14 @@ void QbsProject::parseCurrentBuildConfiguration(bool force)
     m_parsingScheduled = false;
     if (!m_forceParsing)
         m_forceParsing = force;
-    if (m_cancelingParsing)
+    if (m_cancelStatus == CancelStatusCancelingForReparse)
         return;
 
+    // The CancelStatusCancelingAltoghether type can only be set by a build job, during
+    // which no other parse requests come through to this point (except by the build job itself,
+    // but of course not while canceling is in progress).
+    QTC_ASSERT(m_cancelStatus == CancelStatusNone, return);
+
     if (!activeTarget())
         return;
     QbsBuildConfiguration *bc = qobject_cast<QbsBuildConfiguration *>(activeTarget()->activeBuildConfiguration());
@@ -397,7 +405,7 @@ void QbsProject::parseCurrentBuildConfiguration(bool force)
     //       acknowledgment, it might still be doing that when the new one already reads from the
     //       same file.
     if (m_qbsProjectParser) {
-        m_cancelingParsing = true;
+        m_cancelStatus = CancelStatusCancelingForReparse;
         m_qbsProjectParser->cancel();
         return;
     }
@@ -405,6 +413,13 @@ void QbsProject::parseCurrentBuildConfiguration(bool force)
     parse(bc->qbsConfiguration(), bc->environment(), bc->buildDirectory().toString());
 }
 
+void QbsProject::cancelParsing()
+{
+    QTC_ASSERT(m_qbsProjectParser, return);
+    m_cancelStatus = CancelStatusCancelingAltoghether;
+    m_qbsProjectParser->cancel();
+}
+
 void QbsProject::updateAfterBuild()
 {
     updateBuildTargetData();

src/plugins/qbsprojectmanager/qbsproject.h

@@ -96,6 +96,7 @@ public:
     void parseCurrentBuildConfiguration(bool force);
     void scheduleParsing() { m_parsingScheduled = true; }
     bool parsingScheduled() const { return m_parsingScheduled; }
+    void cancelParsing();
     void updateAfterBuild();
 
     void registerQbsProjectParser(QbsProjectParser *p);
@@ -153,7 +154,12 @@ private:
     QFutureInterface<bool> *m_qbsUpdateFutureInterface;
     bool m_forceParsing;
     bool m_parsingScheduled;
-    bool m_cancelingParsing;
+
+    enum CancelStatus {
+        CancelStatusNone,
+        CancelStatusCancelingForReparse,
+        CancelStatusCancelingAltoghether
+    } m_cancelStatus;
 
     QFuture<void> m_codeModelFuture;