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

[Parth Malwankar]

[Parth Malwankar (parthm)]

=== 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.

[Andrew Bennetts (spiv)]

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

[Vincent Ladeuil (vila)]

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

[Vincent Ladeuil (vila)]

@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 on 2010-02-26

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

=== modified file 'NEWS'
--- NEWS	2010-02-19 04:54:24 +0000
+++ NEWS	2010-02-26 05:37:13 +0000
@@ -23,6 +23,9 @@
 
 * Merge correctly when this_tree is not a WorkingTree.  (Aaron Bentley)
 
+* ``bzr mkdir DIR`` will not create DIR unless DIR's parent is a versioned
+   directory. (Parth Malwankar, #138600)
+
 Documentation
 *************
 

=== 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 @@
 
     def run(self, dir_list):
         for d in dir_list:
-            os.mkdir(d)
             wt, dd = WorkingTree.open_containing(d)
-            wt.add([dd])
-            self.outf.write('added %s\n' % d)
+            base = os.path.dirname(dd)
+            id = wt.path2id(base)
+            if id != None:
+                os.mkdir(d)
+                wt.add([dd])
+                self.outf.write('added %s\n' % d)
+            else:
+                raise errors.NotVersionedError(path=base)
 
 
 class cmd_relpath(Command):

=== 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 @@
 from bzrlib.workingtree import WorkingTree
 
 
+class TestMkdir(TestCaseWithTransport):
+
+    def test_mkdir_in_repo(self):
+        """'mkdir' should fail cleanly withing a repo. Bug #138600"""
+        shared_repo = self.make_repository('./foobar')
+        self.run_bzr(['mkdir', 'foobar/abc'], retcode=3)
+        self.failIfExists('foobar/abc')
+
+    def test_mkdir_outside_unversioned_dir(self):
+        """'mkdir' should fail with unversioned directory in path. Bug #138600"""
+        dir = pathjoin('.', 'unversioned_dir0')
+        newdir = pathjoin(dir, 'abc')
+        os.mkdir(dir)
+        self.run_bzr(['mkdir', newdir], retcode=3)
+        self.failIfExists(newdir)
+
+    def test_mkdir_inside_unversioned_dir(self):
+        """'mkdir' should fail cleanly from within an unversioned directory. Bug #138600"""
+        dir = pathjoin('.', 'unversioned_dir1')
+        os.mkdir(dir)
+        self.run_bzr(['mkdir', 'abc'], working_dir=dir, retcode=3)
+        newdir = pathjoin(dir, 'abc')
+        self.failIfExists(newdir)
+
+    def test_mkdir_outside_unversioned_dir_within_branch(self):
+        """'mkdir' should fail with unversioned directory inside branch. Bug #138600"""
+        b = 'foobar0'
+        self.make_branch_and_tree(b)
+        os.chdir(b)
+        dir = pathjoin('.', 'unversioned_dir0')
+        newdir = pathjoin(dir, 'abc')
+        os.mkdir(dir)
+        self.run_bzr(['mkdir', newdir], retcode=3)
+        self.failIfExists(newdir)
+
+    def test_mkdir_inside_unversioned_dir_within_branch(self):
+        """'mkdir' should fail cleanly from within an unversioned directory
+        inside branch. Bug #138600"""
+        b = 'foobar1'
+        self.make_branch_and_tree(b)
+        os.chdir(b)
+        dir = pathjoin('.', 'unversioned_dir1')
+        os.mkdir(dir)
+        self.run_bzr(['mkdir', 'abc'], working_dir=dir, retcode=3)
+        newdir = pathjoin(dir, 'abc')
+        self.failIfExists(newdir)
+
+
 class TestVersioning(TestCaseInTempDir):
 
     def test_mkdir(self):
         """Basic 'bzr mkdir' operation"""
 
         self.run_bzr('init')
-        self.run_bzr('mkdir foo')
+        self.run_bzr(['mkdir', 'foo'])
         self.assert_(os.path.isdir('foo'))
 
-        self.run_bzr('mkdir foo', retcode=3)
+        self.run_bzr(['mkdir', 'foo'], retcode=3)
 
         wt = WorkingTree.open('.')
 
@@ -54,12 +102,12 @@
         """'bzr mkdir' operation in subdirectory"""
 
         self.run_bzr('init')
-        self.run_bzr('mkdir dir')
+        self.run_bzr(['mkdir', 'dir'])
         self.assert_(os.path.isdir('dir'))
 
         os.chdir('dir')
         self.log('Run mkdir in subdir')
-        self.run_bzr('mkdir subdir')
+        self.run_bzr(['mkdir', 'subdir'])
         self.assert_(os.path.isdir('subdir'))
         os.chdir('..')
 
@@ -86,7 +134,7 @@
         self.run_bzr('init')
         os.chdir('../..')
 
-        self.run_bzr('mkdir dir a/dir a/b/dir')
+        self.run_bzr(['mkdir', 'dir', 'a/dir', 'a/b/dir'])
         self.failUnless(os.path.isdir('dir'))
         self.failUnless(os.path.isdir('a/dir'))
         self.failUnless(os.path.isdir('a/b/dir'))