From 659b78832e74e89bc77765380e96d66f02e680e2 Mon Sep 17 00:00:00 2001 From: Roland Rossgotterer Date: Tue, 20 Nov 2018 14:12:19 +0100 Subject: [PATCH] Rewrite 7z uncompressing to work on all supported platforms On Windows we ship 7za.exe in our binaries. On MacOS we do the same, but the executable needs to be called with the full path. On Linux we don't ship anything but assume it to be available from the distribution. --- .../components/installxswiftbuscomponent.cpp | 2 +- src/blackmisc/compressutils.cpp | 128 +++++++++--------- src/blackmisc/compressutils.h | 11 +- tests/blackmisc/testcompress/testcompress.cpp | 2 +- 4 files changed, 75 insertions(+), 68 deletions(-) diff --git a/src/blackgui/components/installxswiftbuscomponent.cpp b/src/blackgui/components/installxswiftbuscomponent.cpp index 9e115fede..11c836de4 100644 --- a/src/blackgui/components/installxswiftbuscomponent.cpp +++ b/src/blackgui/components/installxswiftbuscomponent.cpp @@ -159,7 +159,7 @@ namespace BlackGui // if possible we will unzip QStringList stdOutAndError; - if (CCompressUtils::zip7Uncompress(destFile.absoluteFilePath(), xSwiftBusDirectory, true, &stdOutAndError)) + if (CCompressUtils::zip7Uncompress(destFile.absoluteFilePath(), xSwiftBusDirectory, &stdOutAndError)) { // capture values by copy! const CStatusMessage msg = CStatusMessage(this, CLogCategory::validation()).info("Uncompressed xSwiftBus in '%1'") << xSwiftBusDirectory; diff --git a/src/blackmisc/compressutils.cpp b/src/blackmisc/compressutils.cpp index 0ca814846..07d5349d7 100644 --- a/src/blackmisc/compressutils.cpp +++ b/src/blackmisc/compressutils.cpp @@ -31,7 +31,20 @@ namespace BlackMisc return lengthHeader; } - bool CCompressUtils::zip7Uncompress(const QString &file, const QString &directory, bool wait, QStringList *stdOutAndError) + //! Returns the platform specific 7za command + QString getZip7Executable() + { + QString executable; + if (CBuildConfig::isRunningOnMacOSPlatform()) + { + executable += CDirectoryUtils::binDirectory(); + executable += '/'; + } + executable += QStringLiteral("7za"); + return executable; + } + + bool CCompressUtils::zip7Uncompress(const QString &file, const QString &directory, QStringList *stdOutAndError) { const QFileInfo fi(file); if (!fi.exists()) { return false; } @@ -51,76 +64,26 @@ namespace BlackMisc if (!d.isEmpty()) { args << "-o" + d; } args << f; - QProcess *zipProcess = new QProcess(); - zipProcess->setWorkingDirectory(CDirectoryUtils::binDirectory()); - zipProcess->setProgram(QStringLiteral("7za")); - zipProcess->setArguments(args); - - if (wait) - { - - zipProcess->start(); - const bool finished = zipProcess->waitForFinished(); - if (zipProcess->exitStatus() != QProcess::NormalExit) { return false; } - if (!finished) { return false; } - const int r = zipProcess->exitCode(); - if (stdOutAndError) - { - const QString pStdout = zipProcess->readAllStandardOutput(); - const QString pStderr = zipProcess->readAllStandardError(); - stdOutAndError->clear(); - stdOutAndError->push_back(pStdout); - stdOutAndError->push_back(pStderr); - } - zipProcess->deleteLater(); - return r == 0; - } - else - { - // FIXME: zipProcess is leaked here. - zipProcess->start(); - return true; - } - + QProcess zipProcess; + zipProcess.setProgram(getZip7Executable()); + zipProcess.setArguments(args); + return runZip7Process(&zipProcess, stdOutAndError); } bool CCompressUtils::hasZip7(QStringList *stdOutAndError) { // just display info - const bool isLinux = CBuildConfig::isRunningOnLinuxPlatform(); - if (isLinux) { return CCompressUtils::whichZip7(stdOutAndError); } + if (CBuildConfig::isRunningOnLinuxPlatform()) + { + return CCompressUtils::whichZip7(stdOutAndError); + } - // windows check QStringList args; args << "i"; QProcess zipProcess; - zipProcess.setWorkingDirectory(CDirectoryUtils::binDirectory()); - zipProcess.setProgram(QStringLiteral("7za")); + zipProcess.setProgram(getZip7Executable()); zipProcess.setArguments(args); - zipProcess.start(); - const bool finished = zipProcess.waitForFinished(); - - if (stdOutAndError) - { - stdOutAndError->clear(); - const QString pStdout = zipProcess.readAllStandardOutput(); - const QString pStderr = zipProcess.readAllStandardError(); - if (pStdout.isEmpty() && pStderr.isEmpty()) - { - stdOutAndError->push_back("Checking 7za"); - stdOutAndError->push_back("No 7za or failing"); - } - else - { - stdOutAndError->push_back(pStdout); - stdOutAndError->push_back(pStderr); - } - } - - if (zipProcess.exitStatus() != QProcess::NormalExit) { return false; } - if (!finished) { return false; } - const int r = zipProcess.exitCode(); - return r == 0; + return runZip7Process(&zipProcess, stdOutAndError); } bool CCompressUtils::whichZip7(QStringList *stdOutAndError) @@ -128,8 +91,8 @@ namespace BlackMisc const QString cmd("which 7za"); QProcess zipProcess; zipProcess.start(cmd); - const bool finished = zipProcess.waitForFinished(); - if (!finished) { return false; } + if (!zipProcess.waitForStarted()) { return false; } + if (!zipProcess.waitForFinished()) { return false; } const QString pStdout = zipProcess.readAllStandardOutput(); const QString pStderr = zipProcess.readAllStandardError(); @@ -142,4 +105,43 @@ namespace BlackMisc const int r = zipProcess.exitCode(); return r == 0 && pStdout.contains("7za", Qt::CaseInsensitive); } + + bool CCompressUtils::runZip7Process(QProcess *zipProcess, QStringList *stdOutAndError) + { + zipProcess->start(); + + // If process does not even start, e.g. because no 7za exe found. + if (!zipProcess->waitForStarted()) + { + if (stdOutAndError) + { + stdOutAndError->push_back("7za"); + stdOutAndError->push_back("Command not found"); + } + return false; + } + + // If process does not finish. Very unlikely. + if (!zipProcess->waitForFinished()) + { + if (stdOutAndError) + { + stdOutAndError->push_back("7za"); + stdOutAndError->push_back("Process did not finish."); + } + return false; + } + + if (stdOutAndError) + { + stdOutAndError->clear(); + const QString pStdout = zipProcess->readAllStandardOutput(); + const QString pStderr = zipProcess->readAllStandardError(); + stdOutAndError->push_back(pStdout); + stdOutAndError->push_back(pStderr); + } + + return zipProcess->exitStatus() == QProcess::NormalExit; + } } // ns + diff --git a/src/blackmisc/compressutils.h b/src/blackmisc/compressutils.h index a5fc30939..574f92671 100644 --- a/src/blackmisc/compressutils.h +++ b/src/blackmisc/compressutils.h @@ -16,19 +16,23 @@ #include #include +class QProcess; + namespace BlackMisc { //! Compress utilities class BLACKMISC_EXPORT CCompressUtils { public: + CCompressUtils() = delete; + //! Length header //! \remark 4 bytes -> 32bit static QByteArray lengthHeader(qint32 size); //! Unzip my using 7zip //! \remark relies on external 7zip command line - static bool zip7Uncompress(const QString &file, const QString &directory, bool wait, QStringList *stdOutAndError = nullptr); + static bool zip7Uncompress(const QString &file, const QString &directory, QStringList *stdOutAndError = nullptr); //! External program existing? //! \remark relies on external 7zip command line @@ -39,9 +43,10 @@ namespace BlackMisc static bool whichZip7(QStringList *stdOutAndError = nullptr); private: - //! Ctor - CCompressUtils() {} + static bool runZip7Process(QProcess *zipProcess, QStringList *stdOutAndError); + }; } // ns #endif // guard + diff --git a/tests/blackmisc/testcompress/testcompress.cpp b/tests/blackmisc/testcompress/testcompress.cpp index 7f6beacdf..f12b9eed3 100644 --- a/tests/blackmisc/testcompress/testcompress.cpp +++ b/tests/blackmisc/testcompress/testcompress.cpp @@ -66,7 +66,7 @@ namespace BlackMiscTest const QString td = tempDir.path(); const QString compressedFile(CFileUtils::appendFilePaths(CDirectoryUtils::shareTestDirectory(), "countries.json.gz")); const QString unCompressedFile(CFileUtils::appendFilePaths(td, "countries.json")); - const bool c = CCompressUtils::zip7Uncompress(compressedFile, td, true); + const bool c = CCompressUtils::zip7Uncompress(compressedFile, td); QVERIFY2(c, "Uncompressing failed");