# HG changeset patch # Parent 252f94e03103dec9f9ec19e2fcee898c8d172db8 #9536: use `with` for automatically closing files and releasing locks. Also, made `trac.util.AtomicFile` a context manager. diff -r 252f94e03103 trac/admin/console.py --- a/trac/admin/console.py Wed Dec 29 15:38:06 2010 +0000 +++ b/trac/admin/console.py Wed Dec 29 19:38:06 2010 +0100 @@ -12,6 +12,8 @@ # individuals. For the exact contribution history, see the revision # history and logs, available at http://trac.edgewall.org/log/. +from __future__ import with_statement + import cmd import locale import os.path @@ -44,11 +46,8 @@ def find_readline_lib(): linked to the readline module. """ import readline - f = open(readline.__file__, "rb") - try: + with open(readline.__file__, "rb") as f: data = f.read() - finally: - f.close() import re m = re.search('\0([^\0]*libreadline[^\0]*)\0', data) if m: diff -r 252f94e03103 trac/admin/web_ui.py --- a/trac/admin/web_ui.py Wed Dec 29 15:38:06 2010 +0000 +++ b/trac/admin/web_ui.py Wed Dec 29 19:38:06 2010 +0100 @@ -14,6 +14,8 @@ # # Author: Jonas Borgström +from __future__ import with_statement + from functools import partial import os import pkg_resources @@ -477,14 +479,10 @@ class PluginAdminPanel(Component): except AttributeError: # OS_BINARY not available on every platform pass - target_file = os.fdopen(os.open(target_path, flags, 0666), 'w') - try: + with os.fdopen(os.open(target_path, flags, 0666), 'w') as target_file: shutil.copyfileobj(upload.file, target_file) self.log.info('Plugin %s installed to %s', plugin_filename, target_path) - finally: - target_file.close() - # TODO: Validate that the uploaded file is actually a valid Trac plugin # Make the environment reset itself on the next request diff -r 252f94e03103 trac/attachment.py --- a/trac/attachment.py Wed Dec 29 15:38:06 2010 +0000 +++ b/trac/attachment.py Wed Dec 29 19:38:06 2010 +0100 @@ -265,7 +265,7 @@ class Attachment(object): filename = unicode_quote(filename) path, targetfile = create_unique_file(os.path.join(self.path, filename)) - try: + with targetfile: # Note: `path` is an unicode string because `self.path` was one. # As it contains only quoted chars and numbers, we can use `ascii` basename = os.path.basename(path).encode('ascii') @@ -281,8 +281,6 @@ class Attachment(object): self.env.log.info("New attachment: %s by %s", self.title, self.author) - finally: - targetfile.close() for listener in AttachmentModule(self.env).change_listeners: listener.attachment_added(self) @@ -725,8 +723,7 @@ class AttachmentModule(Component): 'title': get_resource_name(self.env, attachment.resource), 'attachment': attachment} - fd = attachment.open() - try: + with attachment.open() as fd: mimeview = Mimeview(self.env) # MIME type detection @@ -775,8 +772,6 @@ class AttachmentModule(Component): os.fstat(fd.fileno()).st_size, mime_type, attachment.filename, raw_href, annotations=['lineno']) return data - finally: - fd.close() def _format_link(self, formatter, ns, target, label): link, params, fragment = formatter.split_link(target) @@ -946,11 +941,8 @@ class AttachmentAdmin(Component): attachment = Attachment(self.env, realm, id) attachment.author = author attachment.description = description - f = open(path, 'rb') - try: + with open(path, 'rb') as f: attachment.insert(os.path.basename(path), f, os.path.getsize(path)) - finally: - f.close() def _do_remove(self, resource, name): (realm, id) = self.split_resource(resource) @@ -966,8 +958,7 @@ class AttachmentAdmin(Component): if os.path.isfile(destination): raise AdminCommandError(_("File '%(name)s' exists", name=destination)) - input = attachment.open() - try: + with attachment.open() as input: output = (destination is None) and sys.stdout \ or open(destination, "wb") try: @@ -975,6 +966,4 @@ class AttachmentAdmin(Component): finally: if destination is not None: output.close() - finally: - input.close() diff -r 252f94e03103 trac/config.py --- a/trac/config.py Wed Dec 29 15:38:06 2010 +0000 +++ b/trac/config.py Wed Dec 29 19:38:06 2010 +0100 @@ -12,6 +12,8 @@ # individuals. For the exact contribution history, see the revision # history and logs, available at http://trac.edgewall.org/log/. +from __future__ import with_statement + from ConfigParser import ConfigParser from copy import deepcopy import os.path @@ -231,8 +233,7 @@ class Configuration(object): # At this point, all the strings in `sections` are UTF-8 encoded `str` try: - fileobj = AtomicFile(self.filename, 'w') - try: + with AtomicFile(self.filename, 'w') as fileobj: fileobj.write('# -*- coding: utf-8 -*-\n\n') for section, options in sections: fileobj.write('[%s]\n' % section) @@ -244,8 +245,6 @@ class Configuration(object): .replace('\n', '\n ') fileobj.write('%s = %s\n' % (key_str, val_str)) fileobj.write('\n') - finally: - fileobj.close() self._old_sections = deepcopy(self.parser._sections) except Exception: # Revert all changes to avoid inconsistencies diff -r 252f94e03103 trac/env.py --- a/trac/env.py Wed Dec 29 15:38:06 2010 +0000 +++ b/trac/env.py Wed Dec 29 19:38:06 2010 +0100 @@ -350,11 +350,8 @@ class Environment(Component, ComponentMa def verify(self): """Verify that the provided path points to a valid Trac environment directory.""" - fd = open(os.path.join(self.path, 'VERSION'), 'r') - try: + with open(os.path.join(self.path, 'VERSION'), 'r') as fd: assert fd.read(26) == 'Trac Environment Version 1' - finally: - fd.close() def get_db_cnx(self): """Return a database connection from the connection pool @@ -754,8 +751,7 @@ def open_environment(env_path=None, use_ env_path = os.path.normcase(os.path.normpath(env_path)) if use_cache: - env_cache_lock.acquire() - try: + with env_cache_lock: env = env_cache.get(env_path) if env and env.config.parse_if_needed(): # The environment configuration has changed, so shut it down @@ -769,8 +765,6 @@ def open_environment(env_path=None, use_ env = env_cache.setdefault(env_path, open_environment(env_path)) else: CacheManager(env).reset_metadata() - finally: - env_cache_lock.release() else: env = Environment(env_path) needs_upgrade = False @@ -841,11 +835,8 @@ class EnvironmentAdmin(Component): template = Chrome(self.env).load_template('deploy_trac.' + script, 'text') stream = template.generate(**data) - out = file(dest, 'w') - try: + with open(dest, 'w') as out: stream.render('text', out=out) - finally: - out.close() def _do_hotcopy(self, dest, no_db=None): if no_db not in (None, '--no-database'): diff -r 252f94e03103 trac/tests/config.py --- a/trac/tests/config.py Wed Dec 29 15:38:06 2010 +0000 +++ b/trac/tests/config.py Wed Dec 29 19:38:06 2010 +0100 @@ -12,6 +12,8 @@ # individuals. For the exact contribution history, see the revision # history and logs, available at http://trac.edgewall.org/log/. +from __future__ import with_statement + import os import tempfile import time @@ -39,11 +41,8 @@ class ConfigurationTestCase(unittest.Tes return Configuration(self.filename) def _write(self, lines): - fileobj = open(self.filename, 'w') - try: + with open(self.filename, 'w') as fileobj: fileobj.write(('\n'.join(lines + [''])).encode('utf-8')) - finally: - fileobj.close() def test_default(self): config = self._read() @@ -433,12 +432,9 @@ class ConfigurationTestCase(unittest.Tes def _test_with_inherit(self, testcb): sitename = os.path.join(tempfile.gettempdir(), 'trac-site.ini') - sitefile = open(sitename, 'w') try: - try: + with open(sitename, 'w') as sitefile: sitefile.write('[a]\noption = x\n') - finally: - sitefile.close() self._write(['[inherit]', 'file = trac-site.ini']) testcb() diff -r 252f94e03103 trac/util/__init__.py --- a/trac/util/__init__.py Wed Dec 29 15:38:06 2010 +0000 +++ b/trac/util/__init__.py Wed Dec 29 19:38:06 2010 +0100 @@ -17,6 +17,8 @@ # Author: Jonas Borgström # Matthew Good +from __future__ import with_statement + import errno import inspect from itertools import izip, tee @@ -181,7 +183,7 @@ class AtomicFile(object): def __getattr__(self, name): return getattr(self._file, name) - + def commit(self): if self._file is None: return @@ -204,10 +206,18 @@ class AtomicFile(object): os.unlink(self._temp) except: pass - + close = commit __del__ = rollback + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_value, traceback): + self.close() + + closed = property(lambda self: self._file is None or self._file.closed) + def read_file(path, mode='r'): """Read a file and return its content.""" @@ -220,12 +230,9 @@ def read_file(path, mode='r'): def create_file(path, data='', mode='w'): """Create a new file with the given data.""" - f = open(path, mode) - try: + with open(path, mode) as f: if data: f.write(data) - finally: - f.close() def create_unique_file(path): diff -r 252f94e03103 trac/util/daemon.py --- a/trac/util/daemon.py Wed Dec 29 15:38:06 2010 +0000 +++ b/trac/util/daemon.py Wed Dec 29 19:38:06 2010 +0100 @@ -11,6 +11,8 @@ # individuals. For the exact contribution history, see the revision # history and logs, available at http://trac.edgewall.org/log/. +from __future__ import with_statement + import atexit import errno import os @@ -26,14 +28,11 @@ def daemonize(pidfile=None, progname=Non # process running pidfile = os.path.abspath(pidfile) if os.path.exists(pidfile): - fileobj = open(pidfile) - try: + with open(pidfile) as fileobj: try: pid = int(fileobj.read()) except ValueError: sys.exit('Invalid PID in file %s' % pidfile) - finally: - fileobj.close() try: # signal the process to see if it is still running os.kill(pid, 0) @@ -80,11 +79,8 @@ def daemonize(pidfile=None, progname=Non if os.path.exists(pidfile): os.remove(pidfile) atexit.register(remove_pidfile) - fileobj = open(pidfile, 'w') - try: + with open(pidfile, 'w') as fileobj: fileobj.write(str(os.getpid())) - finally: - fileobj.close() def handle_signal(signum, frame): diff -r 252f94e03103 trac/util/dist.py --- a/trac/util/dist.py Wed Dec 29 15:38:06 2010 +0000 +++ b/trac/util/dist.py Wed Dec 29 19:38:06 2010 +0100 @@ -11,6 +11,8 @@ # individuals. For the exact contribution history, see the revision # history and logs, available at http://trac.edgewall.org/log/. +from __future__ import with_statement + """Extra commands for setup.py. In addition to providing a few extra command classes in `l10n_cmdclass`, @@ -131,18 +133,12 @@ try: log.info('generating messages javascript %r to %r', mo_file, js_file) - infile = open(mo_file, 'rb') - try: + with open(mo_file, 'rb') as infile: t = Translations(infile, self.domain) catalog = t._catalog - finally: - infile.close() - outfile = open(js_file, 'w') - try: + with open(js_file, 'w') as outfile: write_js(outfile, catalog, self.domain, locale) - finally: - outfile.close() def write_js(fileobj, catalog, domain, locale): data = {'domain': domain, 'locale': locale} diff -r 252f94e03103 trac/util/tests/__init__.py --- a/trac/util/tests/__init__.py Wed Dec 29 15:38:06 2010 +0000 +++ b/trac/util/tests/__init__.py Wed Dec 29 19:38:06 2010 +0100 @@ -11,6 +11,8 @@ # individuals. For the exact contribution history, see the revision # history and logs, available at http://trac.edgewall.org/log/. +from __future__ import with_statement + import doctest import os.path import random @@ -33,36 +35,28 @@ class AtomicFileTestCase(unittest.TestCa pass def test_non_existing(self): - f = util.AtomicFile(self.path) - try: + with util.AtomicFile(self.path) as f: f.write('test content') - finally: - f.close() + self.assertEqual(True, f.closed) self.assertEqual('test content', util.read_file(self.path)) def test_existing(self): util.create_file(self.path, 'Some content') self.assertEqual('Some content', util.read_file(self.path)) - f = util.AtomicFile(self.path) - try: + with util.AtomicFile(self.path) as f: f.write('Some new content') - finally: - f.close() + self.assertEqual(True, f.closed) self.assertEqual('Some new content', util.read_file(self.path)) if util.can_rename_open_file: def test_existing_open_for_reading(self): util.create_file(self.path, 'Initial file content') self.assertEqual('Initial file content', util.read_file(self.path)) - rf = open(self.path) - try: - f = util.AtomicFile(self.path) - try: + with open(self.path) as rf: + with util.AtomicFile(self.path) as f: f.write('Replaced content') - finally: - f.close() - finally: - rf.close() + self.assertEqual(True, rf.closed) + self.assertEqual(True, f.closed) self.assertEqual('Replaced content', util.read_file(self.path)) # FIXME: It is currently not possible to make this test pass on all @@ -73,11 +67,9 @@ class AtomicFileTestCase(unittest.TestCa # we require Python 3. def _test_unicode_path(self): self.path = os.path.join(tempfile.gettempdir(), u'träc-témpfilè') - f = util.AtomicFile(self.path) - try: + with util.AtomicFile(self.path) as f: f.write('test content') - finally: - f.close() + self.assertEqual(True, f.closed) self.assertEqual('test content', util.read_file(self.path)) diff -r 252f94e03103 trac/util/translation.py --- a/trac/util/translation.py Wed Dec 29 15:38:06 2010 +0000 +++ b/trac/util/translation.py Wed Dec 29 19:38:06 2010 +0100 @@ -11,6 +11,8 @@ # individuals. For the exact contribution history, see the revision # history and logs, available at http://trac.edgewall.org/log/. +from __future__ import with_statement + """Utilities for text translation with gettext.""" import pkg_resources @@ -128,13 +130,10 @@ try: # Public API def add_domain(self, domain, env_path, locales_dir): - self._plugin_domains_lock.acquire() - try: + with self._plugin_domains_lock: if env_path not in self._plugin_domains: self._plugin_domains[env_path] = [] self._plugin_domains[env_path].append((domain, locales_dir)) - finally: - self._plugin_domains_lock.release() def make_activable(self, get_locale, env_path=None): self._current.args = (get_locale, env_path) @@ -149,11 +148,8 @@ try: if not t or t.__class__ is NullTranslations: t = self._null_translations elif env_path: - self._plugin_domains_lock.acquire() - try: + with self._plugin_domains_lock: domains = list(self._plugin_domains.get(env_path, [])) - finally: - self._plugin_domains_lock.release() for domain, dirname in domains: t.add(Translations.load(dirname, locale, domain)) self._current.translations = t diff -r 252f94e03103 trac/versioncontrol/api.py --- a/trac/versioncontrol/api.py Wed Dec 29 15:38:06 2010 +0000 +++ b/trac/versioncontrol/api.py Wed Dec 29 19:38:06 2010 +0100 @@ -594,13 +594,10 @@ class RepositoryManager(Component): def reload_repositories(self): """Reload the repositories from the providers.""" - self._lock.acquire() - try: + with self._lock: # FIXME: trac-admin doesn't reload the environment self._cache = {} self._all_repositories = None - finally: - self._lock.release() self.config.touch() # Force environment reload def notify(self, event, reponame, revs): @@ -650,13 +647,10 @@ class RepositoryManager(Component): def shutdown(self, tid=None): if tid: assert tid == threading._get_ident() - try: - self._lock.acquire() + with self._lock: repositories = self._cache.pop(tid, {}) for reponame, repos in repositories.iteritems(): repos.close() - finally: - self._lock.release() # private methods diff -r 252f94e03103 trac/web/auth.py --- a/trac/web/auth.py Wed Dec 29 15:38:06 2010 +0000 +++ b/trac/web/auth.py Wed Dec 29 19:38:06 2010 +0100 @@ -265,14 +265,11 @@ class PasswordFileAuthentication(HTTPAut self._lock = threading.Lock() def check_reload(self): - self._lock.acquire() - try: + with self._lock: mtime = os.stat(self.filename).st_mtime if mtime > self.mtime: self.mtime = mtime self.load(self.filename) - finally: - self._lock.release() class BasicAuthentication(PasswordFileAuthentication): diff -r 252f94e03103 trac/web/chrome.py --- a/trac/web/chrome.py Wed Dec 29 15:38:06 2010 +0000 +++ b/trac/web/chrome.py Wed Dec 29 19:38:06 2010 +0100 @@ -14,6 +14,8 @@ # # Author: Christopher Lenz +from __future__ import with_statement + """Content presentation for the web layer. The Chrome module deals with delivering and shaping content to the end user, @@ -485,8 +487,7 @@ class Chrome(Component): os.mkdir(templates_dir) site_path = os.path.join(templates_dir, 'site.html.sample') - fileobj = open(site_path, 'w') - try: + with open(site_path, 'w') as fileobj: fileobj.write("""\ """) - finally: - fileobj.close() def environment_needs_upgrade(self, db): return False diff -r 252f94e03103 trac/web/modpython_frontend.py --- a/trac/web/modpython_frontend.py Wed Dec 29 15:38:06 2010 +0000 +++ b/trac/web/modpython_frontend.py Wed Dec 29 19:38:06 2010 +0100 @@ -16,6 +16,8 @@ # Author: Christopher Lenz # Matthew Good +from __future__ import with_statement + import os import pkg_resources import sys @@ -126,8 +128,7 @@ _first_lock = threading.Lock() def handler(req): global _first - try: - _first_lock.acquire() + with _first_lock: if _first: _first = False options = req.get_options() @@ -141,8 +142,6 @@ def handler(req): if egg_cache: pkg_resources.set_extraction_path(egg_cache) reload(sys.modules['trac.web']) - finally: - _first_lock.release() pkg_resources.require('Trac==%s' % VERSION) gateway = ModPythonGateway(req, req.get_options()) from trac.web.main import dispatch_request diff -r 252f94e03103 trac/wiki/admin.py --- a/trac/wiki/admin.py Wed Dec 29 15:38:06 2010 +0000 +++ b/trac/wiki/admin.py Wed Dec 29 19:38:06 2010 +0100 @@ -106,11 +106,8 @@ class WikiAdmin(Component): if os.path.isfile(filename): raise AdminCommandError(_("File '%(name)s' exists", name=filename)) - f = open(filename, 'w') - try: + with open(filename, 'w') as f: f.write(text.encode('utf-8')) - finally: - f.close() break else: raise AdminCommandError(_("Page '%(page)s' not found", page=page))