Patchwork [3,of,6,mergedriver] filemerge: in ':fail' tool, write out other side if local side is deleted

login
register
mail settings
Submitter Siddharth Agarwal
Date Nov. 24, 2015, 10:02 p.m.
Message ID <2ba8e74e9de139060fdb.1448402558@dev666.prn1.facebook.com>
Download mbox | patch
Permalink /patch/11615/
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 1448391421 28800
#      Tue Nov 24 10:57:01 2015 -0800
# Node ID 2ba8e74e9de139060fdbe67ac769e5fcf0f27651
# Parent  f959d838869ced41c19dbcc1c0abd936cc84b972
# Available At http://42.netv6.net/sid0-wip/hg/
#              hg pull http://42.netv6.net/sid0-wip/hg/ -r 2ba8e74e9de1
filemerge: in ':fail' tool, write out other side if local side is deleted

We do this because we don't want to modify the dirstate for failures, and don't
just want to leave the file missing from disk. Plus it's more useful for the
user if the changed side is written out -- it is easier to delete a file than
to get it back via hg revert.
Martin von Zweigbergk - Nov. 28, 2015, 6:21 a.m.
On Tue, Nov 24, 2015 at 2:08 PM Siddharth Agarwal <sid0@fb.com> wrote:

> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1448391421 28800
> #      Tue Nov 24 10:57:01 2015 -0800
> # Node ID 2ba8e74e9de139060fdbe67ac769e5fcf0f27651
> # Parent  f959d838869ced41c19dbcc1c0abd936cc84b972
> # Available At http://42.netv6.net/sid0-wip/hg/
> #              hg pull http://42.netv6.net/sid0-wip/hg/ -r 2ba8e74e9de1
> filemerge: in ':fail' tool, write out other side if local side is deleted
>
> We do this because we don't want to modify the dirstate for failures, and
> don't
> just want to leave the file missing from disk. Plus it's more useful for
> the
> user if the changed side is written out -- it is easier to delete a file
> than
> to get it back via hg revert.


Good, I agree with this. It also seems consistent that the resolution of a
change/delete or delete/change delete is to leave the "change" side there,
whether that's remote or local.

No tests affected? Should we add one?


>
> diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
> --- a/mercurial/filemerge.py
> +++ b/mercurial/filemerge.py
> @@ -285,6 +285,9 @@ def _ifail(repo, mynode, orig, fcd, fco,
>      Rather than attempting to merge files that were modified on both
>      branches, it marks them as unresolved. The resolve command must be
>      used to resolve these conflicts."""
> +    # for change/delete conflicts write out the changed version, then fail
> +    if fcd.isabsent():
> +        repo.wwrite(fcd.path(), fco.data(), fco.flags())
>      return 1, False
>
>  def _premerge(repo, fcd, fco, fca, toolconf, files, labels=None):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
>
Siddharth Agarwal - Nov. 29, 2015, 1:56 a.m.
On 11/27/15 22:21, Martin von Zweigbergk wrote:
> Good, I agree with this. It also seems consistent that the resolution 
> of a change/delete or delete/change delete is to leave the "change" 
> side there, whether that's remote or local.

Right.

>
> No tests affected? Should we add one?

Nothing's affected because change/delete conflicts aren't going through 
this code yet. Once they do this code will all be tested. I have some 
extensive tests coming up in 
http://42.netv6.net/sid0-wip/hg/rev/51c4b0e60f6c.
Martin von Zweigbergk - Nov. 29, 2015, 4:57 a.m.
On Sat, Nov 28, 2015 at 5:56 PM Siddharth Agarwal <sid@less-broken.com>
wrote:

> On 11/27/15 22:21, Martin von Zweigbergk wrote:
> >
> > No tests affected? Should we add one?
>
> Nothing's affected because change/delete conflicts aren't going through
> this code yet.


Oh,  of course. Sorry about my stupid questions :-P

Patch

diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
--- a/mercurial/filemerge.py
+++ b/mercurial/filemerge.py
@@ -285,6 +285,9 @@  def _ifail(repo, mynode, orig, fcd, fco,
     Rather than attempting to merge files that were modified on both
     branches, it marks them as unresolved. The resolve command must be
     used to resolve these conflicts."""
+    # for change/delete conflicts write out the changed version, then fail
+    if fcd.isabsent():
+        repo.wwrite(fcd.path(), fco.data(), fco.flags())
     return 1, False
 
 def _premerge(repo, fcd, fco, fca, toolconf, files, labels=None):