Bazaar

Merge lp:~nmb/bzr/switch-uses-directories into lp:bzr

  1. switch-uses-directories
  2. Merge into bzr.dev
Proposed by Neil Martinsen-Burrell
Status: Merged
Merged at revision: not available
Proposed branch: lp:~nmb/bzr/switch-uses-directories
Merge into: lp:bzr
Diff against target: 64 lines (+21/-0)
3 files modified
NEWS (+4/-0)
bzrlib/builtins.py (+3/-0)
bzrlib/tests/blackbox/test_switch.py (+14/-0)
To merge this branch: bzr merge lp:~nmb/bzr/switch-uses-directories
Reviewer Review Type Date Requested Status
Martin Pool Needs Fixing
John A Meinel Needs Fixing
Review via email: mp+16024@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Neil Martinsen-Burrell (nmb) wrote :

This fixes the bug that switch won't use directory services if the name doesn't contain a '/'. There is a new test that fails for the existing bzr.dev and passes with the change. The rest of the switch tests pass with this change as well.

Revision history for this message
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Neil Martinsen-Burrell wrote:
> Neil Martinsen-Burrell has proposed merging lp:~nmb/bzr/switch-uses-directories into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
> Related bugs:
> #495263 switch -b doesn't respect directory services
> https://bugs.launchpad.net/bugs/495263
>
>
> This fixes the bug that switch won't use directory services if the name doesn't contain a '/'. There is a new test that fails for the existing bzr.dev and passes with the change. The rest of the switch tests pass with this change as well.
>

As long as the existing functionality is preserved (which seems to be
the case), the change seems fine to me.

However this:
+ directories.register('foo:', FooLookup, 'Create branches named
foo-')

Looks like it leaves a permanent entry in the directories registry. So
I'm thinking you need to self.addCleanup() some function to remove that
value. Something like:

self.addCleanup(directories.remove, 'foo:')

 review: needs_fixing

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAksia2MACgkQJdeBCYSNAAO8KgCcCylUrYT+lRDwgQezuwOgLgky
fDgAoIZTZZbrcC+cflBAFBl6w2JnVwhC
=QBAH
-----END PGP SIGNATURE-----

review: Needs Fixing
Revision history for this message
Neil Martinsen-Burrell (nmb) wrote :

Fixed in the branch.

Revision history for this message
Martin Pool (mbp) wrote :

Thanks, that looks good.

The news entry confuses me because switch is primarily used to change to a branch, not to create one. So if this applies only to switch -b you should say so - on the other hand if it may affect plain switching and maybe you should add a test?

review: Needs Fixing
Revision history for this message
Neil Martinsen-Burrell (nmb) wrote :

On 2009-12-13 22:40 , Martin Pool wrote:
> Review: Needs Fixing Thanks, that looks good.
>
> The news entry confuses me because switch is primarily used to change
> to a branch, not to create one. So if this applies only to switch -b
> you should say so - on the other hand if it may affect plain
> switching and maybe you should add a test?

The problem only exists in the creating code path. I'll change the NEWS
to reflect the dependence on -b. Peace,

-Neil

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2009-12-14 09:31:20 +0000
+++ NEWS 2009-12-14 14:46:21 +0000
@@ -105,6 +105,10 @@
105 branch, not just from the root of the branch. 105 branch, not just from the root of the branch.
106 (Neil Martinsen-Burrell, #489102)106 (Neil Martinsen-Burrell, #489102)
107107
108* ``bzr switch -b`` can now create branches that are located using directory
109 services such as ``lp:``, even when the branch name doesn't contain a
110 '/'. (Neil Martinsen-Burrell, #495263)
111
108112
109Improvements113Improvements
110************114************
111115
=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py 2009-12-11 05:48:21 +0000
+++ bzrlib/builtins.py 2009-12-14 14:46:21 +0000
@@ -31,6 +31,7 @@
31 bundle,31 bundle,
32 btree_index,32 btree_index,
33 bzrdir,33 bzrdir,
34 directory_service,
34 delta,35 delta,
35 config,36 config,
36 errors,37 errors,
@@ -5481,6 +5482,8 @@
5481 if branch is None:5482 if branch is None:
5482 raise errors.BzrCommandError('cannot create branch without'5483 raise errors.BzrCommandError('cannot create branch without'
5483 ' source branch')5484 ' source branch')
5485 to_location = directory_service.directories.dereference(
5486 to_location)
5484 if '/' not in to_location and '\\' not in to_location:5487 if '/' not in to_location and '\\' not in to_location:
5485 # This path is meant to be relative to the existing branch5488 # This path is meant to be relative to the existing branch
5486 this_url = self._get_branch_location(control_dir)5489 this_url = self._get_branch_location(control_dir)
54875490
=== modified file 'bzrlib/tests/blackbox/test_switch.py'
--- bzrlib/tests/blackbox/test_switch.py 2009-07-09 15:18:57 +0000
+++ bzrlib/tests/blackbox/test_switch.py 2009-12-14 14:46:21 +0000
@@ -22,6 +22,7 @@
2222
23from bzrlib.workingtree import WorkingTree23from bzrlib.workingtree import WorkingTree
24from bzrlib.tests.blackbox import ExternalBase24from bzrlib.tests.blackbox import ExternalBase
25from bzrlib.directory_service import directories
2526
2627
27class TestSwitch(ExternalBase):28class TestSwitch(ExternalBase):
@@ -183,3 +184,16 @@
183 # The new branch should have been created at the same level as184 # The new branch should have been created at the same level as
184 # 'branch', because we did not have a '/' segment185 # 'branch', because we did not have a '/' segment
185 self.assertEqual(branch.base[:-1] + '2/', tree.branch.base)186 self.assertEqual(branch.base[:-1] + '2/', tree.branch.base)
187
188 def test_create_branch_directory_services(self):
189 branch = self.make_branch('branch')
190 tree = branch.create_checkout('tree', lightweight=True)
191 class FooLookup(object):
192 def look_up(self, name, url):
193 return 'foo-'+name
194 directories.register('foo:', FooLookup, 'Create branches named foo-')
195 self.addCleanup(directories.remove, 'foo:')
196 self.run_bzr('switch -b foo:branch2', working_dir='tree')
197 tree = WorkingTree.open('tree')
198 self.assertEndsWith(tree.branch.base, 'foo-branch2/')
199