Patchwork [6,of,6] largefiles: don't duplicate 'actions' into 'actionbyfile'

login
register
mail settings
Submitter Martin von Zweigbergk
Date Dec. 17, 2014, 9:24 p.m.
Message ID <680ee972300c13784796.1418851478@martinvonz.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/7156/
State Superseded
Commit 43b1be4a152bc27d70d21a698e8db4cd21fe445e
Headers show

Comments

Martin von Zweigbergk - Dec. 17, 2014, 9:24 p.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@google.com>
# Date 1418367089 28800
#      Thu Dec 11 22:51:29 2014 -0800
# Node ID 680ee972300c137847962752fd9b08306b068de9
# Parent  d15256ef6ca65ce717fe48306e06e9d143c37bc9
largefiles: don't duplicate 'actions' into 'actionbyfile'
Pierre-Yves David - Dec. 19, 2014, 12:23 a.m.
On 12/17/2014 01:24 PM, Martin von Zweigbergk wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1418367089 28800
> #      Thu Dec 11 22:51:29 2014 -0800
> # Node ID 680ee972300c137847962752fd9b08306b068de9
> # Parent  d15256ef6ca65ce717fe48306e06e9d143c37bc9
> largefiles: don't duplicate 'actions' into 'actionbyfile'

Except for the couple of question I asked, this series looks good to me.
However, I'm not confident enough in the merge code to push it.

>
> diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
> --- a/hgext/largefiles/overrides.py
> +++ b/hgext/largefiles/overrides.py
> @@ -428,8 +428,7 @@
>
>       # Convert to dictionary with filename as key and action as value.
>       lfiles = set()
> -    actionbyfile = actions
> -    for f in actionbyfile.keys():
> +    for f in actions.keys():
>           splitstandin = f and lfutil.splitstandin(f)
>           if splitstandin in p1:
>               lfiles.add(splitstandin)
> @@ -438,8 +437,8 @@
>
>       for lfile in lfiles:
>           standin = lfutil.standin(lfile)
> -        (lm, largs, lmsg) = actionbyfile.get(lfile, (None, None, None))
> -        (sm, sargs, smsg) = actionbyfile.get(standin, (None, None, None))
> +        (lm, largs, lmsg) = actions.get(lfile, (None, None, None))
> +        (sm, sargs, smsg) = actions.get(standin, (None, None, None))
>           if sm in ('g', 'dc') and lm != 'r':
>               # Case 1: normal file in the working copy, largefile in
>               # the second parent
> @@ -447,16 +446,14 @@
>                           'use (l)argefile or keep (n)ormal file?'
>                           '$$ &Largefile $$ &Normal file') % lfile
>               if repo.ui.promptchoice(usermsg, 0) == 0: # pick remote largefile
> -                actionbyfile[lfile] = ('r', None, 'replaced by standin')
> -                actionbyfile[standin] = ('g', sargs, 'replaces standin')
> +                actions[lfile] = ('r', None, 'replaced by standin')
> +                actions[standin] = ('g', sargs, 'replaces standin')
>               else: # keep local normal file
> -                actionbyfile[lfile] = ('k', None, 'replaces standin')
> +                actions[lfile] = ('k', None, 'replaces standin')
>                   if branchmerge:
> -                    actionbyfile[standin] = ('k', None,
> -                                             'replaced by non-standin')
> +                    actions[standin] = ('k', None, 'replaced by non-standin')
>                   else:
> -                    actionbyfile[standin] = ('r', None,
> -                                             'replaced by non-standin')
> +                    actions[standin] = ('r', None, 'replaced by non-standin')
>           elif lm in ('g', 'dc') and sm != 'r':
>               # Case 2: largefile in the working copy, normal file in
>               # the second parent
> @@ -466,21 +463,21 @@
>               if repo.ui.promptchoice(usermsg, 0) == 0: # keep local largefile
>                   if branchmerge:
>                       # largefile can be restored from standin safely
> -                    actionbyfile[lfile] = ('k', None, 'replaced by standin')
> -                    actionbyfile[standin] = ('k', None, 'replaces standin')
> +                    actions[lfile] = ('k', None, 'replaced by standin')
> +                    actions[standin] = ('k', None, 'replaces standin')
>                   else:
>                       # "lfile" should be marked as "removed" without
>                       # removal of itself
> -                    actionbyfile[lfile] = ('lfmr', None,
> -                                           'forget non-standin largefile')
> +                    actions[lfile] = ('lfmr', None,
> +                                      'forget non-standin largefile')
>
>                       # linear-merge should treat this largefile as 're-added'
> -                    actionbyfile[standin] = ('a', None, 'keep standin')
> +                    actions[standin] = ('a', None, 'keep standin')
>               else: # pick remote normal file
> -                actionbyfile[lfile] = ('g', largs, 'replaces standin')
> -                actionbyfile[standin] = ('r', None, 'replaced by non-standin')
> +                actions[lfile] = ('g', largs, 'replaces standin')
> +                actions[standin] = ('r', None, 'replaced by non-standin')
>
> -    return actionbyfile, diverge, renamedelete
> +    return actions, diverge, renamedelete
>
>   def mergerecordupdates(orig, repo, actions, branchmerge):
>       if 'lfmr' in actions:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Augie Fackler - Dec. 22, 2014, 9:28 p.m.
On Thu, Dec 18, 2014 at 04:23:39PM -0800, Pierre-Yves David wrote:
>
>
> On 12/17/2014 01:24 PM, Martin von Zweigbergk wrote:
> ># HG changeset patch
> ># User Martin von Zweigbergk <martinvonz@google.com>
> ># Date 1418367089 28800
> >#      Thu Dec 11 22:51:29 2014 -0800
> ># Node ID 680ee972300c137847962752fd9b08306b068de9
> ># Parent  d15256ef6ca65ce717fe48306e06e9d143c37bc9
> >largefiles: don't duplicate 'actions' into 'actionbyfile'
>
> Except for the couple of question I asked, this series looks good to me.
> However, I'm not confident enough in the merge code to push it.

I agree that this looks reasonable, but would like to see the promised
V2 to resolve some of your comments.

>
> >
> >diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
> >--- a/hgext/largefiles/overrides.py
> >+++ b/hgext/largefiles/overrides.py
> >@@ -428,8 +428,7 @@
> >
> >      # Convert to dictionary with filename as key and action as value.
> >      lfiles = set()
> >-    actionbyfile = actions
> >-    for f in actionbyfile.keys():
> >+    for f in actions.keys():
> >          splitstandin = f and lfutil.splitstandin(f)
> >          if splitstandin in p1:
> >              lfiles.add(splitstandin)
> >@@ -438,8 +437,8 @@
> >
> >      for lfile in lfiles:
> >          standin = lfutil.standin(lfile)
> >-        (lm, largs, lmsg) = actionbyfile.get(lfile, (None, None, None))
> >-        (sm, sargs, smsg) = actionbyfile.get(standin, (None, None, None))
> >+        (lm, largs, lmsg) = actions.get(lfile, (None, None, None))
> >+        (sm, sargs, smsg) = actions.get(standin, (None, None, None))
> >          if sm in ('g', 'dc') and lm != 'r':
> >              # Case 1: normal file in the working copy, largefile in
> >              # the second parent
> >@@ -447,16 +446,14 @@
> >                          'use (l)argefile or keep (n)ormal file?'
> >                          '$$ &Largefile $$ &Normal file') % lfile
> >              if repo.ui.promptchoice(usermsg, 0) == 0: # pick remote largefile
> >-                actionbyfile[lfile] = ('r', None, 'replaced by standin')
> >-                actionbyfile[standin] = ('g', sargs, 'replaces standin')
> >+                actions[lfile] = ('r', None, 'replaced by standin')
> >+                actions[standin] = ('g', sargs, 'replaces standin')
> >              else: # keep local normal file
> >-                actionbyfile[lfile] = ('k', None, 'replaces standin')
> >+                actions[lfile] = ('k', None, 'replaces standin')
> >                  if branchmerge:
> >-                    actionbyfile[standin] = ('k', None,
> >-                                             'replaced by non-standin')
> >+                    actions[standin] = ('k', None, 'replaced by non-standin')
> >                  else:
> >-                    actionbyfile[standin] = ('r', None,
> >-                                             'replaced by non-standin')
> >+                    actions[standin] = ('r', None, 'replaced by non-standin')
> >          elif lm in ('g', 'dc') and sm != 'r':
> >              # Case 2: largefile in the working copy, normal file in
> >              # the second parent
> >@@ -466,21 +463,21 @@
> >              if repo.ui.promptchoice(usermsg, 0) == 0: # keep local largefile
> >                  if branchmerge:
> >                      # largefile can be restored from standin safely
> >-                    actionbyfile[lfile] = ('k', None, 'replaced by standin')
> >-                    actionbyfile[standin] = ('k', None, 'replaces standin')
> >+                    actions[lfile] = ('k', None, 'replaced by standin')
> >+                    actions[standin] = ('k', None, 'replaces standin')
> >                  else:
> >                      # "lfile" should be marked as "removed" without
> >                      # removal of itself
> >-                    actionbyfile[lfile] = ('lfmr', None,
> >-                                           'forget non-standin largefile')
> >+                    actions[lfile] = ('lfmr', None,
> >+                                      'forget non-standin largefile')
> >
> >                      # linear-merge should treat this largefile as 're-added'
> >-                    actionbyfile[standin] = ('a', None, 'keep standin')
> >+                    actions[standin] = ('a', None, 'keep standin')
> >              else: # pick remote normal file
> >-                actionbyfile[lfile] = ('g', largs, 'replaces standin')
> >-                actionbyfile[standin] = ('r', None, 'replaced by non-standin')
> >+                actions[lfile] = ('g', largs, 'replaces standin')
> >+                actions[standin] = ('r', None, 'replaced by non-standin')
> >
> >-    return actionbyfile, diverge, renamedelete
> >+    return actions, diverge, renamedelete
> >
> >  def mergerecordupdates(orig, repo, actions, branchmerge):
> >      if 'lfmr' in actions:
> >_______________________________________________
> >Mercurial-devel mailing list
> >Mercurial-devel@selenic.com
> >http://selenic.com/mailman/listinfo/mercurial-devel
> >
>
> --
> Pierre-Yves David

Patch

diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -428,8 +428,7 @@ 
 
     # Convert to dictionary with filename as key and action as value.
     lfiles = set()
-    actionbyfile = actions
-    for f in actionbyfile.keys():
+    for f in actions.keys():
         splitstandin = f and lfutil.splitstandin(f)
         if splitstandin in p1:
             lfiles.add(splitstandin)
@@ -438,8 +437,8 @@ 
 
     for lfile in lfiles:
         standin = lfutil.standin(lfile)
-        (lm, largs, lmsg) = actionbyfile.get(lfile, (None, None, None))
-        (sm, sargs, smsg) = actionbyfile.get(standin, (None, None, None))
+        (lm, largs, lmsg) = actions.get(lfile, (None, None, None))
+        (sm, sargs, smsg) = actions.get(standin, (None, None, None))
         if sm in ('g', 'dc') and lm != 'r':
             # Case 1: normal file in the working copy, largefile in
             # the second parent
@@ -447,16 +446,14 @@ 
                         'use (l)argefile or keep (n)ormal file?'
                         '$$ &Largefile $$ &Normal file') % lfile
             if repo.ui.promptchoice(usermsg, 0) == 0: # pick remote largefile
-                actionbyfile[lfile] = ('r', None, 'replaced by standin')
-                actionbyfile[standin] = ('g', sargs, 'replaces standin')
+                actions[lfile] = ('r', None, 'replaced by standin')
+                actions[standin] = ('g', sargs, 'replaces standin')
             else: # keep local normal file
-                actionbyfile[lfile] = ('k', None, 'replaces standin')
+                actions[lfile] = ('k', None, 'replaces standin')
                 if branchmerge:
-                    actionbyfile[standin] = ('k', None,
-                                             'replaced by non-standin')
+                    actions[standin] = ('k', None, 'replaced by non-standin')
                 else:
-                    actionbyfile[standin] = ('r', None,
-                                             'replaced by non-standin')
+                    actions[standin] = ('r', None, 'replaced by non-standin')
         elif lm in ('g', 'dc') and sm != 'r':
             # Case 2: largefile in the working copy, normal file in
             # the second parent
@@ -466,21 +463,21 @@ 
             if repo.ui.promptchoice(usermsg, 0) == 0: # keep local largefile
                 if branchmerge:
                     # largefile can be restored from standin safely
-                    actionbyfile[lfile] = ('k', None, 'replaced by standin')
-                    actionbyfile[standin] = ('k', None, 'replaces standin')
+                    actions[lfile] = ('k', None, 'replaced by standin')
+                    actions[standin] = ('k', None, 'replaces standin')
                 else:
                     # "lfile" should be marked as "removed" without
                     # removal of itself
-                    actionbyfile[lfile] = ('lfmr', None,
-                                           'forget non-standin largefile')
+                    actions[lfile] = ('lfmr', None,
+                                      'forget non-standin largefile')
 
                     # linear-merge should treat this largefile as 're-added'
-                    actionbyfile[standin] = ('a', None, 'keep standin')
+                    actions[standin] = ('a', None, 'keep standin')
             else: # pick remote normal file
-                actionbyfile[lfile] = ('g', largs, 'replaces standin')
-                actionbyfile[standin] = ('r', None, 'replaced by non-standin')
+                actions[lfile] = ('g', largs, 'replaces standin')
+                actions[standin] = ('r', None, 'replaced by non-standin')
 
-    return actionbyfile, diverge, renamedelete
+    return actions, diverge, renamedelete
 
 def mergerecordupdates(orig, repo, actions, branchmerge):
     if 'lfmr' in actions: