Merge branch 'i865-error-reporting'

src/libcalamares/PythonHelper.cpp

@@ -1,6 +1,7 @@
 /* === This file is part of Calamares - <http://github.com/calamares> ===
  *
  *   Copyright 2014, Teo Mrnjavac <teo@kde.org>
+ *   Copyright 2017, Adriaan de Groot <groot@kde.org>
  *
  *   Calamares is free software: you can redistribute it and/or modify
  *   it under the terms of the GNU General Public License as published by
@@ -247,8 +248,11 @@ Helper::createCleanNamespace()
 QString
 Helper::handleLastError()
 {
-    PyObject* type = nullptr, *val = nullptr, *tb = nullptr;
+    PyObject* type = nullptr, *val = nullptr, *traceback_p = nullptr;
-    PyErr_Fetch( &type, &val, &tb );
+    PyErr_Fetch( &type, &val, &traceback_p );
+
+    Logger::CDebug debug;
+    debug.noquote() << "Python Error:\n";
 
     QString typeMsg;
     if ( type != nullptr )
@@ -257,10 +261,11 @@ Helper::handleLastError()
         bp::str pystr( h_type );
         bp::extract< std::string > extracted( pystr );
         if ( extracted.check() )
-            typeMsg = QString::fromStdString( extracted() ).toHtmlEscaped();
+            typeMsg = QString::fromStdString( extracted() ).trimmed();
 
-        if ( typeMsg.trimmed().isEmpty() )
+        if ( typeMsg.isEmpty() )
             typeMsg = tr( "Unknown exception type" );
+        debug << typeMsg << '\n';
     }
 
     QString valMsg;
@@ -270,26 +275,51 @@ Helper::handleLastError()
         bp::str pystr( h_val );
         bp::extract< std::string > extracted( pystr );
         if ( extracted.check() )
-            valMsg = QString::fromStdString( extracted() ).toHtmlEscaped();
+            valMsg = QString::fromStdString( extracted() ).trimmed();
 
-        if ( valMsg.trimmed().isEmpty() )
+        if ( valMsg.isEmpty() )
             valMsg = tr( "unparseable Python error" );
+
+        // Special-case: CalledProcessError has an attribute "output" with the command output,
+        // add that to the printed message.
+        if ( typeMsg.contains( "CalledProcessError" ) )
+        {
+            bp::object exceptionObject( h_val );
+            auto a = exceptionObject.attr( "output" );
+            bp::str outputString( a );
+            bp::extract< std::string > extractedOutput( outputString );
+
+            QString output;
+            if ( extractedOutput.check() )
+            {
+                output = QString::fromStdString( extractedOutput() ).trimmed();
+            }
+            if ( !output.isEmpty() )
+            {
+                // Replace the Type of the error by the warning string,
+                // and use the output of the command (e.g. its stderr) as value.
+                typeMsg = valMsg;
+                valMsg = output;
+            }
+        }
+        debug << valMsg << '\n';
     }
 
     QString tbMsg;
-    if ( tb != nullptr )
+    if ( traceback_p != nullptr )
     {
-        bp::handle<> h_tb( tb );
+        bp::handle<> h_tb( traceback_p );
-        bp::object tb( bp::import( "traceback" ) );
+        bp::object traceback_module( bp::import( "traceback" ) );
-        bp::object format_tb( tb.attr( "format_tb" ) );
+        bp::object format_tb( traceback_module.attr( "format_tb" ) );
         bp::object tb_list( format_tb( h_tb ) );
         bp::object pystr( bp::str( "\n" ).join( tb_list ) );
         bp::extract< std::string > extracted( pystr );
         if ( extracted.check() )
-            tbMsg = QString::fromStdString( extracted() ).toHtmlEscaped();
+            tbMsg = QString::fromStdString( extracted() ).trimmed();
 
-        if ( tbMsg.trimmed().isEmpty() )
+        if ( tbMsg.isEmpty() )
             tbMsg = tr( "unparseable Python traceback" );
+        debug << tbMsg << '\n';
     }
 
     if ( typeMsg.isEmpty() && valMsg.isEmpty() && tbMsg.isEmpty() )
@@ -297,18 +327,15 @@ Helper::handleLastError()
 
 
     QStringList msgList;
-
     if ( !typeMsg.isEmpty() )
-        msgList.append( QString( "<strong>%1</strong>" ).arg( typeMsg ) );
+        msgList.append( QString( "<strong>%1</strong>" ).arg( typeMsg.toHtmlEscaped() ) );
-
     if ( !valMsg.isEmpty() )
-        msgList.append( valMsg );
+        msgList.append( valMsg.toHtmlEscaped() );
 
     if ( !tbMsg.isEmpty() )
     {
-        msgList.append( "Traceback:" );
+        msgList.append( QStringLiteral( "<br/>Traceback:" ) );
-        msgList.append( QString( "<pre>%1</pre>" ).arg( tbMsg ) );
+        msgList.append( QString( "<pre>%1</pre>" ).arg( tbMsg.toHtmlEscaped() ) );
-        cDebug() << "tbMsg" << tbMsg;
     }
 
     // Return a string made of the msgList items, wrapped in <div> tags

src/libcalamares/PythonJobApi.cpp

@@ -36,6 +36,24 @@
 
 namespace bp = boost::python;
 
+static int
+_handle_check_target_env_call_error( const CalamaresUtils::ProcessResult& ec, const QString& cmd )
+{
+    if ( !ec.first )
+        return ec.first;
+
+    QString raise = QString( "import subprocess\n"
+                             "e = subprocess.CalledProcessError(%1,\"%2\")\n" )
+                    .arg( ec.first )
+                    .arg( cmd );
+    if ( !ec.second.isEmpty() )
+        raise.append( QStringLiteral("e.output = \"\"\"%1\"\"\"\n").arg( ec.second ) );
+    raise.append("raise e");
+    bp::exec( raise.toStdString().c_str() );
+    bp::throw_error_already_set();
+    return ec.first;
+}
+
 namespace CalamaresPython
 {
 
@@ -53,16 +71,38 @@ mount( const std::string& device_path,
 }
 
 
+static inline QStringList
+_bp_list_to_qstringlist( const bp::list& args )
+{
+    QStringList list;
+    for ( int i = 0; i < bp::len( args ); ++i )
+    {
+        list.append( QString::fromStdString(
+                         bp::extract< std::string >( args[ i ] ) ) );
+    }
+    return list;
+}
+
+static inline CalamaresUtils::ProcessResult
+_target_env_command(
+    const QStringList& args,
+    const std::string& stdin,
+    int timeout )
+{
+    return CalamaresUtils::System::instance()->
+        targetEnvCommand( args,
+                          QString(),
+                          QString::fromStdString( stdin ),
+                          timeout );
+}
+
 int
 target_env_call( const std::string& command,
                  const std::string& stdin,
                  int timeout )
 {
-    return CalamaresUtils::System::instance()->
+    return _target_env_command(
-           targetEnvCall( QString::fromStdString( command ),
+        QStringList{ QString::fromStdString( command ) }, stdin, timeout ).first;
-                          QString(),
-                          QString::fromStdString( stdin ),
-                          timeout );
 }
 
 
@@ -71,18 +111,8 @@ target_env_call( const bp::list& args,
                  const std::string& stdin,
                  int timeout )
 {
-    QStringList list;
+    return _target_env_command(
-    for ( int i = 0; i < bp::len( args ); ++i )
+        _bp_list_to_qstringlist( args ), stdin, timeout ).first;
-    {
-        list.append( QString::fromStdString(
-                         bp::extract< std::string >( args[ i ] ) ) );
-    }
-
-    return CalamaresUtils::System::instance()->
-           targetEnvCall( list,
-                          QString(),
-                          QString::fromStdString( stdin ),
-                          timeout );
 }
 
 
@@ -91,7 +121,8 @@ check_target_env_call( const std::string& command,
                        const std::string& stdin,
                        int timeout )
 {
-    int ec = target_env_call( command, stdin, timeout );
+    auto ec = _target_env_command(
+        QStringList{ QString::fromStdString( command ) }, stdin, timeout );
     return _handle_check_target_env_call_error( ec, QString::fromStdString( command ) );
 }
 
@@ -101,17 +132,11 @@ check_target_env_call( const bp::list& args,
                        const std::string& stdin,
                        int timeout )
 {
-    int ec = target_env_call( args, stdin, timeout );
+    auto ec = _target_env_command( _bp_list_to_qstringlist( args ), stdin, timeout );
-    if ( !ec )
+    if ( !ec.first )
-        return ec;
+        return ec.first;
-
-    QStringList failedCmdList;
-    for ( int i = 0; i < bp::len( args ); ++i )
-    {
-        failedCmdList.append( QString::fromStdString(
-                                  bp::extract< std::string >( args[ i ] ) ) );
-    }
 
+    QStringList failedCmdList = _bp_list_to_qstringlist( args );
     return _handle_check_target_env_call_error( ec, failedCmdList.join( ' ' ) );
 }
 
@@ -121,15 +146,10 @@ check_target_env_output( const std::string& command,
                          const std::string& stdin,
                          int timeout )
 {
-    QString output;
+    auto ec = _target_env_command(
-    int ec = CalamaresUtils::System::instance()->
+        QStringList{ QString::fromStdString( command ) }, stdin, timeout );
-             targetEnvOutput( QString::fromStdString( command ),
-                              output,
-                              QString(),
-                              QString::fromStdString( stdin ),
-                              timeout );
     _handle_check_target_env_call_error( ec, QString::fromStdString( command ) );
-    return output.toStdString();
+    return ec.second.toStdString();
 }
 
 
@@ -138,41 +158,13 @@ check_target_env_output( const bp::list& args,
                          const std::string& stdin,
                          int timeout )
 {
-    QString output;
+    QStringList list = _bp_list_to_qstringlist( args );
-    QStringList list;
+    auto ec = _target_env_command(
-    for ( int i = 0; i < bp::len( args ); ++i )
+        list, stdin, timeout );
-    {
-        list.append( QString::fromStdString(
-                         bp::extract< std::string >( args[ i ] ) ) );
-    }
-
-    int ec = CalamaresUtils::System::instance()->
-             targetEnvOutput( list,
-                              output,
-                              QString(),
-                              QString::fromStdString( stdin ),
-                              timeout );
     _handle_check_target_env_call_error( ec, list.join( ' ' ) );
-    return output.toStdString();
+    return ec.second.toStdString();
 }
 
-
-int
-_handle_check_target_env_call_error( int ec, const QString& cmd )
-{
-    if ( !ec )
-        return ec;
-
-    QString raise = QString( "import subprocess\n"
-                             "raise subprocess.CalledProcessError(%1,\"%2\")" )
-                    .arg( ec )
-                    .arg( cmd );
-    bp::exec( raise.toStdString().c_str() );
-    bp::throw_error_already_set();
-    return ec;
-}
-
-
 void
 debug( const std::string& s )
 {

src/libcalamares/PythonJobApi.h

@@ -67,8 +67,6 @@ boost::python::list gettext_languages();
 
 void debug( const std::string& s );
 
-inline int _handle_check_target_env_call_error( int ec, const QString& cmd );
-
 class PythonJobInterface
 {
 public:

src/libcalamares/utils/CalamaresUtilsSystem.cpp

@@ -92,42 +92,14 @@ System::mount( const QString& devicePath,
     return QProcess::execute( program, args );
 }
 
-int
+ProcessResult
-System::targetEnvCall( const QStringList& args,
+System::targetEnvCommand(
-            const QString& workingPath,
+    const QStringList& args,
-            const QString& stdInput,
+    const QString& workingPath,
-            int timeoutSec )
+    const QString& stdInput,
+    int timeoutSec )
 {
-    QString discard;
+    QString output;
-    return targetEnvOutput( args,
-                         discard,
-                         workingPath,
-                         stdInput,
-                         timeoutSec );
-}
-
-
-int
-System::targetEnvCall( const QString& command,
-            const QString& workingPath,
-            const QString& stdInput,
-            int timeoutSec )
-{
-    return targetEnvCall( QStringList{ command },
-                       workingPath,
-                       stdInput,
-                       timeoutSec );
-}
-
-
-int
-System::targetEnvOutput( const QStringList& args,
-              QString& output,
-              const QString& workingPath,
-              const QString& stdInput,
-              int timeoutSec )
-{
-    output.clear();
 
     if ( !Calamares::JobQueue::instance() )
         return -3;
@@ -209,25 +181,10 @@ System::targetEnvOutput( const QStringList& args,
     cLog() << "Finished. Exit code:" << r;
     if ( r != 0 )
     {
-        cLog() << "Target cmd" << args;
+        cLog() << "Target cmd:" << args;
-        cLog() << "Target out" << output;
+        cLog().noquote() << "Target output:\n" << output;
     }
-    return r;
+    return ProcessResult(r, output);
-}
-
-
-int
-System::targetEnvOutput( const QString& command,
-              QString& output,
-              const QString& workingPath,
-              const QString& stdInput,
-              int timeoutSec )
-{
-    return targetEnvOutput( QStringList{ command },
-                         output,
-                         workingPath,
-                         stdInput,
-                         timeoutSec );
 }
 
 

src/libcalamares/utils/CalamaresUtilsSystem.h

@@ -1,6 +1,7 @@
 /* === This file is part of Calamares - <http://github.com/calamares> ===
  *
  *   Copyright 2014, Teo Mrnjavac <teo@kde.org>
+ *   Copyright 2017, Adriaan de Groot <groot@kde.org>
  *
  *   Calamares is free software: you can redistribute it and/or modify
  *   it under the terms of the GNU General Public License as published by
@@ -21,10 +22,18 @@
 #include "DllMacro.h"
 
 #include <QObject>
+#include <QPair>
 #include <QString>
 
 namespace CalamaresUtils
 {
+class ProcessResult : public QPair< int, QString >
+{
+public:
+    /** @brief Implicit one-argument constructor has no output, only a return code */
+    ProcessResult( int r ) : QPair< int, QString >( r, QString() ) {}
+    ProcessResult( int r, QString s ) : QPair< int, QString >( r, s ) {}
+} ;
 
 /**
  * @brief The System class is a singleton with utility functions that perform
@@ -61,42 +70,75 @@ public:
                          const QString& filesystemName = QString(),
                          const QString& options = QString() );
 
+
     /**
       * Runs the specified command in the chroot of the target system.
-      * @param args the call with arguments, as a string list.
+      * @param args the command with arguments, as a string list.
       * @param workingPath the current working directory for the QProcess
       * call (optional).
       * @param stdInput the input string to send to the running process as
       * standard input (optional).
       * @param timeoutSec the timeout after which the process will be
       * killed (optional, default is 0 i.e. no timeout).
-      * @returns the program's exit code, or:
+      *
+      * @returns the program's exit code and its output (if any). Special
+      *     exit codes (which will never have any output) are:
       *             -1 = QProcess crash
       *             -2 = QProcess cannot start
       *             -3 = bad arguments
       *             -4 = QProcess timeout
       */
-    DLLEXPORT int targetEnvCall( const QStringList& args,
+    DLLEXPORT ProcessResult targetEnvCommand(
+        const QStringList &args,
+        const QString& workingPath = QString(),
+        const QString& stdInput = QString(),
+        int timeoutSec = 0 );
+
+    /** @brief Convenience wrapper for targetEnvCommand() which returns only the exit code */
+    inline int targetEnvCall( const QStringList& args,
                               const QString& workingPath = QString(),
                               const QString& stdInput = QString(),
-                              int timeoutSec = 0 );
+                              int timeoutSec = 0 )
+    {
+        return targetEnvCommand( args, workingPath, stdInput, timeoutSec ).first;
+    }
 
-    DLLEXPORT int targetEnvCall( const QString& command,
+    /** @brief Convenience wrapper for targetEnvCommand() which returns only the exit code */
+    inline int targetEnvCall( const QString& command,
                               const QString& workingPath = QString(),
                               const QString& stdInput = QString(),
-                              int timeoutSec = 0 );
+                              int timeoutSec = 0 )
+    {
+        return targetEnvCall( QStringList{ command }, workingPath, stdInput, timeoutSec );
+    }
 
-    DLLEXPORT int targetEnvOutput( const QStringList& args,
+    /** @brief Convenience wrapper for targetEnvCommand() which returns only the exit code
+     *
+     * Places the called program's output in the @p output string.
+     */
+    int targetEnvOutput( const QStringList& args,
                                 QString& output,
                                 const QString& workingPath = QString(),
                                 const QString& stdInput = QString(),
-                                int timeoutSec = 0 );
+                                int timeoutSec = 0 )
+    {
+        auto r = targetEnvCommand( args, workingPath, stdInput, timeoutSec );
+        output = r.second;
+        return r.first;
+    }
 
-    DLLEXPORT int targetEnvOutput( const QString& command,
+    /** @brief Convenience wrapper for targetEnvCommand() which returns only the exit code
+     *
+     * Places the called program's output in the @p output string.
+     */
+    inline int targetEnvOutput( const QString& command,
                                 QString& output,
                                 const QString& workingPath = QString(),
                                 const QString& stdInput = QString(),
-                                int timeoutSec = 0 );
+                                int timeoutSec = 0 )
+    {
+        return targetEnvOutput( QStringList{ command }, output, workingPath, stdInput, timeoutSec );
+    }
 
     /**
      * @brief getTotalMemoryB returns the total main memory, in bytes.