Address comments on 058b5a802f

Also address some from 67b1857e58 which got left out of the previous
commit

bpkg/__init__.py

@@ -340,7 +340,13 @@ class USERPREF_PT_packages(bpy.types.Panel):
         return (userpref.active_section == 'PACKAGES')
 
     def draw(self, context):
-        repo = context.user_preferences.addons[__package__].preferences['repo']
+        try:
+            repo = context.user_preferences.addons[__package__].preferences['repo']
+        except KeyError:
+            # HACK: 
+            # If no repositories are initialized, we should try to refresh them. If that doesn't work, display a message
+            repo = {'packages': []}
+
         layout = self.layout
 
         main = layout.row()

bpkg/subproc.py

@@ -101,6 +101,9 @@ class InplaceBackup:
 
     def backup(self):
         """Move 'path' to 'path~'"""
+        if not self.path.exists():
+            raise FileNotFoundError("Can't backup path which doesn't exist")
+
         self.backup_path = pathlib.Path(str(self.path) + '~')
         if self.backup_path.exists():
             self.log.warning("Overwriting existing backup '{}'".format(self.backup_path))
@@ -110,12 +113,17 @@ class InplaceBackup:
 
     def restore(self):
         """Move 'path~' to 'path'"""
-        if not self.backup_path:
+        try:
-            raise RuntimeError("Can't restore file before backing it up")
+            getattr(self, 'backup_path')
+        except AttributeError as err:
+            raise RuntimeError("Can't restore file before backing it up") from err
+
+        if not self.backup_path.exists():
+            raise FileNotFoundError("Can't restore backup which doesn't exist")
 
         if self.path.exists():
             self.log.warning("Overwriting '{0}' with backup file".format(self.path))
-            self._rm(self.backup_path)
+            self._rm(self.path)
 
         shutil.move(str(self.backup_path), str(self.path))
 
@@ -125,10 +133,10 @@ class InplaceBackup:
 
     def _rm(self, path: pathlib.Path):
         """Just delete whatever is specified by `path`"""
-        if path.is_file():
+        if path.is_dir():
-            path.unlink()
-        else:
             shutil.rmtree(str(path))
+        else:
+            path.unlink()
 
 
 
@@ -234,12 +242,9 @@ class Repository:
         """
         Return a dict representation of the repository
         """
-        if self.packages:
+        packages = [p.to_dict() for p in self.packages]
-            packages = [p.to_dict() for p in self.packages]
+        if sort:
-            if sort:
+            packages.sort(key=lambda p: p['bl_info']['name'].lower())
-                packages.sort(key=lambda p: p['bl_info']['name'].lower())
-        else:
-            packages = []
 
         return {
                 'name':     self.name,
@@ -255,7 +260,7 @@ class Repository:
         self.name = repodict.get('name', "")
         self.url = repodict.get('url', "")
         self.packages = [Package(pkg) for pkg in repodict.get('packages', [])]
-        self._headers = repodict.get('_headers', [])
+        self._headers = repodict.get('_headers', {})
 
     @classmethod
     def from_dict(cls, repodict: dict):
@@ -368,14 +373,10 @@ def _install(pipe_to_blender, pkgpath: pathlib.Path, dest: pathlib.Path, searchp
     pipe_to_blender.send(Progress(0.0))
 
     if not pkgpath.is_file():
-        log.error("File to install isn't a file: %s", pkgpath)
+        raise InstallException("Package isn't a file")
-        pipe_to_blender.send(InstallError("Package is not a file"))
-        raise InstallException
 
     if not dest.is_dir():
-        log.error("Destination for install isn't a directory: %s", dest)
+        raise InstallException("Destination is not a directory")
-        pipe_to_blender.send(InstallError("Destination is not a directory"))
-        raise InstallException
 
     # TODO: check to make sure addon/package isn't already installed elsewhere
 
@@ -387,11 +388,7 @@ def _install(pipe_to_blender, pkgpath: pathlib.Path, dest: pathlib.Path, searchp
         try:
             file_to_extract = zipfile.ZipFile(str(pkgpath), 'r')
         except Exception as err:
-            # TODO HACK: see if it makes sense to make a single InstallError class which inherits from both Exception and SubprocError
+            raise InstallException("Failed to read zip file: %s" % err) from err
-            # It sounds weird, but it could avoid the need for duplication here; create a new InstallError with the right message,
-            # then send it to blender and also raise it.
-            pipe_to_blender.send(InstallError("Failed to read zip file: %s" % err))
-            raise InstallException from err
 
         def root_files(filelist: list) -> list:
             """Some string parsing to get a list of the root contents of a zip from its namelist"""
@@ -406,6 +403,7 @@ def _install(pipe_to_blender, pkgpath: pathlib.Path, dest: pathlib.Path, searchp
         conflicts = [dest / f for f in root_files(file_to_extract.namelist()) if (dest / f).exists()]
         backups = []
         for conflict in conflicts:
+            log.debug("Creating backup of conflict %s", conflict)
             backups.append(InplaceBackup(conflict))
 
         try:
@@ -413,8 +411,7 @@ def _install(pipe_to_blender, pkgpath: pathlib.Path, dest: pathlib.Path, searchp
         except Exception as err:
             for backup in backups:
                 backup.restore()
-            pipe_to_blender.send(InstallError("Failed to extract zip file to '%s': %s" % (dest, err)))
+            raise InstallException("Failed to extract zip file to '%s': %s" % (dest, err)) from err
-            raise InstallException from err
 
         for backup in backups:
             backup.remove()
@@ -430,15 +427,14 @@ def _install(pipe_to_blender, pkgpath: pathlib.Path, dest: pathlib.Path, searchp
             shutil.copyfile(str(pkgpath), str(dest_file))
         except Exception as err:
             backup.restore()
-            pipe_to_blender.send(InstallError("Failed to copy file to '%s': %s" % (dest, err)))
+            raise InstallException("Failed to copy file to '%s': %s" % (dest, err)) from err
-            raise InstallException from err
 
     try:
         pkgpath.unlink()
         log.debug("Removed cached package: %s", pkgpath)
     except Exception as err:
-        pipe_to_blender.send(SubprocWarning("Failed to remove package from cache: %s" % err))
+        pipe_to_blender.send(SubprocWarning("Install succeeded, but failed to remove package from cache: %s" % err))
-        raise InstallException from err
+        log.warning("Failed to remove package from cache: %s", err)
 
     pipe_to_blender.send(Progress(1.0))
     return
@@ -464,7 +460,7 @@ def download_and_install(pipe_to_blender, package_url: str, install_path: pathli
         _install(pipe_to_blender, downloaded, install_path, search_paths)
         pipe_to_blender.send(Success())
     except InstallException as err:
-        log.error("Failed to install package: %s", err)
+        log.exception("Failed to install package: %s", err)
         pipe_to_blender.send(InstallError(err))
 
 def refresh(pipe_to_blender, storage_path: pathlib.Path, repository_url: str):