[PATCH] automv: new experimental extension
Pierre-Yves David
pierre-yves.david at ens-lyon.org
Mon Feb 15 13:24:07 UTC 2016
On 02/08/2016 01:56 PM, Martijn Pieters wrote:
> # HG changeset patch
> # User Martijn Pieters <mjpieters at fb.com>
> # Date 1454939571 0
> # Mon Feb 08 13:52:51 2016 +0000
> # Node ID fb91a65f102ca34d69961142ad84955080ada1fc
> # Parent 3026a9107f9e40ba5d63bdae7e81c48db8885fe2
> automv: new experimental extension
>
> Automatically detect moves and record them at commit time.
>
> This extension was originally developed at
> https://bitbucket.org/facebook/hg-experimental
Here is group of feedback to be discussed as potential followup.
> diff --git a/hgext/automv.py b/hgext/automv.py
> new file mode 100644
> --- /dev/null
> +++ b/hgext/automv.py
> @@ -0,0 +1,83 @@
> +# automv.py
> +#
> +# Copyright 2013-2016 Facebook, Inc.
> +#
> +# This software may be used and distributed according to the terms of the
> +# GNU General Public License version 2 or any later version.
> +"""Check for unrecorded moves at commit time (EXPERIMENTAL)
> +
> +This extension checks at commit/amend time if any of the committed files
> +comes from an unrecorded mv.
> +
> +The threshold at which a file is considered a move can be set with the
> +``automv.similarity`` config option; the default value is 1.00.
1) We probably want to unify this config with addremove
2) Not super convinced by the creation of a new config section
3) we probably want a way to disable automv independantly of addremove
> +
> +"""
> +from __future__ import absolute_import
> +
> +from mercurial import (
> + commands,
> + copies,
> + extensions,
> + scmutil,
> + similar
> +)
> +from mercurial.i18n import _
> +
> +def extsetup(ui):
> + entry = extensions.wrapcommand(
> + commands.table, 'commit', mvcheck)
> + entry[1].append(
> + ('', 'no-automv', None,
> + _('disable automatic file move detection')))
--no-automv is the kind of name+behavior that will be a bag of regret in
the futur. I would go for something like '--similarity X' for addremove.
it can take 0 to disable the detection entirely. Augie mention than
'--similarity' could be too generic for somthing we sneak into
commitopts. '--automove X' could be a better alternative.
To be discussed.
> +
> +def mvcheck(orig, ui, repo, *pats, **opts):
I'm a huge proponent of adding basic docstring to everything!
> + disabled = opts.pop('no_automv', False)
> + if not disabled:
> + threshold = float(ui.config('automv', 'similarity', '1.00'))
> + if threshold > 0:
> + match = scmutil.match(repo[None], pats, opts)
> + added, removed = _interestingfiles(repo, match)
> + renames = _findrenames(repo, match, added, removed, threshold)
> + _markchanges(repo, renames)
> +
> + # developer config: automv.testmode
> + if not ui.configbool('automv', 'testmode'):
> + return orig(ui, repo, *pats, **opts)
> +
> +def _interestingfiles(repo, matcher):
I'm still a huge proponent of adding basic docstring to everything!
> + stat = repo.status(repo['.'], repo[None], matcher)
Do we actually need to pass all this ctx, don't we have a good top level
function with the default value for this? (genious noob question here)
> + added = stat[1]
> + removed = stat[2]
> +
> + copy = copies._forwardcopies(repo['.'], repo[None], matcher)
> + # remove the copy files for which we already have copy info
> + added = [f for f in added if f not in copy]
> +
> + return added, removed
> +
> +def _findrenames(repo, matcher, added, removed, similarity):
> + """Find renames from removed files of the current commit/amend files
> + to the added ones"""
<chanting about dosctring>
> + renames = {}
> + if similarity > 0:
> + for src, dst, score in similar.findrenames(
> + repo, added, removed, similarity):
> + if repo.ui.verbose:
> + repo.ui.status(
> + _('detected move of %s as %s (%d%% similar)\n') % (
> + matcher.rel(src), matcher.rel(dst), score * 100))
> + renames[dst] = src
> + if renames:
> + repo.ui.status(_('detected move of %d files\n') % len(renames))
> + return renames
> +
> +def _markchanges(repo, renames):
> + """Marks the files in renames as copied."""
> + wctx = repo[None]
> + wlock = repo.wlock()
> + try:
> + for dst, src in renames.iteritems():
> + wctx.copy(src, dst)
> + finally:
> + wlock.release()
I'm a bit confused by the fact we need our own locking here. should'nt
we be in the locking scope of the commit command already ? if not we are
open to race condition. Actually, we are already open to race condition
in this code because we build the wctx before lock the repository.
We should investigate and cleanup this locking logic.
> diff --git a/tests/test-automv.t b/tests/test-automv.t
> new file mode 100644
> --- /dev/null
> +++ b/tests/test-automv.t
> @@ -0,0 +1,287 @@
Please add some documentation about what this tests file is about.
> + $ cat >> $HGRCPATH << EOF
> + > [extensions]
> + > automv=
> + > rebase=
> + > EOF
> +
> +Setup repo
> +
> + $ hg init repo
> + $ cd repo
> +
> +Test automv command for commit
> +
> + $ echo 'foo' > a.txt
> + $ hg add a.txt
> + $ hg commit -m 'init repo with a'
> +
> +mv/rm/add
> + $ mv a.txt b.txt
> + $ hg rm a.txt
> + $ hg add b.txt
> + $ hg status -C
> + A b.txt
> + R a.txt
> + $ hg commit -m 'msg'
> + detected move of 1 files
> + $ hg status --change . -C
> + A b.txt
> + a.txt
> + R a.txt
> + $ hg up -r 0
> + 1 files updated, 0 files merged, 1 files removed, 0 files unresolved
> +
> +mv/rm/add/modif
> + $ mv a.txt b.txt
> + $ hg rm a.txt
> + $ hg add b.txt
> + $ printf '\nfoo\n' >> b.txt
> + $ hg status -C
> + A b.txt
> + R a.txt
> + $ hg commit -m 'msg'
> + created new head
> + $ hg status --change . -C
> + A b.txt
> + R a.txt
> + $ hg up -r 0
> + 1 files updated, 0 files merged, 1 files removed, 0 files unresolved
> +
> +mv/rm/add/modif/changethreshold
> + $ mv a.txt b.txt
> + $ hg rm a.txt
> + $ hg add b.txt
> + $ printf '\nfoo\n' >> b.txt
> + $ hg status -C
> + A b.txt
> + R a.txt
> + $ hg commit --config automv.similarity='0.6' -m 'msg'
> + detected move of 1 files
> + created new head
> + $ hg status --change . -C
> + A b.txt
> + a.txt
> + R a.txt
> + $ hg up -r 0
> + 1 files updated, 0 files merged, 1 files removed, 0 files unresolved
> +
> +mv
> + $ mv a.txt b.txt
> + $ hg status -C
> + ! a.txt
> + ? b.txt
> + $ hg commit -m 'msg'
> + nothing changed (1 missing files, see 'hg status')
> + [1]
> + $ hg status -C
> + ! a.txt
> + ? b.txt
> + $ hg revert -aqC
> + $ rm b.txt
> +
> +mv/rm/add/notincommitfiles
> + $ mv a.txt b.txt
> + $ hg rm a.txt
> + $ hg add b.txt
> + $ echo 'bar' > c.txt
> + $ hg add c.txt
> + $ hg status -C
> + A b.txt
> + A c.txt
> + R a.txt
> + $ hg commit c.txt -m 'msg'
> + created new head
> + $ hg status --change . -C
> + A c.txt
> + $ hg status -C
> + A b.txt
> + R a.txt
> + $ hg up -r 0
> + 0 files updated, 0 files merged, 1 files removed, 0 files unresolved
> + $ hg rm a.txt
> + $ echo 'bar' > c.txt
> + $ hg add c.txt
> + $ hg commit -m 'msg'
> + detected move of 1 files
> + created new head
> + $ hg status --change . -C
> + A b.txt
> + a.txt
> + A c.txt
> + R a.txt
> + $ hg up -r 0
> + 1 files updated, 0 files merged, 2 files removed, 0 files unresolved
> +
> +mv/rm/add/--no-automv
> + $ mv a.txt b.txt
> + $ hg rm a.txt
> + $ hg add b.txt
> + $ hg status -C
> + A b.txt
> + R a.txt
> + $ hg commit --no-automv -m 'msg'
> + created new head
> + $ hg status --change . -C
> + A b.txt
> + R a.txt
> + $ hg up -r 0
> + 1 files updated, 0 files merged, 1 files removed, 0 files unresolved
> +
> +
> +Test automv command for commit --amend
> +
> +mv/rm/add
> + $ echo 'c' > c.txt
> + $ hg add c.txt
> + $ hg commit -m 'revision to amend to'
> + created new head
> + $ mv a.txt b.txt
> + $ hg rm a.txt
> + $ hg add b.txt
> + $ hg status -C
> + A b.txt
> + R a.txt
> + $ hg commit --amend -m 'amended'
> + detected move of 1 files
> + saved backup bundle to $TESTTMP/repo/.hg/strip-backup/*-amend-backup.hg (glob)
> + $ hg status --change . -C
> + A b.txt
> + a.txt
> + A c.txt
> + R a.txt
> + $ hg up -r 0
> + 1 files updated, 0 files merged, 2 files removed, 0 files unresolved
> +
> +mv/rm/add/modif
> + $ echo 'c' > c.txt
> + $ hg add c.txt
> + $ hg commit -m 'revision to amend to'
> + created new head
> + $ mv a.txt b.txt
> + $ hg rm a.txt
> + $ hg add b.txt
> + $ printf '\nfoo\n' >> b.txt
> + $ hg status -C
> + A b.txt
> + R a.txt
> + $ hg commit --amend -m 'amended'
> + saved backup bundle to $TESTTMP/repo/.hg/strip-backup/*-amend-backup.hg (glob)
> + $ hg status --change . -C
> + A b.txt
> + A c.txt
> + R a.txt
> + $ hg up -r 0
> + 1 files updated, 0 files merged, 2 files removed, 0 files unresolved
> +
> +mv/rm/add/modif/changethreshold
> + $ echo 'c' > c.txt
> + $ hg add c.txt
> + $ hg commit -m 'revision to amend to'
> + created new head
> + $ mv a.txt b.txt
> + $ hg rm a.txt
> + $ hg add b.txt
> + $ printf '\nfoo\n' >> b.txt
> + $ hg status -C
> + A b.txt
> + R a.txt
> + $ hg commit --amend --config automv.similarity='0.6' -m 'amended'
> + detected move of 1 files
> + saved backup bundle to $TESTTMP/repo/.hg/strip-backup/*-amend-backup.hg (glob)
> + $ hg status --change . -C
> + A b.txt
> + a.txt
> + A c.txt
> + R a.txt
> + $ hg up -r 0
> + 1 files updated, 0 files merged, 2 files removed, 0 files unresolved
> +
> +mv
> + $ echo 'c' > c.txt
> + $ hg add c.txt
> + $ hg commit -m 'revision to amend to'
> + created new head
> + $ mv a.txt b.txt
> + $ hg status -C
> + ! a.txt
> + ? b.txt
> + $ hg commit --amend -m 'amended'
> + saved backup bundle to $TESTTMP/repo/.hg/strip-backup/*-amend-backup.hg (glob)
> + $ hg status -C
> + ! a.txt
> + ? b.txt
> + $ hg up -Cr 0
> + 1 files updated, 0 files merged, 1 files removed, 0 files unresolved
> +
> +mv/rm/add/notincommitfiles
> + $ echo 'c' > c.txt
> + $ hg add c.txt
> + $ hg commit -m 'revision to amend to'
> + created new head
> + $ mv a.txt b.txt
> + $ hg rm a.txt
> + $ hg add b.txt
> + $ echo 'bar' > d.txt
> + $ hg add d.txt
> + $ hg status -C
> + A b.txt
> + A d.txt
> + R a.txt
> + $ hg commit --amend -m 'amended' d.txt
> + saved backup bundle to $TESTTMP/repo/.hg/strip-backup/*-amend-backup.hg (glob)
> + $ hg status --change . -C
> + A c.txt
> + A d.txt
> + $ hg status -C
> + A b.txt
> + R a.txt
> + $ hg commit --amend -m 'amended'
> + detected move of 1 files
> + saved backup bundle to $TESTTMP/repo/.hg/strip-backup/*-amend-backup.hg (glob)
> + $ hg status --change . -C
> + A b.txt
> + a.txt
> + A c.txt
> + A d.txt
> + R a.txt
> + $ hg up -r 0
> + 1 files updated, 0 files merged, 3 files removed, 0 files unresolved
> +
> +mv/rm/add/--no-automv
> + $ echo 'c' > c.txt
> + $ hg add c.txt
> + $ hg commit -m 'revision to amend to'
> + created new head
> + $ mv a.txt b.txt
> + $ hg rm a.txt
> + $ hg add b.txt
> + $ hg status -C
> + A b.txt
> + R a.txt
> + $ hg commit --amend -m 'amended' --no-automv
> + saved backup bundle to $TESTTMP/repo/.hg/strip-backup/*-amend-backup.hg (glob)
> + $ hg status --change . -C
> + A b.txt
> + A c.txt
> + R a.txt
> + $ hg up -r 0
> + 1 files updated, 0 files merged, 2 files removed, 0 files unresolved
> +
> +
> +mv/rm/commit/add/amend
> + $ echo 'c' > c.txt
> + $ hg add c.txt
> + $ hg commit -m 'revision to amend to'
> + created new head
> + $ mv a.txt b.txt
> + $ hg rm a.txt
> + $ hg status -C
> + R a.txt
> + ? b.txt
> + $ hg commit -m "removed a"
> + $ hg add b.txt
> + $ hg commit --amend -m 'amended'
> + saved backup bundle to $TESTTMP/repo/.hg/strip-backup/*-amend-backup.hg (glob)
> + $ hg status --change . -C
> + A b.txt
> + R a.txt
--
Pierre-Yves David
More information about the Mercurial-devel
mailing list