[MERGE] TreeTransform avoids many renames when constructing trees

[Aaron Bentley]

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

John Arbash Meinel wrote:
>> Yeah, I've moved away from the cutesy test suites, but most style guides
>> recommend retaining the style of surrounding code, which in this case
>> was wizard-of-oz-based.  I can make it more straightforward.
> 
> Well, I'm fine with biasing local consistency over "correctness", as I don't
> expect you to rename everything.

Well, I did anyhow.

>>> I would be more intrigued if you could create a target directory which
>>> claims to be a child of oz-id and neither one had a name yet.
>> I'm not sure what you mean.  You mean a triply-nested path, like
>> foo/bar/baz?  Creating the contents for all three first, then assigning
>> the name?
> 
> well, I was just meaning 'foo/bar' where 'foo' has not been named yet, but
> 'bar' has.

Okay, I've added a test for that, to ensure it works.

> Ultimately if it is an uncommon code path, we don't need to optimize
> for it. It is just the common case of 50,000 files being built in a single
> large tree that we need to make faster.

Well, as it happens, it only needs one rename, so I'll make sure that
doesn't regress.

>> In theory it could, but creating elphaba2 while elphaba1 is occupying
>> that limbo path causes elphaba2 to be created at the top level of limbo.
>>  I don't consider it worthwhile to rename top-level limbo files into
>> their parents before Transform.apply().  If we did that, we would be
>> renaming a file before Transform.apply() to avoid renaming the file
>> during Transform.apply().
> 
> Well, you are doing that with the test_change_parent test. So I'm confused why
> you aren't doing it consistently. Either always do it, or never, at least from
> what I can see.

In the change_parent test, I must rename elphaba.  If I rename it to the
top, then I will have to rename it again in apply.  If I rename it to
new-3/elphaba, I can avoid that rename.

> Ok, you just had written comments like:
> 
> # limbo/oz => oz

Right.  When I was updating the names in the tests, I noticed this, and
fixed it.

>>> I don't think it is strictly a big deal, but this is O(tree) so it is
>>> nice to avoid extra copies when possible.
>> AIUI, it is faster to iterate through a list than a generator.
> 
> Well, you are creating a list to pass it to sorted, which generates another
> list. I do believe that
>   foo = sorted([list expression])
> is better than
>   foo = [list expression]
>   foo.sort()

I can't parse that.  You either mean "list comprehension" or "generator
expression", but they are significantly different in this context.

Anyhow, submitted.

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

iD8DBQFGZYb90F+nu1YWqI0RAqVNAKCDtcVjG4nhmE1du4WXfFirJu/EzACeN0MB
mPz02WONatsxxgeIZ+c4teQ=
=kXhd
-----END PGP SIGNATURE-----