[Windows|Unix] Reimplement reverter logic to make it cross-platform compatible
diff --git a/certbot/reverter.py b/certbot/reverter.py
index e08b468ac1d..f05e24b01cc 100644
--- a/certbot/reverter.py
+++ b/certbot/reverter.py
@@ -4,17 +4,18 @@
 import logging
 import os
 import shutil
+import sys
 import time
 import traceback
 
 import six
 import zope.component
 
+from certbot.compat import misc
 from certbot import constants
 from certbot import errors
 from certbot import interfaces
 from certbot import util
-from certbot.compat import misc
 
 logger = logging.getLogger(__name__)
 
@@ -237,7 +238,7 @@ def _add_to_checkpoint_dir(self, cp_dir, save_files, save_notes):
                 try:
                     shutil.copy2(filename, os.path.join(
                         cp_dir, os.path.basename(filename) + "_" + str(idx)))
-                    op_fd.write(filename + os.linesep)
+                    op_fd.write('{0}\n'.format(filename))
                 # http://stackoverflow.com/questions/4726260/effective-use-of-python-shutil-copy2
                 except IOError:
                     op_fd.close()
@@ -312,7 +313,10 @@ def _run_undo_commands(self, filepath):  # pylint: disable=no-self-use
         """Run all commands in a file."""
         # NOTE: csv module uses native strings. That is, bytes on Python 2 and
         # unicode on Python 3
-        with open(filepath, 'r') as csvfile:
+        # It is strongly advised to set newline = '' on Python 3 with CSV,
+        # and it fixes problems on Windows.
+        kwargs = {'newline': ''} if sys.version_info[0] > 2 else {}
+        with open(filepath, 'r', **kwargs) as csvfile:  # type: ignore  # pylint: disable=star-args
             csvreader = csv.reader(csvfile)
             for command in reversed(list(csvreader)):
                 try:
@@ -381,7 +385,7 @@ def register_file_creation(self, temporary, *files):
 
             for path in files:
                 if path not in ex_files:
-                    new_fd.write("{0}{1}".format(path, os.linesep))
+                    new_fd.write("{0}\n".format(path))
         except (IOError, OSError):
             logger.error("Unable to register file creation(s) - %s", files)
             raise errors.ReverterError(
@@ -408,11 +412,14 @@ def register_undo_command(self, temporary, command):
         """
         commands_fp = os.path.join(self._get_cp_dir(temporary), "COMMANDS")
         command_file = None
+        # It is strongly advised to set newline = '' on Python 3 with CSV,
+        # and it fixes problems on Windows.
+        kwargs = {'newline': ''} if sys.version_info[0] > 2 else {}
         try:
             if os.path.isfile(commands_fp):
-                command_file = open(commands_fp, "a")
+                command_file = open(commands_fp, "a", **kwargs)  # type: ignore  # pylint: disable=star-args
             else:
-                command_file = open(commands_fp, "w")
+                command_file = open(commands_fp, "w", **kwargs)  # type: ignore  # pylint: disable=star-args
 
             csvwriter = csv.writer(command_file)
             csvwriter.writerow(command)
diff --git a/certbot/tests/reverter_test.py b/certbot/tests/reverter_test.py
index 8e05d4f1c14..18e6984442c 100644
--- a/certbot/tests/reverter_test.py
+++ b/certbot/tests/reverter_test.py
@@ -49,7 +49,6 @@ def test_no_change(self, mock_read):
             x = f.read()
         self.assertTrue("No changes" in x)
 
-    @test_util.broken_on_windows
     def test_basic_add_to_temp_checkpoint(self):
         # These shouldn't conflict even though they are both named config.txt
         self.reverter.add_to_temp_checkpoint(self.sets[0], "save1")
@@ -91,7 +90,6 @@ def test_checkpoint_conflict(self):
         self.assertRaises(errors.ReverterError, self.reverter.add_to_checkpoint,
                           set([config3]), "invalid save")
 
-    @test_util.broken_on_windows
     def test_multiple_saves_and_temp_revert(self):
         self.reverter.add_to_temp_checkpoint(self.sets[0], "save1")
         update_file(self.config1, "updated-directive")
@@ -121,7 +119,6 @@ def test_multiple_registration_fail_and_revert(self):
         self.assertFalse(os.path.isfile(config3))
         self.assertFalse(os.path.isfile(config4))
 
-    @test_util.broken_on_windows
     def test_multiple_registration_same_file(self):
         self.reverter.register_file_creation(True, self.config1)
         self.reverter.register_file_creation(True, self.config1)
@@ -146,7 +143,6 @@ def test_bad_registration(self):
             errors.ReverterError, self.reverter.register_file_creation,
             "filepath")
 
-    @test_util.broken_on_windows
     def test_register_undo_command(self):
         coms = [
             ["a2dismod", "ssl"],
@@ -169,7 +165,6 @@ def test_bad_register_undo_command(self):
                 errors.ReverterError, self.reverter.register_undo_command,
                 True, ["command"])
 
-    @test_util.broken_on_windows
     @mock.patch("certbot.util.run_script")
     def test_run_undo_commands(self, mock_run):
         mock_run.side_effect = ["", errors.SubprocessError]
@@ -233,7 +228,6 @@ def test_recover_checkpoint_rm_failure(self):
             self.assertRaises(
                 errors.ReverterError, self.reverter.revert_temporary_config)
 
-    @test_util.broken_on_windows
     @mock.patch("certbot.reverter.logger.warning")
     def test_recover_checkpoint_missing_new_files(self, mock_warn):
         self.reverter.register_file_creation(
@@ -248,7 +242,6 @@ def test_recover_checkpoint_remove_failure(self, mock_remove):
         self.assertRaises(
             errors.ReverterError, self.reverter.revert_temporary_config)
 
-    @test_util.broken_on_windows
     def test_recovery_routine_temp_and_perm(self):
         # Register a new perm checkpoint file
         config3 = os.path.join(self.dir1, "config3.txt")
@@ -312,7 +305,6 @@ def test_rollback_improper_inputs(self):
         self.assertRaises(
             errors.ReverterError, self.reverter.rollback_checkpoints, "one")
 
-    @test_util.broken_on_windows
     def test_rollback_finalize_checkpoint_valid_inputs(self):
 
         config3 = self._setup_three_checkpoints()
@@ -364,7 +356,6 @@ def test_finalize_checkpoint_no_rename_directory(self, mock_rename):
         self.assertRaises(
             errors.ReverterError, self.reverter.finalize_checkpoint, "Title")
 
-    @test_util.broken_on_windows
     @mock.patch("certbot.reverter.logger")
     def test_rollback_too_many(self, mock_logger):
         # Test no exist warning...
@@ -377,7 +368,6 @@ def test_rollback_too_many(self, mock_logger):
         self.reverter.rollback_checkpoints(4)
         self.assertEqual(mock_logger.warning.call_count, 1)
 
-    @test_util.broken_on_windows
     def test_multi_rollback(self):
         config3 = self._setup_three_checkpoints()
         self.reverter.rollback_checkpoints(3)

This PR is a part of the effort to remove the last broken unit tests in certbot codebase for Windows, as described in #6850.

This PR fixes the logic of `certbot.reverter` module to make it runnable on Windows. This critical module is used to revert modifications done by Certbot on a system when something goes wrong during a certificate deployment.

To do so, several piece of informations are stored on the filesystem, and will be used by `certbot.reverter` to know what need to be reverted at any point during a process. In particular, modified files and new files are stored this way. The form of the storage is a file, where each line represents a path:
```
/path/to/affected_file_1
/path/to/affected_file_2
```

Two problems arises on Windows:
* end of lines in Windows are CRLF, while the current logic of `certbot.reverter` expects LF end lines (Linux style)
* on Windows, `certbot.reverter` generates superfluous empty lines when writing, making the revert process failing when the persisted file is loaded, because each line is expected to contain a valid path.

So the format for theses files is a weak formatting, a separation of statements based on LF end of lines. It seems that it is not the first time that this is a problem, because another file (the shell commands issued by certbot), is using a CSV format. However, the `csv` module behavior is not consistent across Python 2.x/3.x, making necessary some specific mitigations in `certbot.reverter` code.

So I am proposing a new solution here, in order to correct `certbot.reverter` for Windows, while taking advantage of a less brittle and platform independant format for relevant files, and making a consistent implementation accross the various files use during a revert process.

The idea is to use the module `json`. Instead of writing/reading new paths lines by lines, or use the `csv` module, `certbot.revert` will write the list of paths using `json.dumps` in the file, and read the list of files using `json.loads` when needed. Here are the advantages:
* the format used in the files is JSON, which is a strong format, in particular not dependant to LF/CRLF end of lines
* `json` module is working consistently accross Python 2.x/3.x without specific code
* no parallel read/write, or writing line by line: files are read as a whole at one point, then written as a whole later

As a consequence, relevant unit tests in `certbot.tests.reverter_test` are reactivated.