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.

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;

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();
+        QProcess zipProcess;
-        zipProcess->setWorkingDirectory(CDirectoryUtils::binDirectory());
+        zipProcess.setProgram(getZip7Executable());
-        zipProcess->setProgram(QStringLiteral("7za"));
+        zipProcess.setArguments(args);
-        zipProcess->setArguments(args);
+        return runZip7Process(&zipProcess, stdOutAndError);
-
-        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;
-        }
-
     }
 
     bool CCompressUtils::hasZip7(QStringList *stdOutAndError)
     {
         // just display info
-        const bool isLinux = CBuildConfig::isRunningOnLinuxPlatform();
+        if (CBuildConfig::isRunningOnLinuxPlatform())
-        if (isLinux) { return CCompressUtils::whichZip7(stdOutAndError); }
+        {
+            return CCompressUtils::whichZip7(stdOutAndError);
+        }
 
-        // windows check
         QStringList args;
         args << "i";
         QProcess zipProcess;
-        zipProcess.setWorkingDirectory(CDirectoryUtils::binDirectory());
+        zipProcess.setProgram(getZip7Executable());
-        zipProcess.setProgram(QStringLiteral("7za"));
         zipProcess.setArguments(args);
-        zipProcess.start();
+        return runZip7Process(&zipProcess, stdOutAndError);
-        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;
     }
 
     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 (!zipProcess.waitForStarted()) { return false; }
-        if (!finished) { 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
+

src/blackmisc/compressutils.h

@@ -16,19 +16,23 @@
 #include <QByteArray>
 #include <QStringList>
 
+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
+        static bool runZip7Process(QProcess *zipProcess, QStringList *stdOutAndError);
-        CCompressUtils() {}
+
     };
 } // ns
 
 #endif // guard
+

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");