Patchwork [6,of,7] largefiles: rewrite merge code using dictionary with entry per file

login
register
mail settings
Submitter Martin von Zweigbergk
Date Dec. 10, 2014, 9:09 p.m.
Message ID <6527416436b7cfb80192.1418245744@martinvonz.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/7049/
State Accepted
Commit 38e55e55ae4d21c67a82e5663bec1d2b664f7623
Headers show

Comments

Martin von Zweigbergk - Dec. 10, 2014, 9:09 p.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@google.com>
# Date 1418191433 28800
#      Tue Dec 09 22:03:53 2014 -0800
# Node ID 6527416436b7cfb801924e898783791a65a2e912
# Parent  3b6976a44ab9e1ecd2dd228c0269bc5319f9df2b
largefiles: rewrite merge code using dictionary with entry per file

In overridecalculateupdates(), we currently only deal with conflicts
that result in a 'g' action for either the largefile or a standin. We
will soon want to deal cases with 'cd' and 'dc' actions here. It will
be easier to reason about such cases if we rewrite it using a dict
from filename to action.

A side-effect of this change is that the output can only have one
action per file (which should be a good change). Before this change,
when one of the tests in test-issue3084 received this input (the 'a'
in the input was a result of 'cd' conflict resolved in favor of the
modified file):

  'g': [('.hglf/f', ('',), 'remote created')],
  'a': [('f', None, 'prompt keep')],

and the user chose to keep the local largefile, it produced this
output:


  'g': [('.hglf/f', ('',), 'remote created')],
  'r': [('f', None, 'replaced by standin')],
  'a': [('f', None, 'prompt keep')],

Although 'a' actions are processed after 'r' actions by
recordupdates(), it still worked because 'a' actions have no effect on
merges (only on updates). After this change, the output is:

  'g': [('.hglf/f', ('',), 'remote created')],
  'r': [('f', None, 'replaced by standin')],

Similarly, there are several tests in test-largefiles-update that get
inputs like:

  'a': [('.hglf/large2', None, 'prompt keep')],
  'g': [('large2', ('',), 'remote created')],

and when the user chooses to keep the local largefile, they produce
this output:

  'a': [('.hglf/large2', None, 'prompt keep'),
        ('.hglf/large2', None, 'keep standin')],
  'lfmr': [('large2', None, 'forget non-standin largefile')],

In this case, it was not a merge but an update, so the 'a' action does
have an effect. However, since dirstate.add() is idempotent, it still
has no obserable effect.

After this change, the output is:

  'a': [('.hglf/large2', None, 'keep standin')],
  'lfmr': [('large2', None, 'forget non-standin largefile')],

Patch

diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -425,10 +425,14 @@ 
     if overwrite:
         return actions, diverge, renamedelete
 
+    # Convert to dictionary with filename as key and action as value.
+    actionbyfile = {}
+    for m, l in actions.iteritems():
+        for f, args, msg in l:
+            actionbyfile[f] = m, args, msg
+
     removes = set(a[0] for a in actions['r'])
 
-    newglist = []
-    lfmr = [] # LargeFiles: Mark as Removed
     for action in actions['g']:
         f, args, msg = action
         splitstandin = f and lfutil.splitstandin(f)
@@ -442,15 +446,14 @@ 
                         'use (l)argefile or keep (n)ormal file?'
                         '$$ &Largefile $$ &Normal file') % lfile
             if repo.ui.promptchoice(usermsg, 0) == 0: # pick remote largefile
-                actions['r'].append((lfile, None, 'replaced by standin'))
-                newglist.append(action)
+                actionbyfile[lfile] = ('r', None, 'replaced by standin')
             else: # keep local normal file
                 if branchmerge:
-                    actions['k'].append((standin, None,
-                                         'replaced by non-standin'))
+                    actionbyfile[standin] = ('k', None,
+                                             'replaced by non-standin')
                 else:
-                    actions['r'].append((standin, None,
-                                         'replaced by non-standin'))
+                    actionbyfile[standin] = ('r', None,
+                                             'replaced by non-standin')
         elif lfutil.standin(f) in p1 and lfutil.standin(f) not in removes:
             # Case 2: largefile in the working copy, normal file in
             # the second parent
@@ -462,23 +465,24 @@ 
             if repo.ui.promptchoice(usermsg, 0) == 0: # keep local largefile
                 if branchmerge:
                     # largefile can be restored from standin safely
-                    actions['k'].append((lfile, None, 'replaced by standin'))
+                    actionbyfile[lfile] = ('k', None, 'replaced by standin')
                 else:
                     # "lfile" should be marked as "removed" without
                     # removal of itself
-                    lfmr.append((lfile, None, 'forget non-standin largefile'))
+                    actionbyfile[lfile] = ('lfmr', None,
+                                           'forget non-standin largefile')
 
                     # linear-merge should treat this largefile as 're-added'
-                    actions['a'].append((standin, None, 'keep standin'))
+                    actionbyfile[standin] = ('a', None, 'keep standin')
             else: # pick remote normal file
-                actions['r'].append((standin, None, 'replaced by non-standin'))
-                newglist.append(action)
-        else:
-            newglist.append(action)
+                actionbyfile[standin] = ('r', None, 'replaced by non-standin')
 
-    actions['g'] = newglist
-    if lfmr:
-        actions['lfmr'] = lfmr
+    # Convert back to dictionary-of-lists format
+    for l in actions.itervalues():
+        l[:] = []
+    actions['lfmr'] = []
+    for f, (m, args, msg) in actionbyfile.iteritems():
+        actions[m].append((f, args, msg))
 
     return actions, diverge, renamedelete