[MERGE] Allow changing --[no-]trees with reconfigure (bug 145033)

[Marius Kruger]

I'd like this in 1.12, so I gave it a go.

2009/1/19 Aaron Bentley <aaron at aaronbentley.com>

> Matthew D. Fuller wrote:
> >

...

> Actually, considering the change is just
> Repository.set_make_working_trees, it might be simpler to just invoke it
> directly.


done.


> > I did add a test that --with-no-trees doesn't screw with an existing
> > working tree (the rewrite should make that impossible anyway).  I'm
> > not real happy with the test; it sorta takes a long stroll around the
> > garden to check if a door it just opened is open.  There has to be a
> > less absurd way of writing it, but that's far past my competence.
>
> Well, you can skip some of the blackbox tests.  The blackbox tests
> should prove that it works at all, not duplicate the unit tests.


I left them in, as they were already done, and they do add some value:
they test that we support the flags and that the user gets good error
messages.


> > === modified file 'bzrlib/reconfigure.py'
> > --- bzrlib/reconfigure.py     2008-04-24 04:58:42 +0000
> > +++ bzrlib/reconfigure.py     2009-01-19 08:48:21 +0000
> > @@ -24,9 +24,10 @@
> >
> >  class Reconfigure(object):
> >
> > -    def __init__(self, bzrdir, new_bound_location=None):
> > +    def __init__(self, bzrdir, new_bound_location=None,
> with_trees=None):
> >          self.bzrdir = bzrdir
> >          self.new_bound_location = new_bound_location
> > +        self.with_trees = with_trees
>
> What is the purpose of this variable?  AFAICT, it is only used in
> set_repository_trees, and it is set there also.


removed.


> >          try:
> >              self.repository = self.bzrdir.find_repository()
> >          except errors.NoRepositoryPresent:
> > @@ -62,6 +63,8 @@
> >          self._create_tree = False
> >          self._create_repository = False
> >          self._destroy_repository = False
> > +        self._set_with_trees = False
> > +        self._set_with_no_trees = False
> >
> >      @staticmethod
> >      def to_branch(bzrdir):
> > @@ -140,6 +143,25 @@
> >              raise errors.AlreadyStandalone(bzrdir)
> >          return reconfiguration
> >
> > +    @classmethod
> > +    def set_repository_trees(klass, bzrdir, with_trees):
> > +        """Adjust a repository's working tree presense default"""
> > +        reconfiguration = klass(bzrdir, with_trees=with_trees)
>
> You don't need to pass with_trees in-- just use with_trees instead of
> reconfiguration.with_trees.


done


> > +        if not reconfiguration.repository.is_shared():
> > +            raise
> errors.ReconfigurationNotSupported(reconfiguration.bzrdir)
> > +        if reconfiguration.with_trees is True and \
>
> Please use a boolean test like
>
> if reconfiguration.with_trees


done

> +            not reconfiguration.repository.make_working_trees():
> > +            reconfiguration._set_with_trees = True
> > +        elif reconfiguration.with_trees is False and \
>
> Again, use elif not reconfiguration.with_trees


done


> > +            reconfiguration.repository.make_working_trees():
> > +            reconfiguration._set_with_no_trees = True
>
> Except, why not just use reconfiguration.with_trees directly in
> Reconfiguration.apply?


> i.e.
>
> if self.with_trees is not None:
>    if self.repository.set_make_working_trees(self.with_trees)


changes are now done in set_repository_trees() directly.


> > @@ -302,3 +325,7 @@
> >              local_branch.bind(branch.Branch.open(bind_location))
> >          if self._destroy_repository:
> >              self.bzrdir.destroy_repository()
> > +        if self._set_with_trees:
> > +            self.repository.set_make_working_trees(True)
> > +        if self._set_with_no_trees:
> > +            self.repository.set_make_working_trees(False)
>
> self.repository refers to the current repository, but won't refer to a
> newly-created repository.  You'll want to use the local variable 'repo'
> instead.


AFAICT,  newly-created repositories are always shared=False, which is
something the new actions claim not to support.
So since changes are now done in set_repository_trees() directly,
we will ignore newly-created repositories, which is fine IMO,
as I can't see us getting into such a situation from the commandline.


> +    def test_make_without_trees_leaves_tree_alone(self):
> > +        # There has to be a better way of doing this...
> > +        repo = self.make_repository('repo', shared=True)
> > +        branch = self.make_branch('repo/branch')
> > +        self.run_bzr('reconfigure --use-shared',
> > +            working_dir='repo/branch')
> > +        tree = branch.bzrdir.create_workingtree()
>
> Use BzrDir.create_branch_convenience instead of creating the branch and
> tree and then making it use the shared repository.


done,
well I created the repo first and then create_branch_convenience would put
it in the repo.



> > +        self.build_tree(['repo/branch/foo'])
> > +        tree.add('foo')
> > +        tree.commit('x')
>
> You don't have to commit-- you can just supply --force


done


> > +        self.run_bzr('reconfigure --with-no-trees',
> > +            working_dir='repo/branch')
> > +        self.failUnlessExists('repo/branch/foo')
>
> But this whole thing should be a unit test so that you know that
> workingtree.WorkingTree.open works at the end.


I do WorkingTree.open at the end now too, would that satisfy you?


<| regards
U| Marius
H| <><
-------------- next part --------------
An HTML attachment was scrubbed...
URL: https://lists.ubuntu.com/archives/bazaar/attachments/20090206/293b3428/attachment-0001.htm 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 20090206_0347-reconfig-trees.patch
Type: text/x-patch
Size: 18636 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20090206/293b3428/attachment-0001.bin