[MERGE] bug #54172: revert handles new directories nicely

[Aaron Bentley]

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

John Arbash Meinel wrote:
> === modified file NEWS
> --- NEWS
> +++ NEWS
> @@ -3,6 +3,7 @@
>    IMPROVEMENTS:
> 
>    BUG FIXES:
> +  * revert now removes newly-added directories (Aaron Bentley, #54172)
> 
> ^- the indentation here is incorrect. I believe we indent to 4 spaces.
> But you can merge the latest bzr.dev and you'll have to resolve a
> conflict anyway. :)

It's unfortunate the way NEWS makes us serialise our submissions.  Maybe
we could put NEWS entries in alphabetical order to reduce this problem ? :-)

> +                    if file_kind != 'file' and file_kind is not None:
> +                        keep_contents = False
> +                        delete_merge_modified = False
> 
> ^- so this is saying if the object does exist in the working tree
> (file_kind is not None) and it isn't a file, then don't keep the
> contents, and don't delete it from the file system (because it doesn't
> exist anyway, right?)

If it's not a file, and it exists, then don't keep the contents, and
don't remove it from the working tree's merge_modified list (because
only files are in that list).

> +                    else:
> +                        if (file_id in merge_modified and
> +                            merge_modified[file_id] ==
> +                            working_tree.get_file_sha1(file_id)):
> +                            keep_contents = False
> +                            delete_merge_modified = True
> 
> ^- I'm a little concerned that (file_kind is None) will sneak in here,
> and then the get_file_sha1(file_id) will do something weird, because
> id2path will raise a BzrError claiming the file_id doesn't exist in the
> inventory. But checks for this might exist elsewhere.

get_file_sha1 returns None for symlinks, directories, and missing files.
This case is tested by test_inv.TestRevert.dangling_file_ids.  I know,
because I broke it at first.

> Otherwise it looks good. +1 from me if you can confirm that if file_kind
>  is None it won't mess things up.

Cool.  I'll stick it in.

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

iD8DBQFE812b0F+nu1YWqI0RAtVTAJ9xZQP4MQoet0svs3zG9gBEh6ALVwCfVpM9
f6sYgz5rrNALWEPe/MHS4u0=
=vt5w
-----END PGP SIGNATURE-----