reapack

commit cd0766d82fb96eb54d2761cfcac9114275536b73
parent f0cf97ecc807182b1badb91739ed9c0a6275a230
Author: cfillion <cfillion@users.noreply.github.com>
Date:   Sun, 14 Feb 2016 00:12:23 -0800

don't register packages if they failed to install

this was yet another regression caused by the conflict detection code

Diffstat:
M
src/registry.cpp
 | 
39
++++++++++++++++++++++++++++++---------
M
src/registry.hpp
 | 
7
++++---
M
src/transaction.cpp
 | 
62
++++++++++++++++++++++++++++++++++++--------------------------

3 files changed, 70 insertions(+), 38 deletions(-)
diff --git a/src/registry.cpp b/src/registry.cpp
@@ -28,7 +28,7 @@
 using namespace std;
 
 Registry::Registry(const Path &path)
-  : m_db(path.join())
+  : m_db(path.join()), m_savePoint(0)
 {
   migrate();
 
@@ -73,10 +73,6 @@ Registry::Registry(const Path &path)
   );
   m_forgetFiles = m_db.prepare("DELETE FROM files WHERE entry = ?");
 
-  m_savepoint = m_db.prepare("SAVEPOINT savept");
-  m_release = m_db.prepare("RELEASE SAVEPOINT savept");
-  m_restore = m_db.prepare("ROLLBACK TO SAVEPOINT savept");
-
   // lock the database
   m_db.begin();
 }
@@ -122,7 +118,8 @@ void Registry::migrate()
 
 auto Registry::push(const Version *ver, vector<Path> *conflicts) -> Entry
 {
-  m_savepoint->exec();
+  savepoint();
+
   bool hasConflicts = false;
 
   const Package *pkg = ver->package();
@@ -166,7 +163,7 @@ auto Registry::push(const Version *ver, vector<Path> *conflicts) -> Entry
         conflicts->push_back(path);
       }
       else {
-        m_restore->exec();
+        restore();
         throw;
       }
     }
@@ -179,11 +176,11 @@ auto Registry::push(const Version *ver, vector<Path> *conflicts) -> Entry
   }
 
   if(hasConflicts) {
-    m_restore->exec();
+    restore();
     return {};
   }
   else {
-    m_release->exec();
+    release();
     return {entryId, ri->name(), cat->name(),
       pkg->name(), pkg->type(), ver->code()};
   }
@@ -299,6 +296,30 @@ void Registry::forget(const Entry &entry)
   m_forgetEntry->exec();
 }
 
+void Registry::savepoint()
+{
+  char sql[64] = {};
+  sprintf(sql, "SAVEPOINT sp%lu", m_savePoint++);
+
+  m_db.exec(sql);
+}
+
+void Registry::restore()
+{
+  char sql[64] = {};
+  sprintf(sql, "ROLLBACK TO SAVEPOINT sp%lu", --m_savePoint);
+
+  m_db.exec(sql);
+}
+
+void Registry::release()
+{
+  char sql[64] = {};
+  sprintf(sql, "RELEASE SAVEPOINT sp%lu", --m_savePoint);
+
+  m_db.exec(sql);
+}
+
 void Registry::commit()
 {
   m_db.commit();
diff --git a/src/registry.hpp b/src/registry.hpp
@@ -61,6 +61,9 @@ public:
   std::string getMainFile(const Entry &) const;
   Entry push(const Version *, std::vector<Path> *conflicts = nullptr);
   void forget(const Entry &);
+  void savepoint();
+  void restore();
+  void release();
   void commit();
 
 private:
@@ -80,9 +83,7 @@ private:
   Statement *m_clearFiles;
   Statement *m_forgetFiles;
 
-  Statement *m_savepoint;
-  Statement *m_release;
-  Statement *m_restore;
+  size_t m_savePoint;
 };
 
 #endif
diff --git a/src/transaction.cpp b/src/transaction.cpp
@@ -74,8 +74,17 @@ void Transaction::synchronize(const Remote &remote, const bool isUserAction)
       return;
     }
 
+    // upgrade() will register all new packages to test for file conflicts.
+    // Once this is done and all possible installations are queued,
+    // we must restore the registry so the failed packages are not marked as
+    // installed, and to keep the old file list intact (this is needed for
+    // cleaning unused files)
+    m_registry->savepoint();
+
     for(const Package *pkg : ri->packages())
       upgrade(pkg);
+
+    m_registry->restore();
   });
 }
 
@@ -115,31 +124,10 @@ void Transaction::upgrade(const Package *pkg)
       queryRes.status = Registry::Uninstalled;
   }
 
-  m_installQueue.push({ver, queryRes});
-}
-
-void Transaction::installQueued()
-{
-  while(!m_installQueue.empty()) {
-    installTicket(m_installQueue.front());
-    m_installQueue.pop();
-  }
-
-  runTasks();
-}
-
-void Transaction::installTicket(const InstallTicket &ticket)
-{
-  const Version *ver = ticket.first;
-  const Registry::QueryResult &queryRes = ticket.second;
-  const set<Path> &currentFiles = m_registry->getFiles(queryRes.entry);
-
-  Registry::Entry newEntry;
-
+  // prevent file conflicts – pushes to the registry will be reverted!
   try {
-    // register the new version and prevent file conflicts
     vector<Path> conflicts;
-    newEntry = m_registry->push(ver, &conflicts);
+    m_registry->push(ver, &conflicts);
 
     if(!conflicts.empty()) {
       for(const Path &path : conflicts) {
@@ -157,6 +145,26 @@ void Transaction::installTicket(const InstallTicket &ticket)
     return;
   }
 
+  // all green! (pronounce with a japanese accent)
+  m_installQueue.push({ver, queryRes});
+}
+
+void Transaction::installQueued()
+{
+  while(!m_installQueue.empty()) {
+    installTicket(m_installQueue.front());
+    m_installQueue.pop();
+  }
+
+  runTasks();
+}
+
+void Transaction::installTicket(const InstallTicket &ticket)
+{
+  const Version *ver = ticket.first;
+  const Registry::QueryResult &queryRes = ticket.second;
+  const set<Path> &currentFiles = m_registry->getFiles(queryRes.entry);
+
   InstallTask *task = new InstallTask(ver, currentFiles, this);
 
   task->onCommit([=] {
@@ -165,12 +173,14 @@ void Transaction::installTicket(const InstallTicket &ticket)
     else
       m_new.push_back(ticket);
 
-    if(newEntry.type == Package::ExtensionType)
-      m_needRestart = true;
-
     const set<Path> &removedFiles = task->removedFiles();
     m_removals.insert(removedFiles.begin(), removedFiles.end());
 
+    const Registry::Entry newEntry = m_registry->push(ver);
+
+    if(newEntry.type == Package::ExtensionType)
+      m_needRestart = true;
+
     registerInHost(true, newEntry);
   });