From d3828a82fce4d779f9ea259390efa7982d028cbe Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 4 Mar 2020 21:31:47 -0500 Subject: [PATCH 01/10] [packages] Make dummy backend slower - insert sleeps to make it slower (easier when testing) - improve debug logging clarity by noting where the messages are coming from --- src/modules/packages/main.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/modules/packages/main.py b/src/modules/packages/main.py index adca88423..8cbde9de6 100644 --- a/src/modules/packages/main.py +++ b/src/modules/packages/main.py @@ -332,19 +332,23 @@ class PMDummy(PackageManager): backend = "dummy" def install(self, pkgs, from_local=False): - libcalamares.utils.debug("Installing " + str(pkgs)) + from time import sleep + libcalamares.utils.debug("Dummy backend: Installing " + str(pkgs)) + sleep(3) def remove(self, pkgs): - libcalamares.utils.debug("Removing " + str(pkgs)) + from time import sleep + libcalamares.utils.debug("Dummy backend: Removing " + str(pkgs)) + sleep(3) def update_db(self): - libcalamares.utils.debug("Updating DB") + libcalamares.utils.debug("Dummy backend: Updating DB") def update_system(self): - libcalamares.utils.debug("Updating System") + libcalamares.utils.debug("Dummy backend: Updating System") def run(self, script): - libcalamares.utils.debug("Running script '" + str(script) + "'") + libcalamares.utils.debug("Dummy backend: Running script '" + str(script) + "'") class PMPisi(PackageManager): @@ -502,7 +506,7 @@ def run_operations(pkgman, entry): libcalamares.utils.warning("Unknown package-operation key {!s}".format(key)) completed_packages += len(package_list) libcalamares.job.setprogress(completed_packages * 1.0 / total_packages) - libcalamares.utils.debug(pretty_name()) + libcalamares.utils.debug("Pretty name: {!s}, setting progress..".format(pretty_name())) group_packages = 0 _change_mode(None) From 6d29c19e3eaa7db758b3e493a509708492e9c934 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 4 Mar 2020 21:40:40 -0500 Subject: [PATCH 02/10] [libcalamares] Progress is float --- src/libcalamares/PythonJobApi.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcalamares/PythonJobApi.cpp b/src/libcalamares/PythonJobApi.cpp index caa1cb1d2..cf7984c87 100644 --- a/src/libcalamares/PythonJobApi.cpp +++ b/src/libcalamares/PythonJobApi.cpp @@ -171,7 +171,7 @@ PythonJobInterface::PythonJobInterface( Calamares::PythonJob* parent ) void PythonJobInterface::setprogress( qreal progress ) { - if ( progress >= 0 && progress <= 1 ) + if ( progress >= 0.0 && progress <= 1.0 ) { m_parent->emitProgress( progress ); } From 3025c5383b519e796085a435f68b1c832edd7ebc Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 5 Mar 2020 08:54:42 -0500 Subject: [PATCH 03/10] [libcalamares] Document the pretty*() functions for Jobs --- src/libcalamares/Job.h | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/libcalamares/Job.h b/src/libcalamares/Job.h index 862a5f3f5..d93e97cf7 100644 --- a/src/libcalamares/Job.h +++ b/src/libcalamares/Job.h @@ -97,7 +97,7 @@ public: virtual ~Job(); /** @brief The job's (relative) weight. - * + * * The default implementation returns 1.0, which gives all jobs * the same weight, so they advance the overall progress the same * amount. This is nonsense, since some jobs take much longer than @@ -105,8 +105,20 @@ public: * how much work is (relatively) done. */ virtual qreal getJobWeight() const; + /** @brief The human-readable name of this job + * + * This should be a very short statement of what the job does. + * For status and state information, see prettyStatusMessage(). + */ virtual QString prettyName() const = 0; + // TODO: Unused virtual QString prettyDescription() const; + /** @brief A human-readable status for progress reporting + * + * This is called from the JobQueue when progress is made, and should + * return a not-too-long description of the job's status. This + * is made visible in the progress bar of the execution view step. + */ virtual QString prettyStatusMessage() const; virtual JobResult exec() = 0; From 9b5a391c868c3d96c8d1e4552561e64bd52e98ed Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 9 Mar 2020 10:05:01 -0500 Subject: [PATCH 04/10] [libcalamares] Factor out Python helper - the strange construction of Helper and treating it as a singleton can be factored out into a separate singleton-handling instance() function. The Helper should never be destroyed. --- src/libcalamares/PythonHelper.cpp | 66 +++++++++++++++---------------- src/libcalamares/PythonHelper.h | 9 ++--- src/libcalamares/PythonJob.cpp | 17 +------- src/libcalamares/PythonJob.h | 1 - 4 files changed, 37 insertions(+), 56 deletions(-) diff --git a/src/libcalamares/PythonHelper.cpp b/src/libcalamares/PythonHelper.cpp index d9db8581e..0b5d77ac1 100644 --- a/src/libcalamares/PythonHelper.cpp +++ b/src/libcalamares/PythonHelper.cpp @@ -204,8 +204,6 @@ variantHashFromPyDict( const boost::python::dict& pyDict ) } -Helper* Helper::s_instance = nullptr; - static inline void add_if_lib_exists( const QDir& dir, const char* name, QStringList& list ) { @@ -221,48 +219,46 @@ add_if_lib_exists( const QDir& dir, const char* name, QStringList& list ) } } -Helper::Helper( QObject* parent ) - : QObject( parent ) +Helper::Helper() + : QObject( nullptr ) { // Let's make extra sure we only call Py_Initialize once - if ( !s_instance ) + if ( !Py_IsInitialized() ) { - if ( !Py_IsInitialized() ) - { - Py_Initialize(); - } - - m_mainModule = bp::import( "__main__" ); - m_mainNamespace = m_mainModule.attr( "__dict__" ); - - // If we're running from the build dir - add_if_lib_exists( QDir::current(), "libcalamares.so", m_pythonPaths ); - - QDir calaPythonPath( CalamaresUtils::systemLibDir().absolutePath() + QDir::separator() + "calamares" ); - add_if_lib_exists( calaPythonPath, "libcalamares.so", m_pythonPaths ); - - bp::object sys = bp::import( "sys" ); - - foreach ( QString path, m_pythonPaths ) - { - bp::str dir = path.toLocal8Bit().data(); - sys.attr( "path" ).attr( "append" )( dir ); - } - } - else - { - cWarning() << "creating PythonHelper more than once. This is very bad."; - return; + Py_Initialize(); } - s_instance = this; + m_mainModule = bp::import( "__main__" ); + m_mainNamespace = m_mainModule.attr( "__dict__" ); + + // If we're running from the build dir + add_if_lib_exists( QDir::current(), "libcalamares.so", m_pythonPaths ); + + QDir calaPythonPath( CalamaresUtils::systemLibDir().absolutePath() + QDir::separator() + "calamares" ); + add_if_lib_exists( calaPythonPath, "libcalamares.so", m_pythonPaths ); + + bp::object sys = bp::import( "sys" ); + + foreach ( QString path, m_pythonPaths ) + { + bp::str dir = path.toLocal8Bit().data(); + sys.attr( "path" ).attr( "append" )( dir ); + } } -Helper::~Helper() +Helper::~Helper() {} + +Helper* +Helper::instance() { - s_instance = nullptr; -} + static Helper* s_helper = nullptr; + if ( !s_helper ) + { + s_helper = new Helper; + } + return s_helper; +} boost::python::dict Helper::createCleanNamespace() diff --git a/src/libcalamares/PythonHelper.h b/src/libcalamares/PythonHelper.h index 418c75e5f..7528732a0 100644 --- a/src/libcalamares/PythonHelper.h +++ b/src/libcalamares/PythonHelper.h @@ -50,16 +50,15 @@ class Helper : public QObject { Q_OBJECT public: - virtual ~Helper(); - boost::python::dict createCleanNamespace(); QString handleLastError(); + static Helper* instance(); + private: - friend Helper* Calamares::PythonJob::helper(); - explicit Helper( QObject* parent = nullptr ); - static Helper* s_instance; + virtual ~Helper(); + explicit Helper(); boost::python::object m_mainModule; boost::python::object m_mainNamespace; diff --git a/src/libcalamares/PythonJob.cpp b/src/libcalamares/PythonJob.cpp index d94a20981..11a963df0 100644 --- a/src/libcalamares/PythonJob.cpp +++ b/src/libcalamares/PythonJob.cpp @@ -233,7 +233,7 @@ PythonJob::exec() try { - bp::dict scriptNamespace = helper()->createCleanNamespace(); + bp::dict scriptNamespace = CalamaresPython::Helper::instance()->createCleanNamespace(); bp::object calamaresModule = bp::import( "libcalamares" ); bp::dict calamaresNamespace = bp::extract< bp::dict >( calamaresModule.attr( "__dict__" ) ); @@ -299,7 +299,7 @@ PythonJob::exec() QString msg; if ( PyErr_Occurred() ) { - msg = helper()->handleLastError(); + msg = CalamaresPython::Helper::instance()->handleLastError(); } bp::handle_exception(); PyErr_Clear(); @@ -315,17 +315,4 @@ PythonJob::emitProgress( qreal progressValue ) emit progress( progressValue ); } - -CalamaresPython::Helper* -PythonJob::helper() -{ - auto ptr = CalamaresPython::Helper::s_instance; - if ( !ptr ) - { - ptr = new CalamaresPython::Helper; - } - return ptr; -} - - } // namespace Calamares diff --git a/src/libcalamares/PythonJob.h b/src/libcalamares/PythonJob.h index 7cd1b7165..ba2b0a75f 100644 --- a/src/libcalamares/PythonJob.h +++ b/src/libcalamares/PythonJob.h @@ -57,7 +57,6 @@ private: friend class CalamaresPython::PythonJobInterface; void emitProgress( double progressValue ); - CalamaresPython::Helper* helper(); QString m_scriptFile; QString m_workingPath; QString m_description; From aa62ca639b071755789adce99677af5ca31947f8 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 9 Mar 2020 10:32:41 -0500 Subject: [PATCH 05/10] [libcalamares] Start getting prettyDescription from Python --- src/libcalamares/PythonJob.cpp | 6 ++++++ src/libcalamares/PythonJob.h | 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/libcalamares/PythonJob.cpp b/src/libcalamares/PythonJob.cpp index 11a963df0..dd0ea47c1 100644 --- a/src/libcalamares/PythonJob.cpp +++ b/src/libcalamares/PythonJob.cpp @@ -165,6 +165,11 @@ BOOST_PYTHON_MODULE( libcalamares ) namespace Calamares { +struct PythonJob::Private +{ + bp::object m_prettyStatusMessage; + bp::object m_prettyName; +}; PythonJob::PythonJob( const ModuleSystem::InstanceKey& instance, const QString& scriptFile, @@ -172,6 +177,7 @@ PythonJob::PythonJob( const ModuleSystem::InstanceKey& instance, const QVariantMap& moduleConfiguration, QObject* parent ) : Job( parent ) + , m_d( std::make_unique< Private >() ) , m_scriptFile( scriptFile ) , m_workingPath( workingPath ) , m_description() diff --git a/src/libcalamares/PythonJob.h b/src/libcalamares/PythonJob.h index ba2b0a75f..6f8c50a9f 100644 --- a/src/libcalamares/PythonJob.h +++ b/src/libcalamares/PythonJob.h @@ -53,10 +53,12 @@ public: virtual qreal getJobWeight() const override; private: - friend class CalamaresPython::Helper; + struct Private; + friend class CalamaresPython::PythonJobInterface; void emitProgress( double progressValue ); + std::unique_ptr< Private > m_d; QString m_scriptFile; QString m_workingPath; QString m_description; From ed4cdbeacc980b499b7ede9fe4022022e506638b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 9 Mar 2020 14:37:02 -0500 Subject: [PATCH 06/10] [dummypython] Provide status --- src/modules/dummypython/main.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/modules/dummypython/main.py b/src/modules/dummypython/main.py index d2730483d..2da9b4760 100644 --- a/src/modules/dummypython/main.py +++ b/src/modules/dummypython/main.py @@ -43,6 +43,10 @@ _ = gettext.translation("calamares-python", def pretty_name(): return _("Dummy python job.") +status = _("Dummy python step {}").format(0) + +def pretty_status_message(): + return status def run(): """Dummy python job.""" @@ -92,8 +96,10 @@ def run(): except KeyError: configlist = ["no list"] + global status c = 1 for k in configlist: + status = _("Dummy python step {}").format(str(c) + ":" + repr(k)) libcalamares.utils.debug(_("Dummy python step {}").format(str(k))) sleep(1) libcalamares.job.setprogress(c * 1.0 / len(configlist)) From 252089e372a2200fe9f27fb17b15058a649bb207 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 9 Mar 2020 15:01:07 -0500 Subject: [PATCH 07/10] [libcalamares] Refactor pretty_name() call - Split out a general method-that-returns-string caller. --- src/libcalamares/PythonJob.cpp | 39 +++++++++++++++++----------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/libcalamares/PythonJob.cpp b/src/libcalamares/PythonJob.cpp index dd0ea47c1..37859b349 100644 --- a/src/libcalamares/PythonJob.cpp +++ b/src/libcalamares/PythonJob.cpp @@ -215,6 +215,18 @@ PythonJob::prettyStatusMessage() const } } +static QString +pythonStringMethod( bp::dict& script, const char* funcName ) +{ + bp::object func = script.get( funcName, bp::object() ); + if ( !func.is_none() ) + { + bp::extract< std::string > result( func() ); + return result.check() ? QString::fromStdString( result() ).trimmed() : QString(); + } + return QString(); +} + JobResult PythonJob::exec() @@ -248,27 +260,12 @@ PythonJob::exec() calamaresNamespace[ "globalstorage" ] = CalamaresPython::GlobalStoragePythonWrapper( JobQueue::instance()->globalStorage() ); + cDebug() << "Job file" << scriptFI.absoluteFilePath(); bp::object execResult = bp::exec_file( scriptFI.absoluteFilePath().toLocal8Bit().data(), scriptNamespace, scriptNamespace ); - bp::object entryPoint = scriptNamespace[ "run" ]; - bp::object prettyNameFunc = scriptNamespace.get( "pretty_name", bp::object() ); - - cDebug() << "Job file" << scriptFI.absoluteFilePath(); - if ( !prettyNameFunc.is_none() ) - { - bp::extract< std::string > prettyNameResult( prettyNameFunc() ); - if ( prettyNameResult.check() ) - { - m_description = QString::fromStdString( prettyNameResult() ).trimmed(); - } - if ( !m_description.isEmpty() ) - { - cDebug() << "Job description from pretty_name" << prettyName() << "=" << m_description; - emit progress( 0 ); - } - } + m_description = pythonStringMethod( scriptNamespace, "pretty_name" ); if ( m_description.isEmpty() ) { bp::extract< std::string > entryPoint_doc_attr( entryPoint.attr( "__doc__" ) ); @@ -281,10 +278,14 @@ PythonJob::exec() { m_description.truncate( i_newline ); } - cDebug() << "Job description from __doc__" << prettyName() << "=" << m_description; - emit progress( 0 ); + cDebug() << "Job description from __doc__" << prettyName() << '=' << m_description; } } + else + { + cDebug() << "Job description from pretty_name" << prettyName() << '=' << m_description; + } + emit progress( 0 ); bp::object runResult = entryPoint(); From b4aaf85ccf2244de991318b19d0fb14af91e2ffe Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 9 Mar 2020 15:13:40 -0500 Subject: [PATCH 08/10] [libcalamares] Call Python function if available for status --- src/libcalamares/PythonJob.cpp | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/libcalamares/PythonJob.cpp b/src/libcalamares/PythonJob.cpp index 37859b349..8165a4c9c 100644 --- a/src/libcalamares/PythonJob.cpp +++ b/src/libcalamares/PythonJob.cpp @@ -168,7 +168,6 @@ namespace Calamares struct PythonJob::Private { bp::object m_prettyStatusMessage; - bp::object m_prettyName; }; PythonJob::PythonJob( const ModuleSystem::InstanceKey& instance, @@ -205,6 +204,21 @@ PythonJob::prettyName() const QString PythonJob::prettyStatusMessage() const { + if ( m_d && !m_d->m_prettyStatusMessage.is_none() ) + { + cDebug() << "Getting dynamic message"; + QString r; + bp::extract< std::string > result( m_d->m_prettyStatusMessage() ); + r = result.check() ? QString::fromStdString( result() ).trimmed() : QString(); + if ( !r.isEmpty() ) + { + return r; + } + } + else + { + cDebug() << "Getting static message"; + } if ( m_description.isEmpty() ) { return tr( "Running %1 operation." ).arg( QDir( m_workingPath ).dirName() ); From ef249043f986eff12509b96e4b53428037fbe342 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 9 Mar 2020 15:39:35 -0500 Subject: [PATCH 09/10] [libcalamares] call Python method only from Python thread --- src/libcalamares/PythonJob.cpp | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/libcalamares/PythonJob.cpp b/src/libcalamares/PythonJob.cpp index 8165a4c9c..9069c49dc 100644 --- a/src/libcalamares/PythonJob.cpp +++ b/src/libcalamares/PythonJob.cpp @@ -204,21 +204,7 @@ PythonJob::prettyName() const QString PythonJob::prettyStatusMessage() const { - if ( m_d && !m_d->m_prettyStatusMessage.is_none() ) - { - cDebug() << "Getting dynamic message"; - QString r; - bp::extract< std::string > result( m_d->m_prettyStatusMessage() ); - r = result.check() ? QString::fromStdString( result() ).trimmed() : QString(); - if ( !r.isEmpty() ) - { - return r; - } - } - else - { - cDebug() << "Getting static message"; - } + // The description is updated when progress is reported, see emitProgress() if ( m_description.isEmpty() ) { return tr( "Running %1 operation." ).arg( QDir( m_workingPath ).dirName() ); @@ -279,6 +265,7 @@ PythonJob::exec() = bp::exec_file( scriptFI.absoluteFilePath().toLocal8Bit().data(), scriptNamespace, scriptNamespace ); bp::object entryPoint = scriptNamespace[ "run" ]; + m_d->m_prettyStatusMessage = scriptNamespace.get( "pretty_status_message", bp::object() ); m_description = pythonStringMethod( scriptNamespace, "pretty_name" ); if ( m_description.isEmpty() ) { @@ -333,6 +320,20 @@ PythonJob::exec() void PythonJob::emitProgress( qreal progressValue ) { + // This is called from the JobApi (and only from there) from the Job thread, + // so it is safe to call into the Python interpreter. Update the description + // as needed (don't call this from prettyStatusMessage(), which can be + // called from other threads as well). + if ( m_d && !m_d->m_prettyStatusMessage.is_none() ) + { + QString r; + bp::extract< std::string > result( m_d->m_prettyStatusMessage() ); + r = result.check() ? QString::fromStdString( result() ).trimmed() : QString(); + if ( !r.isEmpty() ) + { + m_description = r; + } + } emit progress( progressValue ); } From c4bfad93990e9426e911b7b5ee4458aeed61dee5 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 9 Mar 2020 15:40:45 -0500 Subject: [PATCH 10/10] [packages] Provide status feedback - The status message should be updated; the name is constant. FIXES #1330 --- src/modules/packages/main.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/modules/packages/main.py b/src/modules/packages/main.py index 8cbde9de6..8f58cd6fb 100644 --- a/src/modules/packages/main.py +++ b/src/modules/packages/main.py @@ -56,6 +56,10 @@ def _change_mode(mode): def pretty_name(): + return _("Install packages.") + + +def pretty_status_message(): if not group_packages: if (total_packages > 0): # Outside the context of an operation