[Reproducible-commits] [diffoscope] 03/06: Make container class available on demand instead of bracketing

Jérémy Bobbio lunar at moszumanska.debian.org
Sat Dec 5 23:18:58 UTC 2015


This is an automated email from the git hooks/post-receive script.

lunar pushed a commit to branch master
in repository diffoscope.

commit c359af51cd45dae08701658d82c8616f784cf5e3
Author: Jérémy Bobbio <lunar at debian.org>
Date:   Sat Dec 5 12:26:43 2015 +0000

    Make container class available on demand instead of bracketing
    
    To access the content of a container, we used to require call to the open()
    method. Such bracketed operation makes it difficult to implement parallel
    processing. Instead, we now make container objects associated with a given file
    type available through the as_container property.
    
    The as_container property will lazily initialize an associated container
    object. The reference to the container object will be removed when the
    cleanup() method is called. Ressource can thus be deallocated on garbage
    collection.
    
    With this change, we can finally make Container.compare() return an iterator
    that can be lazily consumed instead of an evaluated list: the various files that
    need to be extracted to perform the comparisosn will be made available when
    required.
    
    File types which can be treated as containers must now define the
    CONTAINER_CLASS constant to point at a Container subclass handling this
    particular file type.
---
 diffoscope/comparators/binary.py     | 18 +++++++++++---
 diffoscope/comparators/bzip2.py      | 18 +++++---------
 diffoscope/comparators/cbfs.py       |  8 +++----
 diffoscope/comparators/cpio.py       |  5 ++--
 diffoscope/comparators/deb.py        | 17 +++++++------
 diffoscope/comparators/debian.py     | 16 ++++++-------
 diffoscope/comparators/dex.py        | 14 +++++------
 diffoscope/comparators/directory.py  | 45 ++++++++++++++++-------------------
 diffoscope/comparators/gzip.py       | 18 +++++---------
 diffoscope/comparators/iso9660.py    |  5 ++--
 diffoscope/comparators/libarchive.py |  2 +-
 diffoscope/comparators/rpm.py        | 17 +++++--------
 diffoscope/comparators/squashfs.py   |  9 ++++---
 diffoscope/comparators/tar.py        |  7 +++---
 diffoscope/comparators/utils.py      | 46 +++++++++++++-----------------------
 diffoscope/comparators/xz.py         | 18 +++++---------
 diffoscope/comparators/zip.py        |  9 ++++---
 17 files changed, 117 insertions(+), 155 deletions(-)

diff --git a/diffoscope/comparators/binary.py b/diffoscope/comparators/binary.py
index b2a40cb..aa74277 100644
--- a/diffoscope/comparators/binary.py
+++ b/diffoscope/comparators/binary.py
@@ -110,10 +110,10 @@ class File(object, metaclass=ABCMeta):
         raise NotImplemented
 
     # Remove any temporary data associated with the file. The function
-    # should be idempotent and work during the destructor. It does nothing by
-    # default.
+    # should be idempotent and work during the destructor.
     def cleanup(self):
-        pass
+        if hasattr(self, '_as_container'):
+            del self._as_container
 
     def __del__(self):
         self.cleanup()
@@ -124,6 +124,18 @@ class File(object, metaclass=ABCMeta):
         return self._name
 
     @property
+    def as_container(self):
+        if not hasattr(self.__class__, 'CONTAINER_CLASS'):
+            if hasattr(self, '_other_file'):
+                return self._other_file.__class__.CONTAINER_CLASS(self)
+            raise NotImplemented('Not a container.')
+        if not hasattr(self, '_as_container'):
+            logger.debug('instanciating %s for %s', self.__class__.CONTAINER_CLASS, self)
+            self._as_container = self.__class__.CONTAINER_CLASS(self)
+        logger.debug('returning a %s for %s', self._as_container.__class__, self)
+        return self._as_container
+
+    @property
     def magic_file_type(self):
         if not hasattr(self, '_magic_file_type'):
             self._magic_file_type = File.guess_file_type(self.path)
diff --git a/diffoscope/comparators/bzip2.py b/diffoscope/comparators/bzip2.py
index aef9c46..6429847 100644
--- a/diffoscope/comparators/bzip2.py
+++ b/diffoscope/comparators/bzip2.py
@@ -27,22 +27,17 @@ from diffoscope import logger, tool_required
 
 
 class Bzip2Container(Archive):
-    @property
-    def path(self):
-        return self._path
-
-    def open_archive(self, path):
-        self._path = path
+    def open_archive(self):
         return self
 
     def close_archive(self):
-        self._path = None
+        pass
 
     def get_members(self):
         return {'bzip2-content': self.get_member(self.get_member_names()[0])}
 
     def get_member_names(self):
-        return [get_compressed_content_name(self.path, '.bz2')]
+        return [get_compressed_content_name(self.source.path, '.bz2')]
 
     @tool_required('bzip2')
     def extract(self, member_name, dest_dir):
@@ -50,12 +45,13 @@ class Bzip2Container(Archive):
         logger.debug('bzip2 extracting to %s', dest_path)
         with open(dest_path, 'wb') as fp:
             subprocess.check_call(
-                ["bzip2", "--decompress", "--stdout", self.path],
+                ["bzip2", "--decompress", "--stdout", self.source.path],
                 shell=False, stdout=fp, stderr=None)
         return dest_path
 
 
 class Bzip2File(File):
+    CONTAINER_CLASS = Bzip2Container
     RE_FILE_TYPE = re.compile(r'^bzip2 compressed data\b')
 
     @staticmethod
@@ -63,6 +59,4 @@ class Bzip2File(File):
         return Bzip2File.RE_FILE_TYPE.match(file.magic_file_type)
 
     def compare_details(self, other, source=None):
-        with Bzip2Container(self).open() as my_container, \
-             Bzip2Container(other).open() as other_container:
-            return my_container.compare(other_container)
+        return self.as_container.compare(other.as_container)
diff --git a/diffoscope/comparators/cbfs.py b/diffoscope/comparators/cbfs.py
index 00efa5a..a24230c 100644
--- a/diffoscope/comparators/cbfs.py
+++ b/diffoscope/comparators/cbfs.py
@@ -59,7 +59,7 @@ class CbfsContainer(Archive):
                 continue
             yield name
 
-    def open_archive(self, path):
+    def open_archive(self):
         return self
 
     def close_archive(self):
@@ -92,6 +92,8 @@ def is_header_valid(buf, size, offset=0):
 
 
 class CbfsFile(File):
+    CONTAINER_CLASS = CbfsContainer
+
     @staticmethod
     def recognizes(file):
         size = os.stat(file.path).st_size
@@ -129,7 +131,5 @@ class CbfsFile(File):
     def compare_details(self, other, source=None):
         differences = []
         differences.append(Difference.from_command(CbfsListing, self.path, other.path))
-        with CbfsContainer(self).open() as my_container, \
-             CbfsContainer(other).open() as other_container:
-            differences.extend(my_container.compare(other_container))
+        differences.extend(self.as_container.compare(other.as_container))
         return differences
diff --git a/diffoscope/comparators/cpio.py b/diffoscope/comparators/cpio.py
index 4f2f76b..536e80c 100644
--- a/diffoscope/comparators/cpio.py
+++ b/diffoscope/comparators/cpio.py
@@ -33,6 +33,7 @@ class CpioContent(Command):
 
 
 class CpioFile(File):
+    CONTAINER_CLASS = LibarchiveContainer
     RE_FILE_TYPE = re.compile(r'\bcpio archive\b')
 
     @staticmethod
@@ -43,7 +44,5 @@ class CpioFile(File):
         differences = []
         differences.append(Difference.from_command(
             CpioContent, self.path, other.path, source="file list"))
-        with LibarchiveContainer(self).open() as my_container, \
-             LibarchiveContainer(other).open() as other_container:
-            differences.extend(my_container.compare(other_container))
+        differences.extend(self.as_container.compare(other.as_container))
         return differences
diff --git a/diffoscope/comparators/deb.py b/diffoscope/comparators/deb.py
index ba1e529..8043389 100644
--- a/diffoscope/comparators/deb.py
+++ b/diffoscope/comparators/deb.py
@@ -33,6 +33,7 @@ class DebContainer(LibarchiveContainer):
 
 
 class DebFile(File):
+    CONTAINER_CLASS = DebContainer
     RE_FILE_TYPE = re.compile(r'^Debian binary package')
 
     @staticmethod
@@ -55,9 +56,7 @@ class DebFile(File):
         other_content = get_ar_content(other.path)
         differences.append(Difference.from_text(
                                my_content, other_content, self.path, other.path, source="metadata"))
-        with DebContainer(self).open() as my_container, \
-             DebContainer(other).open() as other_container:
-            differences.extend(my_container.compare(other_container))
+        differences.extend(self.as_container.compare(other.as_container))
         return differences
 
 
@@ -101,8 +100,9 @@ class Md5sumsFile(File):
 
 
 class DebTarContainer(TarContainer):
-    def __init__(self, archive, ignore_files):
+    def __init__(self, archive):
         super().__init__(archive)
+        ignore_files = archive.container.source.container.source.files_with_same_content_in_data
         assert type(ignore_files) is set
         self._ignore_files = ignore_files
 
@@ -113,6 +113,8 @@ class DebTarContainer(TarContainer):
 
 
 class DebDataTarFile(File):
+    CONTAINER_CLASS = DebTarContainer
+
     @staticmethod
     def recognizes(file):
         return isinstance(file, ArchiveMember) and \
@@ -122,9 +124,6 @@ class DebDataTarFile(File):
 
     def compare_details(self, other, source=None):
         differences = []
-        ignore_files = self.container.source.container.source.files_with_same_content_in_data
-        with DebTarContainer(self, ignore_files).open() as my_container, \
-             DebTarContainer(other, ignore_files).open() as other_container:
-            differences.append(Difference.from_command(TarListing, self.path, other.path))
-            differences.extend(my_container.compare(other_container))
+        differences.append(Difference.from_command(TarListing, self.path, other.path))
+        differences.extend(self.as_container.compare(other.as_container))
         return differences
diff --git a/diffoscope/comparators/debian.py b/diffoscope/comparators/debian.py
index d6a54ba..703804d 100644
--- a/diffoscope/comparators/debian.py
+++ b/diffoscope/comparators/debian.py
@@ -68,6 +68,10 @@ class DebControlMember(File):
 
 
 class DebControlContainer(Container):
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self._version_re = DebControlContainer.get_version_trimming_re(self)
+
     @staticmethod
     def get_version_trimming_re(dcc):
         version = dcc.source.deb822.get('Version')
@@ -79,12 +83,6 @@ class DebControlContainer(Container):
         else:
             return re.compile(re.escape(version))
 
-    @contextmanager
-    def open(self):
-        self._version_re = DebControlContainer.get_version_trimming_re(self)
-        yield self
-        del self._version_re
-
     def get_members(self):
         return {self._trim_version_number(name): self.get_member(name) for name in self.get_member_names()}
 
@@ -99,6 +97,8 @@ class DebControlContainer(Container):
 
 
 class DebControlFile(File):
+    CONTAINER_CLASS = DebControlContainer
+
     @property
     def deb822(self):
         return self._deb822
@@ -122,9 +122,7 @@ class DebControlFile(File):
         differences.append(Difference.from_text(self.deb822.get_as_string('Files'),
                                                 other.deb822.get_as_string('Files'),
                                                 self.path, other.path, source='Files'))
-        with DebControlContainer(self).open() as my_container, \
-             DebControlContainer(other).open() as other_container:
-            differences.extend(my_container.compare(other_container))
+        differences.extend(self.as_container.compare(other.as_container))
 
         return differences
 
diff --git a/diffoscope/comparators/dex.py b/diffoscope/comparators/dex.py
index 3e96324..017f1d4 100644
--- a/diffoscope/comparators/dex.py
+++ b/diffoscope/comparators/dex.py
@@ -31,35 +31,33 @@ class DexContainer(Archive):
     def path(self):
         return self._path
 
-    def open_archive(self, path):
-        self._path = path
+    def open_archive(self):
         return self
 
     def close_archive(self):
-        self._path = None
+        pass
 
     def get_members(self):
         return {'dex-content': self.get_member(self.get_member_names()[0])}
 
     def get_member_names(self):
-        return [get_compressed_content_name(self.path, '.dex') + '.jar']
+        return [get_compressed_content_name(self.source.path, '.dex') + '.jar']
 
     @tool_required('enjarify')
     def extract(self, member_name, dest_dir):
         dest_path = os.path.join(dest_dir, member_name)
         logger.debug('dex extracting to %s', dest_path)
-        subprocess.check_call(['enjarify', '-o', dest_path, self.path],
+        subprocess.check_call(['enjarify', '-o', dest_path, self.source.path],
             shell=False, stderr=None, stdout=subprocess.PIPE)
         return dest_path
 
 class DexFile(File):
     RE_FILE_TYPE = re.compile(r'^Dalvik dex file .*\b')
+    CONTAINER_CLASS = DexContainer
 
     @staticmethod
     def recognizes(file):
         return DexFile.RE_FILE_TYPE.match(file.magic_file_type)
 
     def compare_details(self, other, source=None):
-        with DexContainer(self).open() as my_container, \
-             DexContainer(other).open() as other_container:
-            return my_container.compare(other_container)
+        return self.as_container.compare(other.as_container)
diff --git a/diffoscope/comparators/directory.py b/diffoscope/comparators/directory.py
index 07d7d9a..cfabff8 100644
--- a/diffoscope/comparators/directory.py
+++ b/diffoscope/comparators/directory.py
@@ -129,21 +129,21 @@ class FilesystemDirectory(object):
         except RequiredToolNotFound:
             logger.info("Unable to find 'getfacl'.")
         differences.extend(compare_meta(self.name, other.name))
-        with DirectoryContainer(self).open() as my_container, \
-             DirectoryContainer(other).open() as other_container:
-            my_names = my_container.get_member_names()
-            other_names = other_container.get_member_names()
-            for name in sorted(set(my_names).intersection(other_names)):
-                my_file = my_container.get_member(name)
-                other_file = other_container.get_member(name)
-                inner_difference = diffoscope.comparators.compare_files(
-                                       my_file, other_file, source=name)
-                meta_differences = compare_meta(my_file.name, other_file.name)
-                if meta_differences and not inner_difference:
-                    inner_difference = Difference(None, my_file.path, other_file.path)
-                if inner_difference:
-                    inner_difference.add_details(meta_differences)
-                    differences.append(inner_difference)
+        my_container = DirectoryContainer(self)
+        other_container = DirectoryContainer(other)
+        my_names = my_container.get_member_names()
+        other_names = other_container.get_member_names()
+        for name in sorted(set(my_names).intersection(other_names)):
+            my_file = my_container.get_member(name)
+            other_file = other_container.get_member(name)
+            inner_difference = diffoscope.comparators.compare_files(
+                                   my_file, other_file, source=name)
+            meta_differences = compare_meta(my_file.name, other_file.name)
+            if meta_differences and not inner_difference:
+                inner_difference = Difference(None, my_file.path, other_file.path)
+            if inner_difference:
+                inner_difference.add_details(meta_differences)
+                differences.append(inner_difference)
         if not differences:
             return None
         difference = Difference(None, self.path, other.path, source)
@@ -152,21 +152,16 @@ class FilesystemDirectory(object):
 
 
 class DirectoryContainer(Container):
-    @contextmanager
-    def open(self):
-        self._path = self.source.path.rstrip('/') or '/'
-        yield self
-        self._path = None
-
     def get_member_names(self):
+        path = self.source.path
         names = []
-        for root, _, files in os.walk(self._path):
-            if root == self._path:
+        for root, _, files in os.walk(path):
+            if root == path:
                 root = ''
             else:
-                root = root[len(self._path) + 1:]
+                root = root[len(path) + 1:]
             names.extend([os.path.join(root, f) for f in files])
         return names
 
     def get_member(self, member_name):
-        return FilesystemFile(os.path.join(self._path, member_name))
+        return FilesystemFile(os.path.join(self.source.path, member_name))
diff --git a/diffoscope/comparators/gzip.py b/diffoscope/comparators/gzip.py
index 481b028..f9f585b 100644
--- a/diffoscope/comparators/gzip.py
+++ b/diffoscope/comparators/gzip.py
@@ -27,22 +27,17 @@ from diffoscope.difference import Difference
 
 
 class GzipContainer(Archive):
-    @property
-    def path(self):
-        return self._path
-
-    def open_archive(self, path):
-        self._path = path
+    def open_archive(self):
         return self
 
     def close_archive(self):
-        self._path = None
+        pass
 
     def get_members(self):
         return {'gzip-content': self.get_member(self.get_member_names()[0])}
 
     def get_member_names(self):
-        return [get_compressed_content_name(self.path, '.gz')]
+        return [get_compressed_content_name(self.source.path, '.gz')]
 
     @tool_required('gzip')
     def extract(self, member_name, dest_dir):
@@ -50,12 +45,13 @@ class GzipContainer(Archive):
         logger.debug('gzip extracting to %s', dest_path)
         with open(dest_path, 'wb') as fp:
             subprocess.check_call(
-                ["gzip", "--decompress", "--stdout", self.path],
+                ["gzip", "--decompress", "--stdout", self.source.path],
                 shell=False, stdout=fp, stderr=None)
         return dest_path
 
 
 class GzipFile(object):
+    CONTAINER_CLASS = GzipContainer
     RE_FILE_TYPE = re.compile(r'^gzip compressed data\b')
 
     @staticmethod
@@ -66,7 +62,5 @@ class GzipFile(object):
         differences = []
         differences.append(Difference.from_text(
                                self.magic_file_type, other.magic_file_type, self, other, source='metadata'))
-        with GzipContainer(self).open() as my_container, \
-             GzipContainer(other).open() as other_container:
-            differences.extend(my_container.compare(other_container))
+        differences.extend(self.as_container.compare(other.as_container))
         return differences
diff --git a/diffoscope/comparators/iso9660.py b/diffoscope/comparators/iso9660.py
index 3eca892..f194fef 100644
--- a/diffoscope/comparators/iso9660.py
+++ b/diffoscope/comparators/iso9660.py
@@ -62,6 +62,7 @@ class ISO9660Listing(Command):
 
 
 class Iso9660File(File):
+    CONTAINER_CLASS = LibarchiveContainer
     RE_FILE_TYPE = re.compile(r'\bISO 9660\b')
 
     @staticmethod
@@ -77,7 +78,5 @@ class Iso9660File(File):
                 differences.append(Difference.from_command(ISO9660Listing, self.path, other.path, command_args=(extension,)))
             except subprocess.CalledProcessError:
                 pass # probably no joliet or rockridge data
-        with LibarchiveContainer(self).open() as my_container, \
-             LibarchiveContainer(other).open() as other_container:
-            differences.extend(my_container.compare(other_container))
+        differences.extend(self.as_container.compare(other.as_container))
         return differences
diff --git a/diffoscope/comparators/libarchive.py b/diffoscope/comparators/libarchive.py
index 9a79312..2a49d95 100644
--- a/diffoscope/comparators/libarchive.py
+++ b/diffoscope/comparators/libarchive.py
@@ -105,7 +105,7 @@ class LibarchiveDevice(Device, LibarchiveMember):
 
 
 class LibarchiveContainer(Archive):
-    def open_archive(self, path):
+    def open_archive(self):
         # libarchive is very very stream oriented an not for random access
         # so we are going to reopen the archive everytime
         # not nice, but it'll work
diff --git a/diffoscope/comparators/rpm.py b/diffoscope/comparators/rpm.py
index 7fc7427..bf4ee4c 100644
--- a/diffoscope/comparators/rpm.py
+++ b/diffoscope/comparators/rpm.py
@@ -77,16 +77,11 @@ def compare_rpm_headers(path1, path2):
 
 
 class RpmContainer(Archive):
-    @property
-    def path(self):
-        return self._path
-
-    def open_archive(self, path):
-        self._path = path
+    def open_archive(self):
         return self
 
     def close_archive(self):
-        self._path = None
+        pass
 
     def get_member_names(self):
         return ['content']
@@ -95,17 +90,17 @@ class RpmContainer(Archive):
     def extract(self, member_name, dest_dir):
         assert member_name == 'content'
         dest_path = os.path.join(dest_dir, 'content')
-        cmd = ['rpm2cpio', self._path]
+        cmd = ['rpm2cpio', self.source.path]
         with open(dest_path, 'wb') as dest:
             subprocess.check_call(cmd, shell=False, stdout=dest, stderr=subprocess.PIPE)
         return dest_path
 
 
 class RpmFile(AbstractRpmFile):
+    CONTAINER_CLASS = RpmContainer
+
     def compare_details(self, other, source=None):
         differences = []
         differences.append(compare_rpm_headers(self.path, other.path))
-        with RpmContainer(self).open() as my_container, \
-             RpmContainer(other).open() as other_container:
-            differences.extend(my_container.compare(other_container))
+        differences.extend(self.as_container.compare(other.as_container))
         return differences
diff --git a/diffoscope/comparators/squashfs.py b/diffoscope/comparators/squashfs.py
index 81889a2..b2b5b2f 100644
--- a/diffoscope/comparators/squashfs.py
+++ b/diffoscope/comparators/squashfs.py
@@ -165,8 +165,8 @@ class SquashfsContainer(Archive):
             else:
                 logger.warning('Unknown squashfs entry: %s', line)
 
-    def open_archive(self, path):
-        return dict([(m.name, m) for m in self.entries(path)])
+    def open_archive(self):
+        return dict([(m.name, m) for m in self.entries(self.source.path)])
 
     def close_archive(self):
         pass
@@ -188,6 +188,7 @@ class SquashfsContainer(Archive):
 
 
 class SquashfsFile(File):
+    CONTAINER_CLASS = SquashfsContainer
     RE_FILE_TYPE = re.compile(r'^Squashfs filesystem\b')
 
     @staticmethod
@@ -198,7 +199,5 @@ class SquashfsFile(File):
         differences = []
         differences.append(Difference.from_command(SquashfsSuperblock, self.path, other.path))
         differences.append(Difference.from_command(SquashfsListing, self.path, other.path))
-        with SquashfsContainer(self).open() as my_container, \
-             SquashfsContainer(other).open() as other_container:
-            differences.extend(my_container.compare(other_container))
+        differences.extend(self.as_container.compare(other.as_container))
         return differences
diff --git a/diffoscope/comparators/tar.py b/diffoscope/comparators/tar.py
index 2aba6f7..a876bec 100644
--- a/diffoscope/comparators/tar.py
+++ b/diffoscope/comparators/tar.py
@@ -33,6 +33,7 @@ class TarListing(Command):
 
 
 class TarFile(File):
+    CONTAINER_CLASS = TarContainer
     RE_FILE_TYPE = re.compile(r'\btar archive\b')
 
     @staticmethod
@@ -41,8 +42,6 @@ class TarFile(File):
 
     def compare_details(self, other, source=None):
         differences = []
-        with TarContainer(self).open() as my_container, \
-             TarContainer(other).open() as other_container:
-            differences.append(Difference.from_command(TarListing, self.path, other.path))
-            differences.extend(my_container.compare(other_container))
+        differences.append(Difference.from_command(TarListing, self.path, other.path))
+        differences.extend(self.as_container.compare(other.as_container))
         return differences
diff --git a/diffoscope/comparators/utils.py b/diffoscope/comparators/utils.py
index b0fc36d..ee1b18e 100644
--- a/diffoscope/comparators/utils.py
+++ b/diffoscope/comparators/utils.py
@@ -164,10 +164,6 @@ class Container(object, metaclass=ABCMeta):
     def source(self):
         return self._source
 
-    @contextmanager
-    def open(self):
-        raise NotImplemented
-
     def get_members(self):
         """Returns a directory. The key is what is used to match when comparing containers."""
         return {name: self.get_member(name) for name in self.get_member_names()}
@@ -197,7 +193,7 @@ class Container(object, metaclass=ABCMeta):
                 yield NonExistingFile('/dev/null', other_file), other_file, NO_COMMENT
 
     def compare(self, other, source=None):
-        return list(starmap(diffoscope.comparators.compare_commented_files, self.comparisons(other)))
+        return starmap(diffoscope.comparators.compare_commented_files, self.comparisons(other))
 
 
 class ArchiveMember(File):
@@ -221,8 +217,7 @@ class ArchiveMember(File):
             logger.debug('unpacking %s', self._name)
             assert self._temp_dir is None
             self._temp_dir = get_temporary_directory(suffix='diffoscope')
-            with self._container.open() as container:
-                self._path = container.extract(self._name, self._temp_dir.name)
+            self._path = self.container.extract(self._name, self._temp_dir.name)
         return self._path
 
     def cleanup(self):
@@ -244,30 +239,25 @@ class ArchiveMember(File):
 
 
 class Archive(Container, metaclass=ABCMeta):
+    def __new__(cls, source, *args, **kwargs):
+        if isinstance(source, NonExistingFile):
+            return super(Container, NonExistingArchive).__new__(NonExistingArchive)
+        else:
+            return super(Container, cls).__new__(cls)
+
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        self._archive = None
-
-    @contextmanager
-    def open(self):
-        if isinstance(self.source, NonExistingFile):
-            yield NonExistingArchive(self.source)
-        elif self._archive is not None:
-            yield self
-        else:
-            self._archive = self.open_archive(self.source.path)
-            try:
-                yield self
-            finally:
-                self.close_archive()
-                self._archive = None
+        self._archive = self.open_archive()
+
+    def __del__(self):
+        self.close_archive()
 
     @property
     def archive(self):
         return self._archive
 
     @abstractmethod
-    def open_archive(self, path):
+    def open_archive(self):
         raise NotImplemented
 
     @abstractmethod
@@ -299,16 +289,14 @@ class NonExistingArchiveLikeObject(object):
 
 class NonExistingArchive(Archive):
     @property
-    def archive(self):
-        return NonExistingArchiveLikeObject()
+    def source(self):
+        return None
 
     def open_archive(self):
-        # should never be called
-        raise NotImplemented
+        return NonExistingArchiveLikeObject()
 
     def close_archive(self):
-        # should never be called
-        raise NotImplemented
+        pass
 
     def get_member_names(self):
         return []
diff --git a/diffoscope/comparators/xz.py b/diffoscope/comparators/xz.py
index 9ff9713..98dc81d 100644
--- a/diffoscope/comparators/xz.py
+++ b/diffoscope/comparators/xz.py
@@ -27,22 +27,17 @@ from diffoscope import logger, tool_required
 
 
 class XzContainer(Archive):
-    @property
-    def path(self):
-        return self._path
-
-    def open_archive(self, path):
-        self._path = path
+    def open_archive(self):
         return self
 
     def close_archive(self):
-        self._path = None
+        pass
 
     def get_members(self):
         return {'xz-content': self.get_member(self.get_member_names()[0])}
 
     def get_member_names(self):
-        return [get_compressed_content_name(self.path, '.xz')]
+        return [get_compressed_content_name(self.source.path, '.xz')]
 
     @tool_required('xz')
     def extract(self, member_name, dest_dir):
@@ -50,12 +45,13 @@ class XzContainer(Archive):
         logger.debug('xz extracting to %s', dest_path)
         with open(dest_path, 'wb') as fp:
             subprocess.check_call(
-                ["xz", "--decompress", "--stdout", self.path],
+                ["xz", "--decompress", "--stdout", self.source.path],
                 shell=False, stdout=fp, stderr=None)
         return dest_path
 
 
 class XzFile(File):
+    CONTAINER_CLASS = XzContainer
     RE_FILE_TYPE = re.compile(r'^XZ compressed data$')
 
     @staticmethod
@@ -63,6 +59,4 @@ class XzFile(File):
         return XzFile.RE_FILE_TYPE.match(file.magic_file_type)
 
     def compare_details(self, other, source=None):
-        with XzContainer(self).open() as my_container, \
-             XzContainer(other).open() as other_container:
-            return my_container.compare(other_container)
+        return self.as_container.compare(other.as_container)
diff --git a/diffoscope/comparators/zip.py b/diffoscope/comparators/zip.py
index 00c80ca..103cbf3 100644
--- a/diffoscope/comparators/zip.py
+++ b/diffoscope/comparators/zip.py
@@ -73,8 +73,8 @@ class ZipDirectory(Directory, ArchiveMember):
 
 
 class ZipContainer(Archive):
-    def open_archive(self, path):
-        return zipfile.ZipFile(path, 'r')
+    def open_archive(self):
+        return zipfile.ZipFile(self.source.path, 'r')
 
     def close_archive(self):
         self.archive.close()
@@ -100,6 +100,7 @@ class ZipContainer(Archive):
 
 
 class ZipFile(File):
+    CONTAINER_CLASS = ZipContainer
     RE_FILE_TYPE = re.compile(r'^(Zip archive|Java archive|EPUB document)\b')
 
     @staticmethod
@@ -112,7 +113,5 @@ class ZipFile(File):
         zipinfo_difference = Difference.from_command(Zipinfo, self.path, other.path) or \
                              Difference.from_command(ZipinfoVerbose, self.path, other.path)
         differences.append(zipinfo_difference)
-        with ZipContainer(self).open() as my_container, \
-             ZipContainer(other).open() as other_container:
-            differences.extend(my_container.compare(other_container))
+        differences.extend(self.as_container.compare(other.as_container))
         return differences

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/reproducible/diffoscope.git



More information about the Reproducible-commits mailing list