Bazaar

Merge lp:~parthm/bzr/138600-2.1-mkdir-should-fail-on-invalid-parent into lp:bzr/2.1

  1. 138600-2.1-mkdir-should-fail-on-invalid-parent
  2. Merge into 2.1
Proposed by Parth Malwankar
Status: Rejected
Rejected by: Vincent Ladeuil
Proposed branch: lp:~parthm/bzr/138600-2.1-mkdir-should-fail-on-invalid-parent
Merge into: lp:bzr/2.1
Diff against target: 131 lines (+64/-8)
3 files modified
NEWS (+3/-0)
bzrlib/builtins.py (+8/-3)
bzrlib/tests/blackbox/test_versioning.py (+53/-5)
To merge this branch: bzr merge lp:~parthm/bzr/138600-2.1-mkdir-should-fail-on-invalid-parent
Reviewer Review Type Date Requested Status
Vincent Ladeuil Needs Information
Andrew Bennetts Approve
Review via email: mp+20199@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Parth Malwankar (parthm) wrote :

=== Bug #138600 ===

This is a backport of:
https://code.launchpad.net/~parthm/bzr/138600/+merge/19471

If 'bzr mkdir DIR' fails due to the parent of DIR not being versioned, DIR is not created.

Revision history for this message
Andrew Bennetts (spiv) wrote :

Looks good to me, just like the original :)

It's a low risk change, so seems reasonable for 2.1. On that basis, it's probably just as appropriate for 2.0 too.

One trivial point: to reduce unnecessary conflicts, NEWS entries are supposed to be in alphabetical order, and in practice we have been ignoring punctuation and case when doing that sorting. So "``bzr" would sort before "Merge", not after. It doesn't matter very much.

review: Approve
Revision history for this message
Vincent Ladeuil (vila) wrote :

I'm a bit hesitant to backport against 2.0 and 2.1...

While low risk it's has also been rarely encountered (apparently) and
seems to contradict our policy of minimal changes for stable releases.

Can someone nudge me one way or another ?

review: Needs Information
Revision history for this message
Vincent Ladeuil (vila) wrote :

@Parthm, sorry for the trouble, but I think we should keep the backports
strictly focused to simplify the SRU process.

So I'll mark this mp as rejected on this basis.

Unmerged revisions

4818. By Parth Malwankar

backport of #138600 fix. This prevents mkdir from creating a dir
if parent is not versioned.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2010-02-19 04:54:24 +0000
3+++ NEWS 2010-02-26 05:37:13 +0000
4@@ -23,6 +23,9 @@
5
6 * Merge correctly when this_tree is not a WorkingTree. (Aaron Bentley)
7
8+* ``bzr mkdir DIR`` will not create DIR unless DIR's parent is a versioned
9+ directory. (Parth Malwankar, #138600)
10+
11 Documentation
12 *************
13
14
15=== modified file 'bzrlib/builtins.py'
16--- bzrlib/builtins.py 2010-02-17 17:11:16 +0000
17+++ bzrlib/builtins.py 2010-02-26 05:37:13 +0000
18@@ -677,10 +677,15 @@
19
20 def run(self, dir_list):
21 for d in dir_list:
22- os.mkdir(d)
23 wt, dd = WorkingTree.open_containing(d)
24- wt.add([dd])
25- self.outf.write('added %s\n' % d)
26+ base = os.path.dirname(dd)
27+ id = wt.path2id(base)
28+ if id != None:
29+ os.mkdir(d)
30+ wt.add([dd])
31+ self.outf.write('added %s\n' % d)
32+ else:
33+ raise errors.NotVersionedError(path=base)
34
35
36 class cmd_relpath(Command):
37
38=== modified file 'bzrlib/tests/blackbox/test_versioning.py'
39--- bzrlib/tests/blackbox/test_versioning.py 2009-08-28 05:00:33 +0000
40+++ bzrlib/tests/blackbox/test_versioning.py 2010-02-26 05:37:13 +0000
41@@ -29,16 +29,64 @@
42 from bzrlib.workingtree import WorkingTree
43
44
45+class TestMkdir(TestCaseWithTransport):
46+
47+ def test_mkdir_in_repo(self):
48+ """'mkdir' should fail cleanly withing a repo. Bug #138600"""
49+ shared_repo = self.make_repository('./foobar')
50+ self.run_bzr(['mkdir', 'foobar/abc'], retcode=3)
51+ self.failIfExists('foobar/abc')
52+
53+ def test_mkdir_outside_unversioned_dir(self):
54+ """'mkdir' should fail with unversioned directory in path. Bug #138600"""
55+ dir = pathjoin('.', 'unversioned_dir0')
56+ newdir = pathjoin(dir, 'abc')
57+ os.mkdir(dir)
58+ self.run_bzr(['mkdir', newdir], retcode=3)
59+ self.failIfExists(newdir)
60+
61+ def test_mkdir_inside_unversioned_dir(self):
62+ """'mkdir' should fail cleanly from within an unversioned directory. Bug #138600"""
63+ dir = pathjoin('.', 'unversioned_dir1')
64+ os.mkdir(dir)
65+ self.run_bzr(['mkdir', 'abc'], working_dir=dir, retcode=3)
66+ newdir = pathjoin(dir, 'abc')
67+ self.failIfExists(newdir)
68+
69+ def test_mkdir_outside_unversioned_dir_within_branch(self):
70+ """'mkdir' should fail with unversioned directory inside branch. Bug #138600"""
71+ b = 'foobar0'
72+ self.make_branch_and_tree(b)
73+ os.chdir(b)
74+ dir = pathjoin('.', 'unversioned_dir0')
75+ newdir = pathjoin(dir, 'abc')
76+ os.mkdir(dir)
77+ self.run_bzr(['mkdir', newdir], retcode=3)
78+ self.failIfExists(newdir)
79+
80+ def test_mkdir_inside_unversioned_dir_within_branch(self):
81+ """'mkdir' should fail cleanly from within an unversioned directory
82+ inside branch. Bug #138600"""
83+ b = 'foobar1'
84+ self.make_branch_and_tree(b)
85+ os.chdir(b)
86+ dir = pathjoin('.', 'unversioned_dir1')
87+ os.mkdir(dir)
88+ self.run_bzr(['mkdir', 'abc'], working_dir=dir, retcode=3)
89+ newdir = pathjoin(dir, 'abc')
90+ self.failIfExists(newdir)
91+
92+
93 class TestVersioning(TestCaseInTempDir):
94
95 def test_mkdir(self):
96 """Basic 'bzr mkdir' operation"""
97
98 self.run_bzr('init')
99- self.run_bzr('mkdir foo')
100+ self.run_bzr(['mkdir', 'foo'])
101 self.assert_(os.path.isdir('foo'))
102
103- self.run_bzr('mkdir foo', retcode=3)
104+ self.run_bzr(['mkdir', 'foo'], retcode=3)
105
106 wt = WorkingTree.open('.')
107
108@@ -54,12 +102,12 @@
109 """'bzr mkdir' operation in subdirectory"""
110
111 self.run_bzr('init')
112- self.run_bzr('mkdir dir')
113+ self.run_bzr(['mkdir', 'dir'])
114 self.assert_(os.path.isdir('dir'))
115
116 os.chdir('dir')
117 self.log('Run mkdir in subdir')
118- self.run_bzr('mkdir subdir')
119+ self.run_bzr(['mkdir', 'subdir'])
120 self.assert_(os.path.isdir('subdir'))
121 os.chdir('..')
122
123@@ -86,7 +134,7 @@
124 self.run_bzr('init')
125 os.chdir('../..')
126
127- self.run_bzr('mkdir dir a/dir a/b/dir')
128+ self.run_bzr(['mkdir', 'dir', 'a/dir', 'a/b/dir'])
129 self.failUnless(os.path.isdir('dir'))
130 self.failUnless(os.path.isdir('a/dir'))
131 self.failUnless(os.path.isdir('a/b/dir'))

Subscribers

People subscribed via source and target branches