[PATCH] revsets: makes follow() supports file patterns

[Piotr Listkiewicz]

> It sounds like a bug for me. Do we have to keep compatibility with the old
> implementation that required a canonical path, not a file path?

What do you mean by this? What should be expected implementation then?

Now it just works like this - if we specify --cwd , file arguments to log
are translated to relative path cwd/file. When we additionally specify -f
it invokes follow with those arguments(still with relative files and
repo.getcwd set to cwd). On the other hand if we specify -r "follow(file)"
instead of -f we get file argument to follow  without translation to
relative path.

Is there a way to distinguish between call from -f or follow() , or omit
this problem from follow func?

> I'm not sure what it does, but if it tries to keep the existing behavior,
> it doesn't work as expected.

You are right, i didn't think of given use case trying to keep existing
behaviour.

>  Before this patch,
>
>  $ cd src
>  $ hg log -r 'follow("Makefile")'
>
>  did show the history of the top-level Makefile, which seems wrong though.

Should such a behaviour be kept for old compability, or it was just a bug?




2015-08-21 16:31 GMT+02:00 Yuya Nishihara <yuya at tcha.org>:

> On Fri, 21 Aug 2015 14:10:52 +0200, liscju wrote:
> > # HG changeset patch
> > # User liscju <piotr.listkiewicz at gmail.com>
> > # Date 1440083972 -7200
> > #      Thu Aug 20 17:19:32 2015 +0200
> > # Branch bug4757#followRevset
> > # Node ID 500e3760612a03cfe35a686dba835ee727534fc3
> > # Parent  d9d3d49c4cf77049d12920980e91bf8e4a4ecda2
> > revsets: makes follow() supports file patterns
> >
> > Before this patch, follow only supports full, exact filenames.
> > This patch makes follow argument to be treated like file
> > pattern same way like log treats their arguments.
> >
> > One problem with implementing this was the fact that
> > commands.log function before invoking follow
> > change path of its argument to use current working directory.
> >
> > When log is invoked by command:
> >   $ hg --cwd=dir log -f b
> > it invokes follow with x arg equals 'dir/b'.
> >
> > On the other hand when log is invoked by:
> >   $ hg --cwd=dir log -r "follow(b)"
> > it invokes follow with x arg equals 'b'
>
> It sounds like a bug for me. Do we have to keep compatibility with the old
> implementation that required a canonical path, not a file path?
>
> > To properly handle this situation _follow function
> > check if pattern is declared without kind(so pattern
> > resolves to exact file matching) and adds if needed
> > cwd to pat making matchmod.match matching against
> > project root. Otherwise matchmod.match is simply
> > invoked with cwd equals repo working directory
> >
> > Fix issue4757
>
> We generally put a issue number as follows:
> "revsets: makes follow() supports file patterns (issue4757)"
>
> https://mercurial.selenic.com/wiki/ContributingChanges#Submission_checklist
>
> > diff -r d9d3d49c4cf7 -r 500e3760612a mercurial/revset.py
> > --- a/mercurial/revset.py     Tue Aug 18 18:38:56 2015 -0500
> > +++ b/mercurial/revset.py     Thu Aug 20 17:19:32 2015 +0200
> > @@ -1037,34 +1037,41 @@
> >      return limit(repo, subset, x)
> >
> >  def _follow(repo, subset, x, name, followfirst=False):
> > -    l = getargs(x, 0, 1, _("%s takes no arguments or a filename") %
> name)
> > -    c = repo['.']
> > +    l = getargs(x, 0, 1, _("%s takes no arguments or a filepattern") %
> name)
>
> It looks like the other functions call it a "pattern", not "filepattern".
>
> > +    ctx = repo['.']
> >      if l:
> > -        x = getstring(l[0], _("%s expected a filename") % name)
> > -        if x in c:
> > -            cx = c[x]
> > -            s = set(ctx.rev() for ctx in
> cx.ancestors(followfirst=followfirst))
> > -            # include the revision responsible for the most recent
> version
> > -            s.add(cx.introrev())
> > +        pat = getstring(l[0], _("%s expected a filepattern") % name)
> > +        wtx = repo[None]
>
> A nit, common name for repo[None] is "wctx".
>
> > +        if not matchmod.patkind(pat):
> > +            if not pat.startswith(pathutil.normasprefix(repo.getcwd())):
> > +                pat = pathutil.canonpath(repo.root, repo.getcwd(), pat)
> > +            matcher = matchmod.match(repo.root, '', [pat], ctx=wtx)
>
> I'm not sure what it does, but if it tries to keep the existing behavior,
> it doesn't work as expected.
>
> For example, we have two Makefiles:
>
>   Makefile
>   src/Makefile
>
> Before this patch,
>
>   $ cd src
>   $ hg log -r 'follow("Makefile")'
>
> did show the history of the top-level Makefile, which seems wrong though.
> But now,
>
>   $ cd src
>   $ hg log -r 'follow("Makefile")'
>   $ hg log -r 'follow("src/Makefile")'
>
> both of these commands show the history of src/Makefile.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150821/db0ce82d/attachment.html>