From 972811de236724833cf4fe25f9ac636527bf6800 Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Thu, 15 Sep 2011 12:26:34 +0100 Subject: [PATCH 1/5] Bump version number in trunk. --- src/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.cpp b/src/main.cpp index ecc1573..e6e7c77 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -25,7 +25,7 @@ #include -#define UPDATER_VERSION "0.5" +#define UPDATER_VERSION "0.6" void runWithUi(int argc, char** argv, UpdateInstaller* installer); From 1bad51e3dc74fe5a4707ade4716917492458d7fb Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Thu, 15 Sep 2011 14:45:53 +0100 Subject: [PATCH 2/5] Display a more helpful message to the user if an update cannot be installed because the application was run from a read-only file system. On Mac, this can happen if the user runs the app directly from a disk image. On all platforms, this can happen if the app is run from any kind of read-only media or network share. * Intercept IO errors during installation and try to find a more friendly alternative to the OS error string. * Catch any IO exceptions throw whilst reverting a partial update and log details. --- src/FileUtils.cpp | 37 ++++++++++++++++++++++++++++---- src/FileUtils.h | 17 ++++++++++++++- src/UpdateInstaller.cpp | 47 +++++++++++++++++++++++++++++++++++++++-- src/UpdateInstaller.h | 2 ++ 4 files changed, 96 insertions(+), 7 deletions(-) diff --git a/src/FileUtils.cpp b/src/FileUtils.cpp index 1c8323c..2b44268 100644 --- a/src/FileUtils.cpp +++ b/src/FileUtils.cpp @@ -24,28 +24,57 @@ #endif FileUtils::IOException::IOException(const std::string& error) -: m_errno(0) +{ + init(errno,error); +} + +FileUtils::IOException::IOException(int errorCode, const std::string& error) +{ + init(errorCode,error); +} + +void FileUtils::IOException::init(int errorCode, const std::string& error) { m_error = error; #ifdef PLATFORM_UNIX - m_errno = errno; + m_errorCode = errorCode; - if (m_errno > 0) + if (m_errorCode > 0) { - m_error += " details: " + std::string(strerror(m_errno)); + m_error += " details: " + std::string(strerror(m_errorCode)); } #endif #ifdef PLATFORM_WINDOWS + m_errorCode = 0; m_error += " GetLastError returned: " + intToStr(GetLastError()); #endif + + LOG(Info,"created IOException with errno " + intToStr(m_errorCode) + " rofs " + intToStr(EROFS)); } FileUtils::IOException::~IOException() throw () { } +FileUtils::IOException::Type FileUtils::IOException::type() const +{ +#ifdef PLATFORM_UNIX + switch (m_errorCode) + { + case 0: + return NoError; + case EROFS: + return ReadOnlyFileSystem; + default: + return Unknown; + } +#else + return Unknown; +#endif +} + bool FileUtils::fileExists(const char* path) throw (IOException) { #ifdef PLATFORM_UNIX diff --git a/src/FileUtils.h b/src/FileUtils.h index e8d4b6b..a6cb25d 100644 --- a/src/FileUtils.h +++ b/src/FileUtils.h @@ -21,17 +21,32 @@ class FileUtils { public: IOException(const std::string& error); + IOException(int errno, const std::string& error); virtual ~IOException() throw (); + enum Type + { + NoError, + /** Unknown error type. Call what() to get the description + * provided by the OS. + */ + Unknown, + ReadOnlyFileSystem + }; + virtual const char* what() const throw () { return m_error.c_str(); } + Type type() const; + private: + void init(int errorCode, const std::string& error); + std::string m_error; - int m_errno; + int m_errorCode; }; /** Remove a file. Throws an exception if the file diff --git a/src/UpdateInstaller.cpp b/src/UpdateInstaller.cpp index 3c81dec..4e58565 100644 --- a/src/UpdateInstaller.cpp +++ b/src/UpdateInstaller.cpp @@ -71,6 +71,29 @@ void UpdateInstaller::reportError(const std::string& error) } } +std::string UpdateInstaller::friendlyErrorForError(const FileUtils::IOException& exception) const +{ + std::string friendlyError; + + switch (exception.type()) + { + case FileUtils::IOException::ReadOnlyFileSystem: +#ifdef PLATFORM_MAC + friendlyError = AppInfo::appName() + " was started from a read-only location. " + "Copy it to the Applications folder on your Mac and run " + "it from there."; +#else + friendlyError = AppInfo::appName() + " was started from a read-only location. " + "Re-install it to a location that can be updated and run it from there."; +#endif + break; + default: + break; + } + + return friendlyError; +} + void UpdateInstaller::run() throw () { if (!m_script || !m_script->isValid()) @@ -144,7 +167,13 @@ void UpdateInstaller::run() throw () { LOG(Info,"Starting update installation"); + // the detailed error string returned by the OS std::string error; + // the message to present to the user. This may be the same + // as 'error' or may be different if a more helpful suggestion + // can be made for a particular problem + std::string friendlyError; + try { LOG(Info,"Installing new and updated files"); @@ -161,6 +190,7 @@ void UpdateInstaller::run() throw () catch (const FileUtils::IOException& exception) { error = exception.what(); + friendlyError = friendlyErrorForError(exception); } catch (const std::string& genericError) { @@ -170,10 +200,23 @@ void UpdateInstaller::run() throw () if (!error.empty()) { LOG(Error,std::string("Error installing update ") + error); - revert(); + + try + { + revert(); + } + catch (const FileUtils::IOException& exception) + { + LOG(Error,"Error reverting partial update " + std::string(exception.what())); + } + if (m_observer) { - m_observer->updateError(error); + if (friendlyError.empty()) + { + friendlyError = error; + } + m_observer->updateError(friendlyError); } } diff --git a/src/UpdateInstaller.h b/src/UpdateInstaller.h index 6884966..971479f 100644 --- a/src/UpdateInstaller.h +++ b/src/UpdateInstaller.h @@ -1,6 +1,7 @@ #pragma once #include "Platform.h" +#include "FileUtils.h" #include "UpdateScript.h" #include @@ -52,6 +53,7 @@ class UpdateInstaller void postInstallUpdate(); std::list updaterArgs() const; + std::string friendlyErrorForError(const FileUtils::IOException& ex) const; Mode m_mode; std::string m_installDir; From 92993ea631741395906605aff2e6d75f4a5bc1a8 Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Thu, 15 Sep 2011 14:49:27 +0100 Subject: [PATCH 3/5] Add -d option to the updater test script to run the updater binary under GDB. Running the updater binary under a debugger makes it easier to attach to the process and set breakpoints in GDB than trying to start the ruby script itself under GDB and follow forks until the updater executes. --- src/tests/test-update.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/tests/test-update.rb b/src/tests/test-update.rb index 2e5f6be..4f2660a 100755 --- a/src/tests/test-update.rb +++ b/src/tests/test-update.rb @@ -47,11 +47,15 @@ def zip_supports_bzip2(zip_tool) end force_elevation = false +run_in_debugger = false OptionParser.new do |parser| parser.on("-f","--force-elevated","Force the updater to elevate itself") do force_elevation = true end + parser.on("-d","--debug","Run the updater under GDB") do + run_in_debugger = true + end end.parse! BZIP2_AVAILABLE = zip_supports_bzip2(ZIP_TOOL) @@ -102,7 +106,8 @@ FileUtils.cp("../#{UPDATER_NAME}","#{PACKAGE_DIR}/#{UPDATER_NAME}") install_path = File.expand_path(INSTALL_DIR) Dir.chdir(INSTALL_DIR) do flags = "--force-elevated" if force_elevation - cmd = "#{PACKAGE_DIR}/#{UPDATER_NAME} #{flags} --install-dir \"#{install_path}\" --package-dir \"#{PACKAGE_DIR}\" --script file_list.xml" + debug_flags = "gdb --args" if run_in_debugger + cmd = "#{debug_flags} #{PACKAGE_DIR}/#{UPDATER_NAME} #{flags} --install-dir \"#{install_path}\" --package-dir \"#{PACKAGE_DIR}\" --script file_list.xml" puts "Running '#{cmd}'" system(cmd) end From 274b4331205e6a40d77d97013794324035de1cc7 Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Thu, 15 Sep 2011 15:03:30 +0100 Subject: [PATCH 4/5] Add an extra new-line in the error dialog that appears if the update cannot be installed. --- src/UpdateDialogCocoa.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/UpdateDialogCocoa.mm b/src/UpdateDialogCocoa.mm index 57b8fa8..16ae35d 100644 --- a/src/UpdateDialogCocoa.mm +++ b/src/UpdateDialogCocoa.mm @@ -43,7 +43,7 @@ class UpdateDialogPrivate { dialog->hadError = true; NSMutableString* message = [[NSMutableString alloc] init]; - [message appendString:@"There was a problem installing the update:\n"]; + [message appendString:@"There was a problem installing the update:\n\n"]; [message appendString:arg]; NSAlert* alert = [NSAlert From ef9ceb03f0843325fd5b875669a6578768847fd3 Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Thu, 15 Sep 2011 15:09:19 +0100 Subject: [PATCH 5/5] Add a friendly error message if installation fails due to lack of disk space. --- src/FileUtils.cpp | 2 ++ src/FileUtils.h | 3 ++- src/UpdateInstaller.cpp | 3 +++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/FileUtils.cpp b/src/FileUtils.cpp index 2b44268..d453410 100644 --- a/src/FileUtils.cpp +++ b/src/FileUtils.cpp @@ -67,6 +67,8 @@ FileUtils::IOException::Type FileUtils::IOException::type() const return NoError; case EROFS: return ReadOnlyFileSystem; + case ENOSPC: + return DiskFull; default: return Unknown; } diff --git a/src/FileUtils.h b/src/FileUtils.h index a6cb25d..1e112c6 100644 --- a/src/FileUtils.h +++ b/src/FileUtils.h @@ -32,7 +32,8 @@ class FileUtils * provided by the OS. */ Unknown, - ReadOnlyFileSystem + ReadOnlyFileSystem, + DiskFull }; virtual const char* what() const throw () diff --git a/src/UpdateInstaller.cpp b/src/UpdateInstaller.cpp index 4e58565..866f868 100644 --- a/src/UpdateInstaller.cpp +++ b/src/UpdateInstaller.cpp @@ -87,6 +87,9 @@ std::string UpdateInstaller::friendlyErrorForError(const FileUtils::IOException& "Re-install it to a location that can be updated and run it from there."; #endif break; + case FileUtils::IOException::DiskFull: + friendlyError = "The disk is full. Please free up some space and try again."; + break; default: break; }