diff --git a/kerfuffle/archiveinterface.h b/kerfuffle/archiveinterface.h --- a/kerfuffle/archiveinterface.h +++ b/kerfuffle/archiveinterface.h @@ -75,8 +75,7 @@ /** * List archive contents. * This runs the process of reading archive contents. - * When subclassing, you can block as long as you need, the function runs - * in its own thread. + * When subclassing, you can block as long as you need (unless you called setWaitForFinishedSignal(true)). * @returns whether the listing succeeded. * @note If returning false, make sure to emit the error() signal beforewards to notify * the user of the error condition. @@ -90,8 +89,7 @@ * Globally recognized extraction options: * @li PreservePaths - preserve file paths (extract flat if false) * @li RootNode - node in the archive which will correspond to the @arg destinationDirectory - * When subclassing, you can block as long as you need, the function runs - * in its own thread. + * When subclassing, you can block as long as you need (unless you called setWaitForFinishedSignal(true)). * @returns whether the listing succeeded. * @note If returning false, make sure to emit the error() signal beforewards to notify * the user of the error condition. @@ -120,10 +118,8 @@ protected: /** - * Setting this option to true will not exit the thread with the - * exit of the various functions, but rather when finished(bool) is - * called. Doing this one can use the event loop easily while doing - * the operation. + * Setting this option to true will not run the functions in their own thread. + * Instead it will be necessary to call finished(bool) when the operation is actually finished. */ void setWaitForFinishedSignal(bool value); diff --git a/kerfuffle/cliinterface.h b/kerfuffle/cliinterface.h --- a/kerfuffle/cliinterface.h +++ b/kerfuffle/cliinterface.h @@ -38,6 +38,7 @@ class KPtyProcess; class QDir; +class QTemporaryDir; namespace Kerfuffle { @@ -355,13 +356,12 @@ /** * Run @p programName with the given @p arguments. - * The method waits until @p programName is finished to exit. * * @param programName The program that will be run (not the whole path). * @param arguments A list of arguments that will be passed to the program. * - * @return @c true if the program was found and the process ran correctly, - * @c false otherwise. + * @return @c true if the program was found and the process was started correctly, + * @c false otherwise (in which case finished(false) is emitted). */ bool runProcess(const QStringList& programNames, const QStringList& arguments); @@ -421,6 +421,8 @@ */ bool isEmptyDir(const QDir &dir); + void copyProcessCleanup(); + QByteArray m_stdOutData; QRegularExpression m_passwordPromptPattern; QHash > m_patternCache; @@ -436,8 +438,16 @@ bool m_abortingOperation; QString m_storedFileName; + CompressionOptions m_compressionOptions; + QString m_oldWorkingDir; + QString m_extractDestDir; + QTemporaryDir *m_extractTempDir; + QVariantList m_copiedFiles; + private slots: void processFinished(int exitCode, QProcess::ExitStatus exitStatus); + void copyProcessFinished(int exitCode, QProcess::ExitStatus exitStatus); + }; } diff --git a/kerfuffle/cliinterface.cpp b/kerfuffle/cliinterface.cpp --- a/kerfuffle/cliinterface.cpp +++ b/kerfuffle/cliinterface.cpp @@ -60,7 +60,8 @@ : ReadWriteArchiveInterface(parent, args), m_process(0), m_listEmptyLines(false), - m_abortingOperation(false) + m_abortingOperation(false), + m_extractTempDir(Q_NULLPTR) { //because this interface uses the event loop setWaitForFinishedSignal(true); @@ -108,7 +109,6 @@ return false; } - emit finished(true); return true; } @@ -118,6 +118,9 @@ cacheParameterList(); m_operationMode = Copy; + m_compressionOptions = options; + m_copiedFiles = files; + m_extractDestDir = destinationDirectory; const QStringList extractArgs = m_param.value(ExtractArgs).toStringList(); if (extractArgs.contains(QStringLiteral("$PasswordSwitch")) && @@ -139,77 +142,31 @@ QUrl destDir = QUrl(destinationDirectory); QDir::setCurrent(destDir.adjusted(QUrl::RemoveScheme).url()); - QString oldCurrentDir; bool useTmpExtractDir = options.value(QStringLiteral("DragAndDrop")).toBool() || options.value(QStringLiteral("AlwaysUseTmpDir")).toBool(); - QTemporaryDir tmpExtractDir(QApplication::applicationName() + QLatin1Char('-')); if (useTmpExtractDir) { - qCDebug(ARK) << "Using temporary extraction dir:" << tmpExtractDir.path(); - if (!tmpExtractDir.isValid()) { + + Q_ASSERT(!m_extractTempDir); + m_extractTempDir = new QTemporaryDir(QApplication::applicationName() + QLatin1Char('-')); + + qCDebug(ARK) << "Using temporary extraction dir:" << m_extractTempDir->path(); + if (!m_extractTempDir->isValid()) { qCDebug(ARK) << "Creation of temporary directory failed."; failOperation(); + emit finished(false); return false; } - oldCurrentDir = QDir::currentPath(); - destDir = QUrl(tmpExtractDir.path()); + m_oldWorkingDir = QDir::currentPath(); + destDir = QUrl(m_extractTempDir->path()); QDir::setCurrent(destDir.adjusted(QUrl::RemoveScheme).url()); } if (!runProcess(m_param.value(ExtractProgram).toStringList(), args)) { failOperation(); return false; } - if (options.value(QStringLiteral("AlwaysUseTmpDir")).toBool()) { - // unar exits with code 1 if extraction fails. - // This happens at least with wrong passwords or not enough space in the destination folder. - if (m_exitCode == 1) { - if (password().isEmpty()) { - qCWarning(ARK) << "Extraction aborted, destination folder might not have enough space."; - emit error(i18n("Extraction failed. Make sure that enough space is available.")); - } else { - qCWarning(ARK) << "Extraction aborted, either the password is wrong or the destination folder doesn't have enough space."; - emit error(i18n("Extraction failed. Make sure you provided the correct password and that enough space is available.")); - setPassword(QString()); - } - - emit finished(false); - failOperation(); - // If we don't do this, the temporary directory will not autodelete itself upon destruction. - QDir::setCurrent(oldCurrentDir); - - return false; - } - - if (!options.value(QStringLiteral("DragAndDrop")).toBool()) { - if (!moveToDestination(QDir::current(), QDir(destinationDirectory), options[QStringLiteral("PreservePaths")].toBool())) { - emit error(i18ncp("@info", - "Could not move the extracted file to the destination directory.", - "Could not move the extracted files to the destination directory.", - files.size())); - emit finished(false); - return false; - } - // If we don't do this, the temporary directory will not autodelete itself upon destruction. - QDir::setCurrent(oldCurrentDir); - } - } - - if (options.value(QStringLiteral("DragAndDrop")).toBool()) { - if (!moveDroppedFilesToDest(files, destinationDirectory)) { - emit error(i18ncp("@info", - "Could not move the extracted file to the destination directory.", - "Could not move the extracted files to the destination directory.", - files.size())); - emit finished(false); - return false; - } - // If we don't do this, the temporary directory will not autodelete itself upon destruction. - QDir::setCurrent(oldCurrentDir); - } - - emit finished(true); return true; } @@ -231,7 +188,7 @@ failOperation(); return false; } - emit finished(true); + return true; } @@ -266,12 +223,14 @@ failOperation(); return false; } - emit finished(true); + return true; } bool CliInterface::runProcess(const QStringList& programNames, const QStringList& arguments) { + Q_ASSERT(!m_process); + QString programPath; for (int i = 0; i < programNames.count(); i++) { programPath = QStandardPaths::findExecutable(programNames.at(i)); @@ -288,50 +247,49 @@ qCDebug(ARK) << "Executing" << programPath << arguments << "within directory" << QDir::currentPath(); - if (m_process) { - m_process->waitForFinished(); - delete m_process; - } - #ifdef Q_OS_WIN m_process = new KProcess; #else m_process = new KPtyProcess; m_process->setPtyChannels(KPtyProcess::StdinChannel); - QEventLoop loop; - connect(m_process, static_cast(&KPtyProcess::finished), &loop, &QEventLoop::quit, Qt::DirectConnection); #endif m_process->setOutputChannelMode(KProcess::MergedChannels); m_process->setNextOpenMode(QIODevice::ReadWrite | QIODevice::Unbuffered | QIODevice::Text); m_process->setProgram(programPath, arguments); connect(m_process, SIGNAL(readyReadStandardOutput()), SLOT(readStdout()), Qt::DirectConnection); - connect(m_process, static_cast(&KPtyProcess::finished), this, &CliInterface::processFinished, Qt::DirectConnection); + + if (m_operationMode == Copy) { + // Extraction jobs need a dedicated post-processing function. + connect(m_process, static_cast(&KPtyProcess::finished), this, &CliInterface::copyProcessFinished, Qt::DirectConnection); + } else { + connect(m_process, static_cast(&KPtyProcess::finished), this, &CliInterface::processFinished, Qt::DirectConnection); + } m_stdOutData.clear(); m_process->start(); -#ifdef Q_OS_WIN - bool ret = m_process->waitForFinished(-1); -#else - bool ret = (loop.exec(QEventLoop::WaitForMoreEvents | QEventLoop::ExcludeUserInputEvents) == 0); -#endif - - Q_ASSERT(!m_process); - - return ret; + return true; } void CliInterface::processFinished(int exitCode, QProcess::ExitStatus exitStatus) { m_exitCode = exitCode; qCDebug(ARK) << "Process finished, exitcode:" << exitCode << "exitstatus:" << exitStatus; - //if the m_process pointer is gone, then there is nothing to worry - //about here - if (!m_process) { + if (m_process) { + //handle all the remaining data in the process + readStdout(true); + + delete m_process; + m_process = Q_NULLPTR; + } + + // #193908 - #222392 + // Don't emit finished() if the job was killed quietly. + if (m_abortingOperation) { return; } @@ -341,20 +299,80 @@ } } - //handle all the remaining data in the process - readStdout(true); - - delete m_process; - m_process = 0; - emit progress(1.0); if (m_operationMode == Add) { list(); - return; + } else { + emit finished(true); } } +void CliInterface::copyProcessFinished(int exitCode, QProcess::ExitStatus exitStatus) +{ + Q_ASSERT(m_operationMode == Copy); + + m_exitCode = exitCode; + qCDebug(ARK) << "Extraction process finished, exitcode:" << exitCode << "exitstatus:" << exitStatus; + + if (m_process) { + // Handle all the remaining data in the process. + readStdout(true); + + delete m_process; + m_process = Q_NULLPTR; + } + + if (m_compressionOptions.value(QStringLiteral("AlwaysUseTmpDir")).toBool()) { + // unar exits with code 1 if extraction fails. + // This happens at least with wrong passwords or not enough space in the destination folder. + if (m_exitCode == 1) { + if (password().isEmpty()) { + qCWarning(ARK) << "Extraction aborted, destination folder might not have enough space."; + emit error(i18n("Extraction failed. Make sure that enough space is available.")); + } else { + qCWarning(ARK) << "Extraction aborted, either the password is wrong or the destination folder doesn't have enough space."; + emit error(i18n("Extraction failed. Make sure you provided the correct password and that enough space is available.")); + setPassword(QString()); + } + copyProcessCleanup(); + emit finished(false); + return; + } + + if (!m_compressionOptions.value(QStringLiteral("DragAndDrop")).toBool()) { + if (!moveToDestination(QDir::current(), QDir(m_extractDestDir), m_compressionOptions[QStringLiteral("PreservePaths")].toBool())) { + emit error(i18ncp("@info", + "Could not move the extracted file to the destination directory.", + "Could not move the extracted files to the destination directory.", + m_copiedFiles.size())); + copyProcessCleanup(); + emit finished(false); + return; + } + + copyProcessCleanup(); + } + } + + if (m_compressionOptions.value(QStringLiteral("DragAndDrop")).toBool()) { + if (!moveDroppedFilesToDest(m_copiedFiles, m_extractDestDir)) { + emit error(i18ncp("@info", + "Could not move the extracted file to the destination directory.", + "Could not move the extracted files to the destination directory.", + m_copiedFiles.size())); + copyProcessCleanup(); + emit finished(false); + return; + } + + copyProcessCleanup(); + } + + emit progress(1.0); + emit finished(true); +} + bool CliInterface::moveDroppedFilesToDest(const QVariantList &files, const QString &finalDest) { // Move extracted files from a QTemporaryDir to the final destination. @@ -445,6 +463,18 @@ return d.count() == 0; } +void CliInterface::copyProcessCleanup() +{ + if (!m_oldWorkingDir.isEmpty()) { + QDir::setCurrent(m_oldWorkingDir); + } + + if (m_extractTempDir) { + delete m_extractTempDir; + m_extractTempDir = Q_NULLPTR; + } +} + bool CliInterface::moveToDestination(const QDir &tempDir, const QDir &destDir, bool preservePaths) { qCDebug(ARK) << "Moving extracted files from temp dir" << tempDir.path() << "to final destination" << destDir.path(); diff --git a/part/jobtracker.cpp b/part/jobtracker.cpp --- a/part/jobtracker.cpp +++ b/part/jobtracker.cpp @@ -42,7 +42,6 @@ { foreach(KJob *job, m_jobs) { job->kill(); - delete job; } } diff --git a/plugins/cliunarchiverplugin/cliplugin.cpp b/plugins/cliunarchiverplugin/cliplugin.cpp --- a/plugins/cliunarchiverplugin/cliplugin.cpp +++ b/plugins/cliunarchiverplugin/cliplugin.cpp @@ -76,7 +76,6 @@ } } - emit finished(true); return true; }