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
1=== modified file 'NEWS'
2--- NEWS 2009-12-14 09:31:20 +0000
3+++ NEWS 2009-12-14 14:46:21 +0000
4@@ -105,6 +105,10 @@
5 branch, not just from the root of the branch.
6 (Neil Martinsen-Burrell, #489102)
7
8+* ``bzr switch -b`` can now create branches that are located using directory
9+ services such as ``lp:``, even when the branch name doesn't contain a
10+ '/'. (Neil Martinsen-Burrell, #495263)
11+
12
13 Improvements
14 ************
15
16=== modified file 'bzrlib/builtins.py'
17--- bzrlib/builtins.py 2009-12-11 05:48:21 +0000
18+++ bzrlib/builtins.py 2009-12-14 14:46:21 +0000
19@@ -31,6 +31,7 @@
20 bundle,
21 btree_index,
22 bzrdir,
23+ directory_service,
24 delta,
25 config,
26 errors,
27@@ -5481,6 +5482,8 @@
28 if branch is None:
29 raise errors.BzrCommandError('cannot create branch without'
30 ' source branch')
31+ to_location = directory_service.directories.dereference(
32+ to_location)
33 if '/' not in to_location and '\\' not in to_location:
34 # This path is meant to be relative to the existing branch
35 this_url = self._get_branch_location(control_dir)
36
37=== modified file 'bzrlib/tests/blackbox/test_switch.py'
38--- bzrlib/tests/blackbox/test_switch.py 2009-07-09 15:18:57 +0000
39+++ bzrlib/tests/blackbox/test_switch.py 2009-12-14 14:46:21 +0000
40@@ -22,6 +22,7 @@
41
42 from bzrlib.workingtree import WorkingTree
43 from bzrlib.tests.blackbox import ExternalBase
44+from bzrlib.directory_service import directories
45
46
47 class TestSwitch(ExternalBase):
48@@ -183,3 +184,16 @@
49 # The new branch should have been created at the same level as
50 # 'branch', because we did not have a '/' segment
51 self.assertEqual(branch.base[:-1] + '2/', tree.branch.base)
52+
53+ def test_create_branch_directory_services(self):
54+ branch = self.make_branch('branch')
55+ tree = branch.create_checkout('tree', lightweight=True)
56+ class FooLookup(object):
57+ def look_up(self, name, url):
58+ return 'foo-'+name
59+ directories.register('foo:', FooLookup, 'Create branches named foo-')
60+ self.addCleanup(directories.remove, 'foo:')
61+ self.run_bzr('switch -b foo:branch2', working_dir='tree')
62+ tree = WorkingTree.open('tree')
63+ self.assertEndsWith(tree.branch.base, 'foo-branch2/')
64+