From 9e986201ba342836260c6db6e532db04d31153d9 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 26 Nov 2013 09:25:10 -0500 Subject: MAINT-3489: Make Mac updater script detect prematurely deleted .dmg. Also, delete it when done. --- .../updater/scripts/darwin/update_install.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'indra/viewer_components/updater') diff --git a/indra/viewer_components/updater/scripts/darwin/update_install.py b/indra/viewer_components/updater/scripts/darwin/update_install.py index 10d507c9ef..08f4f0ebb9 100755 --- a/indra/viewer_components/updater/scripts/darwin/update_install.py +++ b/indra/viewer_components/updater/scripts/darwin/update_install.py @@ -199,6 +199,11 @@ def main(dmgfile, markerfile, markertext): # prepare for other cleanup with Janitor(LOGF) as janitor: + # Under some circumstances, this script seems to be invoked with a + # nonexistent pathname. Check for that. + if not os.path.isfile(dmgfile): + fail(dmgfile + " has been deleted") + # Try to derive the name of the running viewer app bundle from our # own pathname. (Hopefully the old viewer won't copy this script # to a temp dir before running!) @@ -376,6 +381,13 @@ def main(dmgfile, markerfile, markertext): log(' '.join(command)) subprocess.check_call(command, stdout=LOGF, stderr=subprocess.STDOUT) + # If all the above succeeded, delete the .dmg file. We don't do this + # as a janitor.later() operation because we only want to do it if we + # get this far successfully. Note that this is out of the scope of the + # Janitor: we must detach the .dmg before removing it! + log("rm " + dmgfile) + os.remove(dmgfile) + except Exception, err: # Because we carefully set sys.excepthook -- and even modify it to log # the problem once we have our log file open -- you might think we -- cgit v1.2.3 From a55f88109fc39061922b518e0d6b309764f72428 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 26 Nov 2013 17:18:28 -0500 Subject: MAINT-3489: Gracefully handle missing downloaded installer file. If a marker file indicates that an installer has been downloaded and verified and is ready to install, but that installer file does not exist, log it and revert to checking for update. Also add logging to every code path that can remove the installer explicitly, in case the missing installer is actually being killed off by some weird sequence of state-machine transitions. --- .../updater/llupdatedownloader.cpp | 52 ++++++++++++-------- .../viewer_components/updater/llupdaterservice.cpp | 56 ++++++++++++++-------- 2 files changed, 69 insertions(+), 39 deletions(-) (limited to 'indra/viewer_components/updater') diff --git a/indra/viewer_components/updater/llupdatedownloader.cpp b/indra/viewer_components/updater/llupdatedownloader.cpp index c28ad76c77..c42112af80 100755 --- a/indra/viewer_components/updater/llupdatedownloader.cpp +++ b/indra/viewer_components/updater/llupdatedownloader.cpp @@ -77,7 +77,8 @@ private: void run(void); void startDownloading(LLURI const & uri, std::string const & hash); void throwOnCurlError(CURLcode code); - bool validateDownload(void); + bool validateDownload(const std::string& filePath); + bool validateOrRemove(const std::string& filePath); LOG_CLASS(LLUpdateDownloader::Implementation); }; @@ -295,9 +296,8 @@ void LLUpdateDownloader::Implementation::resume(void) { resumeDownloading(fileStatus.st_size); } - else if(!validateDownload()) + else if(!validateOrRemove(filePath)) { - LLFile::remove(filePath); download(LLURI(mDownloadData["url"].asString()), mDownloadData["hash"].asString(), mDownloadData["update_channel"].asString(), @@ -421,19 +421,13 @@ void LLUpdateDownloader::Implementation::run(void) if(code == CURLE_OK) { LLFile::remove(mDownloadRecordPath); - if(validateDownload()) + if(validateOrRemove(mDownloadData["path"])) { LL_INFOS("UpdaterService") << "download successful" << LL_ENDL; mClient.downloadComplete(mDownloadData); } else { - LL_INFOS("UpdaterService") << "download failed hash check" << LL_ENDL; - std::string filePath = mDownloadData["path"].asString(); - if(filePath.size() != 0) - { - LLFile::remove(filePath); - } mClient.downloadError("failed hash check"); } } @@ -449,7 +443,9 @@ void LLUpdateDownloader::Implementation::run(void) LLFile::remove(mDownloadRecordPath); if(mDownloadData.has("path")) { - LLFile::remove(mDownloadData["path"].asString()); + std::string filePath = mDownloadData["path"].asString(); + LL_INFOS("UpdaterService") << "removing " << filePath << LL_ENDL; + LLFile::remove(filePath); } mClient.downloadError("curl error"); } @@ -561,31 +557,49 @@ void LLUpdateDownloader::Implementation::throwOnCurlError(CURLcode code) } } +bool LLUpdateDownloader::Implementation::validateOrRemove(const std::string& filePath) +{ + bool valid = validateDownload(filePath); + if (! valid) + { + LL_INFOS("UpdaterService") << "removing " << filePath << LL_ENDL; + LLFile::remove(filePath); + } + return valid; +} -bool LLUpdateDownloader::Implementation::validateDownload(void) +bool LLUpdateDownloader::Implementation::validateDownload(const std::string& filePath) { - std::string filePath = mDownloadData["path"].asString(); llifstream fileStream(filePath, std::ios_base::in | std::ios_base::binary); if(!fileStream) { + LL_INFOS("UpdaterService") << "can't open " << filePath << ", invalid" << LL_ENDL; return false; } std::string hash = mDownloadData["hash"].asString(); - if(hash.size() != 0) + if (! hash.empty()) { - LL_INFOS("UpdaterService") << "checking hash..." << LL_ENDL; char digest[33]; LLMD5(fileStream).hex_digest(digest); - if(hash != digest) + if (hash == digest) + { + LL_INFOS("UpdaterService") << "verified hash " << hash + << " for downloaded " << filePath << LL_ENDL; + return true; + } + else { - LL_WARNS("UpdaterService") << "download hash mismatch; expected " << hash << - " but download is " << digest << LL_ENDL; + LL_WARNS("UpdaterService") << "download hash mismatch for " + << filePath << ": expected " << hash + << " but computed " << digest << LL_ENDL; + return false; } - return hash == digest; } else { + LL_INFOS("UpdaterService") << "no hash specified for " << filePath + << ", unverified" << LL_ENDL; return true; // No hash check provided. } } diff --git a/indra/viewer_components/updater/llupdaterservice.cpp b/indra/viewer_components/updater/llupdaterservice.cpp index 16950e1d62..cb3be5bbdc 100755 --- a/indra/viewer_components/updater/llupdaterservice.cpp +++ b/indra/viewer_components/updater/llupdaterservice.cpp @@ -296,37 +296,49 @@ bool LLUpdaterServiceImpl::checkForInstall(bool launchInstaller) update_marker.close(); // Get the path to the installer file. - LLSD path = update_info.get("path"); - if(update_info["current_version"].asString() != ll_get_version()) + std::string path(update_info.get("path")); + std::string downloader_version(update_info["current_version"]); + if (downloader_version != ll_get_version()) { // This viewer is not the same version as the one that downloaded - // the update. Do not install this update. - if(!path.asString().empty()) + // the update. Do not install this update. + LL_INFOS("UpdaterService") << "ignoring update downloaded by " + << "different viewer version " + << downloader_version << LL_ENDL; + if (! path.empty()) { - LL_INFOS("UpdaterService") << "ignoring update dowloaded by different client version" << LL_ENDL;; - LLFile::remove(path.asString()); + LL_INFOS("UpdaterService") << "removing " << path << LL_ENDL; + LLFile::remove(path); LLFile::remove(update_marker_path()); } - else - { - ; // Nothing to clean up. - } - + foundInstall = false; } - else if(path.isDefined() && !path.asString().empty()) + else if (path.empty()) + { + LL_WARNS("UpdaterService") << "Marker file " << update_marker_path() + << " 'path' entry empty, ignoring" << LL_ENDL; + foundInstall = false; + } + else if (! LLFile::isfile(path)) + { + LL_WARNS("UpdaterService") << "Nonexistent installer " << path + << ", ignoring" << LL_ENDL; + foundInstall = false; + } + else { if(launchInstaller) { setState(LLUpdaterService::INSTALLING); - + LLFile::remove(update_marker_path()); int result = ll_install_update(install_script_path(), - update_info["path"].asString(), + path, update_info["required"].asBoolean(), install_script_mode()); - + if((result == 0) && mAppExitCallback) { mAppExitCallback(); @@ -360,7 +372,8 @@ bool LLUpdaterServiceImpl::checkForResume() LLSD download_info; LLSDSerialize::fromXMLDocument(download_info, download_marker_stream); download_marker_stream.close(); - if(download_info["current_version"].asString() == ll_get_version()) + std::string downloader_version(download_info["current_version"]); + if (downloader_version == ll_get_version()) { mIsDownloading = true; mNewVersion = download_info["update_version"].asString(); @@ -371,10 +384,13 @@ bool LLUpdaterServiceImpl::checkForResume() else { // The viewer that started this download is not the same as this viewer; ignore. - LL_INFOS("UpdaterService") << "ignoring partial download from different viewer version" << LL_ENDL;; + LL_INFOS("UpdaterService") << "ignoring partial download " + << "from different viewer version " + << downloader_version << LL_ENDL; std::string path = download_info["path"].asString(); if(!path.empty()) { + LL_INFOS("UpdaterService") << "removing " << path << LL_ENDL; LLFile::remove(path); } LLFile::remove(download_marker_path); @@ -539,7 +555,7 @@ bool LLUpdaterServiceImpl::onMainLoop(LLSD const & event) // Check for failed install. if(LLFile::isfile(ll_install_failed_marker_path())) { - LL_DEBUGS("UpdaterService") << "found marker " << ll_install_failed_marker_path() << LL_ENDL;; + LL_DEBUGS("UpdaterService") << "found marker " << ll_install_failed_marker_path() << LL_ENDL; int requiredValue = 0; { llifstream stream(ll_install_failed_marker_path()); @@ -552,12 +568,12 @@ bool LLUpdaterServiceImpl::onMainLoop(LLSD const & event) // TODO: notify the user. LL_WARNS("UpdaterService") << "last install attempt failed" << LL_ENDL;; LLFile::remove(ll_install_failed_marker_path()); - + LLSD event; event["type"] = LLSD(LLUpdaterService::INSTALL_ERROR); event["required"] = LLSD(requiredValue); LLEventPumps::instance().obtain(LLUpdaterService::pumpName()).post(event); - + setState(LLUpdaterService::TERMINAL); } else -- cgit v1.2.3