Patchwork [6,of,6,V2] merge: invoke cmdutil.fileprefetchhooks() prior to applying updates

login
register
mail settings
Submitter Matt Harbison
Date Feb. 12, 2018, 3:58 a.m.
Message ID <add5bc8012bbedd24d2b.1518407888@Envy>
Download mbox | patch
Permalink /patch/27651/
State Superseded
Headers show

Comments

Matt Harbison - Feb. 12, 2018, 3:58 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1518373556 18000
#      Sun Feb 11 13:25:56 2018 -0500
# Node ID add5bc8012bbedd24d2b9d3a301bfe0eba892ca4
# Parent  d05ecbfd419747d580100f86a3ffdc3b85574166
merge: invoke cmdutil.fileprefetchhooks() prior to applying updates

This moves the file list calculation into core, so other extensions don't need
to duplicate it.

An alternative to the local import is maybe doing something similar to the
localrepo.prepushoutgoinghooks property.  But both of these things seem like
special cases, and I have no idea which is better/preferred.
Yuya Nishihara - Feb. 12, 2018, 10:29 a.m.
On Sun, 11 Feb 2018 22:58:08 -0500, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1518373556 18000
> #      Sun Feb 11 13:25:56 2018 -0500
> # Node ID add5bc8012bbedd24d2b9d3a301bfe0eba892ca4
> # Parent  d05ecbfd419747d580100f86a3ffdc3b85574166
> merge: invoke cmdutil.fileprefetchhooks() prior to applying updates
> 
> This moves the file list calculation into core, so other extensions don't need
> to duplicate it.
> 
> An alternative to the local import is maybe doing something similar to the
> localrepo.prepushoutgoinghooks property.  But both of these things seem like
> special cases, and I have no idea which is better/preferred.

> diff --git a/mercurial/merge.py b/mercurial/merge.py
> --- a/mercurial/merge.py
> +++ b/mercurial/merge.py
> @@ -1385,6 +1385,19 @@
>      if i > 0:
>          yield i, f
>  
> +def _prefetchfiles(repo, ctx, actions):
> +    """Invoke ``cmdutil.fileprefetchhooks()`` for the files relevant to the dict
> +    of merge actions.  ``ctx`` is the context being merged in."""
> +
> +    # avoid import cycle
> +    from . import cmdutil

cmdutil->merge is reverse dependency. Maybe prefetchfiles() could be moved
to scmutil?

Patch

diff --git a/hgext/lfs/__init__.py b/hgext/lfs/__init__.py
--- a/hgext/lfs/__init__.py
+++ b/hgext/lfs/__init__.py
@@ -333,8 +333,6 @@ 
     wrapfunction(hg, 'clone', wrapper.hgclone)
     wrapfunction(hg, 'postshare', wrapper.hgpostshare)
 
-    wrapfunction(merge, 'applyupdates', wrapper.mergemodapplyupdates)
-
     cmdutil.fileprefetchhooks.add('lfs', wrapper._prefetchfiles)
 
     # Make bundle choose changegroup3 instead of changegroup2. This affects
diff --git a/hgext/lfs/wrapper.py b/hgext/lfs/wrapper.py
--- a/hgext/lfs/wrapper.py
+++ b/hgext/lfs/wrapper.py
@@ -251,9 +251,7 @@ 
 
 def _prefetchfiles(repo, ctx, files):
     """Ensure that required LFS blobs are present, fetching them as a group if
-    needed.
-
-    This is centralized logic for various prefetch hooks."""
+    needed."""
     pointers = []
     localstore = repo.svfs.lfslocalblobstore
 
@@ -266,25 +264,6 @@ 
     if pointers:
         repo.svfs.lfsremoteblobstore.readbatch(pointers, localstore)
 
-def mergemodapplyupdates(orig, repo, actions, wctx, mctx, overwrite,
-                         labels=None):
-    """Ensure that the required LFS blobs are present before applying updates,
-    fetching them as a group if needed.
-
-    This has the effect of ensuring all necessary LFS blobs are present before
-    making working directory changes during an update (including after clone and
-    share) or merge."""
-
-    # Skipping 'a', 'am', 'f', 'r', 'dm', 'e', 'k', 'p' and 'pr', because they
-    # don't touch mctx.  'cd' is skipped, because changed/deleted never resolves
-    # to something from the remote side.
-    oplist = [actions[a] for a in 'g dc dg m'.split()]
-
-    _prefetchfiles(repo, mctx,
-                   [f for sublist in oplist for f, args, msg in sublist])
-
-    return orig(repo, actions, wctx, mctx, overwrite, labels)
-
 def _canskipupload(repo):
     # if remotestore is a null store, upload is a no-op and can be skipped
     return isinstance(repo.svfs.lfsremoteblobstore, blobstore._nullremote)
diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -1385,6 +1385,19 @@ 
     if i > 0:
         yield i, f
 
+def _prefetchfiles(repo, ctx, actions):
+    """Invoke ``cmdutil.fileprefetchhooks()`` for the files relevant to the dict
+    of merge actions.  ``ctx`` is the context being merged in."""
+
+    # avoid import cycle
+    from . import cmdutil
+
+    # Skipping 'a', 'am', 'f', 'r', 'dm', 'e', 'k', 'p' and 'pr', because they
+    # don't touch the context to be merged in.  'cd' is skipped, because
+    # changed/deleted never resolves to something from the remote side.
+    oplist = [actions[a] for a in 'g dc dg m'.split()]
+    prefetch = cmdutil.fileprefetchhooks
+    prefetch(repo, ctx, [f for sublist in oplist for f, args, msg in sublist])
 
 def applyupdates(repo, actions, wctx, mctx, overwrite, labels=None):
     """apply the merge action list to the working directory
@@ -1396,6 +1409,8 @@ 
     describes how many files were affected by the update.
     """
 
+    _prefetchfiles(repo, mctx, actions)
+
     updated, merged, removed = 0, 0, 0
     ms = mergestate.clean(repo, wctx.p1().node(), mctx.node(), labels)
     moves = []