From 2b86d2481c8f17c4b5ef8dffd240868cb80ae405 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 20 Aug 2020 22:28:52 +0200 Subject: [PATCH 1/4] [libcalamares] finish() is a private implementation detail for the job queue --- src/libcalamares/JobQueue.h | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/libcalamares/JobQueue.h b/src/libcalamares/JobQueue.h index 31b5407ef..278d50d44 100644 --- a/src/libcalamares/JobQueue.h +++ b/src/libcalamares/JobQueue.h @@ -50,20 +50,25 @@ public: * of the module. */ void enqueue( int moduleWeight, const JobList& jobs ); + /** @brief Starts all the jobs that are enqueued. + * + * After this, isRunning() returns @c true until + * finished() is emitted. + */ void start(); bool isRunning() const { return !m_finished; } -public slots: - void finish(); - signals: void queueChanged( const JobList& jobs ); + void progress( qreal percent, const QString& prettyName ); void finished(); void failed( const QString& message, const QString& details ); private: + void finish(); + static JobQueue* s_instance; JobThread* m_thread; From 42417ed3b2c633c4f1dc514c4e5c77f8f74c07ef Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 20 Aug 2020 22:30:12 +0200 Subject: [PATCH 2/4] [libcalamares] Rip out the broken jobqueue information signals (break build) --- src/libcalamares/JobQueue.cpp | 1 - src/libcalamares/JobQueue.h | 2 -- 2 files changed, 3 deletions(-) diff --git a/src/libcalamares/JobQueue.cpp b/src/libcalamares/JobQueue.cpp index b39b43759..64c73434e 100644 --- a/src/libcalamares/JobQueue.cpp +++ b/src/libcalamares/JobQueue.cpp @@ -242,7 +242,6 @@ JobQueue::enqueue( int moduleWeight, const JobList& jobs ) { Q_ASSERT( !m_thread->isRunning() ); m_thread->enqueue( moduleWeight, jobs ); - emit queueChanged( jobs ); // FIXME: bogus } void diff --git a/src/libcalamares/JobQueue.h b/src/libcalamares/JobQueue.h index 278d50d44..23977b78f 100644 --- a/src/libcalamares/JobQueue.h +++ b/src/libcalamares/JobQueue.h @@ -60,8 +60,6 @@ public: bool isRunning() const { return !m_finished; } signals: - void queueChanged( const JobList& jobs ); - void progress( qreal percent, const QString& prettyName ); void finished(); void failed( const QString& message, const QString& details ); From 9afe7a37111a1f8c356817ee83546c85c4803820 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 28 Aug 2020 14:24:06 +0200 Subject: [PATCH 3/4] [libcalamares] Document JobQueue signals --- src/libcalamares/JobQueue.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/libcalamares/JobQueue.h b/src/libcalamares/JobQueue.h index 23977b78f..4faf9d420 100644 --- a/src/libcalamares/JobQueue.h +++ b/src/libcalamares/JobQueue.h @@ -60,8 +60,26 @@ public: bool isRunning() const { return !m_finished; } signals: + /** @brief Report progress of the whole queue, with a status message + * + * The @p percent is a value between 0.0 and 1.0 (100%) of the + * overall queue progress (not of the current job), while + * @p prettyName is the status message from the job -- often + * just the name of the job, but some jobs include more information. + */ void progress( qreal percent, const QString& prettyName ); + /** @brief Indicate that the queue is empty, after calling start() + * + * Emitted when the queue empties. The queue may also emit + * failed(), if something went wrong, but finished() is always + * the last one. + */ void finished(); + /** @brief A job in the queue failed. + * + * Contains the (already-translated) text from the job describing + * the failure. + */ void failed( const QString& message, const QString& details ); private: From b37a675657874c3bf20ae9f88dba0f68e8bc4172 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 28 Aug 2020 14:39:32 +0200 Subject: [PATCH 4/4] [libcalamares] Reimplement JobQueue::queueChanged - switch to QStringList as parameter, since consumers (that is, the debug dialog, which is what this is for) are interested just in the **names** of the jobs. - to allow mutex locking in const methods, mark them mutable. --- src/calamares/DebugWindow.cpp | 13 ++++--------- src/libcalamares/JobQueue.cpp | 26 ++++++++++++++++++++++---- src/libcalamares/JobQueue.h | 8 ++++++++ 3 files changed, 34 insertions(+), 13 deletions(-) diff --git a/src/calamares/DebugWindow.cpp b/src/calamares/DebugWindow.cpp index 9e2bdc501..f9d6b9614 100644 --- a/src/calamares/DebugWindow.cpp +++ b/src/calamares/DebugWindow.cpp @@ -102,14 +102,8 @@ DebugWindow::DebugWindow() // JobQueue page m_ui->jobQueueText->setReadOnly( true ); - connect( JobQueue::instance(), &JobQueue::queueChanged, this, [this]( const JobList& jobs ) { - QStringList text; - for ( const auto& job : jobs ) - { - text.append( job->prettyName() ); - } - - m_ui->jobQueueText->setText( text.join( '\n' ) ); + connect( JobQueue::instance(), &JobQueue::queueChanged, this, [this]( const QStringList& jobs ) { + m_ui->jobQueueText->setText( jobs.join( '\n' ) ); } ); // Modules page @@ -193,7 +187,8 @@ DebugWindow::DebugWindow() #endif ] { QString moduleName = m_ui->modulesListView->currentIndex().data().toString(); - Module* module = ModuleManager::instance()->moduleInstance( ModuleSystem::InstanceKey::fromString( moduleName ) ); + Module* module + = ModuleManager::instance()->moduleInstance( ModuleSystem::InstanceKey::fromString( moduleName ) ); if ( module ) { m_module = module->configurationMap(); diff --git a/src/libcalamares/JobQueue.cpp b/src/libcalamares/JobQueue.cpp index 64c73434e..f02170075 100644 --- a/src/libcalamares/JobQueue.cpp +++ b/src/libcalamares/JobQueue.cpp @@ -151,6 +151,24 @@ public: QMetaObject::invokeMethod( m_queue, "finish", Qt::QueuedConnection ); } + /** @brief The names of the queued (not running!) jobs. + */ + QStringList queuedJobs() const + { + QMutexLocker qlock( &m_enqueMutex ); + QStringList l; + l.reserve( m_queuedJobs->count() ); + for ( const auto& j : *m_queuedJobs ) + { + l << j.job->prettyName(); + } + return l; + } + +private: + /* This is called **only** from run(), while m_runMutex is + * already locked, so we can use the m_runningJobs member safely. + */ void emitProgress( qreal percentage ) const { percentage = qBound( 0.0, percentage, 1.0 ); @@ -172,10 +190,8 @@ public: m_queue, "progress", Qt::QueuedConnection, Q_ARG( qreal, progress ), Q_ARG( QString, message ) ); } - -private: - QMutex m_runMutex; - QMutex m_enqueMutex; + mutable QMutex m_runMutex; + mutable QMutex m_enqueMutex; std::unique_ptr< WeightedJobList > m_runningJobs = std::make_unique< WeightedJobList >(); std::unique_ptr< WeightedJobList > m_queuedJobs = std::make_unique< WeightedJobList >(); @@ -242,6 +258,7 @@ JobQueue::enqueue( int moduleWeight, const JobList& jobs ) { Q_ASSERT( !m_thread->isRunning() ); m_thread->enqueue( moduleWeight, jobs ); + emit queueChanged( m_thread->queuedJobs() ); } void @@ -249,6 +266,7 @@ JobQueue::finish() { m_finished = true; emit finished(); + emit queueChanged( m_thread->queuedJobs() ); } GlobalStorage* diff --git a/src/libcalamares/JobQueue.h b/src/libcalamares/JobQueue.h index 4faf9d420..b36d89f26 100644 --- a/src/libcalamares/JobQueue.h +++ b/src/libcalamares/JobQueue.h @@ -82,6 +82,14 @@ signals: */ void failed( const QString& message, const QString& details ); + /** @brief Reports the names of jobs in the queue. + * + * When jobs are added via enqueue(), or when the queue otherwise + * changes, the **names** of the jobs are reported. This is + * primarily for debugging purposes. + */ + void queueChanged( const QStringList& jobNames ); + private: void finish();