From 327ccb241e4ffac7b81f2b09bcd109db8a53f410 Mon Sep 17 00:00:00 2001 From: Otto Winter Date: Thu, 24 Oct 2019 21:53:42 +0200 Subject: [PATCH] Make file generation saving atomic (#792) * Make file generation saving atomic * Lint * Python 2 Compat * Fix * Handle file not found error --- esphome/config_helpers.py | 12 +--- esphome/helpers.py | 113 +++++++++++++++++++++++++++++--------- esphome/py_compat.py | 24 ++++---- esphome/storage_json.py | 15 ++--- esphome/wizard.py | 6 +- esphome/writer.py | 30 +++++----- 6 files changed, 123 insertions(+), 77 deletions(-) diff --git a/esphome/config_helpers.py b/esphome/config_helpers.py index ddad36f8a8..0c508d2202 100644 --- a/esphome/config_helpers.py +++ b/esphome/config_helpers.py @@ -1,10 +1,10 @@ from __future__ import print_function -import codecs import json import os -from esphome.core import CORE, EsphomeError +from esphome.core import CORE +from esphome.helpers import read_file from esphome.py_compat import safe_input @@ -20,10 +20,4 @@ def read_config_file(path): assert data['type'] == 'file_response' return data['content'] - try: - with codecs.open(path, encoding='utf-8') as handle: - return handle.read() - except IOError as exc: - raise EsphomeError(u"Error accessing file {}: {}".format(path, exc)) - except UnicodeDecodeError as exc: - raise EsphomeError(u"Unable to read file {}: {}".format(path, exc)) + return read_file(path) diff --git a/esphome/helpers.py b/esphome/helpers.py index 30a06d842f..6fd1fa2ad7 100644 --- a/esphome/helpers.py +++ b/esphome/helpers.py @@ -1,10 +1,11 @@ from __future__ import print_function import codecs + import logging import os -from esphome.py_compat import char_to_byte, text_type +from esphome.py_compat import char_to_byte, text_type, IS_PY2, encode_text _LOGGER = logging.getLogger(__name__) @@ -79,15 +80,15 @@ def run_system_command(*args): def mkdir_p(path): - import errno - try: os.makedirs(path) - except OSError as exc: - if exc.errno == errno.EEXIST and os.path.isdir(path): + except OSError as err: + import errno + if err.errno == errno.EEXIST and os.path.isdir(path): pass else: - raise + from esphome.core import EsphomeError + raise EsphomeError(u"Error creating directories {}: {}".format(path, err)) def is_ip_address(host): @@ -151,17 +152,6 @@ def is_hassio(): return get_bool_env('ESPHOME_IS_HASSIO') -def copy_file_if_changed(src, dst): - src_text = read_file(src) - if os.path.isfile(dst): - dst_text = read_file(dst) - else: - dst_text = None - if src_text == dst_text: - return - write_file(dst, src_text) - - def walk_files(path): for root, _, files in os.walk(path): for name in files: @@ -172,28 +162,99 @@ def read_file(path): try: with codecs.open(path, 'r', encoding='utf-8') as f_handle: return f_handle.read() - except OSError: + except OSError as err: from esphome.core import EsphomeError - raise EsphomeError(u"Could not read file at {}".format(path)) + raise EsphomeError(u"Error reading file {}: {}".format(path, err)) + except UnicodeDecodeError as err: + from esphome.core import EsphomeError + raise EsphomeError(u"Error reading file {}: {}".format(path, err)) + + +def _write_file(path, text): + import tempfile + directory = os.path.dirname(path) + mkdir_p(directory) + + tmp_path = None + data = encode_text(text) + try: + with tempfile.NamedTemporaryFile(mode="wb", dir=directory, delete=False) as f_handle: + tmp_path = f_handle.name + f_handle.write(data) + # Newer tempfile implementations create the file with mode 0o600 + os.chmod(tmp_path, 0o644) + if IS_PY2: + if os.path.exists(path): + os.remove(path) + os.rename(tmp_path, path) + else: + # If destination exists, will be overwritten + os.replace(tmp_path, path) + finally: + if tmp_path is not None and os.path.exists(tmp_path): + try: + os.remove(tmp_path) + except OSError as err: + _LOGGER.error("Write file cleanup failed: %s", err) def write_file(path, text): try: - mkdir_p(os.path.dirname(path)) - with codecs.open(path, 'w+', encoding='utf-8') as f_handle: - f_handle.write(text) + _write_file(path, text) except OSError: from esphome.core import EsphomeError raise EsphomeError(u"Could not write file at {}".format(path)) -def write_file_if_changed(text, dst): +def write_file_if_changed(path, text): src_content = None - if os.path.isfile(dst): - src_content = read_file(dst) + if os.path.isfile(path): + src_content = read_file(path) if src_content != text: - write_file(dst, text) + write_file(path, text) + + +def copy_file_if_changed(src, dst): + import shutil + if file_compare(src, dst): + return + mkdir_p(os.path.dirname(dst)) + try: + shutil.copy(src, dst) + except OSError as err: + from esphome.core import EsphomeError + raise EsphomeError(u"Error copying file {} to {}: {}".format(src, dst, err)) def list_starts_with(list_, sub): return len(sub) <= len(list_) and all(list_[i] == x for i, x in enumerate(sub)) + + +def file_compare(path1, path2): + """Return True if the files path1 and path2 have the same contents.""" + import stat + + try: + stat1, stat2 = os.stat(path1), os.stat(path2) + except OSError: + # File doesn't exist or another error -> not equal + return False + + if stat.S_IFMT(stat1.st_mode) != stat.S_IFREG or stat.S_IFMT(stat2.st_mode) != stat.S_IFREG: + # At least one of them is not a regular file (or does not exist) + return False + if stat1.st_size != stat2.st_size: + # Different sizes + return False + + bufsize = 8*1024 + # Read files in blocks until a mismatch is found + with open(path1, 'rb') as fh1, open(path2, 'rb') as fh2: + while True: + blob1, blob2 = fh1.read(bufsize), fh2.read(bufsize) + if blob1 != blob2: + # Different content + return False + if not blob1: + # Reached end + return True diff --git a/esphome/py_compat.py b/esphome/py_compat.py index 6833a55801..6cdaa5b047 100644 --- a/esphome/py_compat.py +++ b/esphome/py_compat.py @@ -1,5 +1,6 @@ import functools import sys +import codecs PYTHON_MAJOR = sys.version_info[0] IS_PY2 = PYTHON_MAJOR == 2 @@ -75,15 +76,14 @@ def indexbytes(buf, i): return ord(buf[i]) -if IS_PY2: - def decode_text(data, encoding='utf-8', errors='strict'): - # type: (str, str, str) -> unicode - if isinstance(data, unicode): - return data - return unicode(data, encoding=encoding, errors=errors) -else: - def decode_text(data, encoding='utf-8', errors='strict'): - # type: (bytes, str, str) -> str - if isinstance(data, str): - return data - return data.decode(encoding=encoding, errors=errors) +def decode_text(data, encoding='utf-8', errors='strict'): + if isinstance(data, text_type): + return data + return codecs.decode(data, encoding, errors) + + +def encode_text(data, encoding='utf-8', errors='strict'): + if isinstance(data, binary_type): + return data + + return codecs.encode(data, encoding, errors) diff --git a/esphome/storage_json.py b/esphome/storage_json.py index cac7f9d2fa..0305b59ef5 100644 --- a/esphome/storage_json.py +++ b/esphome/storage_json.py @@ -7,7 +7,7 @@ import os from esphome import const from esphome.core import CORE -from esphome.helpers import mkdir_p +from esphome.helpers import mkdir_p, write_file_if_changed # pylint: disable=unused-import, wrong-import-order from esphome.core import CoreType # noqa @@ -89,8 +89,7 @@ class StorageJSON(object): def save(self, path): mkdir_p(os.path.dirname(path)) - with codecs.open(path, 'w', encoding='utf-8') as f_handle: - f_handle.write(self.to_json()) + write_file_if_changed(path, self.to_json()) @staticmethod def from_esphome_core(esph, old): # type: (CoreType, Optional[StorageJSON]) -> StorageJSON @@ -130,8 +129,7 @@ class StorageJSON(object): @staticmethod def _load_impl(path): # type: (str) -> Optional[StorageJSON] with codecs.open(path, 'r', encoding='utf-8') as f_handle: - text = f_handle.read() - storage = json.loads(text, encoding='utf-8') + storage = json.load(f_handle) storage_version = storage['storage_version'] name = storage.get('name') comment = storage.get('comment') @@ -195,15 +193,12 @@ class EsphomeStorageJSON(object): return json.dumps(self.as_dict(), indent=2) + u'\n' def save(self, path): # type: (str) -> None - mkdir_p(os.path.dirname(path)) - with codecs.open(path, 'w', encoding='utf-8') as f_handle: - f_handle.write(self.to_json()) + write_file_if_changed(path, self.to_json()) @staticmethod def _load_impl(path): # type: (str) -> Optional[EsphomeStorageJSON] with codecs.open(path, 'r', encoding='utf-8') as f_handle: - text = f_handle.read() - storage = json.loads(text, encoding='utf-8') + storage = json.load(f_handle) storage_version = storage['storage_version'] cookie_secret = storage.get('cookie_secret') last_update_check = storage.get('last_update_check') diff --git a/esphome/wizard.py b/esphome/wizard.py index 34ff0ec6c7..67ec2c8960 100644 --- a/esphome/wizard.py +++ b/esphome/wizard.py @@ -1,6 +1,5 @@ from __future__ import print_function -import codecs import os import random import string @@ -9,7 +8,7 @@ import unicodedata import voluptuous as vol import esphome.config_validation as cv -from esphome.helpers import color, get_bool_env +from esphome.helpers import color, get_bool_env, write_file # pylint: disable=anomalous-backslash-in-string from esphome.pins import ESP32_BOARD_PINS, ESP8266_BOARD_PINS from esphome.py_compat import safe_input, text_type @@ -104,8 +103,7 @@ def wizard_write(path, **kwargs): kwargs['platform'] = 'ESP8266' if board in ESP8266_BOARD_PINS else 'ESP32' platform = kwargs['platform'] - with codecs.open(path, 'w', 'utf-8') as f_handle: - f_handle.write(wizard_file(**kwargs)) + write_file(path, wizard_file(**kwargs)) storage = StorageJSON.from_wizard(name, name + '.local', platform, board) storage_path = ext_storage_path(os.path.dirname(path), os.path.basename(path)) storage.save(storage_path) diff --git a/esphome/writer.py b/esphome/writer.py index 8fa239d608..4c2e03569f 100644 --- a/esphome/writer.py +++ b/esphome/writer.py @@ -8,7 +8,8 @@ from esphome.config import iter_components from esphome.const import CONF_BOARD_FLASH_MODE, CONF_ESPHOME, CONF_PLATFORMIO_OPTIONS, \ HEADER_FILE_EXTENSIONS, SOURCE_FILE_EXTENSIONS, __version__ from esphome.core import CORE, EsphomeError -from esphome.helpers import mkdir_p, read_file, write_file_if_changed, walk_files +from esphome.helpers import mkdir_p, read_file, write_file_if_changed, walk_files, \ + copy_file_if_changed from esphome.storage_json import StorageJSON, storage_path _LOGGER = logging.getLogger(__name__) @@ -112,7 +113,7 @@ def migrate_src_version_0_to_1(): "auto-generated again.", main_cpp, main_cpp) _LOGGER.info("Migration: Added include section to %s", main_cpp) - write_file_if_changed(content, main_cpp) + write_file_if_changed(main_cpp, content) def migrate_src_version(old, new): @@ -251,7 +252,7 @@ def write_platformio_ini(content): content_format = INI_BASE_FORMAT full_file = content_format[0] + INI_AUTO_GENERATE_BEGIN + '\n' + content full_file += INI_AUTO_GENERATE_END + content_format[1] - write_file_if_changed(full_file, path) + write_file_if_changed(path, full_file) def write_platformio_project(): @@ -285,7 +286,6 @@ or use the custom_components folder. def copy_src_tree(): - import filecmp import shutil source_files = {} @@ -321,9 +321,7 @@ def copy_src_tree(): os.remove(path) else: src_path = source_files_copy.pop(target) - if not filecmp.cmp(path, src_path): - # Files are not same, copy - shutil.copy(src_path, path) + copy_file_if_changed(src_path, path) # Now copy new files for target, src_path in source_files_copy.items(): @@ -332,14 +330,14 @@ def copy_src_tree(): shutil.copy(src_path, dst_path) # Finally copy defines - write_file_if_changed(generate_defines_h(), - CORE.relative_src_path('esphome', 'core', 'defines.h')) - write_file_if_changed(ESPHOME_README_TXT, - CORE.relative_src_path('esphome', 'README.txt')) - write_file_if_changed(ESPHOME_H_FORMAT.format(include_s), - CORE.relative_src_path('esphome.h')) - write_file_if_changed(VERSION_H_FORMAT.format(__version__), - CORE.relative_src_path('esphome', 'core', 'version.h')) + write_file_if_changed(CORE.relative_src_path('esphome', 'core', 'defines.h'), + generate_defines_h()) + write_file_if_changed(CORE.relative_src_path('esphome', 'README.txt'), + ESPHOME_README_TXT) + write_file_if_changed(CORE.relative_src_path('esphome.h'), + ESPHOME_H_FORMAT.format(include_s)) + write_file_if_changed(CORE.relative_src_path('esphome', 'core', 'version.h'), + VERSION_H_FORMAT.format(__version__)) def generate_defines_h(): @@ -365,7 +363,7 @@ def write_cpp(code_s): full_file = code_format[0] + CPP_INCLUDE_BEGIN + u'\n' + global_s + CPP_INCLUDE_END full_file += code_format[1] + CPP_AUTO_GENERATE_BEGIN + u'\n' + code_s + CPP_AUTO_GENERATE_END full_file += code_format[2] - write_file_if_changed(full_file, path) + write_file_if_changed(path, full_file) def clean_build():