Merge branch 'work/adridg/pybind-fix' into calamares

FIXES #2223

src/libcalamares/python/Api.cpp

@@ -9,12 +9,12 @@
  */
 #include "python/Api.h"
 
-#include "CalamaresVersion.h"
 #include "GlobalStorage.h"
 #include "JobQueue.h"
 #include "compat/Variant.h"
 #include "locale/Global.h"
 #include "partition/Mount.h"
+#include "python/Pybind11Helpers.h"
 #include "python/PythonJob.h"
 #include "utils/Logger.h"
 #include "utils/RAII.h"
@@ -27,10 +27,6 @@
 #include <QDir>
 #include <QStandardPaths>
 
-#undef slots
-#include <pybind11/embed.h>
-#include <pybind11/pybind11.h>
-
 namespace py = pybind11;
 
 /** @namespace
@@ -177,9 +173,9 @@ QVariantList
 variantListFromPyList( const Calamares::Python::List& list )
 {
     QVariantList l;
-    for ( const auto& h : list )
+    for ( const auto item : list )
     {
-        l.append( variantFromPyObject( h ) );
+        l.append( variantFromPyObject( item ) );
     }
     return l;
 }
@@ -188,7 +184,7 @@ QVariantMap
 variantMapFromPyDict( const Calamares::Python::Dictionary& dict )
 {
     QVariantMap m;
-    for ( const auto& item : dict )
+    for ( const auto item : dict )
     {
         m.insert( Calamares::Python::asQString( item.first ), variantFromPyObject( ( item.second ) ) );
     }
@@ -199,9 +195,9 @@ QStringList
 stringListFromPyList( const Calamares::Python::List& list )
 {
     QStringList l;
-    for ( const auto& h : list )
+    for ( const auto item : list )
     {
-        l.append( Calamares::Python::asQString( h ) );
+        l.append( Calamares::Python::asQString( item ) );
     }
     return l;
 }
@@ -577,96 +573,3 @@ GlobalStorageProxy::value( const std::string& key ) const
 
 }  // namespace Python
 }  // namespace Calamares
-
-// Not using EMBEDDED_MODULE because that does not let
-// use use name "libcalamares.utils" and using just "utils"
-// causes command-line use of Python3 followed by `import libcalamares`
-// to crash with an error about adding modules after the interpreter
-// has been initialized.
-static void
-populate_utils( py::module_&& m )
-{
-    m.def( "obscure", &Calamares::Python::obscure, "A function that obscures (encodes) a string" );
-
-    m.def( "debug", &Calamares::Python::debug, "Log a debug-message" );
-    m.def( "warn", &Calamares::Python::warning, "Log a warning-message" );
-    m.def( "warning", &Calamares::Python::warning, "Log a warning-message" );
-    m.def( "error", &Calamares::Python::error, "Log an error-message" );
-
-    m.def( "load_yaml", &Calamares::Python::load_yaml, "Loads YAML from a file." );
-
-    m.def( "target_env_call",
-           &Calamares::Python::target_env_call,
-           "Runs command in target, returns exit code.",
-           py::arg( "command_list" ),
-           py::arg( "input" ) = std::string(),
-           py::arg( "timeout" ) = 0 );
-    m.def( "check_target_env_call",
-           &Calamares::Python::check_target_env_call,
-           "Runs command in target, raises on error exit.",
-           py::arg( "command_list" ),
-           py::arg( "input" ) = std::string(),
-           py::arg( "timeout" ) = 0 );
-    m.def( "check_target_env_output",
-           &Calamares::Python::check_target_env_output,
-           "Runs command in target, returns standard output or raises on error.",
-           py::arg( "command_list" ),
-           py::arg( "input" ) = std::string(),
-           py::arg( "timeout" ) = 0 );
-    m.def( "target_env_process_output",
-           &Calamares::Python::target_env_process_output,
-           "Runs command in target, updating callback and returns standard output or raises on error.",
-           py::arg( "command_list" ),
-           py::arg( "callback" ) = pybind11::none(),
-           py::arg( "input" ) = std::string(),
-           py::arg( "timeout" ) = 0 );
-    m.def( "host_env_process_output",
-           &Calamares::Python::host_env_process_output,
-           "Runs command in target, updating callback and returns standard output or raises on error.",
-           py::arg( "command_list" ),
-           py::arg( "callback" ) = pybind11::none(),
-           py::arg( "input" ) = std::string(),
-           py::arg( "timeout" ) = 0 );
-
-    m.def( "gettext_languages",
-           &Calamares::Python::gettext_languages,
-           "Returns list of languages (most to least-specific) for gettext." );
-    m.def( "gettext_path", &Calamares::Python::gettext_path, "Returns path for gettext search." );
-
-    m.def( "mount",
-           &Calamares::Python::mount,
-           "Runs the mount utility with the specified parameters.\n"
-           "Returns the program's exit code, or:\n"
-           "-1 = QProcess crash\n"
-           "-2 = QProcess cannot start\n"
-           "-3 = bad arguments" );
-}
-
-PYBIND11_MODULE( libcalamares, m )
-{
-    m.doc() = "Calamares API for Python";
-
-    m.add_object( "ORGANIZATION_NAME", Calamares::Python::String( CALAMARES_ORGANIZATION_NAME ) );
-    m.add_object( "ORGANIZATION_DOMAIN", Calamares::Python::String( CALAMARES_ORGANIZATION_DOMAIN ) );
-    m.add_object( "APPLICATION_NAME", Calamares::Python::String( CALAMARES_APPLICATION_NAME ) );
-    m.add_object( "VERSION", Calamares::Python::String( CALAMARES_VERSION ) );
-    m.add_object( "VERSION_SHORT", Calamares::Python::String( CALAMARES_VERSION_SHORT ) );
-
-    populate_utils( m.def_submodule( "utils", "Calamares Utility API for Python" ) );
-
-    py::class_< Calamares::Python::JobProxy >( m, "Job" )
-        .def_readonly( "module_name", &Calamares::Python::JobProxy::moduleName )
-        .def_readonly( "pretty_name", &Calamares::Python::JobProxy::prettyName )
-        .def_readonly( "working_path", &Calamares::Python::JobProxy::workingPath )
-        .def_readonly( "configuration", &Calamares::Python::JobProxy::configuration )
-        .def( "setprogress", &Calamares::Python::JobProxy::setprogress );
-
-    py::class_< Calamares::Python::GlobalStorageProxy >( m, "GlobalStorage" )
-        .def( py::init( []( std::nullptr_t p ) { return new Calamares::Python::GlobalStorageProxy( nullptr ); } ) )
-        .def( "contains", &Calamares::Python::GlobalStorageProxy::contains )
-        .def( "count", &Calamares::Python::GlobalStorageProxy::count )
-        .def( "insert", &Calamares::Python::GlobalStorageProxy::insert )
-        .def( "keys", &Calamares::Python::GlobalStorageProxy::keys )
-        .def( "remove", &Calamares::Python::GlobalStorageProxy::remove )
-        .def( "value", &Calamares::Python::GlobalStorageProxy::value );
-}

src/libcalamares/python/Api.h

@@ -33,7 +33,7 @@ namespace Python __attribute__( ( visibility( "hidden" ) ) )
 
     void debug( const std::string& s );
     void warning( const std::string& s );
-    // void warn( const std::string& s) is an alias of warning()
+    // void warn( const std::string& s) is an alias of warning() defined at the Python level
     void error( const std::string& s );
 
     Dictionary load_yaml( const std::string& path );
@@ -48,12 +48,17 @@ namespace Python __attribute__( ( visibility( "hidden" ) ) )
     int target_env_process_output( const List& args, const Object& callback, const std::string& input, int timeout );
     int host_env_process_output( const List& args, const Object& callback, const std::string& input, int timeout );
 
+    int mount( const std::string& device_path,
+               const std::string& mount_point,
+               const std::string& filesystem_name,
+               const std::string& options );
 
     class Job;
+
     /** @brief Proxy class in Python for the Calamares Job class
-*
+     *
-* This is available as libcalamares.job in Python code.
+     * This is available as libcalamares.job in Python code.
-*/
+     */
     class JobProxy
     {
     public:

src/libcalamares/python/Logger.h

@@ -18,8 +18,7 @@
 
 #include "utils/Logger.h"
 
-#undef slots
+#include "python/Pybind11Helpers.h"
-#include <pybind11/pybind11.h>
 
 #include <string>
 

src/libcalamares/python/Pybind11Helpers.h

@@ -14,9 +14,27 @@
 
 #include <string>
 
+#ifdef __clang__
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wcovered-switch-default"
+#pragma clang diagnostic ignored "-Wfloat-equal"
+#pragma clang diagnostic ignored "-Wweak-vtables"
+#pragma clang diagnostic ignored "-Wmissing-variable-declarations"
+#pragma clang diagnostic ignored "-Wold-style-cast"
+#pragma clang diagnostic ignored "-Wshadow-uncaptured-local"
+#pragma clang diagnostic ignored "-Wshadow-field-in-constructor"
+#pragma clang diagnostic ignored "-Wshadow-field"
+#pragma clang diagnostic ignored "-Wdocumentation"
+#pragma clang diagnostic ignored "-Wmissing-noreturn"
+#pragma clang diagnostic ignored "-Wreserved-identifier"
+#endif
+
 #undef slots
 #include <pybind11/pybind11.h>
 
+#include <pybind11/embed.h>
+#include <pybind11/eval.h>
+
 namespace Calamares
 {
 namespace Python __attribute__( ( visibility( "hidden" ) ) )
@@ -35,4 +53,9 @@ namespace Python __attribute__( ( visibility( "hidden" ) ) )
 
 }  // namespace Python
 }  // namespace Calamares
+
+#ifdef __clang__
+#pragma clang diagnostic pop
+#endif
+
 #endif

src/libcalamares/python/PythonJob.cpp

@@ -8,20 +8,18 @@
  */
 #include "python/PythonJob.h"
 
+#include "CalamaresVersionX.h"
 #include "GlobalStorage.h"
 #include "JobQueue.h"
 #include "python/Api.h"
 #include "python/Logger.h"
+#include "python/Pybind11Helpers.h"
 #include "utils/Logger.h"
 
 #include <QDir>
 #include <QFileInfo>
 #include <QString>
 
-#undef slots
-#include <pybind11/embed.h>
-#include <pybind11/eval.h>
-
 namespace py = pybind11;
 
 namespace
@@ -42,7 +40,7 @@ getPrettyNameFromScope( const py::dict& scope )
             const auto s = func().cast< std::string >();
             return QString::fromUtf8( s.c_str() );
         }
-        catch ( const py::cast_error& e )
+        catch ( const py::cast_error& )
         {
             // Ignore, we will try __doc__ next
         }
@@ -64,7 +62,7 @@ getPrettyNameFromScope( const py::dict& scope )
             }
             // __doc__ is apparently empty, try next fallback
         }
-        catch ( const py::cast_error& e )
+        catch ( const py::cast_error& )
         {
             // Ignore, try next fallback
         }
@@ -74,6 +72,96 @@ getPrettyNameFromScope( const py::dict& scope )
     return QString();
 }
 
+void
+populate_utils( py::module_& m )
+{
+    m.def( "obscure", &Calamares::Python::obscure, "A function that obscures (encodes) a string" );
+
+    m.def( "debug", &Calamares::Python::debug, "Log a debug-message" );
+    m.def( "warn", &Calamares::Python::warning, "Log a warning-message" );
+    m.def( "warning", &Calamares::Python::warning, "Log a warning-message" );
+    m.def( "error", &Calamares::Python::error, "Log an error-message" );
+
+    m.def( "load_yaml", &Calamares::Python::load_yaml, "Loads YAML from a file." );
+
+    m.def( "target_env_call",
+           &Calamares::Python::target_env_call,
+           "Runs command in target, returns exit code.",
+           py::arg( "command_list" ),
+           py::arg( "input" ) = std::string(),
+           py::arg( "timeout" ) = 0 );
+    m.def( "check_target_env_call",
+           &Calamares::Python::check_target_env_call,
+           "Runs command in target, raises on error exit.",
+           py::arg( "command_list" ),
+           py::arg( "input" ) = std::string(),
+           py::arg( "timeout" ) = 0 );
+    m.def( "check_target_env_output",
+           &Calamares::Python::check_target_env_output,
+           "Runs command in target, returns standard output or raises on error.",
+           py::arg( "command_list" ),
+           py::arg( "input" ) = std::string(),
+           py::arg( "timeout" ) = 0 );
+    m.def( "target_env_process_output",
+           &Calamares::Python::target_env_process_output,
+           "Runs command in target, updating callback and returns standard output or raises on error.",
+           py::arg( "command_list" ),
+           py::arg( "callback" ) = pybind11::none(),
+           py::arg( "input" ) = std::string(),
+           py::arg( "timeout" ) = 0 );
+    m.def( "host_env_process_output",
+           &Calamares::Python::host_env_process_output,
+           "Runs command in target, updating callback and returns standard output or raises on error.",
+           py::arg( "command_list" ),
+           py::arg( "callback" ) = pybind11::none(),
+           py::arg( "input" ) = std::string(),
+           py::arg( "timeout" ) = 0 );
+
+    m.def( "gettext_languages",
+           &Calamares::Python::gettext_languages,
+           "Returns list of languages (most to least-specific) for gettext." );
+    m.def( "gettext_path", &Calamares::Python::gettext_path, "Returns path for gettext search." );
+
+    m.def( "mount",
+           &Calamares::Python::mount,
+           "Runs the mount utility with the specified parameters.\n"
+           "Returns the program's exit code, or:\n"
+           "-1 = QProcess crash\n"
+           "-2 = QProcess cannot start\n"
+           "-3 = bad arguments" );
+}
+
+void
+populate_libcalamares( py::module_& m )
+{
+    m.doc() = "Calamares API for Python";
+
+    m.add_object( "ORGANIZATION_NAME", Calamares::Python::String( CALAMARES_ORGANIZATION_NAME ) );
+    m.add_object( "ORGANIZATION_DOMAIN", Calamares::Python::String( CALAMARES_ORGANIZATION_DOMAIN ) );
+    m.add_object( "APPLICATION_NAME", Calamares::Python::String( CALAMARES_APPLICATION_NAME ) );
+    m.add_object( "VERSION", Calamares::Python::String( CALAMARES_VERSION ) );
+    m.add_object( "VERSION_SHORT", Calamares::Python::String( CALAMARES_VERSION_SHORT ) );
+
+    auto utils = m.def_submodule( "utils", "Calamares Utility API for Python" );
+    populate_utils( utils );
+
+    py::class_< Calamares::Python::JobProxy >( m, "Job" )
+        .def_readonly( "module_name", &Calamares::Python::JobProxy::moduleName )
+        .def_readonly( "pretty_name", &Calamares::Python::JobProxy::prettyName )
+        .def_readonly( "working_path", &Calamares::Python::JobProxy::workingPath )
+        .def_readonly( "configuration", &Calamares::Python::JobProxy::configuration )
+        .def( "setprogress", &Calamares::Python::JobProxy::setprogress );
+
+    py::class_< Calamares::Python::GlobalStorageProxy >( m, "GlobalStorage" )
+        .def( py::init( []( std::nullptr_t ) { return new Calamares::Python::GlobalStorageProxy( nullptr ); } ) )
+        .def( "contains", &Calamares::Python::GlobalStorageProxy::contains )
+        .def( "count", &Calamares::Python::GlobalStorageProxy::count )
+        .def( "insert", &Calamares::Python::GlobalStorageProxy::insert )
+        .def( "keys", &Calamares::Python::GlobalStorageProxy::keys )
+        .def( "remove", &Calamares::Python::GlobalStorageProxy::remove )
+        .def( "value", &Calamares::Python::GlobalStorageProxy::value );
+}
+
 }  // namespace
 
 namespace Calamares
@@ -101,7 +189,8 @@ Job::Job( const QString& scriptFile,
           const QString& workingPath,
           const QVariantMap& moduleConfiguration,
           QObject* parent )
-    : m_d( std::make_unique< Job::Private >( scriptFile, workingPath, moduleConfiguration ) )
+    : ::Calamares::Job( parent )
+    , m_d( std::make_unique< Job::Private >( scriptFile, workingPath, moduleConfiguration ) )
 {
 }
 
@@ -150,11 +239,13 @@ Job::exec()
     }
 
     py::scoped_interpreter guard {};
-    auto scope = py::module_::import( "__main__" ).attr( "__dict__" );
+    // Import, but do not keep the handle lying around
-    auto calamaresModule = py::module::import( "libcalamares" );
+    {
-    calamaresModule.attr( "job" ) = Calamares::Python::JobProxy( this );
+        auto calamaresModule = py::module_::import( "libcalamares" );
-    calamaresModule.attr( "globalstorage" )
+        calamaresModule.attr( "job" ) = Calamares::Python::JobProxy( this );
-        = Calamares::Python::GlobalStorageProxy( JobQueue::instance()->globalStorage() );
+        calamaresModule.attr( "globalstorage" )
+            = Calamares::Python::GlobalStorageProxy( JobQueue::instance()->globalStorage() );
+    }
 
     if ( s_preScript )
     {
@@ -174,7 +265,7 @@ Job::exec()
 
     try
     {
-        py::eval_file( scriptFI.absoluteFilePath().toUtf8().constData(), scope );
+        py::eval_file( scriptFI.absoluteFilePath().toUtf8().constData() );
     }
     catch ( const py::error_already_set& e )
     {
@@ -187,6 +278,7 @@ Job::exec()
             JobResult::PythonUncaughtException );
     }
 
+    auto scope = py::module_::import( "__main__" ).attr( "__dict__" );
     m_d->description = getPrettyNameFromScope( scope );
 
     Q_EMIT progress( 0 );
@@ -284,3 +376,8 @@ Job::setInjectedPreScript( const char* script )
 
 }  // namespace Python
 }  // namespace Calamares
+
+PYBIND11_MODULE( libcalamares, m )
+{
+    populate_libcalamares( m );
+}

src/modules/dummypython/main.py

@@ -41,30 +41,24 @@ def pretty_status_message():
 
 def run():
     """Dummy python job."""
-    libcalamares.utils.debug("LocaleDir=" +
+    libcalamares.utils.debug(f"Calamares version: {libcalamares.VERSION} date: {strftime('%Y-%m-%d %H:%M:%S', gmtime())}")
-                             str(libcalamares.utils.gettext_path()))
+    libcalamares.utils.debug(f"Job name         : {libcalamares.job.pretty_name}")
-    libcalamares.utils.debug("Languages=" +
+    libcalamares.utils.debug(f"Job path         : {libcalamares.job.working_path}")
-                             str(libcalamares.utils.gettext_languages()))
+
+    libcalamares.utils.debug(f"LocaleDir        : {libcalamares.utils.gettext_path()}")
+    libcalamares.utils.debug(f"Languages         : {libcalamares.utils.gettext_languages()}")
 
     os.system("/bin/sh -c \"touch ~/calamares-dummypython\"")
-    accumulator = strftime("%Y-%m-%d %H:%M:%S", gmtime()) + "\n"
-    accumulator += "Calamares version: " + libcalamares.VERSION_SHORT + "\n"
-    accumulator += "This job's name: " + libcalamares.job.pretty_name + "\n"
-    accumulator += "This job's path: " + libcalamares.job.working_path
-    libcalamares.utils.debug(accumulator)
 
-    accumulator = "*** Job configuration "
+    libcalamares.utils.debug("*** JOB CONFIGURATION ***")
-    accumulator += str(libcalamares.job.configuration)
+    for k, v in libcalamares.job.configuration.items():
-    libcalamares.utils.debug(accumulator)
+        libcalamares.utils.debug(f"    {k}={v}")
 
-    accumulator = "*** globalstorage test ***"
+    libcalamares.utils.debug("*** GLOBAL STORAGE ***")
-    accumulator += "lala: "
+    for k in libcalamares.globalstorage.keys():
-    accumulator += str(libcalamares.globalstorage.contains("lala")) + "\n"
+        libcalamares.utils.debug(f"    {k}={libcalamares.globalstorage.value(k)}")
-    accumulator += "foo: "
-    accumulator += str(libcalamares.globalstorage.contains("foo")) + "\n"
-    accumulator += "count: " + str(libcalamares.globalstorage.count())
-    libcalamares.utils.debug(accumulator)
 
+    libcalamares.utils.debug("*** GLOBAL STORAGE MODIFICATION ***")
     libcalamares.globalstorage.insert("item2", "value2")
     libcalamares.globalstorage.insert("item3", 3)
     accumulator = "keys: {}\n".format(str(libcalamares.globalstorage.keys()))
@@ -78,7 +72,7 @@ def run():
         str(libcalamares.globalstorage.value("item3")))
     libcalamares.utils.debug(accumulator)
 
-    libcalamares.utils.debug("Run dummy python")
+    libcalamares.utils.debug("*** ACTIVITY ***")
 
     sleep(1)