diff --git a/src/libcalamares/JobQueue.cpp b/src/libcalamares/JobQueue.cpp index 6ef055ffc..3a76aa099 100644 --- a/src/libcalamares/JobQueue.cpp +++ b/src/libcalamares/JobQueue.cpp @@ -44,7 +44,7 @@ public: } virtual ~JobThread() override; - + void setJobs( const JobList& jobs ) { m_jobs = jobs; @@ -157,6 +157,14 @@ JobQueue::JobQueue( QObject* parent ) JobQueue::~JobQueue() { + if ( m_thread->isRunning() ) + { + m_thread->terminate(); + if ( !m_thread->wait(300) ) + cError() << "Could not terminate job thread (expect a crash now)."; + delete m_thread; + } + delete m_storage; } diff --git a/src/libcalamares/ProcessJob.cpp b/src/libcalamares/ProcessJob.cpp index 3cf4eec49..47fcee05d 100644 --- a/src/libcalamares/ProcessJob.cpp +++ b/src/libcalamares/ProcessJob.cpp @@ -48,10 +48,8 @@ ProcessJob::~ProcessJob() QString ProcessJob::prettyName() const { - //TODO: show something more meaningful - return tr( "Run command %1 %2" ) - .arg( m_command ) - .arg( m_runInChroot ? "in chroot." : " ." ); + return ( m_runInChroot ? tr( "Run command '%1' in target system." ) : tr( " Run command '%1'." ) ) + .arg( m_command ); } @@ -67,83 +65,23 @@ ProcessJob::prettyStatusMessage() const JobResult ProcessJob::exec() { - int ec = 0; - QString output; + using CalamaresUtils::System; + if ( m_runInChroot ) - ec = CalamaresUtils::System::instance()-> - targetEnvOutput( m_command, - output, + return CalamaresUtils::System::instance()-> + targetEnvCommand( { m_command }, m_workingPath, QString(), - m_timeoutSec ); + m_timeoutSec ) + .explainProcess( m_command, m_timeoutSec ); else - ec = callOutput( m_command, - output, - m_workingPath, - QString(), - m_timeoutSec ); - - return CalamaresUtils::ProcessResult::explainProcess( ec, m_command, output, m_timeoutSec ); -} - - -int -ProcessJob::callOutput( const QString& command, - QString& output, - const QString& workingPath, - const QString& stdInput, - int timeoutSec ) -{ - output.clear(); - - QProcess process; - process.setProgram( "/bin/sh" ); - process.setArguments( { "-c", command } ); - process.setProcessChannelMode( QProcess::MergedChannels ); - - if ( !workingPath.isEmpty() ) - { - if ( QDir( workingPath ).exists() ) - process.setWorkingDirectory( QDir( workingPath ).absolutePath() ); - else - { - cWarning() << "Invalid working directory:" << workingPath; - return -3; - } - } - - cDebug() << "Running" << command; - process.start(); - if ( !process.waitForStarted() ) - { - cWarning() << "Process failed to start" << process.error(); - return -2; - } - - if ( !stdInput.isEmpty() ) - { - process.write( stdInput.toLocal8Bit() ); - process.closeWriteChannel(); - } - - if ( !process.waitForFinished( timeoutSec ? ( timeoutSec * 1000 ) : -1 ) ) - { - cWarning() << "Timed out. output so far:"; - output.append( QString::fromLocal8Bit( process.readAllStandardOutput() ).trimmed() ); - cWarning() << output; - return -4; - } - - output.append( QString::fromLocal8Bit( process.readAllStandardOutput() ).trimmed() ); - - if ( process.exitStatus() == QProcess::CrashExit ) - { - cWarning() << "Process crashed"; - return -1; - } - - cDebug() << "Finished. Exit code:" << process.exitCode(); - return process.exitCode(); + return + System::runCommand( System::RunLocation::RunInHost, + { "/bin/sh", "-c", m_command }, + m_workingPath, + QString(), + m_timeoutSec ) + .explainProcess( m_command, m_timeoutSec ); } } // namespace Calamares diff --git a/src/libcalamares/ProcessJob.h b/src/libcalamares/ProcessJob.h index d01dbb676..224ebdaf0 100644 --- a/src/libcalamares/ProcessJob.h +++ b/src/libcalamares/ProcessJob.h @@ -1,7 +1,7 @@ /* === This file is part of Calamares - === * * Copyright 2014, Teo Mrnjavac - * Copyright 2017-2018, Adriaan de Groot + * Copyright 2017-2019, Adriaan de Groot * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -40,11 +40,6 @@ public: JobResult exec() override; private: - int callOutput( const QString& command, - QString& output, - const QString& workingPath = QString(), - const QString& stdInput = QString(), - int timeoutSec = 0 ); QString m_command; QString m_workingPath; bool m_runInChroot; diff --git a/src/libcalamares/Tests.cpp b/src/libcalamares/Tests.cpp index 615cb51a7..3b7624537 100644 --- a/src/libcalamares/Tests.cpp +++ b/src/libcalamares/Tests.cpp @@ -18,9 +18,12 @@ #include "Tests.h" +#include "utils/CalamaresUtilsSystem.h" #include "utils/Logger.h" #include "utils/Yaml.h" +#include + #include QTEST_GUILESS_MAIN( LibCalamaresTests ) @@ -113,3 +116,45 @@ LibCalamaresTests::testLoadSaveYamlExtended() } QFile::remove( "out.yaml" ); } + +void +LibCalamaresTests::testCommands() +{ + using CalamaresUtils::System; + auto r = System::runCommand( + System::RunLocation::RunInHost, + { "/bin/ls", "/tmp" } + ); + + QVERIFY( r.getExitCode() == 0 ); + + QTemporaryFile tf( "/tmp/calamares-test-XXXXXX" ); + QVERIFY( tf.open() ); + QVERIFY( !tf.fileName().isEmpty() ); + + QFileInfo tfn( tf.fileName() ); + QVERIFY( !r.getOutput().contains( tfn.fileName() ) ); + + // Run ls again, now that the file exists + r = System::runCommand( + System::RunLocation::RunInHost, + { "/bin/ls", "/tmp" } + ); + QVERIFY( r.getOutput().contains( tfn.fileName() ) ); + + // .. and without a working directory set, assume builddir != /tmp + r = System::runCommand( + System::RunLocation::RunInHost, + { "/bin/ls" } + ); + QVERIFY( !r.getOutput().contains( tfn.fileName() ) ); + + r = System::runCommand( + System::RunLocation::RunInHost, + { "/bin/ls" }, + "/tmp" + ); + QVERIFY( r.getOutput().contains( tfn.fileName() ) ); + + +} diff --git a/src/libcalamares/Tests.h b/src/libcalamares/Tests.h index 8d0aee1ff..5cdb3912b 100644 --- a/src/libcalamares/Tests.h +++ b/src/libcalamares/Tests.h @@ -34,6 +34,8 @@ private Q_SLOTS: void testLoadSaveYaml(); // Just settings.conf void testLoadSaveYamlExtended(); // Do a find() in the src dir + + void testCommands(); }; #endif diff --git a/src/libcalamares/utils/CalamaresUtilsSystem.cpp b/src/libcalamares/utils/CalamaresUtilsSystem.cpp index 1b603a7e7..5990fbc42 100644 --- a/src/libcalamares/utils/CalamaresUtilsSystem.cpp +++ b/src/libcalamares/utils/CalamaresUtilsSystem.cpp @@ -114,14 +114,24 @@ System::mount( const QString& devicePath, const QString& options ) { if ( devicePath.isEmpty() || mountPoint.isEmpty() ) - return -3; + { + if ( devicePath.isEmpty() ) + cWarning() << "Can't mount an empty device."; + if ( mountPoint.isEmpty() ) + cWarning() << "Can't mount on an empty mountpoint."; + + return static_cast(ProcessResult::Code::NoWorkingDirectory); + } QDir mountPointDir( mountPoint ); if ( !mountPointDir.exists() ) { bool ok = mountPointDir.mkpath( mountPoint ); if ( !ok ) - return -3; + { + cWarning() << "Could not create mountpoint" << mountPoint; + return static_cast(ProcessResult::Code::NoWorkingDirectory); + } } QString program( "mount" ); @@ -146,15 +156,13 @@ System::runCommand( { QString output; - if ( !Calamares::JobQueue::instance() ) - return -3; + Calamares::GlobalStorage* gs = Calamares::JobQueue::instance() ? Calamares::JobQueue::instance()->globalStorage() : nullptr; - Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage(); if ( ( location == System::RunLocation::RunInTarget ) && ( !gs || !gs->contains( "rootMountPoint" ) ) ) { cWarning() << "No rootMountPoint in global storage"; - return -3; + return ProcessResult::Code::NoWorkingDirectory; } QProcess process; @@ -167,7 +175,7 @@ System::runCommand( if ( !QDir( destDir ).exists() ) { cWarning() << "rootMountPoint points to a dir which does not exist"; - return -3; + return ProcessResult::Code::NoWorkingDirectory; } program = "chroot"; @@ -189,8 +197,10 @@ System::runCommand( if ( QDir( workingPath ).exists() ) process.setWorkingDirectory( QDir( workingPath ).absolutePath() ); else + { cWarning() << "Invalid working directory:" << workingPath; - return -3; + return ProcessResult::Code::NoWorkingDirectory; + } } cDebug() << "Running" << program << RedactedList( arguments ); @@ -198,20 +208,20 @@ System::runCommand( if ( !process.waitForStarted() ) { cWarning() << "Process failed to start" << process.error(); - return -2; + return ProcessResult::Code::FailedToStart; } if ( !stdInput.isEmpty() ) { process.write( stdInput.toLocal8Bit() ); - process.closeWriteChannel(); } + process.closeWriteChannel(); if ( !process.waitForFinished( timeoutSec ? ( timeoutSec * 1000 ) : -1 ) ) { cWarning().noquote().nospace() << "Timed out. Output so far:\n" << process.readAllStandardOutput(); - return -4; + return ProcessResult::Code::TimedOut; } output.append( QString::fromLocal8Bit( process.readAllStandardOutput() ).trimmed() ); @@ -219,12 +229,13 @@ System::runCommand( if ( process.exitStatus() == QProcess::CrashExit ) { cWarning().noquote().nospace() << "Process crashed. Output so far:\n" << output; - return -1; + return ProcessResult::Code::Crashed; } auto r = process.exitCode(); cDebug() << "Finished. Exit code:" << r; - if ( ( r != 0 ) || Calamares::Settings::instance()->debugMode() ) + bool showDebug = ( !Calamares::Settings::instance() ) || ( Calamares::Settings::instance()->debugMode() ); + if ( ( r != 0 ) || showDebug ) { cDebug() << "Target cmd:" << RedactedList( args ); cDebug().noquote().nospace() << "Target output:\n" << output; @@ -306,22 +317,22 @@ ProcessResult::explainProcess( int ec, const QString& command, const QString& ou ? QCoreApplication::translate( "ProcessResult", "\nThere was no output from the command.") : (QCoreApplication::translate( "ProcessResult", "\nOutput:\n") + output); - if ( ec == -1 ) //Crash! + if ( ec == static_cast(ProcessResult::Code::Crashed) ) //Crash! return JobResult::error( QCoreApplication::translate( "ProcessResult", "External command crashed." ), QCoreApplication::translate( "ProcessResult", "Command %1 crashed." ) .arg( command ) + outputMessage ); - if ( ec == -2 ) + if ( ec == static_cast(ProcessResult::Code::FailedToStart) ) return JobResult::error( QCoreApplication::translate( "ProcessResult", "External command failed to start." ), QCoreApplication::translate( "ProcessResult", "Command %1 failed to start." ) .arg( command ) ); - if ( ec == -3 ) + if ( ec == static_cast(ProcessResult::Code::NoWorkingDirectory) ) return JobResult::error( QCoreApplication::translate( "ProcessResult", "Internal error when starting command." ), QCoreApplication::translate( "ProcessResult", "Bad parameters for process job call." ) ); - if ( ec == -4 ) + if ( ec == static_cast(ProcessResult::Code::TimedOut) ) return JobResult::error( QCoreApplication::translate( "ProcessResult", "External command failed to finish." ), QCoreApplication::translate( "ProcessResult", "Command %1 failed to finish in %2 seconds." ) .arg( command ) diff --git a/src/libcalamares/utils/CalamaresUtilsSystem.h b/src/libcalamares/utils/CalamaresUtilsSystem.h index 6809859ee..c17d52e93 100644 --- a/src/libcalamares/utils/CalamaresUtilsSystem.h +++ b/src/libcalamares/utils/CalamaresUtilsSystem.h @@ -32,8 +32,16 @@ namespace CalamaresUtils class ProcessResult : public QPair< int, QString > { public: + enum class Code : int + { + Crashed = -1, // Must match special return values from QProcess + FailedToStart = -2, // Must match special return values from QProcess + NoWorkingDirectory = -3, + TimedOut = -4 + } ; + /** @brief Implicit one-argument constructor has no output, only a return code */ - ProcessResult( int r ) : QPair< int, QString >( r, QString() ) {} + ProcessResult( Code r ) : QPair< int, QString >( static_cast(r), QString() ) {} ProcessResult( int r, QString s ) : QPair< int, QString >( r, s ) {} int getExitCode() const { return first; } @@ -93,9 +101,9 @@ public: * @param filesystemName the name of the filesystem (optional). * @param options any additional options as passed to mount -o (optional). * @returns the program's exit code, or: - * -1 = QProcess crash - * -2 = QProcess cannot start - * -3 = bad arguments + * Crashed = QProcess crash + * FailedToStart = QProcess cannot start + * NoWorkingDirectory = bad arguments */ DLLEXPORT int mount( const QString& devicePath, const QString& mountPoint, @@ -120,10 +128,10 @@ public: * * @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 + * Crashed = QProcess crash + * FailedToStart = QProcess cannot start + * NoWorkingDirectory = bad arguments + * TimedOut = QProcess timeout */ static DLLEXPORT ProcessResult runCommand( RunLocation location,