I'd like this in 1.12, so I gave it a go.<br><br><div class="gmail_quote">2009/1/19 Aaron Bentley <span dir="ltr"><<a href="mailto:aaron@aaronbentley.com">aaron@aaronbentley.com</a>></span><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="Ih2E3d">
Matthew D. Fuller wrote:<br>
</div>></blockquote><div>... <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Actually, considering the change is just<br>
Repository.set_make_working_trees, it might be simpler to just invoke it<br>
directly.</blockquote><div><br>done.<br><br>
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div class="Ih2E3d"><br>
> I did add a test that --with-no-trees doesn't screw with an existing<br>
> working tree (the rewrite should make that impossible anyway). I'm<br>
> not real happy with the test; it sorta takes a long stroll around the<br>
> garden to check if a door it just opened is open. There has to be a<br>
> less absurd way of writing it, but that's far past my competence.<br>
<br>
</div>Well, you can skip some of the blackbox tests. The blackbox tests<br>
should prove that it works at all, not duplicate the unit tests.</blockquote><div><br>I left them in, as they were already done, and they do add some value:<br>they test that we support the flags and that the user gets good error messages.<br>
<br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
> === modified file 'bzrlib/reconfigure.py'<br>
> --- bzrlib/reconfigure.py 2008-04-24 04:58:42 +0000<br>
> +++ bzrlib/reconfigure.py 2009-01-19 08:48:21 +0000<br>
> @@ -24,9 +24,10 @@<br>
><br>
> class Reconfigure(object):<br>
><br>
> - def __init__(self, bzrdir, new_bound_location=None):<br>
> + def __init__(self, bzrdir, new_bound_location=None, with_trees=None):<br>
> self.bzrdir = bzrdir<br>
> self.new_bound_location = new_bound_location<br>
> + self.with_trees = with_trees<br>
<br>
What is the purpose of this variable? AFAICT, it is only used in<br>
set_repository_trees, and it is set there also.</blockquote><div><br>removed.<br> <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
> try:<br>
> self.repository = self.bzrdir.find_repository()<br>
> except errors.NoRepositoryPresent:<br>
> @@ -62,6 +63,8 @@<br>
> self._create_tree = False<br>
> self._create_repository = False<br>
> self._destroy_repository = False<br>
> + self._set_with_trees = False<br>
> + self._set_with_no_trees = False<br>
><br>
> @staticmethod<br>
> def to_branch(bzrdir):<br>
> @@ -140,6 +143,25 @@<br>
> raise errors.AlreadyStandalone(bzrdir)<br>
> return reconfiguration<br>
><br>
> + @classmethod<br>
> + def set_repository_trees(klass, bzrdir, with_trees):<br>
> + """Adjust a repository's working tree presense default"""<br>
> + reconfiguration = klass(bzrdir, with_trees=with_trees)<br>
<br>
You don't need to pass with_trees in-- just use with_trees instead of<br>
reconfiguration.with_trees.</blockquote><div><br>done<br> <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
> + if not reconfiguration.repository.is_shared():<br>
> + raise errors.ReconfigurationNotSupported(reconfiguration.bzrdir)<br>
> + if reconfiguration.with_trees is True and \<br>
<br>
Please use a boolean test like<br>
<br>
if reconfiguration.with_trees</blockquote><div><br>done<br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">> + not reconfiguration.repository.make_working_trees():<br>
> + reconfiguration._set_with_trees = True<br>
> + elif reconfiguration.with_trees is False and \<br>
<br>
Again, use elif not reconfiguration.with_trees</blockquote><div><br>done<br> <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
> + reconfiguration.repository.make_working_trees():<br>
> + reconfiguration._set_with_no_trees = True<br>
<br>
Except, why not just use reconfiguration.with_trees directly in<br>
Reconfiguration.apply?</blockquote><div></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
i.e.<br>
<br>
if self.with_trees is not None:<br>
if self.repository.set_make_working_trees(self.with_trees)</blockquote><div><br>changes are now done in set_repository_trees() directly.<br> <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
> @@ -302,3 +325,7 @@<br>
> local_branch.bind(branch.Branch.open(bind_location))<br>
> if self._destroy_repository:<br>
> self.bzrdir.destroy_repository()<br>
> + if self._set_with_trees:<br>
<div class="Ih2E3d">> + self.repository.set_make_working_trees(True)<br>
> + if self._set_with_no_trees:<br>
> + self.repository.set_make_working_trees(False)<br>
<br>
</div>self.repository refers to the current repository, but won't refer to a<br>
newly-created repository. You'll want to use the local variable 'repo'<br>
instead.</blockquote><div><br>AFAICT, newly-created repositories are always shared=False, which is <br>something the new actions claim not to support. <br>So since changes are now done in set_repository_trees() directly,<br>
we will ignore newly-created repositories, which is fine IMO, <br>as I can't see us getting into such a situation from the commandline.<br><br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
> + def test_make_without_trees_leaves_tree_alone(self):<br>
> + # There has to be a better way of doing this...<br>
> + repo = self.make_repository('repo', shared=True)<br>
> + branch = self.make_branch('repo/branch')<br>
> + self.run_bzr('reconfigure --use-shared',<br>
> + working_dir='repo/branch')<br>
> + tree = branch.bzrdir.create_workingtree()<br>
<br>
Use BzrDir.create_branch_convenience instead of creating the branch and<br>
tree and then making it use the shared repository.</blockquote><div><br>done, <br>well I created the repo first and then create_branch_convenience would put it in the repo.<br><br> <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
> + self.build_tree(['repo/branch/foo'])<br>
> + tree.add('foo')<br>
> + tree.commit('x')<br>
<br>
You don't have to commit-- you can just supply --force</blockquote><div><br>done<br> <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
> + self.run_bzr('reconfigure --with-no-trees',<br>
> + working_dir='repo/branch')<br>
> + self.failUnlessExists('repo/branch/foo')<br>
<br>
But this whole thing should be a unit test so that you know that<br>
workingtree.WorkingTree.open works at the end.</blockquote><div> </div><div>I do WorkingTree.open at the end now too, would that satisfy you?<br> </div></div><br><| regards<br>U| Marius<br>H| <>< <br>