From 938b1ac4aabaaabf1158f5341ca3d4bf005c3650 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 4 Jul 2019 11:47:27 +0200 Subject: [PATCH 1/5] [libcalamares] Make API more type-explicit with std::chrono - Having an int timeoutSec is suggestive -- it's probably a number of seconds -- but having an explicit type that says it's seconds is better. - Doesn't compile, because the implementation and consumers have not changed. --- src/libcalamares/utils/CalamaresUtilsSystem.h | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/libcalamares/utils/CalamaresUtilsSystem.h b/src/libcalamares/utils/CalamaresUtilsSystem.h index c17d52e93..734dbff4b 100644 --- a/src/libcalamares/utils/CalamaresUtilsSystem.h +++ b/src/libcalamares/utils/CalamaresUtilsSystem.h @@ -27,6 +27,8 @@ #include #include +#include + namespace CalamaresUtils { class ProcessResult : public QPair< int, QString > @@ -138,7 +140,7 @@ public: const QStringList &args, const QString& workingPath = QString(), const QString& stdInput = QString(), - int timeoutSec = 0 ); + std::chrono::seconds timeoutSec = std::chrono::seconds(0) ); /** @brief Convenience wrapper for runCommand(). * Runs the command in the location specified through the boolean @@ -149,7 +151,7 @@ public: const QStringList &args, const QString& workingPath = QString(), const QString& stdInput = QString(), - int timeoutSec = 0 ) + std::chrono::seconds timeoutSec = std::chrono::seconds(0) ) { return runCommand( m_doChroot ? RunLocation::RunInTarget : RunLocation::RunInHost, @@ -163,7 +165,7 @@ public: inline int targetEnvCall( const QStringList& args, const QString& workingPath = QString(), const QString& stdInput = QString(), - int timeoutSec = 0 ) + std::chrono::seconds timeoutSec = std::chrono::seconds(0) ) { return targetEnvCommand( args, workingPath, stdInput, timeoutSec ).first; } @@ -172,7 +174,7 @@ public: inline int targetEnvCall( const QString& command, const QString& workingPath = QString(), const QString& stdInput = QString(), - int timeoutSec = 0 ) + std::chrono::seconds timeoutSec = std::chrono::seconds(0) ) { return targetEnvCall( QStringList{ command }, workingPath, stdInput, timeoutSec ); } @@ -185,7 +187,7 @@ public: QString& output, const QString& workingPath = QString(), const QString& stdInput = QString(), - int timeoutSec = 0 ) + std::chrono::seconds timeoutSec = std::chrono::seconds(0) ) { auto r = targetEnvCommand( args, workingPath, stdInput, timeoutSec ); output = r.second; @@ -200,7 +202,7 @@ public: QString& output, const QString& workingPath = QString(), const QString& stdInput = QString(), - int timeoutSec = 0 ) + std::chrono::seconds timeoutSec = std::chrono::seconds(0) ) { return targetEnvOutput( QStringList{ command }, output, workingPath, stdInput, timeoutSec ); } From 91644b4ba22b837a7c1f5927f0065d8e51592775 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 4 Jul 2019 11:50:14 +0200 Subject: [PATCH 2/5] [libcalamares] Partially fix implementation of timeout - Adjust most call sites to use std::chrono::duration, - Call to QProcess::waitForFinished() needs work, since that takes milliseconds. --- src/libcalamares/utils/CalamaresUtilsSystem.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcalamares/utils/CalamaresUtilsSystem.cpp b/src/libcalamares/utils/CalamaresUtilsSystem.cpp index 5990fbc42..73e457239 100644 --- a/src/libcalamares/utils/CalamaresUtilsSystem.cpp +++ b/src/libcalamares/utils/CalamaresUtilsSystem.cpp @@ -152,7 +152,7 @@ System::runCommand( const QStringList& args, const QString& workingPath, const QString& stdInput, - int timeoutSec ) + std::chrono::seconds timeoutSec ) { QString output; From cac07c1472fdbec47eaa05458598c13e3a83b011 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 1 Aug 2019 22:47:42 +0200 Subject: [PATCH 3/5] [libcalamares] Use std::chrono::seconds for timeouts - Distinguish just-an-int from seconds all across the API --- src/libcalamares/ProcessJob.cpp | 2 +- src/libcalamares/ProcessJob.h | 6 ++-- src/libcalamares/PythonJobApi.cpp | 4 ++- .../utils/CalamaresUtilsSystem.cpp | 22 +++++++-------- src/libcalamares/utils/CalamaresUtilsSystem.h | 28 +++++++++---------- src/libcalamares/utils/CommandList.cpp | 12 ++++---- src/libcalamares/utils/CommandList.h | 22 ++++++++------- 7 files changed, 51 insertions(+), 45 deletions(-) diff --git a/src/libcalamares/ProcessJob.cpp b/src/libcalamares/ProcessJob.cpp index 47fcee05d..f952fefe6 100644 --- a/src/libcalamares/ProcessJob.cpp +++ b/src/libcalamares/ProcessJob.cpp @@ -31,7 +31,7 @@ namespace Calamares { ProcessJob::ProcessJob( const QString& command, const QString& workingPath, bool runInChroot, - int secondsTimeout, + std::chrono::seconds secondsTimeout, QObject* parent ) : Job( parent ) , m_command( command ) diff --git a/src/libcalamares/ProcessJob.h b/src/libcalamares/ProcessJob.h index 224ebdaf0..84f84e550 100644 --- a/src/libcalamares/ProcessJob.h +++ b/src/libcalamares/ProcessJob.h @@ -22,6 +22,8 @@ #include "Job.h" +#include + namespace Calamares { class ProcessJob : public Job @@ -31,7 +33,7 @@ public: explicit ProcessJob( const QString& command, const QString& workingPath, bool runInChroot = false, - int secondsTimeout = 30, + std::chrono::seconds secondsTimeout = std::chrono::seconds(30), QObject* parent = nullptr ); virtual ~ProcessJob() override; @@ -43,7 +45,7 @@ private: QString m_command; QString m_workingPath; bool m_runInChroot; - int m_timeoutSec; + std::chrono::seconds m_timeoutSec; }; } // namespace Calamares diff --git a/src/libcalamares/PythonJobApi.cpp b/src/libcalamares/PythonJobApi.cpp index 8e8b8b2ab..750d8ea73 100644 --- a/src/libcalamares/PythonJobApi.cpp +++ b/src/libcalamares/PythonJobApi.cpp @@ -89,11 +89,13 @@ _target_env_command( const std::string& stdin, int timeout ) { + // Since Python doesn't give us the type system for distinguishing + // seconds from other integral types, massage to seconds here. return CalamaresUtils::System::instance()-> targetEnvCommand( args, QString(), QString::fromStdString( stdin ), - timeout ); + std::chrono::seconds( timeout ) ); } int diff --git a/src/libcalamares/utils/CalamaresUtilsSystem.cpp b/src/libcalamares/utils/CalamaresUtilsSystem.cpp index 4fe571e6d..894c434be 100644 --- a/src/libcalamares/utils/CalamaresUtilsSystem.cpp +++ b/src/libcalamares/utils/CalamaresUtilsSystem.cpp @@ -219,7 +219,7 @@ System::runCommand( } process.closeWriteChannel(); - if ( !process.waitForFinished( timeoutSec ? ( timeoutSec * 1000 ) : -1 ) ) + if ( !process.waitForFinished( timeoutSec > std::chrono::seconds::zero() ? ( std::chrono::milliseconds( timeoutSec ).count() ) : -1 ) ) { cWarning().noquote().nospace() << "Timed out. Output so far:\n" << process.readAllStandardOutput(); @@ -249,7 +249,7 @@ QString System::targetPath( const QString& path ) const { QString completePath; - + if ( doChroot() ) { Calamares::GlobalStorage* gs = Calamares::JobQueue::instance() ? Calamares::JobQueue::instance()->globalStorage() : nullptr; @@ -259,14 +259,14 @@ System::targetPath( const QString& path ) const cWarning() << "No rootMountPoint in global storage, cannot create target file" << path; return QString(); } - + completePath = gs->value( "rootMountPoint" ).toString() + '/' + path; } else { completePath = QStringLiteral( "/" ) + path; } - + return completePath; } @@ -278,32 +278,32 @@ System::createTargetFile( const QString& path, const QByteArray& contents ) cons { return QString(); } - + QFile f( completePath ); if ( f.exists() ) { return QString(); } - + QIODevice::OpenMode m = #if QT_VERSION >= QT_VERSION_CHECK( 5, 11, 0 ) // New flag from Qt 5.11, implies WriteOnly QIODevice::NewOnly | #endif QIODevice::WriteOnly | QIODevice::Truncate; - + if ( !f.open( m ) ) { return QString(); } - + if ( f.write( contents ) != contents.size() ) { f.close(); f.remove(); return QString(); } - + f.close(); return QFileInfo( f ).canonicalFilePath(); } @@ -371,7 +371,7 @@ System::doChroot() const } Calamares::JobResult -ProcessResult::explainProcess( int ec, const QString& command, const QString& output, int timeout ) +ProcessResult::explainProcess( int ec, const QString& command, const QString& output, std::chrono::seconds timeout ) { using Calamares::JobResult; @@ -401,7 +401,7 @@ ProcessResult::explainProcess( int ec, const QString& command, const QString& ou return JobResult::error( QCoreApplication::translate( "ProcessResult", "External command failed to finish." ), QCoreApplication::translate( "ProcessResult", "Command %1 failed to finish in %2 seconds." ) .arg( command ) - .arg( timeout ) + .arg( timeout.count() ) + outputMessage ); //Any other exit code diff --git a/src/libcalamares/utils/CalamaresUtilsSystem.h b/src/libcalamares/utils/CalamaresUtilsSystem.h index d39bcaab1..5b10f7d1a 100644 --- a/src/libcalamares/utils/CalamaresUtilsSystem.h +++ b/src/libcalamares/utils/CalamaresUtilsSystem.h @@ -62,16 +62,16 @@ public: * @param timeout Timeout passed to the process runner, for explaining * error code -4 (timeout). */ - static Calamares::JobResult explainProcess( int errorCode, const QString& command, const QString& output, int timeout ); + static Calamares::JobResult explainProcess( int errorCode, const QString& command, const QString& output, std::chrono::seconds timeout ); /// @brief Convenience wrapper for explainProcess() - inline Calamares::JobResult explainProcess( const QString& command, int timeout ) const + inline Calamares::JobResult explainProcess( const QString& command, std::chrono::seconds timeout ) const { return explainProcess( getExitCode(), command, getOutput(), timeout ); } /// @brief Convenience wrapper for explainProcess() - inline Calamares::JobResult explainProcess( const QStringList& command, int timeout ) const + inline Calamares::JobResult explainProcess( const QStringList& command, std::chrono::seconds timeout ) const { return explainProcess( getExitCode(), command.join( ' ' ), getOutput(), timeout ); } @@ -207,40 +207,40 @@ public: return targetEnvOutput( QStringList{ command }, output, workingPath, stdInput, timeoutSec ); } - + /** @brief Gets a path to a file in the target system, from the host. - * + * * @param path Path to the file; this is interpreted * from the root of the target system (whatever that may be, * but / in the chroot, or / in OEM modes). - * + * * @return The complete path to the target file, from * the root of the host system, or empty on failure. - * + * * For instance, during installation where the target root is * mounted on /tmp/calamares-something, asking for targetPath("/etc/passwd") * will give you /tmp/calamares-something/etc/passwd. - * + * * No attempt is made to canonicalize anything, since paths might not exist. - */ + */ DLLEXPORT QString targetPath( const QString& path ) const; - + /** @brief Create a (small-ish) file in the target system. - * + * * @param path Path to the file; this is interpreted * from the root of the target system (whatever that may be, * but / in the chroot, or / in OEM modes). * @param contents Actual content of the file. - * + * * Will not overwrite files. Returns an empty string if the * target file already exists. - * + * * @return The complete canonical path to the target file from the * root of the host system, or empty on failure. (Here, it is * possible to be canonical because the file exists). */ DLLEXPORT QString createTargetFile( const QString& path, const QByteArray& contents ) const; - + /** * @brief getTotalMemoryB returns the total main memory, in bytes. * diff --git a/src/libcalamares/utils/CommandList.cpp b/src/libcalamares/utils/CommandList.cpp index 9916c71fe..414cfa9e5 100644 --- a/src/libcalamares/utils/CommandList.cpp +++ b/src/libcalamares/utils/CommandList.cpp @@ -35,10 +35,10 @@ namespace CalamaresUtils static CommandLine get_variant_object( const QVariantMap& m ) { QString command = CalamaresUtils::getString( m, "command" ); - int timeout = CalamaresUtils::getInteger( m, "timeout", CommandLine::TimeoutNotSet ); + int timeout = CalamaresUtils::getInteger( m, "timeout", -1 ); if ( !command.isEmpty() ) - return CommandLine( command, timeout ); + return CommandLine( command, timeout >= 0 ? std::chrono::seconds( timeout ) : CommandLine::TimeoutNotSet() ); cWarning() << "Bad CommandLine element" << m; return CommandLine(); } @@ -50,7 +50,7 @@ static CommandList_t get_variant_stringlist( const QVariantList& l ) for ( const auto& v : l ) { if ( v.type() == QVariant::String ) - retl.append( CommandLine( v.toString(), CommandLine::TimeoutNotSet ) ); + retl.append( CommandLine( v.toString(), CommandLine::TimeoutNotSet() ) ); else if ( v.type() == QVariant::Map ) { auto command( get_variant_object( v.toMap() ) ); @@ -65,13 +65,13 @@ static CommandList_t get_variant_stringlist( const QVariantList& l ) return retl; } -CommandList::CommandList( bool doChroot, int timeout ) +CommandList::CommandList( bool doChroot, std::chrono::seconds timeout ) : m_doChroot( doChroot ) , m_timeout( timeout ) { } -CommandList::CommandList::CommandList( const QVariant& v, bool doChroot, int timeout ) +CommandList::CommandList::CommandList( const QVariant& v, bool doChroot, std::chrono::seconds timeout ) : CommandList( doChroot, timeout ) { if ( v.type() == QVariant::List ) @@ -155,7 +155,7 @@ Calamares::JobResult CommandList::run() QStringList shell_cmd { "/bin/sh", "-c" }; shell_cmd << processed_cmd; - int timeout = i->timeout() >= 0 ? i->timeout() : m_timeout; + std::chrono::seconds timeout = i->timeout() >= std::chrono::seconds::zero() ? i->timeout() : m_timeout; ProcessResult r = System::runCommand( location, shell_cmd, QString(), QString(), timeout ); diff --git a/src/libcalamares/utils/CommandList.h b/src/libcalamares/utils/CommandList.h index 3dccdec6a..7453ae6ce 100644 --- a/src/libcalamares/utils/CommandList.h +++ b/src/libcalamares/utils/CommandList.h @@ -24,6 +24,8 @@ #include #include +#include + namespace CalamaresUtils { @@ -31,23 +33,23 @@ namespace CalamaresUtils * Each command can have an associated timeout in seconds. The timeout * defaults to 10 seconds. Provide some convenience naming and construction. */ -struct CommandLine : public QPair< QString, int > +struct CommandLine : public QPair< QString, std::chrono::seconds > { - enum { TimeoutNotSet = -1 }; + static inline constexpr std::chrono::seconds TimeoutNotSet() { return std::chrono::seconds(-1); } /// An invalid command line CommandLine() - : QPair< QString, int >( QString(), TimeoutNotSet ) + : QPair( QString(), TimeoutNotSet() ) { } CommandLine( const QString& s ) - : QPair< QString, int >( s, TimeoutNotSet ) + : QPair( s, TimeoutNotSet() ) { } - CommandLine( const QString& s, int t ) - : QPair< QString, int >( s, t) + CommandLine( const QString& s, std::chrono::seconds t ) + : QPair( s, t) { } @@ -56,7 +58,7 @@ struct CommandLine : public QPair< QString, int > return first; } - int timeout() const + std::chrono::seconds timeout() const { return second; } @@ -82,8 +84,8 @@ class CommandList : protected CommandList_t { public: /** @brief empty command-list with timeout to apply to entries. */ - CommandList( bool doChroot = true, int timeout = 10 ); - CommandList( const QVariant& v, bool doChroot = true, int timeout = 10 ); + CommandList( bool doChroot = true, std::chrono::seconds timeout = std::chrono::seconds( 10 ) ); + CommandList( const QVariant& v, bool doChroot = true, std::chrono::seconds timeout = std::chrono::seconds( 10 ) ); ~CommandList(); bool doChroot() const @@ -106,7 +108,7 @@ protected: private: bool m_doChroot; - int m_timeout; + std::chrono::seconds m_timeout; } ; } // namespace From e2504627aaf3420ec7ee0b9f6ccc5c76a306c8a7 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 1 Aug 2019 22:51:52 +0200 Subject: [PATCH 4/5] [libcalamaresui] Chase timeout-type into the UI library (TODO: move ProcessJobModule to libcalamares, it has no UI dependency) --- src/libcalamaresui/modulesystem/ProcessJobModule.cpp | 9 ++++++--- src/libcalamaresui/modulesystem/ProcessJobModule.h | 4 +++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/libcalamaresui/modulesystem/ProcessJobModule.cpp b/src/libcalamaresui/modulesystem/ProcessJobModule.cpp index f15e5c457..74c195b6b 100644 --- a/src/libcalamaresui/modulesystem/ProcessJobModule.cpp +++ b/src/libcalamaresui/modulesystem/ProcessJobModule.cpp @@ -72,10 +72,13 @@ ProcessJobModule::initFrom( const QVariantMap& moduleDescriptor ) m_command = moduleDescriptor.value( "command" ).toString(); } - m_secondsTimeout = 30; + m_secondsTimeout = std::chrono::seconds( 30 ); if ( moduleDescriptor.contains( "timeout" ) && !moduleDescriptor.value( "timeout" ).isNull() ) { - m_secondsTimeout = moduleDescriptor.value( "timeout" ).toInt(); + int sec = moduleDescriptor.value( "timeout" ).toInt(); + if ( sec < 0 ) + sec = 0; + m_secondsTimeout = std::chrono::seconds( sec ); } m_runInChroot = false; @@ -88,7 +91,7 @@ ProcessJobModule::initFrom( const QVariantMap& moduleDescriptor ) ProcessJobModule::ProcessJobModule() : Module() - , m_secondsTimeout( 30 ) + , m_secondsTimeout( std::chrono::seconds( 30 ) ) , m_runInChroot( false ) { } diff --git a/src/libcalamaresui/modulesystem/ProcessJobModule.h b/src/libcalamaresui/modulesystem/ProcessJobModule.h index 9c20aa4f4..96fb2ed47 100644 --- a/src/libcalamaresui/modulesystem/ProcessJobModule.h +++ b/src/libcalamaresui/modulesystem/ProcessJobModule.h @@ -24,6 +24,8 @@ #include "UiDllMacro.h" +#include + namespace Calamares { @@ -46,7 +48,7 @@ private: QString m_command; QString m_workingPath; - int m_secondsTimeout; + std::chrono::seconds m_secondsTimeout; bool m_runInChroot; job_ptr m_job; }; From a0854a999e4c4a1a5020d0f9316b13b6b7dc00fb Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 1 Aug 2019 22:59:06 +0200 Subject: [PATCH 5/5] Modules: chase API change, use std::chrono::seconds --- .../ContextualProcessJob.cpp | 2 +- src/modules/initcpio/InitcpioJob.cpp | 6 ++-- src/modules/initramfs/InitramfsJob.cpp | 6 ++-- .../luksbootkeyfile/LuksBootKeyFileJob.cpp | 4 +-- src/modules/shellprocess/ShellProcessJob.cpp | 2 +- src/modules/shellprocess/Tests.cpp | 35 +++++++++++-------- src/modules/tracking/TrackingJobs.cpp | 2 +- src/modules/users/CreateUserJob.cpp | 6 ++-- 8 files changed, 34 insertions(+), 29 deletions(-) diff --git a/src/modules/contextualprocess/ContextualProcessJob.cpp b/src/modules/contextualprocess/ContextualProcessJob.cpp index 428e54cd5..5d6b20afc 100644 --- a/src/modules/contextualprocess/ContextualProcessJob.cpp +++ b/src/modules/contextualprocess/ContextualProcessJob.cpp @@ -170,7 +170,7 @@ ContextualProcessJob::setConfigurationMap( const QVariantMap& configurationMap ) continue; } - CalamaresUtils::CommandList* commands = new CalamaresUtils::CommandList( valueiter.value(), !dontChroot, timeout ); + CalamaresUtils::CommandList* commands = new CalamaresUtils::CommandList( valueiter.value(), !dontChroot, std::chrono::seconds( timeout ) ); binding->append( valueString, commands ); } diff --git a/src/modules/initcpio/InitcpioJob.cpp b/src/modules/initcpio/InitcpioJob.cpp index 38017d83e..38f3a8961 100644 --- a/src/modules/initcpio/InitcpioJob.cpp +++ b/src/modules/initcpio/InitcpioJob.cpp @@ -74,8 +74,8 @@ InitcpioJob::exec() cDebug() << "Updating initramfs with kernel" << m_kernel; auto r = CalamaresUtils::System::instance()->targetEnvCommand( - { "mkinitcpio", "-p", m_kernel }, QString(), QString(), 0 ); - return r.explainProcess( "mkinitcpio", 10 ); + { "mkinitcpio", "-p", m_kernel }, QString(), QString() /* no timeout , 0 */ ); + return r.explainProcess( "mkinitcpio", std::chrono::seconds( 10 ) /* fake timeout */ ); } void @@ -89,7 +89,7 @@ InitcpioJob::setConfigurationMap( const QVariantMap& configurationMap ) else if ( m_kernel == "$uname" ) { auto r = CalamaresUtils::System::runCommand( - CalamaresUtils::System::RunLocation::RunInHost, { "/bin/uname", "-r" }, QString(), QString(), 3 ); + CalamaresUtils::System::RunLocation::RunInHost, { "/bin/uname", "-r" }, QString(), QString(), std::chrono::seconds( 3 ) ); if ( r.getExitCode() == 0 ) { m_kernel = r.getOutput(); diff --git a/src/modules/initramfs/InitramfsJob.cpp b/src/modules/initramfs/InitramfsJob.cpp index 01d400443..af8e07ca7 100644 --- a/src/modules/initramfs/InitramfsJob.cpp +++ b/src/modules/initramfs/InitramfsJob.cpp @@ -63,8 +63,8 @@ InitramfsJob::exec() // And then do the ACTUAL work. auto r = CalamaresUtils::System::instance()->targetEnvCommand( - { "update-initramfs", "-k", m_kernel, "-c", "-t" }, QString(), QString(), 0 ); - return r.explainProcess( "update-initramfs", 10 ); + { "update-initramfs", "-k", m_kernel, "-c", "-t" }, QString(), QString() /* no timeout, 0 */ ); + return r.explainProcess( "update-initramfs", std::chrono::seconds( 10 ) /* fake timeout */ ); } @@ -79,7 +79,7 @@ InitramfsJob::setConfigurationMap( const QVariantMap& configurationMap ) else if ( m_kernel == "$uname" ) { auto r = CalamaresUtils::System::runCommand( - CalamaresUtils::System::RunLocation::RunInHost, { "/bin/uname", "-r" }, QString(), QString(), 3 ); + CalamaresUtils::System::RunLocation::RunInHost, { "/bin/uname", "-r" }, QString(), QString(), std::chrono::seconds( 3 ) ); if ( r.getExitCode() == 0 ) { m_kernel = r.getOutput(); diff --git a/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp b/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp index 292c768a9..9bcea6538 100644 --- a/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp +++ b/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp @@ -71,7 +71,7 @@ struct LuksDevice * Given a list of partitions (as set up by the partitioning module, * so there's maps with keys inside), returns just the list of * luks passphrases for each device. - */ + */ static QList< LuksDevice > getLuksDevices( const QVariantList& list ) { @@ -130,7 +130,7 @@ static bool setupLuks( const LuksDevice& d ) { auto r = CalamaresUtils::System::instance()->targetEnvCommand( - { "cryptsetup", "luksAddKey", d.device, keyfile }, QString(), d.passphrase, 15 ); + { "cryptsetup", "luksAddKey", d.device, keyfile }, QString(), d.passphrase, std::chrono::seconds( 15 ) ); if ( r.getExitCode() != 0 ) { cWarning() << "Could not configure LUKS keyfile on" << d.device << ':' << r.getOutput() << "(exit code" diff --git a/src/modules/shellprocess/ShellProcessJob.cpp b/src/modules/shellprocess/ShellProcessJob.cpp index d688540ae..ba3e9a299 100644 --- a/src/modules/shellprocess/ShellProcessJob.cpp +++ b/src/modules/shellprocess/ShellProcessJob.cpp @@ -75,7 +75,7 @@ ShellProcessJob::setConfigurationMap( const QVariantMap& configurationMap ) if ( configurationMap.contains( "script" ) ) { - m_commands = new CalamaresUtils::CommandList( configurationMap.value( "script" ), !dontChroot, timeout ); + m_commands = new CalamaresUtils::CommandList( configurationMap.value( "script" ), !dontChroot, std::chrono::seconds( timeout ) ); if ( m_commands->isEmpty() ) cDebug() << "ShellProcessJob: \"script\" contains no commands for" << moduleInstanceKey(); } diff --git a/src/modules/shellprocess/Tests.cpp b/src/modules/shellprocess/Tests.cpp index 488f4a7af..149fbc2d3 100644 --- a/src/modules/shellprocess/Tests.cpp +++ b/src/modules/shellprocess/Tests.cpp @@ -34,6 +34,8 @@ QTEST_GUILESS_MAIN( ShellProcessTests ) using CommandList = CalamaresUtils::CommandList; +using std::operator""s; + ShellProcessTests::ShellProcessTests() { @@ -68,8 +70,9 @@ ShellProcessTests::testProcessListSampleConfig() CalamaresUtils::yamlMapToVariant( doc ).toMap().value( "script" ) ); QVERIFY( !cl.isEmpty() ); QCOMPARE( cl.count(), 3 ); - QCOMPARE( cl.at(0).timeout(), -1 ); - QCOMPARE( cl.at(2).timeout(), 3600 ); // slowloris + + QCOMPARE( cl.at(0).timeout(), CalamaresUtils::CommandLine::TimeoutNotSet() ); + QCOMPARE( cl.at(2).timeout(), 3600s ); // slowloris } void ShellProcessTests::testProcessListFromList() @@ -105,9 +108,10 @@ script: "ls /tmp" )" ); CommandList cl( CalamaresUtils::yamlMapToVariant( doc ).toMap().value( "script" ) ); + QVERIFY( !cl.isEmpty() ); QCOMPARE( cl.count(), 1 ); - QCOMPARE( cl.at(0).timeout(), 10 ); + QCOMPARE( cl.at(0).timeout(), 10s ); QCOMPARE( cl.at(0).command(), QStringLiteral( "ls /tmp" ) ); // Not a string @@ -118,7 +122,6 @@ script: false CalamaresUtils::yamlMapToVariant( doc ).toMap().value( "script" ) ); QVERIFY( cl1.isEmpty() ); QCOMPARE( cl1.count(), 0 ); - } void ShellProcessTests::testProcessFromObject() @@ -130,9 +133,10 @@ script: )" ); CommandList cl( CalamaresUtils::yamlMapToVariant( doc ).toMap().value( "script" ) ); + QVERIFY( !cl.isEmpty() ); QCOMPARE( cl.count(), 1 ); - QCOMPARE( cl.at(0).timeout(), 20 ); + QCOMPARE( cl.at(0).timeout(), 20s ); QCOMPARE( cl.at(0).command(), QStringLiteral( "ls /tmp" ) ); } @@ -148,9 +152,9 @@ script: CalamaresUtils::yamlMapToVariant( doc ).toMap().value( "script" ) ); QVERIFY( !cl.isEmpty() ); QCOMPARE( cl.count(), 2 ); - QCOMPARE( cl.at(0).timeout(), 12 ); + QCOMPARE( cl.at(0).timeout(), 12s ); QCOMPARE( cl.at(0).command(), QStringLiteral( "ls /tmp" ) ); - QCOMPARE( cl.at(1).timeout(), -1 ); // not set + QCOMPARE( cl.at(1).timeout(), CalamaresUtils::CommandLine::TimeoutNotSet() ); // not set } void ShellProcessTests::testRootSubstitution() @@ -182,30 +186,31 @@ script: QVERIFY( gs != nullptr ); qDebug() << "Expect WARNING, ERROR, WARNING"; + // Doesn't use @@ROOT@@, so no failures - QVERIFY( bool(CommandList(plainScript, false, 10 ).run()) ); + QVERIFY( bool(CommandList(plainScript, false, 10s ).run()) ); // Doesn't use @@ROOT@@, but does chroot, so fails - QVERIFY( !bool(CommandList(plainScript, true, 10 ).run()) ); + QVERIFY( !bool(CommandList(plainScript, true, 10s ).run()) ); // Does use @@ROOT@@, which is not set, so fails - QVERIFY( !bool(CommandList(rootScript, false, 10 ).run()) ); + QVERIFY( !bool(CommandList(rootScript, false, 10s ).run()) ); // .. fails for two reasons - QVERIFY( !bool(CommandList(rootScript, true, 10 ).run()) ); + QVERIFY( !bool(CommandList(rootScript, true, 10s ).run()) ); gs->insert( "rootMountPoint", "/tmp" ); // Now that the root is set, two variants work .. still can't // chroot, unless the rootMountPoint contains a full system, // *and* we're allowed to chroot (ie. running tests as root). qDebug() << "Expect no output."; - QVERIFY( bool(CommandList(plainScript, false, 10 ).run()) ); - QVERIFY( bool(CommandList(rootScript, false, 10 ).run()) ); + QVERIFY( bool(CommandList(plainScript, false, 10s ).run()) ); + QVERIFY( bool(CommandList(rootScript, false, 10s ).run()) ); qDebug() << "Expect ERROR"; // But no user set yet - QVERIFY( !bool(CommandList(userScript, false, 10 ).run()) ); + QVERIFY( !bool(CommandList(userScript, false, 10s ).run()) ); // Now play dangerous games with shell expansion gs->insert( "username", "`id -u`" ); - QVERIFY( bool(CommandList(userScript, false, 10 ).run()) ); + QVERIFY( bool(CommandList(userScript, false, 10s ).run()) ); } diff --git a/src/modules/tracking/TrackingJobs.cpp b/src/modules/tracking/TrackingJobs.cpp index 7875ee6db..53884445e 100644 --- a/src/modules/tracking/TrackingJobs.cpp +++ b/src/modules/tracking/TrackingJobs.cpp @@ -125,7 +125,7 @@ sed -i "s,URI =.*,URI = http://releases.neon.kde.org/meta-release/${MACHINE_ID}, sed -i "s,URI_LTS =.*,URI_LTS = http://releases.neon.kde.org/meta-release-lts/${MACHINE_ID}," /etc/update-manager/meta-release true )x"), - 1); + std::chrono::seconds( 1 ) ); if ( r == 0 ) return Calamares::JobResult::ok(); diff --git a/src/modules/users/CreateUserJob.cpp b/src/modules/users/CreateUserJob.cpp index d7e356231..788ba0195 100644 --- a/src/modules/users/CreateUserJob.cpp +++ b/src/modules/users/CreateUserJob.cpp @@ -158,7 +158,7 @@ CreateUserJob::exec() if ( commandResult.getExitCode() ) { cError() << "useradd failed" << commandResult.getExitCode(); - return commandResult.explainProcess( useradd, 10 /* bogus timeout */ ); + return commandResult.explainProcess( useradd, std::chrono::seconds( 10 ) /* bogus timeout */ ); } commandResult = CalamaresUtils::System::instance()->targetEnvCommand( @@ -166,7 +166,7 @@ CreateUserJob::exec() if ( commandResult.getExitCode() ) { cError() << "usermod failed" << commandResult.getExitCode(); - return commandResult.explainProcess( "usermod", 10 ); + return commandResult.explainProcess( "usermod", std::chrono::seconds( 10 ) /* bogus timeout */ ); } QString userGroup = QString( "%1:%2" ).arg( m_userName ).arg( m_userName ); @@ -176,7 +176,7 @@ CreateUserJob::exec() if ( commandResult.getExitCode() ) { cError() << "chown failed" << commandResult.getExitCode(); - return commandResult.explainProcess( "chown", 10 ); + return commandResult.explainProcess( "chown", std::chrono::seconds( 10 ) /* bogus timeout */ ); } return Calamares::JobResult::ok();