← Back to branch summary

~bzr-pqm/bzr/bzr.dev

« back to all changes in this revision

Viewing changes to bzrlib/config.py

  • Committer: Vincent Ladeuil
  • Date: 2012-01-05 14:26:58 UTC
  • mto: This revision was merged to the branch mainline in revision 6468.
  • Revision ID: v.ladeuil+lp@free.fr-20120105142658-vek3v6pzlxb751s2
Tests passing for a first rough version of a cached branch config store. The changes here are too invasive and several parallel proposals have been made. 

@only_raises is evil and gave a hard time since any exception during
save_changes() was swallowed.

Possible improvements: 

- add some needs_write_lock decorators to crucial
  methods (_set_config_location ?) but keep locking the branch at higher levels

- decorate branch.unlock to call stack.save if last_lock() it True
  outside of @only_raises scope (evil decorator)

- add @needs_write_lock to stack.set and stack.remove (will probably get
  rid of most testing issues) we probably need a specialized decorator
  that can relay to the store and from there to the branch or whatever is
  needed. This will also helps bzr config to get it right. The
  get_mutable_section trick should not be needed anymore either.

- decorate branch.unlock to call stack.save if last_lock() it True outside
  of @only_raises scope (evil decorator)

Show diffs side-by-side

added added

removed removed

Lines of Context:
bzrlib/config.py
1
 
# Copyright (C) 2005-2011 Canonical Ltd
 
1
# Copyright (C) 2005-2012 Canonical Ltd
2
2
#   Authors: Robert Collins <robert.collins@canonical.com>
3
3
#            and others
4
4
#
3019
3019
        # From now on we need a lock on the store to ensure changes can be
3020
3020
        # saved atomically (daughter classes will provide locking around
3021
3021
        # save_changes)
3022
 
        self.load()
3023
3022
        # Apply the changes from the preserved dirty sections
3024
3023
        for dirty in dirty_sections:
3025
3024
            clean = self.get_mutable_section(dirty.id)
3351
3350
        self.branch = branch
3352
3351
        self.id = 'branch'
3353
3352
 
3354
 
    def lock_write(self, token=None):
3355
 
        return self.branch.lock_write(token)
3356
 
 
3357
 
    def unlock(self):
3358
 
        return self.branch.unlock()
3359
 
 
3360
 
    @needs_write_lock
3361
 
    def save(self):
3362
 
        # We need to be able to override the undecorated implementation
3363
 
        self.save_without_locking()
3364
 
 
3365
 
    def save_without_locking(self):
3366
 
        super(BranchStore, self).save()
 
3353
    # FIXME: This is very handy to detect which callers forgot to lock the
 
3354
    # branch but break many bt.test_config tests. Either these tests should
 
3355
    # parametrized differently or better ways to achieve the branch locking
 
3356
    # should be found that don't require this.
 
3357
    def xget_mutable_section(self, section_id=None):
 
3358
        if self.branch.peek_lock_mode() != 'w':
 
3359
            from bzrlib import debug ; debug.set_trace()
 
3360
            raise AssertionError('The branch for %s is not write-locked'
 
3361
                                 % self.external_url())
 
3362
        return super(BranchStore, self).get_mutable_section(section_id)
3367
3363
 
3368
3364
 
3369
3365
class ControlStore(LockableIniFileStore):
3819
3815
            lstore, mutable_section_id=location)
3820
3816
 
3821
3817
 
3822
 
class BranchStack(_CompatibleStack):
 
3818
class BranchStack(Stack):
3823
3819
    """Per-location options falling back to branch then global options stack.
3824
3820
 
3825
3821
    The following sections are queried:
3866
3862
        self.bzrdir = bzrdir
3867
3863
 
3868
3864
 
3869
 
class BranchOnlyStack(_CompatibleStack):
 
3865
class BranchOnlyStack(Stack):
3870
3866
    """Branch-only options stack."""
3871
3867
 
3872
3868
    # FIXME: _BranchOnlyStack only uses branch.conf and is used only for the
3952
3948
                # Set the option value
3953
3949
                self._set_config_option(name, value, directory, scope)
3954
3950
 
3955
 
    def _get_stack(self, directory, scope=None):
 
3951
    def _get_stack(self, directory, scope=None, write_access=False):
3956
3952
        """Get the configuration stack specified by ``directory`` and ``scope``.
3957
3953
 
3958
3954
        :param directory: Where the configurations are derived from.
3959
3955
 
3960
3956
        :param scope: A specific config to start from.
 
3957
 
 
3958
        :param write_access: Whether a write access to the stack will be
 
3959
            attempted.
3961
3960
        """
3962
3961
        # FIXME: scope should allow access to plugin-specific stacks (even
3963
3962
        # reduced to the plugin-specific store), related to
3971
3970
                (_, br, _) = (
3972
3971
                    controldir.ControlDir.open_containing_tree_or_branch(
3973
3972
                        directory))
 
3973
                if write_access:
 
3974
                    self.add_cleanup(br.lock_write().unlock)
3974
3975
                return br.get_config_stack()
3975
3976
            raise errors.NoSuchConfig(scope)
3976
3977
        else:
3978
3979
                (_, br, _) = (
3979
3980
                    controldir.ControlDir.open_containing_tree_or_branch(
3980
3981
                        directory))
 
3982
                if write_access:
 
3983
                    self.add_cleanup(br.lock_write().unlock)
3981
3984
                return br.get_config_stack()
3982
3985
            except errors.NotBranchError:
3983
3986
                return LocationStack(directory)
4028
4031
                        self.outf.write('  %s = %s\n' % (oname, value))
4029
4032
 
4030
4033
    def _set_config_option(self, name, value, directory, scope):
4031
 
        conf = self._get_stack(directory, scope)
 
4034
        conf = self._get_stack(directory, scope, write_access=True)
4032
4035
        conf.set(name, value)
4033
4036
 
4034
4037
    def _remove_config_option(self, name, directory, scope):
4035
4038
        if name is None:
4036
4039
            raise errors.BzrCommandError(
4037
4040
                '--remove expects an option to remove.')
4038
 
        conf = self._get_stack(directory, scope)
 
4041
        conf = self._get_stack(directory, scope, write_access=True)
4039
4042
        try:
4040
4043
            conf.remove(name)
4041
4044
        except KeyError: