Merge lp:~parthm/bzr/138600-2.1-mkdir-should-fail-on-invalid-parent into lp:bzr/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
=== modified file 'NEWS'
--- NEWS 2010-02-19 04:54:24 +0000
+++ NEWS 2010-02-26 05:37:13 +0000
@@ -23,6 +23,9 @@
2323
24* Merge correctly when this_tree is not a WorkingTree. (Aaron Bentley)24* Merge correctly when this_tree is not a WorkingTree. (Aaron Bentley)
2525
26* ``bzr mkdir DIR`` will not create DIR unless DIR's parent is a versioned
27 directory. (Parth Malwankar, #138600)
28
26Documentation29Documentation
27*************30*************
2831
2932
=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py 2010-02-17 17:11:16 +0000
+++ bzrlib/builtins.py 2010-02-26 05:37:13 +0000
@@ -677,10 +677,15 @@
677677
678 def run(self, dir_list):678 def run(self, dir_list):
679 for d in dir_list:679 for d in dir_list:
680 os.mkdir(d)
681 wt, dd = WorkingTree.open_containing(d)680 wt, dd = WorkingTree.open_containing(d)
682 wt.add([dd])681 base = os.path.dirname(dd)
683 self.outf.write('added %s\n' % d)682 id = wt.path2id(base)
683 if id != None:
684 os.mkdir(d)
685 wt.add([dd])
686 self.outf.write('added %s\n' % d)
687 else:
688 raise errors.NotVersionedError(path=base)
684689
685690
686class cmd_relpath(Command):691class cmd_relpath(Command):
687692
=== modified file 'bzrlib/tests/blackbox/test_versioning.py'
--- bzrlib/tests/blackbox/test_versioning.py 2009-08-28 05:00:33 +0000
+++ bzrlib/tests/blackbox/test_versioning.py 2010-02-26 05:37:13 +0000
@@ -29,16 +29,64 @@
29from bzrlib.workingtree import WorkingTree29from bzrlib.workingtree import WorkingTree
3030
3131
32class TestMkdir(TestCaseWithTransport):
33
34 def test_mkdir_in_repo(self):
35 """'mkdir' should fail cleanly withing a repo. Bug #138600"""
36 shared_repo = self.make_repository('./foobar')
37 self.run_bzr(['mkdir', 'foobar/abc'], retcode=3)
38 self.failIfExists('foobar/abc')
39
40 def test_mkdir_outside_unversioned_dir(self):
41 """'mkdir' should fail with unversioned directory in path. Bug #138600"""
42 dir = pathjoin('.', 'unversioned_dir0')
43 newdir = pathjoin(dir, 'abc')
44 os.mkdir(dir)
45 self.run_bzr(['mkdir', newdir], retcode=3)
46 self.failIfExists(newdir)
47
48 def test_mkdir_inside_unversioned_dir(self):
49 """'mkdir' should fail cleanly from within an unversioned directory. Bug #138600"""
50 dir = pathjoin('.', 'unversioned_dir1')
51 os.mkdir(dir)
52 self.run_bzr(['mkdir', 'abc'], working_dir=dir, retcode=3)
53 newdir = pathjoin(dir, 'abc')
54 self.failIfExists(newdir)
55
56 def test_mkdir_outside_unversioned_dir_within_branch(self):
57 """'mkdir' should fail with unversioned directory inside branch. Bug #138600"""
58 b = 'foobar0'
59 self.make_branch_and_tree(b)
60 os.chdir(b)
61 dir = pathjoin('.', 'unversioned_dir0')
62 newdir = pathjoin(dir, 'abc')
63 os.mkdir(dir)
64 self.run_bzr(['mkdir', newdir], retcode=3)
65 self.failIfExists(newdir)
66
67 def test_mkdir_inside_unversioned_dir_within_branch(self):
68 """'mkdir' should fail cleanly from within an unversioned directory
69 inside branch. Bug #138600"""
70 b = 'foobar1'
71 self.make_branch_and_tree(b)
72 os.chdir(b)
73 dir = pathjoin('.', 'unversioned_dir1')
74 os.mkdir(dir)
75 self.run_bzr(['mkdir', 'abc'], working_dir=dir, retcode=3)
76 newdir = pathjoin(dir, 'abc')
77 self.failIfExists(newdir)
78
79
32class TestVersioning(TestCaseInTempDir):80class TestVersioning(TestCaseInTempDir):
3381
34 def test_mkdir(self):82 def test_mkdir(self):
35 """Basic 'bzr mkdir' operation"""83 """Basic 'bzr mkdir' operation"""
3684
37 self.run_bzr('init')85 self.run_bzr('init')
38 self.run_bzr('mkdir foo')86 self.run_bzr(['mkdir', 'foo'])
39 self.assert_(os.path.isdir('foo'))87 self.assert_(os.path.isdir('foo'))
4088
41 self.run_bzr('mkdir foo', retcode=3)89 self.run_bzr(['mkdir', 'foo'], retcode=3)
4290
43 wt = WorkingTree.open('.')91 wt = WorkingTree.open('.')
4492
@@ -54,12 +102,12 @@
54 """'bzr mkdir' operation in subdirectory"""102 """'bzr mkdir' operation in subdirectory"""
55103
56 self.run_bzr('init')104 self.run_bzr('init')
57 self.run_bzr('mkdir dir')105 self.run_bzr(['mkdir', 'dir'])
58 self.assert_(os.path.isdir('dir'))106 self.assert_(os.path.isdir('dir'))
59107
60 os.chdir('dir')108 os.chdir('dir')
61 self.log('Run mkdir in subdir')109 self.log('Run mkdir in subdir')
62 self.run_bzr('mkdir subdir')110 self.run_bzr(['mkdir', 'subdir'])
63 self.assert_(os.path.isdir('subdir'))111 self.assert_(os.path.isdir('subdir'))
64 os.chdir('..')112 os.chdir('..')
65113
@@ -86,7 +134,7 @@
86 self.run_bzr('init')134 self.run_bzr('init')
87 os.chdir('../..')135 os.chdir('../..')
88136
89 self.run_bzr('mkdir dir a/dir a/b/dir')137 self.run_bzr(['mkdir', 'dir', 'a/dir', 'a/b/dir'])
90 self.failUnless(os.path.isdir('dir'))138 self.failUnless(os.path.isdir('dir'))
91 self.failUnless(os.path.isdir('a/dir'))139 self.failUnless(os.path.isdir('a/dir'))
92 self.failUnless(os.path.isdir('a/b/dir'))140 self.failUnless(os.path.isdir('a/b/dir'))

Subscribers

People subscribed via source and target branches