[MERGE] Implement hard-link support for branch and checkout

[Ian Clatworthy]

Aaron Bentley wrote:
> Hi all,
> 
> This patch implements hard-link support for branch and checkout,
> building on my previous accelerator_tree work.
> 
> The primary advantage of this is space savings, not speed.  But using
> hardlinks is 1.19 x as fast:

bb:tweak

As I explained in an earlier email, I think this is an important
enhancement for the right crowd. After this lands, I'd like to write
(with assistance) a section towards the end of the User Guide presenting
it as a feature for advanced users, together with the right warnings,
directions for setting popular editors to break hard links, etc.

Some minor tweaks and a follow-up patch requested below.

> +    def create_hardlink(self, path, trans_id):
> +        """Schedule creation of a hard link"""
> +        name = self._limbo_name(trans_id)
> +        os.link(path, name)
> +        try:
> +            unique_add(self._new_contents, trans_id, 'file')
> +        except:
> +            # Clean up the file, it never got registered so
> +            # TreeTransform.finalize() won't clean it up.
> +            os.unlink(name)
> +            raise
> +

Bare except clauses are considered bad. Please be explicit about the
class of exceptions being caught.

> +            offset = num +1 - len(deferred_contents)

Need a space between the + and 1.

>          iter = accelerator_tree._iter_changes(tree, include_unchanged=True)
>          unchanged = dict((f, p[0]) for (f, p, c, v, d, n, k, e)
> -                         in iter if not c)
> +                         in iter if not (c or e[0] != e[1]))

This edit caught my attention. Why isn't a changed execute flag detected
by _iter_changes() as a change?

> +    count = 0
> +    for count, (trans_id, contents) in enumerate(tree.iter_files_bytes(
> +                                                 new_desired_files)):
> +        tt.create_file(contents, trans_id)
> +        pb.update('Adding file contents', count+offset, total)

The 'count = 0' line is redundant.

As I mentioned earlier, I'm happy to add an appendix (say) to the User
Guide explaining when and how to use this feature. As part of this patch
though, can you please add a note, over and above the option text, to
the online help for branch and checkout saying something like:

"If you use the hardlink option, be sure to configure your editor to
break hardlinks when saving." or something like that.

Now to the requested follow-up patch ....

I wonder what makes the most sense UI wise if the hardlink option is
specified but not supported on a given platform? I think this ought to
fail cleanly with an explicit error message. This looks easy to trap in
cmd_branch and cmd_checkout but the code and tests can come in a
follow-up patch.

I'm also thinking that the users wanting this feature will be working on
large trees most the time, will use it more often than not and will
therefore make it an alias, i.e.

  branch=branch --hardlink

Given this, if the platform supports it but the filesystem doesn't or
they cross filesystem boundaries, we might want to think about whether
failing outright (as I think the code will do now) is really the right
thing to do. We could output a warning that the request for hardlinking
is being ignored and continue, perhaps? Once again, this isn't necessary
to land this particular patch though.

Ian C.