[MERGE] Allow changing --[no-]trees with reconfigure (bug 145033)
Aaron Bentley
aaron at aaronbentley.com
Mon Feb 9 19:13:36 GMT 2009
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
bb:resubmit
Marius Kruger wrote:
> Actually, considering the change is just
> Repository.set_make_working_trees, it might be simpler to just invoke it
> directly.
>
>
> done.
No, I meant not using Reconfigure at all. If you're using Reconfigure,
then the change should be caused by apply.
> > 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.
It's okay to leave them it, but the error messages should be tested in
the unit tests as well.
> > @@ -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.
This is an API, not the commandline. We may have other clients that use
it differently. We may extend the commandline later.
It does not make sense to write fragile code when it's just as easy to
write robust code.
> > + 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?
That's fine.
> # Bazaar merge directive format 2 (Bazaar 0.90)
> # revision_id: amanic at gmail.com-20090206014740-6gbah25yvrkocm6t
> # target_branch: ../bzr.dev/
^^^ Please set the public_branch for your local mirror
> === modified file 'bzrlib/builtins.py'
> --- bzrlib/builtins.py 2009-02-02 05:43:13 +0000
> +++ bzrlib/builtins.py 2009-02-05 23:31:14 +0000
> @@ -4851,7 +4851,11 @@
> ' checkout (with no local history).',
> standalone='Reconfigure to be a standalone branch '
> '(i.e. stop using shared repository).',
> - use_shared='Reconfigure to use a shared repository.'),
> + use_shared='Reconfigure to use a shared repository.',
> + with_trees='Reconfigure repository to create '
> + 'working trees on branches by default',
> + with_no_trees='Reconfigure repository to not create '
> + 'working trees on branches by default'),
> Option('bind-to', help='Branch to bind checkout to.',
> type=str),
> Option('force',
> @@ -4877,6 +4881,12 @@
> reconfiguration = reconfigure.Reconfigure.to_use_shared(directory)
> elif target_type == 'standalone':
> reconfiguration = reconfigure.Reconfigure.to_standalone(directory)
> + elif target_type == 'with-trees':
> + reconfiguration = reconfigure.Reconfigure.set_repository_trees(
> + directory, True)
> + elif target_type == 'with-no-trees':
> + reconfiguration = reconfigure.Reconfigure.set_repository_trees(
> + directory, False)
> reconfiguration.apply(force)
^^^ It doesn't make sense to do reconfiguration.apply for with-trees or
with-no-trees.
> === modified file 'bzrlib/errors.py'
> --- bzrlib/errors.py 2009-01-27 05:04:43 +0000
> +++ bzrlib/errors.py 2009-02-05 23:31:14 +0000
> @@ -2744,6 +2744,18 @@
> _fmt = "'%(display_url)s' is already standalone."
>
>
> +class AlreadyWithTrees(BzrDirError):
> +
> + _fmt = "Shared repository '%(display_url)s' already creates " \
> + "working trees."
We generally use parentheses to split lines, e.g.:
_fmt = ("Shared repository '%(display_url)s' already creates"
" working trees.")
> === modified file 'bzrlib/reconfigure.py'
> --- bzrlib/reconfigure.py 2008-04-24 04:58:42 +0000
> +++ bzrlib/reconfigure.py 2009-02-06 01:47:40 +0000
> @@ -140,6 +140,21 @@
> raise errors.AlreadyStandalone(bzrdir)
> return reconfiguration
>
> + @classmethod
> + def set_repository_trees(klass, bzrdir, with_trees):
> + """Adjust a repository's working tree presence default"""
> + reconfiguration = klass(bzrdir)
> + if not reconfiguration.repository.is_shared():
> + raise errors.ReconfigurationNotSupported(reconfiguration.bzrdir)
> + if with_trees and reconfiguration.repository.make_working_trees():
> + raise errors.AlreadyWithTrees(bzrdir)
> + elif (not with_trees and
> + not reconfiguration.repository.make_working_trees()):
> + raise errors.AlreadyWithNoTrees(bzrdir)
> + else:
> + reconfiguration.repository.set_make_working_trees(with_trees)
> + return reconfiguration
See above.
> === modified file 'bzrlib/tests/test_reconfigure.py'
> --- bzrlib/tests/test_reconfigure.py 2008-04-25 22:16:00 +0000
> +++ bzrlib/tests/test_reconfigure.py 2009-01-08 08:13:29 +0000
> @@ -377,3 +377,33 @@
> def test_unsynced_branch_to_lightweight_checkout_forced(self):
> reconfiguration = self.make_unsynced_branch_reconfiguration()
> reconfiguration.apply(force=True)
> +
> + def make_repository_with_without_trees(self, with_trees):
> + repo = self.make_repository('repo', shared=True)
> + repo.set_make_working_trees(with_trees)
> + return repo
> +
> + def test_make_with_trees(self):
> + repo = self.make_repository_with_without_trees(False)
> + reconfiguration = reconfigure.Reconfigure.set_repository_trees(
> + repo.bzrdir, True)
> + reconfiguration.apply()
> + self.assertIs(True, repo.make_working_trees())
> +
> + def test_make_without_trees(self):
> + repo = self.make_repository_with_without_trees(True)
> + reconfiguration = reconfigure.Reconfigure.set_repository_trees(
> + repo.bzrdir, False)
> + reconfiguration.apply()
> + self.assertIs(False, repo.make_working_trees())
> +
> + def test_make_with_trees_already_with_trees(self):
> + repo = self.make_repository_with_without_trees(True)
> + self.assertRaises(errors.AlreadyWithTrees,
> + reconfigure.Reconfigure.set_repository_trees, repo.bzrdir, True)
You should also test the text of the error, like so:
e = self.assertRaises(errors.AlreadyWithTrees,
reconfigure.Reconfigure.set_repository_trees, repo.bzrdir, True)
self.assertEqual('Already has trees!', str(e))
> +
> + def test_make_without_trees_already_no_trees(self):
> + repo = self.make_repository_with_without_trees(False)
> + self.assertRaises(errors.AlreadyWithNoTrees,
> + reconfigure.Reconfigure.set_repository_trees, repo.bzrdir, False)
> +
You also need to test the case where errors.ReconfigurationNotSupported
is raised.
Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iEYEARECAAYFAkmQgFwACgkQ0F+nu1YWqI26nQCfemOBq5vBt+Gts5r/cfoS1o2p
iF8AnRm6hZ/jd2OzTYEakWJrMfFzaSe7
=i+DO
-----END PGP SIGNATURE-----
More information about the bazaar
mailing list