all: Fix all instances of ResourceWarning about unclosed files

10 files changed, 134 insertions, 123 deletions

diff --git a/apt/cache.py b/apt/cache.py
index bfa41edc..3bbae923 100644
--- a/apt/cache.py
+++ b/apt/cache.py
@@ -107,7 +107,7 @@ class Cache(object):
                 os.makedirs(rootdir + d)
         for f in files:
             if not os.path.exists(rootdir + f):
-                open(rootdir + f, "w")
+                open(rootdir + f, "w").close()
 
     def _run_callbacks(self, name):
         """ internal helper to run a callback """
diff --git a/apt/cdrom.py b/apt/cdrom.py
index 01caa12f..9688de9e 100644
--- a/apt/cdrom.py
+++ b/apt/cdrom.py
@@ -79,9 +79,10 @@ class Cdrom(apt_pkg.Cdrom):
         src.append(apt_pkg.config.find_file("Dir::Etc::sourcelist"))
         # Check each file
         for fname in src:
-            for line in open(fname):
-                if not line.lstrip().startswith("#") and cd_id in line:
-                    return True
+            with open(fname) as fobj:
+                for line in fobj:
+                    if not line.lstrip().startswith("#") and cd_id in line:
+                        return True
         return False
 
     if apt_pkg._COMPAT_0_7:
diff --git a/apt/debfile.py b/apt/debfile.py
index fb4312a1..d0f41def 100644
--- a/apt/debfile.py
+++ b/apt/debfile.py
@@ -64,7 +64,7 @@ class DebPackage(object):
         self._installed_conflicts = set()
         self._failure_string = ""
         self.filename = filename
-        self._debfile = apt_inst.DebFile(open(self.filename))
+        self._debfile = apt_inst.DebFile(self.filename)
         control = self._debfile.control.extractdata("control")
         self._sections = apt_pkg.TagSection(control)
         self.pkgname = self._sections["Package"]
diff --git a/apt/package.py b/apt/package.py
index d7d5d167..54ef1c01 100644
--- a/apt/package.py
+++ b/apt/package.py
@@ -50,9 +50,9 @@ __all__ = ('BaseDependency', 'Dependency', 'Origin', 'Package', 'Record',
 
 def _file_is_same(path, size, md5):
     """Return ``True`` if the file is the same."""
-    if (os.path.exists(path) and os.path.getsize(path) == size and
-        apt_pkg.md5sum(open(path)) == md5):
-        return True
+    if os.path.exists(path) and os.path.getsize(path) == size:
+        with open(path) as fobj:
+            return apt_pkg.md5sum(fobj) == md5
 
 
 class FetchError(Exception):
@@ -994,11 +994,8 @@ class Package(object):
         """
         path = "/var/lib/dpkg/info/%s.list" % self.name
         try:
-            file_list = open(path, "rb")
-            try:
+            with open(path, "rb") as file_list:
                 return file_list.read().decode("utf-8").split(u"\n")
-            finally:
-                file_list.close()
         except EnvironmentError:
             return []
 
@@ -1104,6 +1101,7 @@ class Package(object):
                 # Check if the download was canceled
                 if cancel_lock and cancel_lock.isSet():
                     return u""
+                # FIXME: python3.2: Should be closed manually
                 changelog_file = urllib2.urlopen(uri)
                 # do only get the lines that are new
                 changelog = u""
diff --git a/aptsources/distinfo.py b/aptsources/distinfo.py
index 6374f185..48a72719 100644
--- a/aptsources/distinfo.py
+++ b/aptsources/distinfo.py
@@ -176,89 +176,93 @@ class DistInfo(object):
         map_mirror_sets = {}
 
         dist_fname = "%s/%s.info" % (base_dir, dist)
-        dist_file = open(dist_fname)
-        if not dist_file:
-            return
-        template = None
-        component = None
-        for line in dist_file:
-            tokens = line.split(':', 1)
-            if len(tokens) < 2:
-                continue
-            field = tokens[0].strip()
-            value = tokens[1].strip()
-            if field == 'ChangelogURI':
-                self.changelogs_uri = _(value)
-            elif field == 'MetaReleaseURI':
-                self.metarelease_uri = value
-            elif field == 'Suite':
-                self.finish_template(template, component)
-                component=None
-                template = Template()
-                template.name = value
-                template.distribution = dist
-                template.match_name = "^%s$" % value
-            elif field == 'MatchName':
-                template.match_name = value
-            elif field == 'ParentSuite':
-                template.child = True
-                for nanny in self.templates:
-                    # look for parent and add back ref to it
-                    if nanny.name == value:
-                        template.parents.append(nanny)
-                        nanny.children.append(template)
-            elif field == 'Available':
-                template.available = apt_pkg.string_to_bool(value)
-            elif field == 'Official':
-                template.official = apt_pkg.string_to_bool(value)
-            elif field == 'RepositoryType':
-                template.type = value
-            elif field == 'BaseURI' and not template.base_uri:
-                template.base_uri = value
-            elif field == 'BaseURI-%s' % self.arch:
-                template.base_uri = value
-            elif field == 'MatchURI' and not template.match_uri:
-                template.match_uri = value
-            elif field == 'MatchURI-%s' % self.arch:
-                template.match_uri = value
-            elif (field == 'MirrorsFile' or
-                  field == 'MirrorsFile-%s' % self.arch):
-                # Make the path absolute.
-                value = os.path.isabs(value) and value or \
-                        os.path.abspath(os.path.join(base_dir, value))
-                if value not in map_mirror_sets:
-                    mirror_set = {}
-                    try:
-                        mirror_data = filter(match_mirror_line.match,
-                                             [x.strip() for x in open(value)])
-                    except Exception:
-                        print "WARNING: Failed to read mirror file"
-                        mirror_data = []
-                    for line in mirror_data:
-                        if line.startswith("#LOC:"):
-                            location = match_loc.sub(r"\1", line)
-                            continue
-                        (proto, hostname, dir) = split_url(line)
-                        if hostname in mirror_set:
-                            mirror_set[hostname].add_repository(proto, dir)
-                        else:
-                            mirror_set[hostname] = Mirror(
-                                proto, hostname, dir, location)
-                    map_mirror_sets[value] = mirror_set
-                template.mirror_set = map_mirror_sets[value]
-            elif field == 'Description':
-                template.description = _(value)
-            elif field == 'Component':
-                if component and not template.has_component(component.name):
-                    template.components.append(component)
-                component = Component(value)
-            elif field == 'CompDescription':
-                component.set_description(_(value))
-            elif field == 'CompDescriptionLong':
-                component.set_description_long(_(value))
-        self.finish_template(template, component)
-        template=None
-        component=None
+        with open(dist_fname) as dist_file:
+
+
+
+            template = None
+            component = None
+            for line in dist_file:
+                tokens = line.split(':', 1)
+                if len(tokens) < 2:
+                    continue
+                field = tokens[0].strip()
+                value = tokens[1].strip()
+                if field == 'ChangelogURI':
+                    self.changelogs_uri = _(value)
+                elif field == 'MetaReleaseURI':
+                    self.metarelease_uri = value
+                elif field == 'Suite':
+                    self.finish_template(template, component)
+                    component=None
+                    template = Template()
+                    template.name = value
+                    template.distribution = dist
+                    template.match_name = "^%s$" % value
+                elif field == 'MatchName':
+                    template.match_name = value
+                elif field == 'ParentSuite':
+                    template.child = True
+                    for nanny in self.templates:
+                        # look for parent and add back ref to it
+                        if nanny.name == value:
+                            template.parents.append(nanny)
+                            nanny.children.append(template)
+                elif field == 'Available':
+                    template.available = apt_pkg.string_to_bool(value)
+                elif field == 'Official':
+                    template.official = apt_pkg.string_to_bool(value)
+                elif field == 'RepositoryType':
+                    template.type = value
+                elif field == 'BaseURI' and not template.base_uri:
+                    template.base_uri = value
+                elif field == 'BaseURI-%s' % self.arch:
+                    template.base_uri = value
+                elif field == 'MatchURI' and not template.match_uri:
+                    template.match_uri = value
+                elif field == 'MatchURI-%s' % self.arch:
+                    template.match_uri = value
+                elif (field == 'MirrorsFile' or
+                      field == 'MirrorsFile-%s' % self.arch):
+                    # Make the path absolute.
+                    value = os.path.isabs(value) and value or \
+                            os.path.abspath(os.path.join(base_dir, value))
+                    if value not in map_mirror_sets:
+                        mirror_set = {}
+                        try:
+                            with open(value) as value_f:
+                                mirror_data = filter(match_mirror_line.match,
+                                                     [x.strip() for x in
+                                                      value_f])
+                        except Exception:
+                            print "WARNING: Failed to read mirror file"
+                            mirror_data = []
+                        for line in mirror_data:
+                            if line.startswith("#LOC:"):
+                                location = match_loc.sub(r"\1", line)
+                                continue
+                            (proto, hostname, dir) = split_url(line)
+                            if hostname in mirror_set:
+                                mirror_set[hostname].add_repository(proto, dir)
+                            else:
+                                mirror_set[hostname] = Mirror(
+                                    proto, hostname, dir, location)
+                        map_mirror_sets[value] = mirror_set
+                    template.mirror_set = map_mirror_sets[value]
+                elif field == 'Description':
+                    template.description = _(value)
+                elif field == 'Component':
+                    if (component and not
+                        template.has_component(component.name)):
+                        template.components.append(component)
+                    component = Component(value)
+                elif field == 'CompDescription':
+                    component.set_description(_(value))
+                elif field == 'CompDescriptionLong':
+                    component.set_description_long(_(value))
+            self.finish_template(template, component)
+            template=None
+            component=None
 
     def finish_template(self, template, component):
         " finish the current tempalte "
diff --git a/aptsources/sourceslist.py b/aptsources/sourceslist.py
index 38831e9c..22770c10 100644
--- a/aptsources/sourceslist.py
+++ b/aptsources/sourceslist.py
@@ -368,15 +368,12 @@ class SourcesList(object):
     def load(self, file):
         """ (re)load the current sources """
         try:
-            f = open(file, "r")
-            lines = f.readlines()
-            for line in lines:
-                source = SourceEntry(line, file)
-                self.list.append(source)
+            with open(file, "r") as f:
+                for line in f:
+                    source = SourceEntry(line, file)
+                    self.list.append(source)
         except:
             print "could not open file '%s'" % file
-        else:
-            f.close()
 
     def save(self):
         """ save the current sources """
@@ -388,14 +385,19 @@ class SourcesList(object):
                 "## See sources.list(5) for more information, especialy\n"
                 "# Remember that you can only use http, ftp or file URIs\n"
                 "# CDROMs are managed through the apt-cdrom tool.\n")
-            open(path, "w").write(header)
+
+            with open(path, "w") as f:
+                f.write(header)
             return
-        for source in self.list:
-            if source.file not in files:
-                files[source.file] = open(source.file, "w")
-            files[source.file].write(source.str())
-        for f in files:
-            files[f].close()
+
+        try:
+            for source in self.list:
+                if source.file not in files:
+                    files[source.file] = open(source.file, "w")
+                files[source.file].write(source.str())
+        finally:
+            for f in files:
+                files[f].close()
 
     def check_for_relations(self, sources_list):
         """get all parent and child channels in the sources list"""
diff --git a/debian/changelog b/debian/changelog
index 2864126a..cbe92343 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -3,6 +3,7 @@ python-apt (0.8.0~exp2) UNRELEASED; urgency=low
   * aptsources: Parse multi-arch sources.list files correctly
   * aptsources: Allow insertion of new multi-arch entries
   * aptsources: Various cleanup work
+  * all: Fix all instances of ResourceWarning about unclosed files
 
  -- Julian Andres Klode <jak@debian.org>  Wed, 06 Apr 2011 09:46:52 +0200
 
diff --git a/tests/test_apt_cache.py b/tests/test_apt_cache.py
index b4cc650d..151a20e2 100644
--- a/tests/test_apt_cache.py
+++ b/tests/test_apt_cache.py
@@ -89,17 +89,17 @@ class TestAptCache(unittest.TestCase):
         tmpdir = tempfile.mkdtemp()
         dpkg_dir = os.path.join(tmpdir,"var","lib","dpkg")
         os.makedirs(os.path.join(dpkg_dir,"updates"))
-        open(os.path.join(dpkg_dir,"status"), "w")
+        open(os.path.join(dpkg_dir,"status"), "w").close()
         apt_pkg.config.set("Dir::State::status",
                        os.path.join(dpkg_dir,"status"))
         cache = apt.Cache()
         # test empty
         self.assertFalse(cache.dpkg_journal_dirty)
         # that is ok, only [0-9] are dpkg jounral entries
-        open(os.path.join(dpkg_dir,"updates","xxx"), "w")
+        open(os.path.join(dpkg_dir,"updates","xxx"), "w").close()
         self.assertFalse(cache.dpkg_journal_dirty)        
         # that is a dirty journal
-        open(os.path.join(dpkg_dir,"updates","000"), "w")
+        open(os.path.join(dpkg_dir,"updates","000"), "w").close()
         self.assertTrue(cache.dpkg_journal_dirty)
         # reset config value
         apt_pkg.config.set("Dir::State::status", old_status)
@@ -121,20 +121,19 @@ class TestAptCache(unittest.TestCase):
         apt_pkg.config.set("dir::etc::sourceparts", "xxx")
         # main sources.list
         sources_list = base_sources
-        f=open(sources_list, "w")
-        repo = os.path.abspath("./data/test-repo2")
-        f.write("deb copy:%s /\n" % repo)
-        f.close()
+        with open(sources_list, "w") as f:
+            repo = os.path.abspath("./data/test-repo2")
+            f.write("deb copy:%s /\n" % repo)
 
         # test single sources.list fetching
         sources_list = os.path.join(rootdir, "test.list")
-        f=open(sources_list, "w")
-        repo_dir = os.path.abspath("./data/test-repo")
-        f.write("deb copy:%s /\n" % repo_dir)
-        f.close()
+        with open(sources_list, "w") as f:
+            repo_dir = os.path.abspath("./data/test-repo")
+            f.write("deb copy:%s /\n" % repo_dir)
+
         self.assertTrue(os.path.exists(sources_list))
         # write marker to ensure listcleaner is not run
-        open("./data/tmp/var/lib/apt/lists/marker", "w")
+        open("./data/tmp/var/lib/apt/lists/marker", "w").close()
 
         # update a single sources.list
         cache = apt.Cache()
diff --git a/tests/test_hashes.py b/tests/test_hashes.py
index e0aabe09..660373cb 100644
--- a/tests/test_hashes.py
+++ b/tests/test_hashes.py
@@ -82,11 +82,16 @@ class TestHashString(unittest.TestCase):
 
     def setUp(self):
         """Prepare the test by reading the file."""
-        self.hashes = apt_pkg.Hashes(open(apt_pkg.__file__))
+        self.file = open(apt_pkg.__file__)
+        self.hashes = apt_pkg.Hashes(self.file)
         self.md5 = apt_pkg.HashString("MD5Sum", self.hashes.md5)
         self.sha1 = apt_pkg.HashString("SHA1", self.hashes.sha1)
         self.sha256 = apt_pkg.HashString("SHA256", self.hashes.sha256)
 
+    def tearDown(self):
+        """Cleanup, Close the file object used for the tests."""
+        self.file.close()        
+
     def test_md5(self):
         """hashes: Test apt_pkg.HashString().md5"""
         self.assertEqual("MD5Sum:%s" % self.hashes.md5, str(self.md5))
diff --git a/tests/test_progress.py b/tests/test_progress.py
index ffab5bc0..73853dfa 100644
--- a/tests/test_progress.py
+++ b/tests/test_progress.py
@@ -34,7 +34,8 @@ class TestProgress(unittest.TestCase):
         apt_pkg.config.set("Dir::state::lists", "./tmp")
         # create artifical line
         deb_line = "deb file:%s/data/fake-packages/ /\n" % basedir
-        open("fetch_sources.list","w").write(deb_line)
+        with open("fetch_sources.list","w") as fobj:
+            fobj.write(deb_line)
         apt_pkg.config.set("Dir::Etc::sourcelist", "fetch_sources.list")
 
     def test_acquire_progress(self):