Append multiple history backups on import.

Previously, a successful import of the text history into sqlite would
move 'history' to 'history.bak'. If history.bak already existed, this
would overwrite it on unix and fail on windows.

With this patch, the most recently imported history is appended to
history.bak to avoid data loss.

Resolves #3005.

A few other options I considered:

1. os.replace:
    - fast, simple, no error on Windows
    - potential data loss
2. numbered backups (.bak.1, .bak.2, ...):
    - fast, no data loss, but more complex
3. append each line to the backup as it is read:
    - more efficient than current patch (no need to read history twice)
    - potentially duplicate data if backup fails

qutebrowser/browser/history.py

@@ -252,10 +252,7 @@ class WebHistory(sql.SqlTable):
                 except ValueError as ex:
                     message.error('Failed to import history: {}'.format(ex))
                 else:
-                    bakpath = path + '.bak'
-                    message.info('History import complete. Moving {} to {}'
-                                 .format(path, bakpath))
-                    os.rename(path, bakpath)
+                    self._write_backup(path)
 
         # delay to give message time to appear before locking down for import
         message.info('Converting {} to sqlite...'.format(path))
@@ -287,6 +284,16 @@ class WebHistory(sql.SqlTable):
         self.insert_batch(data)
         self.completion.insert_batch(completion_data, replace=True)
 
+    def _write_backup(self, path):
+        bak = path + '.bak'
+        message.info('History import complete. Appending {} to {}'
+                     .format(path, bak))
+        with open(path, 'r', encoding='utf-8') as infile:
+            with open(bak, 'a', encoding='utf-8') as outfile:
+                for line in infile:
+                    outfile.write('\n' + line)
+        os.remove(path)
+
     def _format_url(self, url):
         return url.toString(QUrl.RemovePassword | QUrl.FullyEncoded)
 

tests/unit/browser/test_history.py

@@ -284,6 +284,22 @@ def test_import_txt(hist, data_tmpdir, monkeypatch, stubs):
     assert (data_tmpdir / 'history.bak').exists()
 
 
+def test_import_txt_existing_backup(hist, data_tmpdir, monkeypatch, stubs):
+    monkeypatch.setattr(history, 'QTimer', stubs.InstaTimer)
+    histfile = data_tmpdir / 'history'
+    bakfile = data_tmpdir / 'history.bak'
+    histfile.write('12345 http://example.com/ title')
+    bakfile.write('12346 http://qutebrowser.org/')
+
+    hist.import_txt()
+
+    assert list(hist) == [('http://example.com/', 'title', 12345, False)]
+
+    assert not histfile.exists()
+    assert bakfile.read().split('\n') == ['12346 http://qutebrowser.org/',
+                                          '12345 http://example.com/ title']
+
+
 @pytest.mark.parametrize('line', [
     '',
     '#12345 http://example.com/commented',