GH-2475 fix reporting missing local libraries on launch

4 files changed, 61 insertions, 62 deletions

diff --git a/api/logic/launch/steps/Update.cpp b/api/logic/launch/steps/Update.cpp
index 65f24391..0e138cf6 100644
--- a/api/logic/launch/steps/Update.cpp
+++ b/api/logic/launch/steps/Update.cpp
@@ -49,7 +49,7 @@ void Update::updateFinished()
     }
     else
     {
-        QString reason = tr("Instance update failed because: %1.\n\n").arg(m_updateTask->failReason());
+        QString reason = tr("Instance update failed because: %1\n\n").arg(m_updateTask->failReason());
         m_updateTask.reset();
         emit logLine(reason, MessageLevel::Fatal);
         emitFailed(reason);
diff --git a/api/logic/minecraft/Library.cpp b/api/logic/minecraft/Library.cpp
index 4c032ef3..a56e8110 100644
--- a/api/logic/minecraft/Library.cpp
+++ b/api/logic/minecraft/Library.cpp
@@ -45,58 +45,50 @@ void Library::getApplicableFiles(OpSys system, QStringList& jar, QStringList& na
     }
 }
 
-QList< std::shared_ptr< NetAction > > Library::getDownloads(OpSys system, class HttpMetaCache* cache,
-                                                            QStringList& failedFiles, const QString & overridePath) const
+QList< std::shared_ptr< NetAction > > Library::getDownloads(
+    OpSys system,
+    class HttpMetaCache* cache,
+    QStringList& failedLocalFiles,
+    const QString & overridePath
+) const
 {
     QList<NetActionPtr> out;
-    bool isAlwaysStale = (hint() == "always-stale");
+    bool stale = isAlwaysStale();
     bool local = isLocal();
-    bool isForge = (hint() == "forge-pack-xz");
+
+    auto check_local_file = [&](QString storage)
+    {
+        QFileInfo fileinfo(storage);
+        QString fileName = fileinfo.fileName();
+        auto fullPath = FS::PathCombine(overridePath, fileName);
+        QFileInfo localFileInfo(fullPath);
+        if(!localFileInfo.exists())
+        {
+            failedLocalFiles.append(localFileInfo.filePath());
+            return false;
+        }
+        return true;
+    };
 
     auto add_download = [&](QString storage, QString url, QString sha1)
     {
+        if(local)
+        {
+            return check_local_file(storage);
+        }
         auto entry = cache->resolveEntry("libraries", storage);
-        if(isAlwaysStale)
+        if(stale)
         {
             entry->setStale(true);
         }
         if (!entry->isStale())
             return true;
-        if(local)
-        {
-            if(!overridePath.isEmpty())
-            {
-                QString fileName;
-                int position = storage.lastIndexOf('/');
-                if(position == -1)
-                {
-                    fileName = storage;
-                }
-                else
-                {
-                    fileName = storage.mid(position);
-                }
-                auto fullPath = FS::PathCombine(overridePath, fileName);
-                QFileInfo fileinfo(fullPath);
-                if(fileinfo.exists())
-                {
-                    return true;
-                }
-            }
-            QFileInfo fileinfo(entry->getFullPath());
-            if(!fileinfo.exists())
-            {
-                failedFiles.append(entry->getFullPath());
-                return false;
-            }
-            return true;
-        }
         Net::Download::Options options;
-        if(isAlwaysStale)
+        if(stale)
         {
             options |= Net::Download::Option::AcceptLocalFiles;
         }
-        if (isForge)
+        if (isForge())
         {
             qDebug() << "XzDownload for:" << rawName() << "storage:" << storage << "url:" << url;
             out.append(ForgeXzDownload::make(url, storage, entry));
@@ -178,7 +170,8 @@ QList< std::shared_ptr< NetAction > > Library::getDownloads(OpSys system, class
     }
     else
     {
-        auto raw_dl = [&](){
+        auto raw_dl = [&]()
+        {
             if (!m_absoluteURL.isEmpty())
             {
                 return m_absoluteURL;
@@ -245,6 +238,16 @@ bool Library::isLocal() const
     return m_hint == "local";
 }
 
+bool Library::isAlwaysStale() const
+{
+    return m_hint == "always-stale";
+}
+
+bool Library::isForge() const
+{
+    return m_hint == "forge-pack-xz";
+}
+
 void Library::setStoragePrefix(QString prefix)
 {
     m_storagePrefix = prefix;
diff --git a/api/logic/minecraft/Library.h b/api/logic/minecraft/Library.h
index 5fcff316..014f6745 100644
--- a/api/logic/minecraft/Library.h
+++ b/api/logic/minecraft/Library.h
@@ -148,9 +148,15 @@ public: /* methods */
     /// Returns true if the library is contained in an instance and false if it is shared
     bool isLocal() const;
 
+    /// Returns true if the library is to always be checked for updates
+    bool isAlwaysStale() const;
+
+    /// Return true if the library requires forge XZ hacks
+    bool isForge() const;
+
     // Get a list of downloads for this library
     QList<NetActionPtr> getDownloads(OpSys system, class HttpMetaCache * cache,
-                                     QStringList & failedFiles, const QString & overridePath) const;
+                                     QStringList & failedLocalFiles, const QString & overridePath) const;
 
 private: /* methods */
     /// the default storage prefix used by MultiMC
diff --git a/api/logic/minecraft/update/LibrariesTask.cpp b/api/logic/minecraft/update/LibrariesTask.cpp
index 6dcb149c..56fa1fc4 100644
--- a/api/logic/minecraft/update/LibrariesTask.cpp
+++ b/api/logic/minecraft/update/LibrariesTask.cpp
@@ -22,42 +22,32 @@ void LibrariesTask::executeTask()
     downloadJob.reset(job);
 
     auto metacache = ENV.metacache();
-    QList<LibraryPtr> brokenLocalLibs;
-    QStringList failedFiles;
-    auto createJob = [&](const LibraryPtr & lib)
+    QStringList failedLocalFiles;
+
+    QList<LibraryPtr> artifactPool;
+    artifactPool.append(profile->getLibraries());
+    artifactPool.append(profile->getNativeLibraries());
+    artifactPool.append(profile->getJarMods());
+    artifactPool.append(profile->getMainJar());
+    for (auto lib : artifactPool)
     {
         if(!lib)
         {
             emitFailed(tr("Null jar is specified in the metadata, aborting."));
             return;
         }
-        auto dls = lib->getDownloads(currentSystem, metacache.get(), failedFiles, inst->getLocalLibraryPath());
+        auto dls = lib->getDownloads(currentSystem, metacache.get(), failedLocalFiles, inst->getLocalLibraryPath());
         for(auto dl : dls)
         {
             downloadJob->addNetAction(dl);
         }
-    };
-    auto createJobs = [&](const QList<LibraryPtr> & libs)
-    {
-        for (auto lib : libs)
-        {
-            createJob(lib);
-        }
-    };
-    createJobs(profile->getLibraries());
-    createJobs(profile->getNativeLibraries());
-    createJobs(profile->getJarMods());
-    createJob(profile->getMainJar());
+    }
 
-    // FIXME: this is never filled!!!!
-    if (!brokenLocalLibs.empty())
+    if (!failedLocalFiles.empty())
     {
         downloadJob.reset();
-        QString failed_all = failedFiles.join("\n");
-        emitFailed(tr("Some libraries marked as 'local' are missing their jar "
-                    "files:\n%1\n\nYou'll have to correct this problem manually. If this is "
-                    "an externally tracked instance, make sure to run it at least once "
-                    "outside of MultiMC.").arg(failed_all));
+        QString failed_all = failedLocalFiles.join("\n");
+        emitFailed(tr("Some libraries marked as 'local' are missing their jar files:\n%1\n\nYou'll have to correct this problem manually.").arg(failed_all));
         return;
     }
     connect(downloadJob.get(), &NetJob::succeeded, this, &LibrariesTask::emitSucceeded);