Patchwork [1,of,7] match: if the repo root is a root, that is the only root we need

login
register
mail settings
Submitter Mads Kiilerich
Date Jan. 16, 2014, 1:18 a.m.
Message ID <7a74b5ba8e5e66378677.1389835089@localhost.localdomain>
Download mbox | patch
Permalink /patch/3339/
State Rejected
Headers show

Comments

Mads Kiilerich - Jan. 16, 2014, 1:18 a.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1380816081 -7200
#      Thu Oct 03 18:01:21 2013 +0200
# Node ID 7a74b5ba8e5e663786774fe84ec4ecca0207a9c5
# Parent  47d0843647d1e32f6af4482867327cec5db11a1f
match: if the repo root is a root, that is the only root we need

It will change what ends up in .files() and what is .exact(), in a way that is
more correct and efficient than before.
Matt Mackall - Jan. 20, 2014, 11:37 p.m.
On Thu, 2014-01-16 at 02:18 +0100, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1380816081 -7200
> #      Thu Oct 03 18:01:21 2013 +0200
> # Node ID 7a74b5ba8e5e663786774fe84ec4ecca0207a9c5
> # Parent  47d0843647d1e32f6af4482867327cec5db11a1f
> match: if the repo root is a root, that is the only root we need
> 
> It will change what ends up in .files() and what is .exact(), in a way that is
> more correct and efficient than before.

What was it doing before, how was it incorrect, and what is it doing
now? You have this info at your fingertips, I have to acquire it by
expensive, error-prone reverse-engineering of your patch, so please
include it in the description.

>    $ hg debugwalk 'relglob:Procyonidae/**' fennel
> -  f  fennel                          fennel                          exact
> +  f  fennel                          fennel

If 'fennel' is not an exact match here now, then I'm pretty sure this
patch must be wrong. The purpose of exact() is to distinguish between
things that match command line args exactly and things that simply match
patterns.
Mads Kiilerich - Jan. 21, 2014, 1:52 a.m.
On 01/21/2014 12:37 AM, Matt Mackall wrote:
> On Thu, 2014-01-16 at 02:18 +0100, Mads Kiilerich wrote:
>> # HG changeset patch
>> # User Mads Kiilerich <madski@unity3d.com>
>> # Date 1380816081 -7200
>> #      Thu Oct 03 18:01:21 2013 +0200
>> # Node ID 7a74b5ba8e5e663786774fe84ec4ecca0207a9c5
>> # Parent  47d0843647d1e32f6af4482867327cec5db11a1f
>> match: if the repo root is a root, that is the only root we need
>>
>> It will change what ends up in .files() and what is .exact(), in a way that is
>> more correct and efficient than before.
> What was it doing before, how was it incorrect, and what is it doing
> now? You have this info at your fingertips, I have to acquire it by
> expensive, error-prone reverse-engineering of your patch, so please
> include it in the description.

Hmm. Right. I failed in the error-prone reverse-engineering of the code.

I now see that _roots() also returns exact matches and that .exact() 
also returns roots (when .anypats() is true).

.exact() can thus only be used after checking that .matchfn matches and 
that it not just is a root, and .files() can also contain both files and 
roots.

I think that the thing that initially led me in the wrong direction was 
that .files() can contain the same root multiple times and thus in some 
cases would walk the same tree multiple times.

I will revisit later.

/Mads

Patch

diff --git a/mercurial/match.py b/mercurial/match.py
--- a/mercurial/match.py
+++ b/mercurial/match.py
@@ -345,11 +345,15 @@  def _roots(patterns):
                 if '[' in p or '{' in p or '*' in p or '?' in p:
                     break
                 root.append(p)
-            r.append('/'.join(root) or '.')
+            if not root:
+                return ['.']
+            r.append('/'.join(root))
         elif kind in ('relpath', 'path'):
-            r.append(name or '.')
+            if not name:
+                return ['.']
+            r.append(name)
         else: # relglob, re, relre
-            r.append('.')
+            return ['.']
     return r
 
 def _anypats(patterns):
diff --git a/tests/test-walk.t b/tests/test-walk.t
--- a/tests/test-walk.t
+++ b/tests/test-walk.t
@@ -259,7 +259,7 @@  Test patterns:
   f  mammals/Procyonidae/coatimundi  mammals/Procyonidae/coatimundi
   f  mammals/Procyonidae/raccoon     mammals/Procyonidae/raccoon
   $ hg debugwalk 'relglob:Procyonidae/**' fennel
-  f  fennel                          fennel                          exact
+  f  fennel                          fennel
   f  mammals/Procyonidae/cacomistle  mammals/Procyonidae/cacomistle
   f  mammals/Procyonidae/coatimundi  mammals/Procyonidae/coatimundi
   f  mammals/Procyonidae/raccoon     mammals/Procyonidae/raccoon
@@ -276,7 +276,7 @@  Test patterns:
   f  mammals/Procyonidae/raccoon     mammals/Procyonidae/raccoon
   f  mammals/skunk                   mammals/skunk
   $ hg debugwalk 'glob:mamm**' fennel
-  f  fennel                          fennel                          exact
+  f  fennel                          fennel
   f  mammals/Procyonidae/cacomistle  mammals/Procyonidae/cacomistle
   f  mammals/Procyonidae/coatimundi  mammals/Procyonidae/coatimundi
   f  mammals/Procyonidae/raccoon     mammals/Procyonidae/raccoon