From e63945593621e87c8afd64624f29e5a04ff6e1b6 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Mon, 14 Jul 2014 14:22:27 +0200 Subject: [PATCH] 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 --- .../qbsprojectmanager/qbsbuildstep.cpp | 104 +++++++++++------- src/plugins/qbsprojectmanager/qbsbuildstep.h | 8 +- src/plugins/qbsprojectmanager/qbsproject.cpp | 31 ++++-- src/plugins/qbsprojectmanager/qbsproject.h | 8 +- 4 files changed, 103 insertions(+), 48 deletions(-) diff --git a/src/plugins/qbsprojectmanager/qbsbuildstep.cpp b/src/plugins/qbsprojectmanager/qbsbuildstep.cpp index 24c03a94ec2..9ee071ace64 100644 --- a/src/plugins/qbsprojectmanager/qbsbuildstep.cpp +++ b/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 &fi) { m_fi = &fi; - QbsProject *pro = static_cast(project()); - qbs::BuildOptions options(m_qbsBuildOptions); - options.setChangedFiles(m_changedFiles); - 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))); + // We need a pre-build parsing step in order not to lose project file changes done + // right before building (but before the delay has elapsed). + parseProject(); } 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()) { - connect(pro, SIGNAL(projectParsingDone(bool)), this, SLOT(reparsingDone())); - pro->parseCurrentBuildConfiguration(true); - } else { + if (pro->parsingScheduled()) + parseProject(); + else finish(); - } } -void QbsBuildStep::reparsingDone() +void QbsBuildStep::reparsingDone(bool success) { - disconnect(static_cast(project()), SIGNAL(projectParsingDone(bool)), - this, SLOT(reparsingDone())); - finish(); + disconnect(qbsProject(), SIGNAL(projectParsingDone(bool)), this, SLOT(reparsingDone(bool))); + 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 - m_job->deleteLater(); - m_job = 0; + if (m_job) { + m_job->deleteLater(); + m_job = 0; + } emit finished(); } +QbsProject *QbsBuildStep::qbsProject() const +{ + return static_cast(project()); +} + // -------------------------------------------------------------------- // QbsBuildStepConfigWidget: // -------------------------------------------------------------------- diff --git a/src/plugins/qbsprojectmanager/qbsbuildstep.h b/src/plugins/qbsprojectmanager/qbsbuildstep.h index dded12290a0..69511437014 100644 --- a/src/plugins/qbsprojectmanager/qbsbuildstep.h +++ b/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; }; diff --git a/src/plugins/qbsprojectmanager/qbsproject.cpp b/src/plugins/qbsprojectmanager/qbsproject.cpp index deb49bdfc7d..682c65395bd 100644 --- a/src/plugins/qbsprojectmanager/qbsproject.cpp +++ b/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. - if (m_cancelingParsing) { - m_cancelingParsing = false; + const CancelStatus cancelStatus = m_cancelStatus; + m_cancelStatus = CancelStatusNone; + + // 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(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(); diff --git a/src/plugins/qbsprojectmanager/qbsproject.h b/src/plugins/qbsprojectmanager/qbsproject.h index ab757cc82f4..ab86c24074a 100644 --- a/src/plugins/qbsprojectmanager/qbsproject.h +++ b/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 *m_qbsUpdateFutureInterface; bool m_forceParsing; bool m_parsingScheduled; - bool m_cancelingParsing; + + enum CancelStatus { + CancelStatusNone, + CancelStatusCancelingForReparse, + CancelStatusCancelingAltoghether + } m_cancelStatus; QFuture m_codeModelFuture;