D623: copytrace: move fast heuristic copytracing algorithm to core

yuja (Yuya Nishihara) phabricator at mercurial-scm.org
Wed Sep 6 10:18:27 EDT 2017


yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.


  Generally looks good.
  
  We might need something to handle copies between `anc` (real common
  ancestor) and `base` (pseudo merge base) as we do in the "full" copy tracing,
  but I'm not pretty sure. Since this is an experimental feature, and it may miss
  some copies by design, I think this is mostly good to go.

INLINE COMMENTS

> copies.py:22
>  
> +defaultdict = collections.defaultdict
> +

Nit: there are only two use sites, so let's remove the module-level
alias.

> copies.py:608
>  
> +def _heuristicscopytracing(repo, cdst, csrc, base):
> +    """ Fast copytracing using filename heuristics

This cdst/csrc naming is confusing because c1 is actually the
source revision (= the original wctx) in "update" scenario. And
IIUC, we are searching for copies from c1 to c2.

Can you rename them?

> copies.py:638
> +    mdst = cdst.manifest()
> +    while ctx != base:
> +        if len(ctx.parents()) == 2:

Perhaps this wouldn't stop if the base were in the other side.
I don't think that would happen thanks to how mergecopies()
are used currently, but it's probably better to error out early.

> copies.py:641
> +            # To keep things simple let's not handle merges
> +            repo.ui.debug("swicthing to full copytracing because of merges")
> +            return _fullcopytracing(repo, cdst, csrc, base)

Typo: "switching", and missing "\n".

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D623

To: pulkit, #hg-reviewers, yuja
Cc: yuja, mercurial-devel


More information about the Mercurial-devel mailing list