[MERGE] Allow changing --[no-]trees with reconfigure (bug 145033)
Marius Kruger
amanic at gmail.com
Sun Feb 15 23:58:25 GMT 2009
2009/2/9 Aaron Bentley <aaron at aaronbentley.com>
> 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.
ok. Because Reconfigure gives other advantages like checking if we
have a repository present, I chose to keep on using it.
So I moved the actual change to apply() again.
> > > 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.
>
done.
> > @@ -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.
fair enough. sorry. done: using 'repo' now.
> > # 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
eish <\me scratches head>. `bzr send` seems to be a black art
with this public_branch location that I just don't seem to get right :(
It seems to work great, but not being able to set it directly
with `reconfigure` or something makes it a bit obscure.
But that is another story.
fixed now. thanks.
> === 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.
we now do the actual change in apply() again.
> > === 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.")
done.
> > === 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))
done for both tests (with and without).
> > +
> > + 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.
done.
thanks a lot for the review and spoon feeding me the fixes:)
--
<| regards
U| Marius
H| <><
-------------- next part --------------
An HTML attachment was scrubbed...
URL: https://lists.ubuntu.com/archives/bazaar/attachments/20090216/c8d79728/attachment-0001.htm
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 20090216_0150-reconfig-trees.patch
Type: text/x-diff
Size: 21682 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20090216/c8d79728/attachment-0001.bin
More information about the bazaar
mailing list