Bug 1520683 - When the file generation fails, don't write to the underlying file. r=mshal,froydnj
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 17 Jan 2019 15:52:10 +0000
changeset 514269 cc32943d74cc1623f28d6dedd23beabdfbbbf753
parent 514268 b1a1231573cd3922090c48dc9676db8d8371ef58
child 514270 de634684229e8c53ceb03b56dd45e52a1a0af9e1
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmshal, froydnj
bugs1520683
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1520683 - When the file generation fails, don't write to the underlying file. r=mshal,froydnj That way we avoid massive rebuilds due to errors that are recoverable and that would make the file generation succeed with the same contents as before. Differential Revision: https://phabricator.services.mozilla.com/D16858
python/mozbuild/mozbuild/action/file_generate.py
python/mozbuild/mozbuild/util.py
--- a/python/mozbuild/mozbuild/action/file_generate.py
+++ b/python/mozbuild/mozbuild/action/file_generate.py
@@ -62,17 +62,23 @@ def main(argv):
     if not hasattr(module, method):
         print('Error: script "{0}" is missing a {1} method'.format(script, method),
               file=sys.stderr)
         return 1
 
     ret = 1
     try:
         with FileAvoidWrite(args.output_file, mode='rb') as output:
-            ret = module.__dict__[method](output, *args.additional_arguments, **kwargs)
+            try:
+                ret = module.__dict__[method](output, *args.additional_arguments, **kwargs)
+            except:
+                # Ensure that we don't overwrite the file if the script failed.
+                output.avoid_writing_to_file()
+                raise
+
             # The following values indicate a statement of success:
             #  - a set() (see below)
             #  - 0
             #  - False
             #  - None
             #
             # Everything else is an error (so scripts can conveniently |return
             # 1| or similar). If a set is returned, the elements of the set
@@ -95,16 +101,20 @@ def main(argv):
                 # Add dependencies on any buildconfig items that were accessed
                 # by the script.
                 deps |= set(buildconfig.get_dependencies())
 
                 mk = Makefile()
                 mk.create_rule([args.dep_target]).add_dependencies(deps)
                 with FileAvoidWrite(args.dep_file) as dep_file:
                     mk.dump(dep_file)
+            else:
+                # Ensure that we don't overwrite the file if the script failed.
+                output.avoid_writing_to_file()
+
     except IOError as e:
         print('Error opening file "{0}"'.format(e.filename), file=sys.stderr)
         traceback.print_exc()
         return 1
     return ret
 
 if __name__ == '__main__':
     sys.exit(main(sys.argv[1:]))
--- a/python/mozbuild/mozbuild/util.py
+++ b/python/mozbuild/mozbuild/util.py
@@ -214,25 +214,28 @@ class FileAvoidWrite(BytesIO):
     Additionally, there is dry run mode where the file is not actually written
     out, but reports whether the file was existing and would have been updated
     still occur, as well as diff capture if requested.
     """
     def __init__(self, filename, capture_diff=False, dry_run=False, mode='rU'):
         BytesIO.__init__(self)
         self.name = filename
         self._capture_diff = capture_diff
-        self._dry_run = dry_run
+        self._write_to_file = not dry_run
         self.diff = None
         self.mode = mode
 
     def write(self, buf):
         if isinstance(buf, unicode):
             buf = buf.encode('utf-8')
         BytesIO.write(self, buf)
 
+    def avoid_writing_to_file(self):
+        self._write_to_file = False
+
     def close(self):
         """Stop accepting writes, compare file contents, and rewrite if needed.
 
         Returns a tuple of bools indicating what action was performed:
 
             (file existed, file updated)
 
         If ``capture_diff`` was specified at construction time and the
@@ -254,17 +257,17 @@ class FileAvoidWrite(BytesIO):
                 old_content = existing.read()
                 if old_content == buf:
                     return True, False
             except IOError:
                 pass
             finally:
                 existing.close()
 
-        if not self._dry_run:
+        if self._write_to_file:
             ensureParentDir(self.name)
             # Maintain 'b' if specified.  'U' only applies to modes starting with
             # 'r', so it is dropped.
             writemode = 'w'
             if 'b' in self.mode:
                 writemode += 'b'
             with open(self.name, writemode) as file:
                 file.write(buf)