[Bf-blender-cvs] [4179be64993] blender-v2.83-release: Codesign: Report codesign errors from server to worker

Sergey Sharybin noreply at git.blender.org
Fri Nov 13 11:37:33 CET 2020


Commit: 4179be649934d138f631b1ee69c15592a2da2670
Author: Sergey Sharybin
Date:   Thu Nov 12 10:12:45 2020 +0100
Branches: blender-v2.83-release
https://developer.blender.org/rB4179be649934d138f631b1ee69c15592a2da2670

Codesign: Report codesign errors from server to worker

Pass codesign errors (if any) from codesign buildbot server to the
buildbot worker, so that the latter one can abort build process if
the error happens. This solves issues when non-properly-notarized
DMG package gets uploaded to the buildbot website.

===================================================================

M	build_files/buildbot/codesign/archive_with_indicator.py
M	build_files/buildbot/codesign/base_code_signer.py
A	build_files/buildbot/codesign/exception.py
M	build_files/buildbot/codesign/macos_code_signer.py
M	build_files/buildbot/codesign/windows_code_signer.py

===================================================================

diff --git a/build_files/buildbot/codesign/archive_with_indicator.py b/build_files/buildbot/codesign/archive_with_indicator.py
index 085026fcf98..aebf5a15417 100644
--- a/build_files/buildbot/codesign/archive_with_indicator.py
+++ b/build_files/buildbot/codesign/archive_with_indicator.py
@@ -18,12 +18,72 @@
 
 # <pep8 compliant>
 
+import dataclasses
+import json
 import os
+
 from pathlib import Path
+from typing import Optional
 
 import codesign.util as util
 
 
+class ArchiveStateError(Exception):
+    message: str
+
+    def __init__(self, message):
+        self.message = message
+        super().__init__(self.message)
+
+
+ at dataclasses.dataclass
+class ArchiveState:
+    """
+    Additional information (state) of the archive
+
+    Includes information like expected file size of the archive file in the case
+    the archive file is expected to be successfully created.
+
+    If the archive can not be created, this state will contain error message
+    indicating details of error.
+    """
+
+    # Size in bytes of the corresponding archive.
+    file_size: Optional[int] = None
+
+    # Non-empty value indicates that error has happenned.
+    error_message: str = ''
+
+    def has_error(self) -> bool:
+        """
+        Check whether the archive is at error state
+        """
+
+        return self.error_message
+
+    def serialize_to_string(self) -> str:
+        payload = dataclasses.asdict(self)
+        return json.dumps(payload, sort_keys=True, indent=4)
+
+    def serialize_to_file(self, filepath: Path) -> None:
+        string = self.serialize_to_string()
+        filepath.write_text(string)
+
+    @classmethod
+    def deserialize_from_string(cls, string: str) -> 'ArchiveState':
+        try:
+            object_as_dict = json.loads(string)
+        except json.decoder.JSONDecodeError:
+            raise ArchiveStateError('Error parsing JSON')
+
+        return cls(**object_as_dict)
+
+    @classmethod
+    def deserialize_from_file(cls, filepath: Path):
+        string = filepath.read_text()
+        return cls.deserialize_from_string(string)
+
+
 class ArchiveWithIndicator:
     """
     The idea of this class is to wrap around logic which takes care of keeping
@@ -79,6 +139,19 @@ class ArchiveWithIndicator:
         if not self.ready_indicator_filepath.exists():
             return False
 
+        try:
+            archive_state = ArchiveState.deserialize_from_file(
+                self.ready_indicator_filepath)
+        except ArchiveStateError as error:
+            print(f'Error deserializing archive state: {error.message}')
+            return False
+
+        if archive_state.has_error():
+            # If the error did happen during codesign procedure there will be no
+            # corresponding archive file.
+            # The caller code will deal with the error check further.
+            return True
+
         # Sometimes on macOS indicator file appears prior to the actual archive
         # despite the order of creation and os.sync() used in tag_ready().
         # So consider archive not ready if there is an indicator without an
@@ -88,23 +161,11 @@ class ArchiveWithIndicator:
                   f'({self.archive_filepath}) to appear.')
             return False
 
-        # Read archive size from indicator/
-        #
-        # Assume that file is either empty or is fully written. This is being checked
-        # by performing ValueError check since empty string will throw this exception
-        # when attempted to be converted to int.
-        expected_archive_size_str = self.ready_indicator_filepath.read_text()
-        try:
-            expected_archive_size = int(expected_archive_size_str)
-        except ValueError:
-            print(f'Invalid archive size "{expected_archive_size_str}"')
-            return False
-
         # Wait for until archive is fully stored.
         actual_archive_size = self.archive_filepath.stat().st_size
-        if  actual_archive_size != expected_archive_size:
+        if actual_archive_size != archive_state.file_size:
             print('Partial/invalid archive size (expected '
-                  f'{expected_archive_size} got {actual_archive_size})')
+                  f'{archive_state.file_size} got {actual_archive_size})')
             return False
 
         return True
@@ -129,7 +190,7 @@ class ArchiveWithIndicator:
             print(f'Exception checking archive: {e}')
             return False
 
-    def tag_ready(self) -> None:
+    def tag_ready(self, error_message='') -> None:
         """
         Tag the archive as ready by creating the corresponding indication file.
 
@@ -138,13 +199,34 @@ class ArchiveWithIndicator:
               If it is violated, an assert will fail.
         """
         assert not self.is_ready()
+
         # Try the best to make sure everything is synced to the file system,
         # to avoid any possibility of stamp appearing on a network share prior to
         # an actual file.
         if util.get_current_platform() != util.Platform.WINDOWS:
             os.sync()
-        archive_size = self.archive_filepath.stat().st_size
-        self.ready_indicator_filepath.write_text(str(archive_size))
+
+        archive_size = -1
+        if self.archive_filepath.exists():
+            archive_size = self.archive_filepath.stat().st_size
+
+        archive_info = ArchiveState(
+            file_size=archive_size, error_message=error_message)
+
+        self.ready_indicator_filepath.write_text(
+            archive_info.serialize_to_string())
+
+    def get_state(self) -> ArchiveState:
+        """
+        Get state object for this archive
+
+        The state is read from the corresponding state file.
+        """
+
+        try:
+            return ArchiveState.deserialize_from_file(self.ready_indicator_filepath)
+        except ArchiveStateError as error:
+            return ArchiveState(error_message=f'Error in information format: {error}')
 
     def clean(self) -> None:
         """
diff --git a/build_files/buildbot/codesign/base_code_signer.py b/build_files/buildbot/codesign/base_code_signer.py
index 66953bfc5e5..b816415e7e4 100644
--- a/build_files/buildbot/codesign/base_code_signer.py
+++ b/build_files/buildbot/codesign/base_code_signer.py
@@ -58,6 +58,7 @@ import codesign.util as util
 
 from codesign.absolute_and_relative_filename import AbsoluteAndRelativeFileName
 from codesign.archive_with_indicator import ArchiveWithIndicator
+from codesign.exception import CodeSignException
 
 
 logger = logging.getLogger(__name__)
@@ -145,13 +146,13 @@ class BaseCodeSigner(metaclass=abc.ABCMeta):
     def cleanup_environment_for_builder(self) -> None:
         # TODO(sergey): Revisit need of cleaning up the existing files.
         # In practice it wasn't so helpful, and with multiple clients
-        # talking to the same server it becomes even mor etricky.
+        # talking to the same server it becomes even more tricky.
         pass
 
     def cleanup_environment_for_signing_server(self) -> None:
         # TODO(sergey): Revisit need of cleaning up the existing files.
         # In practice it wasn't so helpful, and with multiple clients
-        # talking to the same server it becomes even mor etricky.
+        # talking to the same server it becomes even more tricky.
         pass
 
     def generate_request_id(self) -> str:
@@ -220,9 +221,15 @@ class BaseCodeSigner(metaclass=abc.ABCMeta):
         """
         Wait until archive with signed files is available.
 
+        Will only return if the archive with signed files is available. If there
+        was an error during code sign procedure the SystemExit exception is
+        raised, with the message set to the error reported by the codesign
+        server.
+
         Will only wait for the configured time. If that time exceeds and there
         is still no responce from the signing server the application will exit
         with a non-zero exit code.
+
         """
 
         signed_archive_info = self.signed_archive_info_for_request_id(
@@ -236,9 +243,17 @@ class BaseCodeSigner(metaclass=abc.ABCMeta):
             time.sleep(1)
             time_slept_in_seconds = time.monotonic() - time_start
             if time_slept_in_seconds > timeout_in_seconds:
+                signed_archive_info.clean()
                 unsigned_archive_info.clean()
                 raise SystemExit("Signing server didn't finish signing in "
-                                 f"{timeout_in_seconds} seconds, dying :(")
+                                 f'{timeout_in_seconds} seconds, dying :(')
+
+        archive_state = signed_archive_info.get_state()
+        if archive_state.has_error():
+            signed_archive_info.clean()
+            unsigned_archive_info.clean()
+            raise SystemExit(
+                f'Error happenned during codesign procedure: {archive_state.error_message}')
 
     def copy_signed_files_to_directory(
             self, signed_dir: Path, destination_dir: Path) -> None:
@@ -391,7 +406,13 @@ class BaseCodeSigner(metaclass=abc.ABCMeta):
                 temp_dir)
 
             logger_server.info('Signing all requested files...')
-            self.sign_all_files(files)
+            try:
+                self.sign_all_files(files)
+            except CodeSignException as error:
+                signed_archive_info.tag_ready(error_message=error.message)
+                unsigned_archive_info.clean()
+                logger_server.info('Signing is complete with errors.')
+                return
 
             logger_server.info('Packing signed files...')
             pack_files(files=files,
diff --git a/build_files/buildbot/codesign/exception.py b/build_files/buildbot/codesign/exception.py
new file mode 100644
index 00000000000..6c8a9f262a5
--- /dev/null
+++ b/build_files/buildbot/codesign/exception.py
@@ -0,0 +1,26 @@
+# ##### BEGIN GPL LICENSE BLOCK #####
+#
+#  This program is free software; you can redistribute it and/or
+#  modify it under the terms of the GNU General Public License
+#  as published by the Free Software Foundation; either version 2
+#  of the License, or (at your option) any later version.
+#
+#  This program is distributed in the hope that it will be u

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list