Bazaar

Merge lp:~mathepic/bzr/mkdir-recursive into lp:bzr

  1. mkdir-recursive
  2. Merge into bzr.dev
Proposed by Jared Hance
Status: Superseded
Proposed branch: lp:~mathepic/bzr/mkdir-recursive
Merge into: lp:bzr
Diff against target: 49 lines
2 files modified
NEWS (+4/-0)
bzrlib/builtins.py (+19/-3)
To merge this branch: bzr merge lp:~mathepic/bzr/mkdir-recursive
Reviewer Review Type Date Requested Status
Martin Pool Needs Fixing
Ian Clatworthy Needs Fixing
bzr-core Pending
Review via email: mp+13380@code.launchpad.net

This proposal has been superseded by a proposal from 2011-12-10.

To post a comment you must log in.
Revision history for this message
Jared Hance (mathepic) wrote :

This fixes bug #253529 (https://bugs.edge.launchpad.net/bzr/+bug/253529) by providing a -p (--parents) option for "bzr mkdir"

Revision history for this message
Jared Hance (mathepic) wrote :

Don't merge this yet, I need to make an alternative for smart_add since it adds the files in the directory, something I forgot about in the original implementation.

Revision history for this message
Jared Hance (mathepic) wrote :

> Don't merge this yet, I need to make an alternative for smart_add since it
> adds the files in the directory, something I forgot about in the original
> implementation.

Actually, I just checked and "bzr add" does not add files in the parent directories of the target directories. Since the target directory is always empty, it should not be a problem.

Revision history for this message
Ian Clatworthy (ian-clatworthy) wrote :

Thanks for the patch. Some minor things to fix ...

1. This is a boolean option so the parameter ought to be declared as "parent=False" instead of "parent=None".

2. It should also be tested as simply "if parents" instead of testing against None.

3. You'll need to tweak the help argument in the Option declaration so that it starts with a capital letter and ends with a full stop. Otherwise, it won't merge because our style checker will fall over!

BTW, "bzr init" has a "create-prefix" option that does a very similar thing. I think consistency with posix mkdir (as you've done) makes more sense. Maybe we ought to change init down the track to be consistent with this? If so, it's outside the scope of this patch. I was just mentioning it for completeness.

review: Needs Fixing
Revision history for this message
Ian Clatworthy (ian-clatworthy) wrote :

Ian Clatworthy wrote:
> Review: Needs Fixing
> Thanks for the patch. Some minor things to fix ...
>
> 1. This is a boolean option so the parameter ought to be declared as "parent=False" instead of "parent=None".

Sorry. I meant "parents=False".

Ian C.

Revision history for this message
Jared Hance (mathepic) wrote :

Okay, fixed.

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

What do you mean by "No error if existing, "? If the target directory exists, os.makedirs will error. And because of the way you rely on smart_add, this is probably a good thing?

I will tweak that text and land it.

review: Approve
Revision history for this message
Martin Pool (mbp) wrote :
Download full text (3.6 KiB)

This seems to cause a failure like so

blackbox.test_versioning.TestVersioning.test_mkdir_in_subdir FAIL 54ms
   Unexpected return code
not equal:
a = 0
b = 3

======================================================================
FAIL: test_mkdir_in_subdir (bzrlib.tests.blackbox.test_versioning.TestVersioning)

vvvv[log from bzrlib.tests.blackbox.test_versioning.TestVersioning.test_mkdir_in_subdir]
329.240 run bzr: ['init']
329.240 bzr arguments: ['init']
329.243 encoding stdout as sys.stdout encoding 'UTF-8'
329.250 creating repository in file:///tmp/testbzr-oemsyS.tmp/bzrlib.tests.blackbox.test_versioning.TestVersioning.test_mkdir_in_subdir/work/.bzr/.
329.253 creating branch <bzrlib.branch.BzrBranchFormat7 object at 0x69da250> in file:///tmp/testbzr-oemsyS.tmp/bzrlib.tests.blackbox.test_versioning.TestVersioning.test_mkdir_in_subdir/work/.bzr/
329.263 trying to create missing lock '/tmp/testbzr-oemsyS.tmp/bzrlib.tests.blackbox.test_versioning.TestVersioning.test_mkdir_in_subdir/work/.bzr/checkout/dirstate'
329.263 opening working tree '/tmp/testbzr-oemsyS.tmp/bzrlib.tests.blackbox.test_versioning.TestVersioning.test_mkdir_in_subdir/work'
329.270 opening working tree '/tmp/testbzr-oemsyS.tmp/bzrlib.tests.blackbox.test_versioning.TestVersioning.test_mkdir_in_subdir/work'
329.272 output:
'Created a standalone tree (format: 2a)\n'
329.272 run bzr: ['mkdir', 'dir']
329.272 bzr arguments: ['mkdir', 'dir']
329.274 encoding stdout as sys.stdout encoding 'UTF-8'
329.278 opening working tree '/tmp/testbzr-oemsyS.tmp/bzrlib.tests.blackbox.test_versioning.TestVersioning.test_mkdir_in_subdir/work'
329.283 output:
'added dir\n'
329.283 Run mkdir in subdir
329.283 run bzr: ['mkdir', 'subdir']
329.283 bzr arguments: ['mkdir', 'subdir']
329.285 encoding stdout as sys.stdout encoding 'UTF-8'
329.289 opening working tree '/tmp/testbzr-oemsyS.tmp/bzrlib.tests.blackbox.test_versioning.TestVersioning.test_mkdir_in_subdir/work'
329.292 Traceback (most recent call last):
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/commands.py", line 1140, in run_bzr_catch_user_errors
   return run_bzr(argv)
make: *** [check-nodocs] Error 1
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/commands.py", line 1038, in run_bzr
   ret = run(*run_argv)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/commands.py", line 655, in run_argv_aliases
   return self.run(**all_cmd_args)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/builtins.py", line 706, in run
   wt.smart_add([dd])
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/mutabletree.py", line 49, in tree_write_locked
   return unbound(self, *args, **kwargs)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/mutabletree.py", line 403, in smart_add
   kind = osutils.file_kind(abspath)
 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/osutils.py", line 1860, in file_kind
   raise errors.NoSuchFile(f)
NoSuchFile: No such file: u'/tmp/testbzr-oemsyS.tmp/bzrlib.tests.blackbox.test_versioning.TestVersioning.test_mkdir_in_subdir/work/dir/dir/subdir'

329.293 errors:
"bzr: ERROR: No such file: u'/tmp/testbzr-oemsyS.tmp/bzrlib.tests.blackbox.test_versioning.TestVersioning.test_mkdir_in_s...

Read more...

review: Needs Fixing

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2009-10-23 16:31:03 +0000
+++ NEWS 2009-10-25 13:31:12 +0000
@@ -22,6 +22,10 @@
22Bug Fixes22Bug Fixes
23*********23*********
2424
25* "bzr mkdir" now includes -p (--parents) option for recursively adding
26 parent directories.
27 (Jared Hance, #253529)
28
25* ``bzr+http`` servers no longer give spurious jail break errors when29* ``bzr+http`` servers no longer give spurious jail break errors when
26 serving branches inside a shared repository. (Andrew Bennetts, #348308)30 serving branches inside a shared repository. (Andrew Bennetts, #348308)
2731
2832
=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py 2009-10-12 22:09:19 +0000
+++ bzrlib/builtins.py 2009-10-25 13:31:12 +0000
@@ -677,13 +677,29 @@
677 """677 """
678678
679 takes_args = ['dir+']679 takes_args = ['dir+']
680 takes_options = [
681 Option(
682 'parents',
683 help='No error if existing, make parent directories as needed.',
684 short_name='p'
685 )
686 ]
680 encoding_type = 'replace'687 encoding_type = 'replace'
681688
682 def run(self, dir_list):689 def run(self, dir_list, parents=False):
683 for d in dir_list:690 for d in dir_list:
684 os.mkdir(d)691 if parents:
692 os.mkdir(d)
693 else:
694 os.makedirs(d)
695
685 wt, dd = WorkingTree.open_containing(d)696 wt, dd = WorkingTree.open_containing(d)
686 wt.add([dd])697
698 # This should work since the target directory of "bzr mkdir"
699 # is always empty - Therefore, no files except the directory
700 # will be added, making this equivalent to recursively adding
701 # the directories, if needed.
702 wt.smart_add([dd])
687 self.outf.write('added %s\n' % d)703 self.outf.write('added %s\n' % d)
688704
689705