[MERGE] new locks branch
Martin Pool
mbp at sourcefrog.net
Wed Feb 22 05:13:41 GMT 2006
diff from bzr.dev attached; much of this has already been approve but
here's what I intend to merge. I think it's uptodate with review
comments. It does adds a lock-breaking API but not a command, which I
will do shortly.
http://people.ubuntu.com/~mbp/bzr.mbp.locks
--
Martin
-------------- next part --------------
=== added file 'bzrlib/lockdir.py'
--- /dev/null
+++ bzrlib/lockdir.py
@@ -0,0 +1,334 @@
+# Copyright (C) 2006 Canonical Ltd
+
+# 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 useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+
+"""lock file protecting a resource
+
+bzr objects are locked by the existence of a file with a particular name
+within the control directory. We use this rather than OS internal locks
+(such as flock etc) because they can be seen across all transports,
+including http.
+
+Objects can be read if there is only physical read access; therefore
+readers can never be required to create a lock, though they will
+check whether a writer is using the lock. Writers can't detect
+whether anyone else is reading from the resource as they write.
+This works because of ordering constraints that make sure readers
+see a consistent view of existing data.
+
+Waiting for a lock must be done by polling; this can be aborted after
+a timeout.
+
+Locks must always be explicitly released, typically from a try/finally
+block -- they are not released from a finalizer or when Python
+exits.
+
+Locks may fail to be released if the process is abruptly terminated
+(machine stop, SIGKILL) or if a remote transport becomes permanently
+disconnected. There is therefore a method to break an existing lock.
+This should rarely be used, and generally only with user approval.
+Locks contain some information on when the lock was taken and by who
+which may guide in deciding whether it can safely be broken. (This is
+similar to the messages displayed by emacs and vim.) Note that if the
+lock holder is still alive they will get no notification that the lock
+has been broken and will continue their work -- so it is important to be
+sure they are actually dead.
+
+A lock is represented on disk by a directory of a particular name,
+containing an information file. Taking a lock is done by renaming a
+temporary directory into place. We use temporary directories because
+for all known transports and filesystems we believe that exactly one
+attempt to claim the lock will succeed and the others will fail. (Files
+won't do because some filesystems or transports only have
+rename-and-overwrite, making it hard to tell who won.)
+
+The desired characteristics are:
+
+* Locks are not reentrant. (That is, a client that tries to take a
+ lock it already holds may deadlock or fail.)
+* Stale locks can be guessed at by a heuristic
+* Lost locks can be broken by any client
+* Failed lock operations leave little or no mess
+* Deadlocks are avoided by having a timeout always in use, clients
+ desiring indefinite waits can retry or set a silly big timeout.
+
+Storage formats use the locks, and also need to consider concurrency
+issues underneath the lock. A format may choose not to use a lock
+at all for some operations.
+
+LockDirs always operate over a Transport. The transport may be readonly, in
+which case the lock can be queried but not acquired.
+
+Locks are identified by a path name, relative to a base transport.
+
+Calling code will typically want to make sure there is exactly one LockDir
+object per actual lock on disk. This module does nothing to prevent aliasing
+and deadlocks will likely occur if the locks are aliased.
+
+In the future we may add a "freshen" method which can be called
+by a lock holder to check that their lock has not been broken, and to
+update the timestamp within it.
+
+Example usage:
+
+>>> from bzrlib.transport.memory import MemoryTransport
+>>> # typically will be obtained from a BzrDir, Branch, etc
+>>> t = MemoryTransport()
+>>> l = LockDir(t, 'sample-lock')
+>>> l.wait_lock()
+>>> # do something here
+>>> l.unlock()
+
+"""
+
+import os
+import time
+from StringIO import StringIO
+
+import bzrlib.config
+from bzrlib.errors import (
+ DirectoryNotEmpty,
+ FileExists,
+ LockBreakMismatch,
+ LockBroken,
+ LockContention,
+ LockError,
+ LockNotHeld,
+ NoSuchFile,
+ UnlockableTransport,
+ )
+from bzrlib.transport import Transport
+from bzrlib.osutils import rand_chars
+from bzrlib.rio import RioWriter, read_stanza, Stanza
+
+# XXX: At the moment there is no consideration of thread safety on LockDir
+# objects. This should perhaps be updated - e.g. if two threads try to take a
+# lock at the same time they should *both* get it. But then that's unlikely
+# to be a good idea.
+
+# TODO: After renaming the directory, check the contents are what we
+# expected. It's possible that the rename failed but the transport lost
+# the failure indication.
+
+# TODO: Transport could offer a simpler put() method that avoids the
+# rename-into-place for cases like creating the lock template, where there is
+# no chance that the file already exists.
+
+# TODO: Perhaps store some kind of note like the bzr command line in the lock
+# info?
+
+# TODO: Some kind of callback run while polling a lock to show progress
+# indicators.
+
+_DEFAULT_TIMEOUT_SECONDS = 300
+_DEFAULT_POLL_SECONDS = 0.5
+
+class LockDir(object):
+ """Write-lock guarding access to data.
+ """
+
+ INFO_NAME = '/info'
+
+ def __init__(self, transport, path):
+ """Create a new LockDir object.
+
+ The LockDir is initially unlocked - this just creates the object.
+
+ :param transport: Transport which will contain the lock
+
+ :param path: Path to the lock within the base directory of the
+ transport.
+ """
+ assert isinstance(transport, Transport), \
+ ("not a transport: %r" % transport)
+ self.transport = transport
+ self.path = path
+ self._lock_held = False
+ self._info_path = path + self.INFO_NAME
+ self.nonce = rand_chars(20)
+
+ def __repr__(self):
+ return '%s(%s%s)' % (self.__class__.__name__,
+ self.transport.base,
+ self.path)
+
+ is_held = property(lambda self: self._lock_held)
+
+ def attempt_lock(self):
+ """Take the lock; fail if it's already held
+
+ If you wish to block until the lock can be obtained, call wait_lock()
+ instead.
+ """
+ if self.transport.is_readonly():
+ raise UnlockableTransport(self.transport)
+ try:
+ tmpname = '%s.pending.%s.tmp' % (self.path, rand_chars(20))
+ self.transport.mkdir(tmpname)
+ sio = StringIO()
+ self._prepare_info(sio)
+ sio.seek(0)
+ self.transport.put(tmpname + self.INFO_NAME, sio)
+ # FIXME: this turns into os.rename on posix, but into a fancy rename
+ # on Windows that may overwrite existing directory trees.
+ # NB: posix rename will overwrite empty directories, but not
+ # non-empty directories.
+ self.transport.move(tmpname, self.path)
+ self._lock_held = True
+ return
+ except (DirectoryNotEmpty, FileExists), e:
+ pass
+ # fall through to here on contention
+ raise LockContention(self)
+
+ def unlock(self):
+ if not self._lock_held:
+ raise LockNotHeld(self)
+ # rename before deleting, because we can't atomically remove the whole
+ # tree
+ tmpname = '%s.releasing.%s.tmp' % (self.path, rand_chars(20))
+ self.transport.rename(self.path, tmpname)
+ self._lock_held = False
+ self.transport.delete(tmpname + self.INFO_NAME)
+ self.transport.rmdir(tmpname)
+
+ def force_break(self, dead_holder_info):
+ """Release a lock held by another process.
+
+ WARNING: This should only be used when the other process is dead; if
+ it still thinks it has the lock there will be two concurrent writers.
+ In general the user's approval should be sought for lock breaks.
+
+ dead_holder_info must be the result of a previous LockDir.peek() call;
+ this is used to check that it's still held by the same process that
+ the user decided was dead. If this is not the current holder,
+ LockBreakMismatch is raised.
+
+ After the lock is broken it will not be held by any process.
+ It is possible that another process may sneak in and take the
+ lock before the breaking process acquires it.
+ """
+ if not isinstance(dead_holder_info, dict):
+ raise ValueError("dead_holder_info: %r" % dead_holder_info)
+ if self._lock_held:
+ raise AssertionError("can't break own lock: %r" % self)
+ current_info = self.peek()
+ if current_info is None:
+ # must have been recently released
+ return
+ if current_info != dead_holder_info:
+ raise LockBreakMismatch(self, current_info, dead_holder_info)
+ tmpname = '%s.broken.%s.tmp' % (self.path, rand_chars(20))
+ self.transport.rename(self.path, tmpname)
+ # check that we actually broke the right lock, not someone else;
+ # there's a small race window between checking it and doing the
+ # rename.
+ broken_info_path = tmpname + self.INFO_NAME
+ broken_info = self._parse_info(self.transport.get(broken_info_path))
+ if broken_info != dead_holder_info:
+ raise LockBreakMismatch(self, broken_info, dead_holder_info)
+ self.transport.delete(broken_info_path)
+ self.transport.rmdir(tmpname)
+
+ def confirm(self):
+ """Make sure that the lock is still held by this locker.
+
+ This should only fail if the lock was broken by user intervention,
+ or if the lock has been affected by a bug.
+
+ If the lock is not thought to be held, raises LockNotHeld. If
+ the lock is thought to be held but has been broken, raises
+ LockBroken.
+ """
+ if not self._lock_held:
+ raise LockNotHeld(self)
+ info = self.peek()
+ if info is None:
+ # no lock there anymore!
+ raise LockBroken(self)
+ if info.get('nonce') != self.nonce:
+ # there is a lock, but not ours
+ raise LockBroken(self)
+
+ def peek(self):
+ """Check if the lock is held by anyone.
+
+ If it is held, this returns the lock info structure as a rio Stanza,
+ which contains some information about the current lock holder.
+ Otherwise returns None.
+ """
+ try:
+ info = self._parse_info(self.transport.get(self._info_path))
+ assert isinstance(info, dict), \
+ "bad parse result %r" % info
+ return info
+ except NoSuchFile, e:
+ return None
+
+ def _prepare_info(self, outf):
+ """Write information about a pending lock to a temporary file.
+ """
+ import socket
+ # XXX: is creating this here inefficient?
+ config = bzrlib.config.GlobalConfig()
+ s = Stanza(hostname=socket.gethostname(),
+ pid=str(os.getpid()),
+ start_time=str(int(time.time())),
+ nonce=self.nonce,
+ user=config.user_email(),
+ )
+ RioWriter(outf).write_stanza(s)
+
+ def _parse_info(self, info_file):
+ return read_stanza(info_file.readlines()).as_dict()
+
+ def wait_lock(self, timeout=_DEFAULT_TIMEOUT_SECONDS,
+ poll=_DEFAULT_POLL_SECONDS):
+ """Wait a certain period for a lock.
+
+ If the lock can be acquired within the bounded time, it
+ is taken and this returns. Otherwise, LockContention
+ is raised. Either way, this function should return within
+ approximately `timeout` seconds. (It may be a bit more if
+ a transport operation takes a long time to complete.)
+ """
+ # XXX: the transport interface doesn't let us guard
+ # against operations there taking a long time.
+ deadline = time.time() + timeout
+ while True:
+ try:
+ self.attempt_lock()
+ return
+ except LockContention:
+ pass
+ if time.time() + poll < deadline:
+ time.sleep(poll)
+ else:
+ raise LockContention(self)
+
+ def wait(self, timeout=20, poll=0.5):
+ """Wait a certain period for a lock to be released.
+ """
+ # XXX: the transport interface doesn't let us guard
+ # against operations there taking a long time.
+ deadline = time.time() + timeout
+ while True:
+ if self.peek():
+ return
+ if time.time() + poll < deadline:
+ time.sleep(poll)
+ else:
+ raise LockContention(self)
+
=== added file 'bzrlib/tests/test_lockdir.py'
--- /dev/null
+++ bzrlib/tests/test_lockdir.py
@@ -0,0 +1,269 @@
+# Copyright (C) 2006 Canonical Ltd
+
+# 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 useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+
+"""Tests for LockDir"""
+
+from threading import Thread
+import time
+
+from bzrlib.errors import (
+ LockBreakMismatch,
+ LockContention, LockError, UnlockableTransport,
+ LockNotHeld, LockBroken
+ )
+from bzrlib.lockdir import LockDir
+from bzrlib.tests import TestCaseInTempDir, TestCaseWithTransport
+
+# These tests sometimes use threads to test the behaviour of lock files with
+# concurrent actors. This is not a typical (or necessarily supported) use;
+# they're really meant for guarding between processes.
+
+class TestLockDir(TestCaseWithTransport):
+ """Test LockDir operations"""
+
+ def test_00_lock_creation(self):
+ """Creation of lock file on a transport"""
+ t = self.get_transport()
+ lf = LockDir(t, 'test_lock')
+ self.assertFalse(lf.is_held)
+
+ def test_01_lock_repr(self):
+ """Lock string representation"""
+ lf = LockDir(self.get_transport(), 'test_lock')
+ r = repr(lf)
+ self.assertContainsRe(r, r'^LockDir\(.*/test_lock\)$')
+
+ def test_02_unlocked_peek(self):
+ lf = LockDir(self.get_transport(), 'test_lock')
+ self.assertEqual(lf.peek(), None)
+
+ def test_03_readonly_peek(self):
+ lf = LockDir(self.get_readonly_transport(), 'test_lock')
+ self.assertEqual(lf.peek(), None)
+
+ def test_10_lock_uncontested(self):
+ """Acquire and release a lock"""
+ t = self.get_transport()
+ lf = LockDir(t, 'test_lock')
+ lf.attempt_lock()
+ try:
+ self.assertTrue(lf.is_held)
+ finally:
+ lf.unlock()
+ self.assertFalse(lf.is_held)
+
+ def test_11_lock_readonly_transport(self):
+ """Fail to lock on readonly transport"""
+ t = self.get_readonly_transport()
+ lf = LockDir(t, 'test_lock')
+ self.assertRaises(UnlockableTransport, lf.attempt_lock)
+
+ def test_20_lock_contested(self):
+ """Contention to get a lock"""
+ t = self.get_transport()
+ lf1 = LockDir(t, 'test_lock')
+ lf1.attempt_lock()
+ lf2 = LockDir(t, 'test_lock')
+ try:
+ # locking is between LockDir instances; aliases within
+ # a single process are not detected
+ lf2.attempt_lock()
+ self.fail('Failed to detect lock collision')
+ except LockContention, e:
+ self.assertEqual(e.lock, lf2)
+ self.assertContainsRe(str(e),
+ r'^Could not acquire.*test_lock.*$')
+ lf1.unlock()
+
+ def test_20_lock_peek(self):
+ """Peek at the state of a lock"""
+ t = self.get_transport()
+ lf1 = LockDir(t, 'test_lock')
+ lf1.attempt_lock()
+ # lock is held, should get some info on it
+ info1 = lf1.peek()
+ self.assertEqual(set(info1.keys()),
+ set(['user', 'nonce', 'hostname', 'pid', 'start_time']))
+ # should get the same info if we look at it through a different
+ # instance
+ info2 = LockDir(t, 'test_lock').peek()
+ self.assertEqual(info1, info2)
+ # locks which are never used should be not-held
+ self.assertEqual(LockDir(t, 'other_lock').peek(), None)
+
+ def test_21_peek_readonly(self):
+ """Peek over a readonly transport"""
+ t = self.get_transport()
+ lf1 = LockDir(t, 'test_lock')
+ lf2 = LockDir(self.get_readonly_transport(), 'test_lock')
+ self.assertEqual(lf2.peek(), None)
+ lf1.attempt_lock()
+ info2 = lf2.peek()
+ self.assertTrue(info2)
+ self.assertEqual(info2['nonce'], lf1.nonce)
+
+ def test_30_lock_wait_fail(self):
+ """Wait on a lock, then fail
+
+ We ask to wait up to 400ms; this should fail within at most one
+ second. (Longer times are more realistic but we don't want the test
+ suite to take too long, and this should do for now.)
+ """
+ t = self.get_transport()
+ lf1 = LockDir(t, 'test_lock')
+ lf2 = LockDir(t, 'test_lock')
+ lf1.attempt_lock()
+ try:
+ before = time.time()
+ self.assertRaises(LockContention, lf2.wait_lock,
+ timeout=0.4, poll=0.1)
+ after = time.time()
+ self.assertTrue(after - before <= 1.0)
+ finally:
+ lf1.unlock()
+
+ def test_31_lock_wait_easy(self):
+ """Succeed when waiting on a lock with no contention.
+ """
+ t = self.get_transport()
+ lf2 = LockDir(t, 'test_lock')
+ try:
+ before = time.time()
+ lf2.wait_lock(timeout=0.4, poll=0.1)
+ after = time.time()
+ self.assertTrue(after - before <= 1.0)
+ finally:
+ lf2.unlock()
+
+ def test_32_lock_wait_succeed(self):
+ """Succeed when trying to acquire a lock that gets released
+
+ One thread holds on a lock and then releases it; another tries to lock it.
+ """
+ t = self.get_transport()
+ lf1 = LockDir(t, 'test_lock')
+ lf1.attempt_lock()
+
+ def wait_and_unlock():
+ time.sleep(0.1)
+ lf1.unlock()
+ unlocker = Thread(target=wait_and_unlock)
+ unlocker.start()
+ try:
+ lf2 = LockDir(t, 'test_lock')
+ before = time.time()
+ # wait and then lock
+ lf2.wait_lock(timeout=0.4, poll=0.1)
+ after = time.time()
+ self.assertTrue(after - before <= 1.0)
+ finally:
+ unlocker.join()
+
+ def test_33_wait(self):
+ """Succeed when waiting on a lock that gets released
+
+ The difference from test_32_lock_wait_succeed is that the second
+ caller does not actually acquire the lock, but just waits for it
+ to be released. This is done over a readonly transport.
+ """
+ t = self.get_transport()
+ lf1 = LockDir(t, 'test_lock')
+ lf1.attempt_lock()
+
+ def wait_and_unlock():
+ time.sleep(0.1)
+ lf1.unlock()
+ unlocker = Thread(target=wait_and_unlock)
+ unlocker.start()
+ try:
+ lf2 = LockDir(self.get_readonly_transport(), 'test_lock')
+ before = time.time()
+ # wait but don't lock
+ lf2.wait(timeout=0.4, poll=0.1)
+ after = time.time()
+ self.assertTrue(after - before <= 1.0)
+ finally:
+ unlocker.join()
+
+ def test_40_confirm_easy(self):
+ """Confirm a lock that's already held"""
+ t = self.get_transport()
+ lf1 = LockDir(t, 'test_lock')
+ lf1.attempt_lock()
+ lf1.confirm()
+
+ def test_41_confirm_not_held(self):
+ """Confirm a lock that's already held"""
+ t = self.get_transport()
+ lf1 = LockDir(t, 'test_lock')
+ self.assertRaises(LockNotHeld, lf1.confirm)
+
+ def test_42_confirm_broken_manually(self):
+ """Confirm a lock broken by hand"""
+ t = self.get_transport()
+ lf1 = LockDir(t, 'test_lock')
+ lf1.attempt_lock()
+ t.move('test_lock', 'lock_gone_now')
+ self.assertRaises(LockBroken, lf1.confirm)
+
+ def test_43_break(self):
+ """Break a lock whose caller has forgotten it"""
+ t = self.get_transport()
+ lf1 = LockDir(t, 'test_lock')
+ lf1.attempt_lock()
+ # we incorrectly discard the lock object without unlocking it
+ del lf1
+ # someone else sees it's still locked
+ lf2 = LockDir(t, 'test_lock')
+ holder_info = lf2.peek()
+ self.assertTrue(holder_info)
+ lf2.force_break(holder_info)
+ # now we should be able to take it
+ lf2.attempt_lock()
+ lf2.confirm()
+
+ def test_44_break_already_released(self):
+ """Lock break races with regular release"""
+ t = self.get_transport()
+ lf1 = LockDir(t, 'test_lock')
+ lf1.attempt_lock()
+ # someone else sees it's still locked
+ lf2 = LockDir(t, 'test_lock')
+ holder_info = lf2.peek()
+ # in the interim the lock is released
+ lf1.unlock()
+ # break should succeed
+ lf2.force_break(holder_info)
+ # now we should be able to take it
+ lf2.attempt_lock()
+ lf2.confirm()
+
+ def test_45_break_mismatch(self):
+ """Lock break races with someone else acquiring it"""
+ t = self.get_transport()
+ lf1 = LockDir(t, 'test_lock')
+ lf1.attempt_lock()
+ # someone else sees it's still locked
+ lf2 = LockDir(t, 'test_lock')
+ holder_info = lf2.peek()
+ # in the interim the lock is released
+ lf1.unlock()
+ lf3 = LockDir(t, 'test_lock')
+ lf3.attempt_lock()
+ # break should now *fail*
+ self.assertRaises(LockBreakMismatch, lf2.force_break,
+ holder_info)
+ lf3.unlock()
=== modified file 'bzrlib/errors.py'
--- bzrlib/errors.py
+++ bzrlib/errors.py
@@ -1,4 +1,4 @@
-# (C) 2005 Canonical
+# Copyright (C) 2005, 2006 Canonical
# 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
@@ -189,6 +189,10 @@
"""File exists: %(path)r%(extra)s"""
+class DirectoryNotEmpty(PathError):
+ """Directory not empty: %(path)r%(extra)s"""
+
+
class PermissionDenied(PathError):
"""Permission denied: %(path)r%(extra)s"""
@@ -267,23 +271,85 @@
"""Cannot operate on a file because it is a control file."""
-class LockError(Exception):
- """Lock error"""
+class LockError(BzrNewError):
+ """Lock error: %(message)s"""
# All exceptions from the lock/unlock functions should be from
# this exception class. They will be translated as necessary. The
# original exception is available as e.original_error
+ #
+ # New code should prefer to raise specific subclasses
+ def __init__(self, message):
+ self.message = message
class CommitNotPossible(LockError):
"""A commit was attempted but we do not have a write lock open."""
+ def __init__(self):
+ pass
class AlreadyCommitted(LockError):
"""A rollback was requested, but is not able to be accomplished."""
+ def __init__(self):
+ pass
class ReadOnlyError(LockError):
"""A write attempt was made in a read only transaction."""
+ def __init__(self):
+ pass
+
+
+class BranchNotLocked(LockError):
+ """Branch %(branch)r is not locked"""
+ def __init__(self, branch):
+ # XXX: sometimes called with a LockableFiles instance not a Branch
+ self.branch = branch
+
+
+class ReadOnlyObjectDirtiedError(ReadOnlyError):
+ """Cannot change object %(obj)r in read only transaction"""
+ def __init__(self, obj):
+ self.obj = obj
+
+
+class UnlockableTransport(LockError):
+ """Cannot lock: transport is read only: %(transport)s"""
+ def __init__(self, transport):
+ self.transport = transport
+
+
+class LockContention(LockError):
+ """Could not acquire lock %(lock)s"""
+ # TODO: show full url for lock, combining the transport and relative bits?
+ def __init__(self, lock):
+ self.lock = lock
+
+
+class LockBroken(LockError):
+ """Lock was broken while still open: %(lock)s - check storage consistency!"""
+ def __init__(self, lock):
+ self.lock = lock
+
+
+class LockBreakMismatch(LockError):
+ """Lock was released and re-acquired before being broken: %(lock)s: held by %(holder)r, wanted to break %(target)r"""
+ def __init__(self, lock, holder, target):
+ self.lock = lock
+ self.holder = holder
+ self.target = target
+
+
+class LockNotHeld(LockError):
+ """Lock not held: %(lock)s"""
+ def __init__(self, lock):
+ self.lock = lock
+
+
+class BranchNotLocked(LockError):
+ """Branch %(branch)r not locked"""
+ def __init__(self, branch):
+ self.branch = branch
class PointlessCommit(BzrNewError):
=== modified file 'bzrlib/help.py'
--- bzrlib/help.py
+++ bzrlib/help.py
@@ -162,5 +162,5 @@
cmd_help = cmd_object.help()
if cmd_help:
firstline = cmd_help.split('\n', 1)[0]
- print >>outfile, ' ' + firstline
+ print >>outfile, ' ' + firstline
=== modified file 'bzrlib/lockable_files.py'
--- bzrlib/lockable_files.py
+++ bzrlib/lockable_files.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2005 Canonical Ltd
+# Copyright (C) 2005, 2006 Canonical Ltd
# 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
@@ -167,8 +167,7 @@
# and potentially a remote locking protocol
if self._lock_mode:
if self._lock_mode != 'w':
- raise ReadOnlyError("can't upgrade to a write lock from %r" %
- self._lock_mode)
+ raise ReadOnlyError()
self._lock_count += 1
else:
self._lock = self._transport.lock_write(
@@ -195,8 +194,7 @@
def unlock(self):
# mutter("unlock: %s (%s)", self, self._lock_count)
if not self._lock_mode:
- raise LockError('branch %r is not locked' % (self))
-
+ raise errors.BranchNotLocked(self)
if self._lock_count > 1:
self._lock_count -= 1
else:
=== modified file 'bzrlib/osutils.py'
--- bzrlib/osutils.py
+++ bzrlib/osutils.py
@@ -129,7 +129,7 @@
import random
base = os.path.basename(new)
dirname = os.path.dirname(new)
- tmp_name = u'tmp.%s.%.9f.%d.%d' % (base, time.time(), os.getpid(), random.randint(0, 0x7FFFFFFF))
+ tmp_name = u'tmp.%s.%.9f.%d.%s' % (base, time.time(), os.getpid(), rand_chars(10))
tmp_name = pathjoin(dirname, tmp_name)
# Rename the file out of the way, but keep track if it didn't exist
@@ -441,6 +441,7 @@
"""Return size of given open file."""
return os.fstat(f.fileno())[ST_SIZE]
+
# Define rand_bytes based on platform.
try:
# Python 2.4 and later have os.urandom,
@@ -462,6 +463,20 @@
s += chr(random.randint(0, 255))
n -= 1
return s
+
+
+ALNUM = '0123456789abcdefghijklmnopqrstuvwxyz'
+def rand_chars(num):
+ """Return a random string of num alphanumeric characters
+
+ The result only contains lowercase chars because it may be used on
+ case-insensitive filesystems.
+ """
+ s = ''
+ for raw_byte in rand_bytes(num):
+ s += ALNUM[ord(raw_byte) % 36]
+ return s
+
## TODO: We could later have path objects that remember their list
## decomposition (might be too tricksy though.)
=== modified file 'bzrlib/rio.py'
--- bzrlib/rio.py
+++ bzrlib/rio.py
@@ -1,7 +1,7 @@
# Copyright (C) 2005 by Canonical Ltd
#
# Distributed under the GNU General Public Licence v2
-#
+
# \subsection{\emph{rio} - simple text metaformat}
#
# \emph{r} stands for `restricted', `reproducible', or `rfc822-like'.
@@ -18,10 +18,6 @@
# stream representation of an object and vice versa, and that this relation
# will continue to hold for future versions of bzr.
-# In comments, $\min(1,10)$
-
-min(1,10)
-
import re
# XXX: some redundancy is allowing to write stanzas in isolation as well as
@@ -97,7 +93,8 @@
## elif isinstance(value, (int, long)):
## value = str(value) # XXX: python2.4 without L-suffix
else:
- raise ValueError("invalid value %r" % value)
+ raise TypeError("invalid type for rio value: %r of type %s"
+ % (value, type(value)))
self.items.append((tag, value))
def __contains__(self, find_tag):
@@ -174,6 +171,15 @@
if t == tag:
r.append(v)
return r
+
+ def as_dict(self):
+ """Return a dict containing the unique values of the stanza.
+ """
+ d = {}
+ for tag, value in self.items:
+ assert tag not in d
+ d[tag] = value
+ return d
_tag_re = re.compile(r'^[-a-zA-Z0-9_]+$')
def valid_tag(tag):
=== modified file 'bzrlib/sign_my_commits.py'
--- bzrlib/sign_my_commits.py
+++ bzrlib/sign_my_commits.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2005 Canonical Ltd
+# Copyright (C) 2005, 2006 Canonical Ltd
# 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
@@ -61,7 +61,8 @@
raise errors.BzrCommandError('cannot sign revisions on non-listable transports')
count = 0
- for rev_id in repo.revision_store:
+ # return in partial topological order for the sake of reproducibility
+ for rev_id in repo.all_revision_ids():
if repo.revision_store.has_id(rev_id, suffix='sig'):
continue
=== modified file 'bzrlib/tests/__init__.py'
--- bzrlib/tests/__init__.py
+++ bzrlib/tests/__init__.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2005 by Canonical Ltd
+# Copyright (C) 2005, 2006 by Canonical Ltd
# 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
@@ -42,13 +42,14 @@
import bzrlib.errors as errors
import bzrlib.inventory
import bzrlib.iterablefile
+import bzrlib.lockdir
import bzrlib.merge3
import bzrlib.osutils
import bzrlib.osutils as osutils
import bzrlib.plugin
import bzrlib.store
import bzrlib.trace
-from bzrlib.transport import urlescape
+from bzrlib.transport import urlescape, get_transport
import bzrlib.transport
from bzrlib.transport.local import LocalRelpathServer
from bzrlib.transport.readonly import ReadonlyServer
@@ -66,6 +67,7 @@
bzrlib.errors,
bzrlib.inventory,
bzrlib.iterablefile,
+ bzrlib.lockdir,
bzrlib.merge3,
bzrlib.option,
bzrlib.osutils,
@@ -299,7 +301,7 @@
raise AssertionError('pattern "%s" not found in "%s"'
% (needle_re, haystack))
- def AssertSubset(self, sublist, superlist):
+ def assertSubset(self, sublist, superlist):
"""Assert that every entry in sublist is present in superlist."""
missing = []
for entry in sublist:
@@ -616,7 +618,7 @@
"""
# XXX: It's OK to just create them using forward slashes on windows?
if transport is None or transport.is_readonly():
- transport = bzrlib.transport.get_transport(".")
+ transport = get_transport(".")
for name in shape:
self.assert_(isinstance(name, basestring))
if name[-1] == '/':
@@ -729,6 +731,22 @@
base = base + relpath
return base
+ def get_transport(self):
+ """Return a writeable transport for the test scratch space"""
+ t = get_transport(self.get_url())
+ self.assertFalse(t.is_readonly())
+ return t
+
+ def get_readonly_transport(self):
+ """Return a readonly transport for the test scratch space
+
+ This can be used to test that operations which should only need
+ readonly access in fact do not try to write.
+ """
+ t = get_transport(self.get_readonly_url())
+ self.assertTrue(t.is_readonly())
+ return t
+
def make_branch(self, relpath):
"""Create a branch on the transport at relpath."""
repo = self.make_repository(relpath)
@@ -737,10 +755,10 @@
def make_bzrdir(self, relpath):
try:
url = self.get_url(relpath)
- segments = url.split('/')
+ segments = relpath.split('/')
if segments and segments[-1] not in ('', '.'):
- parent = '/'.join(segments[:-1])
- t = bzrlib.transport.get_transport(parent)
+ parent = self.get_url('/'.join(segments[:-1]))
+ t = get_transport(parent)
try:
t.mkdir(segments[-1])
except errors.FileExists:
@@ -876,6 +894,7 @@
'bzrlib.tests.test_http',
'bzrlib.tests.test_identitymap',
'bzrlib.tests.test_inv',
+ 'bzrlib.tests.test_lockdir',
'bzrlib.tests.test_lockable_files',
'bzrlib.tests.test_log',
'bzrlib.tests.test_merge',
=== modified file 'bzrlib/tests/repository_implementations/test_fileid_involved.py'
--- bzrlib/tests/repository_implementations/test_fileid_involved.py
+++ bzrlib/tests/repository_implementations/test_fileid_involved.py
@@ -225,4 +225,4 @@
l2 = self.compare_tree_fileids(self.branch, old_rev, new_rev)
self.assertNotEqual(l2, l1)
- self.AssertSubset(l2, l1)
+ self.assertSubset(l2, l1)
=== modified file 'bzrlib/tests/test_osutils.py'
--- bzrlib/tests/test_osutils.py
+++ bzrlib/tests/test_osutils.py
@@ -59,8 +59,17 @@
self.check_file_contents('a', 'something in a\n')
+ # TODO: test fancy_rename using a MemoryTransport
- # TODO: test fancy_rename using a MemoryTransport
+ def test_01_rand_chars_empty(self):
+ result = osutils.rand_chars(0)
+ self.assertEqual(result, '')
+
+ def test_02_rand_chars_100(self):
+ result = osutils.rand_chars(100)
+ self.assertEqual(len(result), 100)
+ self.assertEqual(type(result), str)
+ self.assertContainsRe(result, r'^[a-z0-9]{100}$')
class TestSafeUnicode(TestCase):
=== modified file 'bzrlib/tests/test_rio.py'
--- bzrlib/tests/test_rio.py
+++ bzrlib/tests/test_rio.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2005 by Canonical Ltd
+# Copyright (C) 2005, 2006 by Canonical Ltd
#
# 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
@@ -63,6 +63,12 @@
self.assertEquals(list(s.to_lines()),
['name: fred\n',
'number: 42\n'])
+
+ def test_as_dict(self):
+ """Convert rio Stanza to dictionary"""
+ s = Stanza(number='42', name='fred')
+ sd = s.as_dict()
+ self.assertEquals(sd, dict(number='42', name='fred'))
def test_to_file(self):
"""Write rio to file"""
@@ -280,3 +286,13 @@
"""Write empty stanza"""
l = list(Stanza().to_lines())
self.assertEquals(l, [])
+
+ def test_rio_raises_type_error(self):
+ """TypeError on adding invalid type to Stanza"""
+ s = Stanza()
+ self.assertRaises(TypeError, s.add, 'foo', {})
+
+ def test_rio_raises_type_error_key(self):
+ """TypeError on adding invalid type to Stanza"""
+ s = Stanza()
+ self.assertRaises(TypeError, s.add, 10, {})
=== modified file 'bzrlib/tests/test_smart_add.py'
--- bzrlib/tests/test_smart_add.py
+++ bzrlib/tests/test_smart_add.py
@@ -106,10 +106,10 @@
self.build_tree(['inertiatic/', 'inertiatic/esp', 'inertiatic/CVS',
'inertiatic/foo.pyc'])
added, ignored = smart_add_tree(wt, u'.')
- self.AssertSubset(('inertiatic', 'inertiatic/esp'), added)
- self.AssertSubset(('CVS', '*.py[oc]'), ignored)
- self.AssertSubset(('inertiatic/CVS',), ignored['CVS'])
- self.AssertSubset(('inertiatic/foo.pyc',), ignored['*.py[oc]'])
+ self.assertSubset(('inertiatic', 'inertiatic/esp'), added)
+ self.assertSubset(('CVS', '*.py[oc]'), ignored)
+ self.assertSubset(('inertiatic/CVS',), ignored['CVS'])
+ self.assertSubset(('inertiatic/foo.pyc',), ignored['*.py[oc]'])
class TestSmartAddBranch(TestCaseWithTransport):
=== modified file 'bzrlib/tests/test_transport_implementations.py'
--- bzrlib/tests/test_transport_implementations.py
+++ bzrlib/tests/test_transport_implementations.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2004, 2005 by Canonical Ltd
+# Copyright (C) 2004, 2005, 2006 by Canonical Ltd
# 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
@@ -25,8 +25,9 @@
import stat
import sys
-from bzrlib.errors import (NoSuchFile, FileExists,
+from bzrlib.errors import (DirectoryNotEmpty, NoSuchFile, FileExists,
LockError,
+ PathError,
TransportNotPossible, ConnectionError)
from bzrlib.tests import TestCaseInTempDir, TestSkipped
from bzrlib.transport import memory, urlescape
@@ -552,6 +553,44 @@
t.rmdir('adir')
self.assertRaises(NoSuchFile, t.stat, 'adir')
+ def test_rmdir_not_empty(self):
+ """Deleting a non-empty directory raises an exception
+
+ sftp (and possibly others) don't give us a specific "directory not
+ empty" exception -- we can just see that the operation failed.
+ """
+ t = self.get_transport()
+ if t.is_readonly():
+ return
+ t.mkdir('adir')
+ t.mkdir('adir/bdir')
+ self.assertRaises(PathError, t.rmdir, 'adir')
+
+ def test_rename_dir_succeeds(self):
+ t = self.get_transport()
+ if t.is_readonly():
+ raise TestSkipped("transport is readonly")
+ t.mkdir('adir')
+ t.mkdir('adir/asubdir')
+ t.rename('adir', 'bdir')
+ self.assertTrue(t.has('bdir/asubdir'))
+ self.assertFalse(t.has('adir'))
+
+ def test_rename_dir_nonempty(self):
+ """Attempting to replace a nonemtpy directory should fail"""
+ t = self.get_transport()
+ if t.is_readonly():
+ raise TestSkipped("transport is readonly")
+ t.mkdir('adir')
+ t.mkdir('adir/asubdir')
+ t.mkdir('bdir')
+ t.mkdir('bdir/bsubdir')
+ self.assertRaises(PathError, t.rename, 'bdir', 'adir')
+ # nothing was changed so it should still be as before
+ self.assertTrue(t.has('bdir/bsubdir'))
+ self.assertFalse(t.has('adir/bdir'))
+ self.assertFalse(t.has('adir/bsubdir'))
+
def test_delete_tree(self):
t = self.get_transport()
@@ -790,17 +829,23 @@
def test_iter_files_recursive(self):
transport = self.get_transport()
if not transport.listable():
- self.assertRaises(TransportNotPossible,
+ self.assertRaises(TransportNotPossible,
transport.iter_files_recursive)
return
- self.build_tree(['isolated/',
+ self.build_tree(['isolated/',
'isolated/dir/',
'isolated/dir/foo',
'isolated/dir/bar',
'isolated/bar'],
transport=transport)
- transport = transport.clone('isolated')
paths = set(transport.iter_files_recursive())
+ # nb the directories are not converted
+ self.assertEqual(paths,
+ set(['isolated/dir/foo',
+ 'isolated/dir/bar',
+ 'isolated/bar']))
+ sub_transport = transport.clone('isolated')
+ paths = set(sub_transport.iter_files_recursive())
self.assertEqual(set(['dir/foo', 'dir/bar', 'bar']), paths)
def test_connect_twice_is_same_content(self):
=== modified file 'bzrlib/tests/test_ui.py'
--- bzrlib/tests/test_ui.py
+++ bzrlib/tests/test_ui.py
@@ -61,4 +61,7 @@
result = pb.note('t')
self.assertEqual(None, result)
self.assertEqual("t\n", stdout.getvalue())
- self.assertEqual("\r \r", stderr.getvalue())
+ # the exact contents will depend on the terminal width and we don't
+ # care about that right now - but you're probably running it on at
+ # least a 10-character wide terminal :)
+ self.assertContainsRe(r'^\r {10,}\r$', stderr.getvalue())
=== modified file 'bzrlib/transactions.py'
--- bzrlib/transactions.py
+++ bzrlib/transactions.py
@@ -51,7 +51,7 @@
def commit(self):
"""ReadOnlyTransactions cannot commit."""
- raise errors.CommitNotPossible('In a read only transaction')
+ raise errors.CommitNotPossible()
def finish(self):
"""Clean up this transaction
@@ -82,8 +82,7 @@
def register_dirty(self, an_object):
"""Register an_object as being dirty."""
- raise errors.ReadOnlyError(
- "Cannot dirty objects in a read only transaction")
+ raise errors.ReadOnlyError()
def rollback(self):
"""Let people call this even though nothing has to happen."""
=== modified file 'bzrlib/transport/__init__.py'
--- bzrlib/transport/__init__.py
+++ bzrlib/transport/__init__.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2005 Canonical Ltd
+# Copyright (C) 2005, 2006 Canonical Ltd
# 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
@@ -107,6 +107,8 @@
raise errors.FileExists(path, extra=e)
if e.errno == errno.EACCES:
raise errors.PermissionDenied(path, extra=e)
+ if e.errno == errno.ENOTEMPTY:
+ raise errors.DirectoryNotEmpty(path, extra=e)
if raise_generic:
raise errors.TransportError(orig_error=e)
@@ -223,6 +225,8 @@
def iter_files_recursive(self):
"""Iter the relative paths of files in the transports sub-tree.
+
+ *NOTE*: This only lists *files*, not subdirectories!
As with other listing functions, only some transports implement this,.
you may check via is_listable to determine if it will.
@@ -358,8 +362,30 @@
files.append(path)
source.copy_to(files, target)
+ def rename(self, rel_from, rel_to):
+ """Rename a file or directory.
+
+ This *must* fail if the destination is a nonempty directory - it must
+ not automatically remove it. It should raise DirectoryNotEmpty, or
+ some other PathError if the case can't be specifically detected.
+
+ If the destination is an empty directory or a file this function may
+ either fail or succeed, depending on the underlying transport. It
+ should not attempt to remove the destination if overwriting is not the
+ native transport behaviour. If at all possible the transport should
+ ensure that the rename either completes or not, without leaving the
+ destination deleted and the new file not moved in place.
+
+ This is intended mainly for use in implementing LockDir.
+ """
+ # transports may need to override this
+ raise NotImplementedError(self.rename)
+
def move(self, rel_from, rel_to):
"""Move the item at rel_from to the location at rel_to.
+
+ The destination is deleted if possible, even if it's a non-empty
+ directory tree.
If a transport can directly implement this it is suggested that
it do so for efficiency.
=== modified file 'bzrlib/transport/local.py'
--- bzrlib/transport/local.py
+++ bzrlib/transport/local.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2005 Canonical Ltd
+# Copyright (C) 2005, 2006 Canonical Ltd
# 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
@@ -151,12 +151,23 @@
# TODO: What about path_to?
self._translate_error(e, path_from)
+ def rename(self, rel_from, rel_to):
+ path_from = self.abspath(rel_from)
+ try:
+ # *don't* call bzrlib.osutils.rename, because we want to
+ # detect errors on rename
+ os.rename(path_from, self.abspath(rel_to))
+ except (IOError, OSError),e:
+ # TODO: What about path_to?
+ self._translate_error(e, path_from)
+
def move(self, rel_from, rel_to):
"""Move the item at rel_from to the location at rel_to"""
path_from = self.abspath(rel_from)
path_to = self.abspath(rel_to)
try:
+ # this version will delete the destination if necessary
rename(path_from, path_to)
except (IOError, OSError),e:
# TODO: What about path_to?
=== modified file 'bzrlib/transport/memory.py'
--- bzrlib/transport/memory.py
+++ bzrlib/transport/memory.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2005 Canonical Ltd
+# Copyright (C) 2005, 2006 Canonical Ltd
# 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
@@ -13,11 +13,17 @@
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
-"""Implementation of Transport that uses memory for its storage."""
+
+"""Implementation of Transport that uses memory for its storage.
+
+The contents of the transport will be lost when the object is discarded,
+so this is primarily useful for testing.
+"""
from copy import copy
import os
import errno
+import re
from stat import *
from cStringIO import StringIO
@@ -48,6 +54,7 @@
url = url + '/'
super(MemoryTransport, self).__init__(url)
self._cwd = url[url.find(':') + 1:]
+ # dictionaries from absolute path to file mode
self._dirs = {}
self._files = {}
self._locks = {}
@@ -151,6 +158,27 @@
path[len(_abspath)] == '/'):
result.append(path[len(_abspath) + 1:])
return result
+
+ def rename(self, rel_from, rel_to):
+ """Rename a file or directory; fail if the destination exists"""
+ abs_from = self._abspath(rel_from)
+ abs_to = self._abspath(rel_to)
+ def replace(x):
+ if x == abs_from:
+ x = abs_to
+ elif x.startswith(abs_from + '/'):
+ x = abs_to + x[len(abs_from):]
+ return x
+ def do_renames(container):
+ for path in container:
+ new_path = replace(path)
+ if new_path != path:
+ if new_path in container:
+ raise FileExists(new_path)
+ container[new_path] = container[path]
+ del container[path]
+ do_renames(self._files)
+ do_renames(self._dirs)
def rmdir(self, relpath):
"""See Transport.rmdir."""
@@ -159,10 +187,11 @@
self._translate_error(IOError(errno.ENOTDIR, relpath), relpath)
for path in self._files:
if path.startswith(_abspath):
- self._translate_error(IOError(errno.EBUSY, relpath), relpath)
+ self._translate_error(IOError(errno.ENOTEMPTY, relpath),
+ relpath)
for path in self._dirs:
if path.startswith(_abspath) and path != _abspath:
- self._translate_error(IOError(errno.EBUSY, relpath), relpath)
+ self._translate_error(IOError(errno.ENOTEMPTY, relpath), relpath)
if not _abspath in self._dirs:
raise NoSuchFile(relpath)
del self._dirs[_abspath]
=== modified file 'bzrlib/transport/sftp.py'
--- bzrlib/transport/sftp.py
+++ bzrlib/transport/sftp.py
@@ -417,7 +417,7 @@
self._sftp.chmod(tmp_abspath, mode)
fout.close()
closed = True
- self._rename(tmp_abspath, abspath)
+ self._rename_and_overwrite(tmp_abspath, abspath)
except Exception, e:
# If we fail, try to clean up the temporary file
# before we throw the exception
@@ -505,7 +505,16 @@
except (IOError, paramiko.SSHException), e:
self._translate_io_exception(e, relpath, ': unable to append')
- def _rename(self, abs_from, abs_to):
+ def rename(self, rel_from, rel_to):
+ """Rename without special overwriting"""
+ try:
+ self._sftp.rename(self._remote_path(rel_from),
+ self._remote_path(rel_to))
+ except (IOError, paramiko.SSHException), e:
+ self._translate_io_exception(e, rel_from,
+ ': unable to rename to %r' % (rel_to))
+
+ def _rename_and_overwrite(self, abs_from, abs_to):
"""Do a fancy rename on the remote server.
Using the implementation provided by osutils.
@@ -521,7 +530,7 @@
"""Move the item at rel_from to the location at rel_to"""
path_from = self._remote_path(rel_from)
path_to = self._remote_path(rel_to)
- self._rename(path_from, path_to)
+ self._rename_and_overwrite(path_from, path_to)
def delete(self, relpath):
"""Delete the item at relpath"""
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: Digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060222/53267ad0/attachment.pgp
More information about the bazaar
mailing list