Merge branch 'issue-1330'

- When Python modules emit progress, update their status message
  by calling an optional pretty_status_message() in the Python code.
  This is polled (later) by the execution progress bar to display
  the message.

FIXES #1330

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;
 

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();
-        }
+        Py_Initialize();
+    }
 
-        m_mainModule = bp::import( "__main__" );
-        m_mainNamespace = m_mainModule.attr( "__dict__" );
+    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 );
+    // 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 );
+    QDir calaPythonPath( CalamaresUtils::systemLibDir().absolutePath() + QDir::separator() + "calamares" );
+    add_if_lib_exists( calaPythonPath, "libcalamares.so", m_pythonPaths );
 
-        bp::object sys = bp::import( "sys" );
+    bp::object sys = bp::import( "sys" );
 
-        foreach ( QString path, m_pythonPaths )
-        {
-            bp::str dir = path.toLocal8Bit().data();
-            sys.attr( "path" ).attr( "append" )( dir );
-        }
-    }
-    else
+    foreach ( QString path, m_pythonPaths )
     {
-        cWarning() << "creating PythonHelper more than once. This is very bad.";
-        return;
+        bp::str dir = path.toLocal8Bit().data();
+        sys.attr( "path" ).attr( "append" )( dir );
     }
-
-    s_instance = this;
 }
 
-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()

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;

src/libcalamares/PythonJob.cpp

@@ -165,6 +165,10 @@ BOOST_PYTHON_MODULE( libcalamares )
 namespace Calamares
 {
 
+struct PythonJob::Private
+{
+    bp::object m_prettyStatusMessage;
+};
 
 PythonJob::PythonJob( const ModuleSystem::InstanceKey& instance,
                       const QString& scriptFile,
@@ -172,6 +176,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()
@@ -199,6 +204,7 @@ PythonJob::prettyName() const
 QString
 PythonJob::prettyStatusMessage() const
 {
+    // The description is updated when progress is reported, see emitProgress()
     if ( m_description.isEmpty() )
     {
         return tr( "Running %1 operation." ).arg( QDir( m_workingPath ).dirName() );
@@ -209,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()
@@ -233,7 +251,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__" ) );
@@ -242,27 +260,13 @@ 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_d->m_prettyStatusMessage = scriptNamespace.get( "pretty_status_message", bp::object() );
+        m_description = pythonStringMethod( scriptNamespace, "pretty_name" );
         if ( m_description.isEmpty() )
         {
             bp::extract< std::string > entryPoint_doc_attr( entryPoint.attr( "__doc__" ) );
@@ -275,10 +279,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();
 
@@ -299,7 +307,7 @@ PythonJob::exec()
         QString msg;
         if ( PyErr_Occurred() )
         {
-            msg = helper()->handleLastError();
+            msg = CalamaresPython::Helper::instance()->handleLastError();
         }
         bp::handle_exception();
         PyErr_Clear();
@@ -312,20 +320,21 @@ PythonJob::exec()
 void
 PythonJob::emitProgress( qreal progressValue )
 {
-    emit progress( progressValue );
-}
-
-
-CalamaresPython::Helper*
-PythonJob::helper()
-{
-    auto ptr = CalamaresPython::Helper::s_instance;
-    if ( !ptr )
+    // 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() )
     {
-        ptr = new CalamaresPython::Helper;
+        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;
+        }
     }
-    return ptr;
+    emit progress( progressValue );
 }
 
-
 }  // namespace Calamares

src/libcalamares/PythonJob.h

@@ -53,11 +53,12 @@ public:
     virtual qreal getJobWeight() const override;
 
 private:
-    friend class CalamaresPython::Helper;
+    struct Private;
+
     friend class CalamaresPython::PythonJobInterface;
     void emitProgress( double progressValue );
 
-    CalamaresPython::Helper* helper();
+    std::unique_ptr< Private > m_d;
     QString m_scriptFile;
     QString m_workingPath;
     QString m_description;

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 );
     }

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))

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
@@ -332,19 +336,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 +510,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)