Patchwork [1,of,4,V2] scmutil: teach the file prefetch hook to handle multiple commits

login
register
mail settings
Submitter Matt Harbison
Date April 16, 2018, 7:07 p.m.
Message ID <fe6824bcc6b8228f2eb1.1523905629@MATT7H-PC.attotech.com>
Download mbox | patch
Permalink /patch/31097/
State Accepted
Headers show

Comments

Matt Harbison - April 16, 2018, 7:07 p.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1523746245 14400
#      Sat Apr 14 18:50:45 2018 -0400
# Node ID fe6824bcc6b8228f2eb141ea96efb6797b5530d7
# Parent  1859b9a7ddefe90971259da0793e9289fd65c63f
scmutil: teach the file prefetch hook to handle multiple commits

The remainder of the commands that need prefetch deal with multiple revisions.
I initially coded this as a separate hook, but then it needed a list of files
to handle `diff` and `grep`, so it didn't seem worth keeping them separate.

Not every matcher will emit bad file messages (some are built from a list of
files that are known to exist).  But it seems better to filter this in one place
than to push this on either each caller or each hook implementation.
Yuya Nishihara - April 17, 2018, 11:34 a.m.
On Mon, 16 Apr 2018 15:07:09 -0400, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1523746245 14400
> #      Sat Apr 14 18:50:45 2018 -0400
> # Node ID fe6824bcc6b8228f2eb141ea96efb6797b5530d7
> # Parent  1859b9a7ddefe90971259da0793e9289fd65c63f
> scmutil: teach the file prefetch hook to handle multiple commits

Queued, thanks.

> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -2258,16 +2258,15 @@ def cat(ui, repo, ctx, matcher, basefm, 
>          mfnode = ctx.manifestnode()
>          try:
>              if mfnode and mfl[mfnode].find(file)[0]:
> -                scmutil.fileprefetchhooks(repo, ctx, [file])
> +                scmutil.prefetchfiles(repo, [ctx.rev()], matcher)
>                  write(file)
>                  return 0
>          except KeyError:
>              pass
>  
> -    files = [f for f in ctx.walk(matcher)]
> -    scmutil.fileprefetchhooks(repo, ctx, files)
> -
> -    for abs in files:
> +    scmutil.prefetchfiles(repo, [ctx.rev()], matcher)

Nit: these prefetchfiles() could be moved before the fast path.

Patch

diff --git a/hgext/lfs/wrapper.py b/hgext/lfs/wrapper.py
--- a/hgext/lfs/wrapper.py
+++ b/hgext/lfs/wrapper.py
@@ -244,17 +244,21 @@  def hgpostshare(orig, sourcerepo, destre
     if 'lfs' in destrepo.requirements:
         destrepo.vfs.append('hgrc', util.tonativeeol('\n[extensions]\nlfs=\n'))
 
-def _prefetchfiles(repo, ctx, files):
+def _prefetchfiles(repo, revs, match):
     """Ensure that required LFS blobs are present, fetching them as a group if
     needed."""
     pointers = []
+    oids = set()
     localstore = repo.svfs.lfslocalblobstore
 
-    for f in files:
-        p = pointerfromctx(ctx, f)
-        if p and not localstore.has(p.oid()):
-            p.filename = f
-            pointers.append(p)
+    for rev in revs:
+        ctx = repo[rev]
+        for f in ctx.walk(match):
+            p = pointerfromctx(ctx, f)
+            if p and p.oid() not in oids and not localstore.has(p.oid()):
+                p.filename = f
+                pointers.append(p)
+                oids.add(p.oid())
 
     if pointers:
         # Recalculating the repo store here allows 'paths.default' that is set
diff --git a/mercurial/archival.py b/mercurial/archival.py
--- a/mercurial/archival.py
+++ b/mercurial/archival.py
@@ -320,7 +320,8 @@  def archive(repo, dest, node, kind, deco
     total = len(files)
     if total:
         files.sort()
-        scmutil.fileprefetchhooks(repo, ctx, files)
+        scmutil.prefetchfiles(repo, [ctx.rev()],
+                              scmutil.matchfiles(repo, files))
         repo.ui.progress(_('archiving'), 0, unit=_('files'), total=total)
         for i, f in enumerate(files):
             ff = ctx.flags(f)
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -2258,16 +2258,15 @@  def cat(ui, repo, ctx, matcher, basefm, 
         mfnode = ctx.manifestnode()
         try:
             if mfnode and mfl[mfnode].find(file)[0]:
-                scmutil.fileprefetchhooks(repo, ctx, [file])
+                scmutil.prefetchfiles(repo, [ctx.rev()], matcher)
                 write(file)
                 return 0
         except KeyError:
             pass
 
-    files = [f for f in ctx.walk(matcher)]
-    scmutil.fileprefetchhooks(repo, ctx, files)
-
-    for abs in files:
+    scmutil.prefetchfiles(repo, [ctx.rev()], matcher)
+
+    for abs in ctx.walk(matcher):
         write(abs)
         err = 0
 
@@ -2945,8 +2944,11 @@  def revert(ui, repo, ctx, parents, *pats
                 _revertprefetch(repo, ctx,
                                 *[actions[name][0] for name in needdata])
             oplist = [actions[name][0] for name in needdata]
-            prefetch = scmutil.fileprefetchhooks
-            prefetch(repo, ctx, [f for sublist in oplist for f in sublist])
+            prefetch = scmutil.prefetchfiles
+            matchfiles = scmutil.matchfiles
+            prefetch(repo, [ctx.rev()],
+                     matchfiles(repo,
+                                [f for sublist in oplist for f in sublist]))
             _performrevert(repo, parents, ctx, actions, interactive, tobackup)
 
         if targetsubs:
diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -1473,8 +1473,11 @@  def _prefetchfiles(repo, ctx, actions):
     # changed/deleted never resolves to something from the remote side.
     oplist = [actions[a] for a in (ACTION_GET, ACTION_DELETED_CHANGED,
                                    ACTION_LOCAL_DIR_RENAME_GET, ACTION_MERGE)]
-    prefetch = scmutil.fileprefetchhooks
-    prefetch(repo, ctx, [f for sublist in oplist for f, args, msg in sublist])
+    prefetch = scmutil.prefetchfiles
+    matchfiles = scmutil.matchfiles
+    prefetch(repo, [ctx.rev()],
+             matchfiles(repo,
+                        [f for sublist in oplist for f, args, msg in sublist]))
 
 @attr.s(frozen=True)
 class updateresult(object):
diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -1357,9 +1357,20 @@  class simplekeyvaluefile(object):
     'unbundle',
 ]
 
-# a list of (repo, ctx, files) functions called by various commands to allow
-# extensions to ensure the corresponding files are available locally, before the
-# command uses them.
+def prefetchfiles(repo, revs, match):
+    """Invokes the registered file prefetch functions, allowing extensions to
+    ensure the corresponding files are available locally, before the command
+    uses them."""
+    if match:
+        # The command itself will complain about files that don't exist, so
+        # don't duplicate the message.
+        match = matchmod.badmatch(match, lambda fn, msg: None)
+    else:
+        match = matchall(repo)
+
+    fileprefetchhooks(repo, revs, match)
+
+# a list of (repo, revs, match) prefetch functions
 fileprefetchhooks = util.hooks()
 
 # A marker that tells the evolve extension to suppress its own reporting
diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -562,7 +562,8 @@  class hgsubrepo(abstractsubrepo):
             files = [f for f in files if match(f)]
         rev = self._state[1]
         ctx = self._repo[rev]
-        scmutil.fileprefetchhooks(self._repo, ctx, files)
+        scmutil.prefetchfiles(self._repo, [ctx.rev()],
+                              scmutil.matchfiles(self._repo, files))
         total = abstractsubrepo.archive(self, archiver, prefix, match)
         for subpath in ctx.substate:
             s = subrepo(ctx, subpath, True)
diff --git a/tests/test-lfs-test-server.t b/tests/test-lfs-test-server.t
--- a/tests/test-lfs-test-server.t
+++ b/tests/test-lfs-test-server.t
@@ -616,7 +616,7 @@  Archive will prefetch blobs in a group
 Cat will prefetch blobs in a group
 
   $ rm -rf .hg/store/lfs `hg config lfs.usercache`
-  $ hg cat --debug -r 1 a b c
+  $ hg cat --debug -r 1 a b c nonexistent
   http auth: user foo, password ***
   http auth: user foo, password ***
   Status: 200
@@ -681,6 +681,7 @@  Cat will prefetch blobs in a group
   THIS-IS-LFS
   lfs: found d11e1a642b60813aee592094109b406089b8dff4cb157157f753418ec7857998 in the local lfs store
   ANOTHER-LARGE-FILE
+  nonexistent: no such file in rev dfca2c9e2ef2
 
 Revert will prefetch blobs in a group