From 232e0ea50fa7472803510260b8368aac33478ff8 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Mon, 1 Apr 2019 18:50:08 +0200 Subject: [PATCH] Rely on universal newline mode on python 3 for windows (#6866) --- certbot/reverter.py | 19 +++++++++++++------ certbot/tests/reverter_test.py | 10 ---------- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/certbot/reverter.py b/certbot/reverter.py index e08b468ac..f05e24b01 100644 --- a/certbot/reverter.py +++ b/certbot/reverter.py @@ -4,17 +4,18 @@ import glob 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 @@ class Reverter(object): 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 @@ class Reverter(object): """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 @@ class Reverter(object): 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 @@ class Reverter(object): """ 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 8e05d4f1c..18e698444 100644 --- a/certbot/tests/reverter_test.py +++ b/certbot/tests/reverter_test.py @@ -49,7 +49,6 @@ class ReverterCheckpointLocalTest(test_util.ConfigTestCase): 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 @@ class ReverterCheckpointLocalTest(test_util.ConfigTestCase): 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 @@ class ReverterCheckpointLocalTest(test_util.ConfigTestCase): 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 @@ class ReverterCheckpointLocalTest(test_util.ConfigTestCase): 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 @@ class ReverterCheckpointLocalTest(test_util.ConfigTestCase): 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 @@ class ReverterCheckpointLocalTest(test_util.ConfigTestCase): 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 @@ class ReverterCheckpointLocalTest(test_util.ConfigTestCase): 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 @@ class TestFullCheckpointsReverter(test_util.ConfigTestCase): 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 @@ class TestFullCheckpointsReverter(test_util.ConfigTestCase): 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 @@ class TestFullCheckpointsReverter(test_util.ConfigTestCase): 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)