[MERGE] Handle a file turning in to a directory in TreeTransform.

Aaron Bentley aaron at aaronbentley.com
Thu Jul 17 16:49:16 BST 2008


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

James Westby wrote:
> When a file turns in to a directory in a TreeTransform and a child
> is added to this new directory stat is called on the child path.
> stat throws "ENOTDIR" as there is a file on disk that is named the
> same as one of the parents of the path it is given. This means
> that the file is not present, and so we should handle ENOTDIR
> in the same way as ENOENT. The attached patch does this.

That sounds fine.  Tests need work.

bb:resubmit


> +    def test_file_to_directory(self):
> +        wt = self.make_branch_and_tree('.')
> +        f = open('foo', 'wb')
> +        try:
> +            f.write("contents\n")
> +        finally:
> +            f.close() 

We generally use self.build_tree for this purpose.

self.build_tree(['foo']) will reduce the size of the test case by 5 lines.

> +        wt.add(['foo'])
> +        wt.commit("one")

There's no need to commit.


> +        tt = TreeTransform(wt)
> +        try:
> +            old_trans_id = tt.trans_id_tree_path("foo")
> +            tt.delete_contents(old_trans_id)
> +            tt.new_directory('foo', tt.root)
> +            new_trans_id = tt.trans_id_tree_path("foo/bar")
> +            tt.create_file(["new\n", "content\n"], new_trans_id)
> +            tt.apply(no_conflicts=True)

no_conflicts=True should be avoided except for very rare circumstances.

> +        except:
> +            tt.finalize()
> +            raise

Instead of wrapping the TreeTransform operations in a try/except, I
recommend doing:

        tt = TreeTransform(wt)
        self.addCleanup(tt.finalize)


> === modified file 'bzrlib/transform.py'
> --- bzrlib/transform.py	2008-07-02 19:18:03 +0000
> +++ bzrlib/transform.py	2008-07-16 16:38:55 +0000
> @@ -356,6 +356,12 @@
>          except OSError, e:
>              if e.errno == errno.ENOENT:
>                  return
> +            elif e.errno == errno.ENOTDIR:
> +                # If the file is in a directory that was a file in
> +                # tree on disk ENOTDIR will be raised. This error tells
> +                # us that the file is not present on disk, so we handle
> +                # it the same as ENOENT. See bug 248448.
> +                return

This looks fine, though I might do it as:
   if e.errno in (errno.ENOENT, errno.ENOTDIR):
       return

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFIf2n80F+nu1YWqI0RAkPoAJkBPrlrnNp2RbRdm4B9ur5F5t3OHgCfdMzV
+bjTygoJdqF69lulI3uVnqI=
=9LDB
-----END PGP SIGNATURE-----



More information about the bazaar mailing list