Patchwork [2,of,6,mergedriver] mergestate: explicitly forget 'dc' conflicts where the deleted side is picked

login
register
mail settings
Submitter Siddharth Agarwal
Date Nov. 24, 2015, 10:02 p.m.
Message ID <f959d838869ced41c19d.1448402557@dev666.prn1.facebook.com>
Download mbox | patch
Permalink /patch/11611/
State Accepted
Delegated to: Martin von Zweigbergk
Headers show

Comments

Siddharth Agarwal - Nov. 24, 2015, 10:02 p.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1448330605 28800
#      Mon Nov 23 18:03:25 2015 -0800
# Node ID f959d838869ced41c19dbcc1c0abd936cc84b972
# Parent  4adc3f379dc67ac23d5377206b3179ebe5099765
# Available At http://42.netv6.net/sid0-wip/hg/
#              hg pull http://42.netv6.net/sid0-wip/hg/ -r f959d838869c
mergestate: explicitly forget 'dc' conflicts where the deleted side is picked

Once we move change/delete conflicts into the resolve phase, a 'dc' file might
first be resolved by picking the other side, then later be resolved by picking
the local side. For this transition we want to make sure that the file goes
back to not being in the dirstate.

This has no impact on conflicts during the initial merge.
Martin von Zweigbergk - Nov. 28, 2015, 6:14 a.m.
On Tue, Nov 24, 2015 at 2:04 PM Siddharth Agarwal <sid0@fb.com> wrote:

> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1448330605 28800
> #      Mon Nov 23 18:03:25 2015 -0800
> # Node ID f959d838869ced41c19dbcc1c0abd936cc84b972
> # Parent  4adc3f379dc67ac23d5377206b3179ebe5099765
> # Available At http://42.netv6.net/sid0-wip/hg/
> #              hg pull http://42.netv6.net/sid0-wip/hg/ -r f959d838869c
> mergestate: explicitly forget 'dc' conflicts where the deleted side is
> picked
>
> Once we move change/delete conflicts into the resolve phase, a 'dc' file
> might
> first be resolved by picking the other side, then later be resolved by
> picking
> the local side. For this transition we want to make sure that the file goes
> back to not being in the dirstate.
>
> This has no impact on conflicts during the initial merge.
>
> diff --git a/mercurial/merge.py b/mercurial/merge.py
> --- a/mercurial/merge.py
> +++ b/mercurial/merge.py
> @@ -458,10 +458,13 @@ class mergestate(object):
>          if complete:
>              action = None
>              if deleted:
> -                if not fcd.isabsent():
> +                if fcd.isabsent():
> +                    # dc: local picked. Need to drop if present, which may
> +                    # happen on re-resolves.
> +                    action = 'f'
>

Does 'f' stand for 'forget'? Will the file also be removed if it exists, or
will it just become untracked (eventually, when this feature is done))?


> +                else:
>                      # cd: remote picked (or otherwise deleted)
>                      action = 'r'
> -                # else: dc: local picked (no action necessary)
>              else:
>                  if fcd.isabsent(): # dc: remote picked
>                      action = 'g'
> @@ -511,7 +514,7 @@ class mergestate(object):
>
>      def actions(self):
>          """return lists of actions to perform on the dirstate"""
> -        actions = {'r': [], 'a': [], 'g': []}
> +        actions = {'r': [], 'f': [], 'a': [], 'g': []}
>          for f, (r, action) in self._results.iteritems():
>              if action is not None:
>                  actions[action].append((f, None, "merge result"))
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
>
Siddharth Agarwal - Nov. 29, 2015, 1:58 a.m.
On 11/27/15 22:14, Martin von Zweigbergk wrote:
> Does 'f' stand for 'forget'? Will the file also be removed if it 
> exists, or will it just become untracked (eventually, when this 
> feature is done))?

Yeah, 'f' stands for forget (or 'drop', as the internal APIs call it). 
The file will just become untracked -- it will not be removed from disk.
Siddharth Agarwal - Nov. 29, 2015, 1:59 a.m.
On 11/28/15 17:58, Siddharth Agarwal wrote:
> On 11/27/15 22:14, Martin von Zweigbergk wrote:
>> Does 'f' stand for 'forget'? Will the file also be removed if it 
>> exists, or will it just become untracked (eventually, when this 
>> feature is done))?
>
> Yeah, 'f' stands for forget (or 'drop', as the internal APIs call it). 
> The file will just become untracked -- it will not be removed from disk.

Actually, at this point since the file is deleted ('deleted' is true) 
the file isn't on disk anyway, so the point is moot. However we won't 
take any further action.
Martin von Zweigbergk - Nov. 29, 2015, 5:01 a.m.
On Sat, Nov 28, 2015 at 5:59 PM Siddharth Agarwal <sid@less-broken.com>
wrote:

> On 11/28/15 17:58, Siddharth Agarwal wrote:
> > On 11/27/15 22:14, Martin von Zweigbergk wrote:
> >> Does 'f' stand for 'forget'? Will the file also be removed if it
> >> exists, or will it just become untracked (eventually, when this
> >> feature is done))?
> >
> > Yeah, 'f' stands for forget (or 'drop', as the internal APIs call it).
> > The file will just become untracked -- it will not be removed from disk.
>
> Actually, at this point since the file is deleted ('deleted' is true)
> the file isn't on disk anyway, so the point is moot. However we won't
> take any further action.
>

I see, it was deleted from disk in filemerge.py, and this is just about
updating the dirstate.

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -458,10 +458,13 @@  class mergestate(object):
         if complete:
             action = None
             if deleted:
-                if not fcd.isabsent():
+                if fcd.isabsent():
+                    # dc: local picked. Need to drop if present, which may
+                    # happen on re-resolves.
+                    action = 'f'
+                else:
                     # cd: remote picked (or otherwise deleted)
                     action = 'r'
-                # else: dc: local picked (no action necessary)
             else:
                 if fcd.isabsent(): # dc: remote picked
                     action = 'g'
@@ -511,7 +514,7 @@  class mergestate(object):
 
     def actions(self):
         """return lists of actions to perform on the dirstate"""
-        actions = {'r': [], 'a': [], 'g': []}
+        actions = {'r': [], 'f': [], 'a': [], 'g': []}
         for f, (r, action) in self._results.iteritems():
             if action is not None:
                 actions[action].append((f, None, "merge result"))