[PATCH 1 of 3] log: reorganize if-else and for loop in logcmdutil._makematcher()

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 1 of 3] log: reorganize if-else and for loop in logcmdutil._makematcher()

Yuya Nishihara
# HG changeset patch
# User Yuya Nishihara <[hidden email]>
# Date 1599804538 -32400
#      Fri Sep 11 15:08:58 2020 +0900
# Node ID 306c09f80dd9659aa9f0eebe1baa328afa3b7977
# Parent  4532e7ebde4d9eb5011b78fb537879bcc67b30c4
log: reorganize if-else and for loop in logcmdutil._makematcher()

The test conditions are branchy depending on --follow and --rev options,
so it should be better to switch first by --follow --rev.

Note that revs is not empty so "if follow and startctxs" can be replaced
with "if follow and opts.get(b'rev')".

diff --git a/mercurial/logcmdutil.py b/mercurial/logcmdutil.py
--- a/mercurial/logcmdutil.py
+++ b/mercurial/logcmdutil.py
@@ -691,39 +691,47 @@ def _makematcher(repo, revs, pats, opts)
     slowpath = match.anypats() or (not match.always() and opts.get(b'removed'))
     if not slowpath:
         follow = opts.get(b'follow') or opts.get(b'follow_first')
-        startctxs = []
         if follow and opts.get(b'rev'):
             startctxs = [repo[r] for r in revs]
-        for f in match.files():
-            if follow and startctxs:
+            for f in match.files():
                 # No idea if the path was a directory at that revision, so
                 # take the slow path.
                 if any(f not in c for c in startctxs):
                     slowpath = True
                     continue
-            elif follow and f not in wctx:
-                # If the file exists, it may be a directory, so let it
-                # take the slow path.
-                if os.path.exists(repo.wjoin(f)):
-                    slowpath = True
-                    continue
-                else:
-                    raise error.Abort(
-                        _(
-                            b'cannot follow file not in parent '
-                            b'revision: "%s"'
-                        )
-                        % f
-                    )
-            filelog = repo.file(f)
-            if not filelog:
-                # A zero count may be a directory or deleted file, so
-                # try to find matching entries on the slow path.
-                if follow:
+                filelog = repo.file(f)
+                if not filelog:
                     raise error.Abort(
                         _(b'cannot follow nonexistent file: "%s"') % f
                     )
-                slowpath = True
+        elif follow:
+            for f in match.files():
+                if f not in wctx:
+                    # If the file exists, it may be a directory, so let it
+                    # take the slow path.
+                    if os.path.exists(repo.wjoin(f)):
+                        slowpath = True
+                        continue
+                    else:
+                        raise error.Abort(
+                            _(
+                                b'cannot follow file not in parent '
+                                b'revision: "%s"'
+                            )
+                            % f
+                        )
+                filelog = repo.file(f)
+                if not filelog:
+                    raise error.Abort(
+                        _(b'cannot follow nonexistent file: "%s"') % f
+                    )
+        else:
+            for f in match.files():
+                filelog = repo.file(f)
+                if not filelog:
+                    # A zero count may be a directory or deleted file, so
+                    # try to find matching entries on the slow path.
+                    slowpath = True
 
         # We decided to fall back to the slowpath because at least one
         # of the paths was not a file. Check to see if at least one of them
diff --git a/tests/test-log.t b/tests/test-log.t
--- a/tests/test-log.t
+++ b/tests/test-log.t
@@ -2308,6 +2308,20 @@ follow files from wdir
   $ hg log -T '== {rev} ==\n' -fr'wdir()' --git --stat notfound
   notfound: $ENOENT$
 
+follow added/removed files from wdir parent
+
+  $ hg log -T '{rev}\n' -f d1/f2
+  abort: cannot follow nonexistent file: "d1/f2"
+  [255]
+
+  $ hg log -T '{rev}\n' -f f1-copy
+  abort: cannot follow nonexistent file: "f1-copy"
+  [255]
+
+  $ hg log -T '{rev}\n' -f .d6/f1
+  abort: cannot follow file not in parent revision: ".d6/f1"
+  [255]
+
   $ hg revert -aqC
 
 Check that adding an arbitrary name shows up in log automatically
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 2 of 3] log: fix -fr'wdir()' PATH to follow newly added file

Yuya Nishihara
# HG changeset patch
# User Yuya Nishihara <[hidden email]>
# Date 1599804815 -32400
#      Fri Sep 11 15:13:35 2020 +0900
# Node ID 21487b54666e457374fa669ffe3005f3840353c6
# Parent  306c09f80dd9659aa9f0eebe1baa328afa3b7977
log: fix -fr'wdir()' PATH to follow newly added file

Testing filelog doesn't make sense in this case because the file existence
is tested against the specified changectxs. If the filelog is empty and
if startctxs != [wctx], 'f not in c' should be triggered.

diff --git a/mercurial/logcmdutil.py b/mercurial/logcmdutil.py
--- a/mercurial/logcmdutil.py
+++ b/mercurial/logcmdutil.py
@@ -698,12 +698,7 @@ def _makematcher(repo, revs, pats, opts)
                 # take the slow path.
                 if any(f not in c for c in startctxs):
                     slowpath = True
-                    continue
-                filelog = repo.file(f)
-                if not filelog:
-                    raise error.Abort(
-                        _(b'cannot follow nonexistent file: "%s"') % f
-                    )
+                    break
         elif follow:
             for f in match.files():
                 if f not in wctx:
@@ -722,6 +717,8 @@ def _makematcher(repo, revs, pats, opts)
                         )
                 filelog = repo.file(f)
                 if not filelog:
+                    # A file exists in wdir but not in history, which means
+                    # the file isn't committed yet.
                     raise error.Abort(
                         _(b'cannot follow nonexistent file: "%s"') % f
                     )
diff --git a/tests/test-log.t b/tests/test-log.t
--- a/tests/test-log.t
+++ b/tests/test-log.t
@@ -2295,15 +2295,21 @@ follow files from wdir
    1 files changed, 1 insertions(+), 0 deletions(-)
   
 
- BROKEN: added file should exist in wdir
   $ hg log -T '== {rev} ==\n' -fr'wdir()' --git --stat d1/f2
-  abort: cannot follow nonexistent file: "d1/f2"
-  [255]
+  == 2147483647 ==
+   d1/f2 |  1 +
+   1 files changed, 1 insertions(+), 0 deletions(-)
+  
 
- BROKEN: copied file should exist in wdir
   $ hg log -T '== {rev} ==\n' -fr'wdir()' --git --stat f1-copy
-  abort: cannot follow nonexistent file: "f1-copy"
-  [255]
+  == 2147483647 ==
+   f1-copy |  1 +
+   1 files changed, 1 insertions(+), 0 deletions(-)
+  
+  == 0 ==
+   d1/f1 |  1 +
+   1 files changed, 1 insertions(+), 0 deletions(-)
+  
 
   $ hg log -T '== {rev} ==\n' -fr'wdir()' --git --stat notfound
   notfound: $ENOENT$
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 3 of 3] log: make -frREV PATH detect missing files before falling back to slow path

Yuya Nishihara
In reply to this post by Yuya Nishihara
# HG changeset patch
# User Yuya Nishihara <[hidden email]>
# Date 1599863027 -32400
#      Sat Sep 12 07:23:47 2020 +0900
# Node ID 78f4530ec2ed09dce0146ab2dfb6f42b323abe76
# Parent  21487b54666e457374fa669ffe3005f3840353c6
log: make -frREV PATH detect missing files before falling back to slow path

If -rREV isn't specified, "log --follow" would abort on nonexistent paths.
Let's implement this behavior for "-frREV" case as we have ctx.hasdir() now.
Otherwise "log -frREV PATH" would silently fall back to slow path and files
wouldn't be followed across renames.

The loop is quadratic (as before), but the size of the startctxs and
match.files() should be small in general.

Some tests are marked as BROKEN since file renames aren't tracked in the
slow path. This is a known limitation of the current history traversal
function.

diff --git a/mercurial/logcmdutil.py b/mercurial/logcmdutil.py
--- a/mercurial/logcmdutil.py
+++ b/mercurial/logcmdutil.py
@@ -692,13 +692,27 @@ def _makematcher(repo, revs, pats, opts)
     if not slowpath:
         follow = opts.get(b'follow') or opts.get(b'follow_first')
         if follow and opts.get(b'rev'):
+            # There may be the case that a path doesn't exist in some (but
+            # not all) of the specified start revisions, but let's consider
+            # the path is valid. Missing files will be warned by the matcher.
             startctxs = [repo[r] for r in revs]
             for f in match.files():
-                # No idea if the path was a directory at that revision, so
-                # take the slow path.
-                if any(f not in c for c in startctxs):
-                    slowpath = True
-                    break
+                found = False
+                for c in startctxs:
+                    if f in c:
+                        found = True
+                    elif c.hasdir(f):
+                        # If a directory exists in any of the start revisions,
+                        # take the slow path.
+                        found = slowpath = True
+                if not found:
+                    raise error.Abort(
+                        _(
+                            b'cannot follow file not in any of the specified '
+                            b'revisions: "%s"'
+                        )
+                        % f
+                    )
         elif follow:
             for f in match.files():
                 if f not in wctx:
diff --git a/tests/test-log.t b/tests/test-log.t
--- a/tests/test-log.t
+++ b/tests/test-log.t
@@ -504,14 +504,50 @@ follow files from the specified revision
   0 (false !)
 
 follow files from the specified revisions with missing patterns
-(BROKEN: should follow copies from e@4)
 
   $ hg log -T '{rev}\n' -fr4 e x
-  4
-  2 (false !)
+  abort: cannot follow file not in any of the specified revisions: "x"
+  [255]
+
+follow files from the specified revisions with directory patterns
+(BROKEN: should follow copies from dir/b@2)
+
+  $ hg log -T '{rev}\n' -fr2 dir/b dir
+  2
   1 (false !)
   0 (false !)
 
+follow files from multiple revisions, but the pattern is missing in
+one of the specified revisions
+
+  $ hg log -T '{rev}\n' -fr'2+4' dir/b e
+  e: no such file in rev f8954cd4dc1f
+  dir/b: no such file in rev 7e4639b4691b
+  4
+  2
+  1
+  0
+
+follow files from multiple revisions, and the pattern matches a file in
+one revision but matches a directory in another:
+(BROKEN: should follow copies from dir/b@2 and dir/b/g@5)
+(BROKEN: the revision 4 should not be included since dir/b/g@5 is unchanged)
+
+  $ mkdir -p dir/b
+  $ hg mv g dir/b
+  $ hg ci -m 'make dir/b a directory'
+
+  $ hg log -T '{rev}\n' -fr'2+5' dir/b
+  5
+  4
+  3 (false !)
+  2
+  1 (false !)
+  0 (false !)
+
+  $ hg --config extensions.strip= strip -r. --no-backup
+  1 files updated, 0 files merged, 1 files removed, 0 files unresolved
+
 follow files from the specified revisions across copies with -p/--patch
 
   $ hg log -T '== rev: {rev},{file_copies % " {source}->{name}"} ==\n' -fpr 4 e g
@@ -2312,7 +2348,15 @@ follow files from wdir
   
 
   $ hg log -T '== {rev} ==\n' -fr'wdir()' --git --stat notfound
-  notfound: $ENOENT$
+  abort: cannot follow file not in any of the specified revisions: "notfound"
+  [255]
+
+follow files from wdir and non-wdir revision:
+
+  $ hg log -T '{rev}\n' -fr'wdir()+.' f1-copy
+  f1-copy: no such file in rev 65624cd9070a
+  2147483647
+  0
 
 follow added/removed files from wdir parent
 
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1 of 3] log: reorganize if-else and for loop in logcmdutil._makematcher()

Pulkit Goyal
In reply to this post by Yuya Nishihara
On Mon, Sep 14, 2020 at 6:58 PM Yuya Nishihara <[hidden email]> wrote:

>
> # HG changeset patch
> # User Yuya Nishihara <[hidden email]>
> # Date 1599804538 -32400
> #      Fri Sep 11 15:08:58 2020 +0900
> # Node ID 306c09f80dd9659aa9f0eebe1baa328afa3b7977
> # Parent  4532e7ebde4d9eb5011b78fb537879bcc67b30c4
> log: reorganize if-else and for loop in logcmdutil._makematcher()
>
> The test conditions are branchy depending on --follow and --rev options,
> so it should be better to switch first by --follow --rev.
>
> Note that revs is not empty so "if follow and startctxs" can be replaced
> with "if follow and opts.get(b'rev')".

Queued the series, many thanks!
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel