reapack

commit 775ad7f81cb0ca8cd2bb40cd0b48919381ee0ce5
parent e319850bfd893b4d30edfddf418b4d301fccff6c
Author: cfillion <cfillion@users.noreply.github.com>
Date:   Tue, 19 Jan 2016 15:57:36 -0500

don't leave empty directories behind when rolling back an install task

Diffstat:
M
src/task.cpp
 | 
74
++++++++++++++++++++++++++++++++++++--------------------------------------
M
src/task.hpp
 | 
8
+++-----

2 files changed, 39 insertions(+), 43 deletions(-)
diff --git a/src/task.cpp b/src/task.cpp
@@ -54,32 +54,50 @@ void Task::commit()
     m_onCommit();
 }
 
-int Task::removeFile(const Path &path) const
+bool Task::renameFile(const Path &from, const Path &to) const
+{
+  const string &fullFrom = m_transaction->prefixPath(from).join();
+  const string &fullTo = m_transaction->prefixPath(to).join();
+
+#ifdef _WIN32
+  return !_wrename(make_autostring(fullFrom).c_str(),
+    make_autostring(fullTo).c_str());
+#else
+  return !rename(fullFrom.c_str(), fullTo.c_str());
+#endif
+}
+
+bool Task::removeFile(const Path &path) const
 {
   const auto_string &fullPath =
     make_autostring(m_transaction->prefixPath(path).join());
 
 #ifdef _WIN32
   if(GetFileAttributes(fullPath.c_str()) & FILE_ATTRIBUTE_DIRECTORY)
-    return !RemoveDirectory(fullPath.c_str());
+    return RemoveDirectory(fullPath.c_str()) != 0;
   else
-    return _wremove(fullPath.c_str());
+    return !_wremove(fullPath.c_str());
 #else
-  return remove(fullPath.c_str());
+  return !remove(fullPath.c_str());
 #endif
 }
 
-int Task::renameFile(const Path &from, const Path &to) const
+bool Task::removeFileRecursive(const Path &file) const
 {
-  const string &fullFrom = m_transaction->prefixPath(from).join();
-  const string &fullTo = m_transaction->prefixPath(to).join();
+  if(!removeFile(file))
+    return false;
 
-#ifdef _WIN32
-  return _wrename(make_autostring(fullFrom).c_str(),
-    make_autostring(fullTo).c_str());
-#else
-  return rename(fullFrom.c_str(), fullTo.c_str());
-#endif
+  Path dir = file;
+
+  // remove empty directories, but not top-level ones that were created by REAPER
+  while(dir.size() > 2) {
+    dir.removeLast();
+
+    if(!removeFile(dir))
+      break;
+  }
+
+  return true;
 }
 
 InstallTask::InstallTask(Version *ver, const set<Path> &oldFiles, Transaction *t)
@@ -133,7 +151,7 @@ bool InstallTask::doCommit()
   for(const PathPair &paths : m_newFiles) {
     removeFile(paths.second);
 
-    if(renameFile(paths.first, paths.second)) {
+    if(!renameFile(paths.first, paths.second)) {
       transaction()->addError(strerror(errno), paths.first.join());
 
       // it's a bit late to rollback here as some files might already have been
@@ -149,7 +167,7 @@ bool InstallTask::doCommit()
 void InstallTask::doRollback()
 {
   for(const PathPair &paths : m_newFiles)
-    removeFile(paths.first);
+    removeFileRecursive(paths.first);
 
   m_newFiles.clear();
 }
@@ -162,31 +180,11 @@ RemoveTask::RemoveTask(const vector<Path> &files, Transaction *t)
 bool RemoveTask::doCommit()
 {
   for(const Path &path : m_files) {
-    if(!remove(path))
+    if(removeFileRecursive(path))
+      m_removedFiles.insert(path);
+    else
       return false;
   }
 
   return true;
 }
-
-bool RemoveTask::remove(const Path &file)
-{
-  if(removeFile(file)) {
-    transaction()->addError(strerror(errno), file.join());
-    return false;
-  }
-  else
-    m_removedFiles.insert(file);
-
-  Path dir = file;
-
-  // remove empty directories, but not top-level ones that were created by REAPER
-  while(dir.size() > 2) {
-    dir.removeLast();
-
-    if(removeFile(dir))
-      break;
-  }
-
-  return true;
-}
diff --git a/src/task.hpp b/src/task.hpp
@@ -43,8 +43,9 @@ public:
   void rollback();
 
 protected:
-  int removeFile(const Path &) const;
-  int renameFile(const Path &, const Path &) const;
+  bool renameFile(const Path &, const Path &) const;
+  bool removeFile(const Path &) const;
+  bool removeFileRecursive(const Path &) const;
 
   Transaction *transaction() const { return m_transaction; }
 
@@ -52,7 +53,6 @@ protected:
   virtual void doRollback() = 0;
 
 private:
-
   Transaction *m_transaction;
   bool m_isCancelled;
 
@@ -89,8 +89,6 @@ protected:
   void doRollback() override {}
 
 private:
-  bool remove(const Path &);
-
   std::vector<Path> m_files;
   std::set<Path> m_removedFiles;
 };