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

@@ -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:
+    return read_file(path)
-        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))

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:
+    except OSError as err:
-        if exc.errno == errno.EEXIST and os.path.isdir(path):
+        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))
+        _write_file(path, text)
-        with codecs.open(path, 'w+', encoding='utf-8') as f_handle:
-            f_handle.write(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):
+    if os.path.isfile(path):
-        src_content = read_file(dst)
+        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

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, text_type):
-        if isinstance(data, unicode):
         return data
-        return unicode(data, encoding=encoding, errors=errors)
+    return codecs.decode(data, encoding, errors)
-else:
+
-    def decode_text(data, encoding='utf-8', errors='strict'):
+
-        # type: (bytes, str, str) -> str
+def encode_text(data, encoding='utf-8', errors='strict'):
-        if isinstance(data, str):
+    if isinstance(data, binary_type):
         return data
-        return data.decode(encoding=encoding, errors=errors)
+
+    return codecs.encode(data, encoding, errors)

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:
+        write_file_if_changed(path, self.to_json())
-            f_handle.write(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.load(f_handle)
-        storage = json.loads(text, encoding='utf-8')
         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))
+        write_file_if_changed(path, self.to_json())
-        with codecs.open(path, 'w', encoding='utf-8') as f_handle:
-            f_handle.write(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.load(f_handle)
-        storage = json.loads(text, encoding='utf-8')
         storage_version = storage['storage_version']
         cookie_secret = storage.get('cookie_secret')
         last_update_check = storage.get('last_update_check')

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:
+    write_file(path, wizard_file(**kwargs))
-        f_handle.write(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)

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):
+            copy_file_if_changed(src_path, path)
-                # Files are not same, copy
-                shutil.copy(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(),
+    write_file_if_changed(CORE.relative_src_path('esphome', 'core', 'defines.h'),
-                          CORE.relative_src_path('esphome', 'core', 'defines.h'))
+                          generate_defines_h())
-    write_file_if_changed(ESPHOME_README_TXT,
+    write_file_if_changed(CORE.relative_src_path('esphome', 'README.txt'),
-                          CORE.relative_src_path('esphome', 'README.txt'))
+                          ESPHOME_README_TXT)
-    write_file_if_changed(ESPHOME_H_FORMAT.format(include_s),
+    write_file_if_changed(CORE.relative_src_path('esphome.h'),
-                          CORE.relative_src_path('esphome.h'))
+                          ESPHOME_H_FORMAT.format(include_s))
-    write_file_if_changed(VERSION_H_FORMAT.format(__version__),
+    write_file_if_changed(CORE.relative_src_path('esphome', 'core', 'version.h'),
-                          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():