Patchwork [6,of,6] merge: use None as filename for base in 'both created' conflicts

login
register
mail settings
Submitter Martin von Zweigbergk
Date Nov. 25, 2014, 5:22 a.m.
Message ID <701701b84bba376ab63a.1416892972@martinvonz.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/6848/
State Accepted
Commit 9da5a7413eb8bc3e708eee62bc38342c8ff7f917
Delegated to: Pierre-Yves David
Headers show

Comments

Martin von Zweigbergk - Nov. 25, 2014, 5:22 a.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@google.com>
# Date 1416874622 28800
#      Mon Nov 24 16:17:02 2014 -0800
# Node ID 701701b84bba376ab63af472f3dc57730ba4c251
# Parent  9a17f0a1c28705f0d7fdf7732d94f27087892dad
merge: use None as filename for base in 'both created' conflicts

Instead of using a file that we know is not in the common ancestor's
maniffest, let's use None. This is safe as the only place that cares
about the value (applyupdates) already checks if the item exists in
the ancestor.
Augie Fackler - Nov. 25, 2014, 3:38 p.m.
On Mon, Nov 24, 2014 at 09:22:52PM -0800, Martin von Zweigbergk wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1416874622 28800
> #      Mon Nov 24 16:17:02 2014 -0800
> # Node ID 701701b84bba376ab63af472f3dc57730ba4c251
> # Parent  9a17f0a1c28705f0d7fdf7732d94f27087892dad
> merge: use None as filename for base in 'both created' conflicts

Series looks good, but I think Pierre-Yves should review before this
goes in.

>
> Instead of using a file that we know is not in the common ancestor's
> maniffest, let's use None. This is safe as the only place that cares

Typo here: s/ff/f/

(don't resend just for that though.)

> about the value (applyupdates) already checks if the item exists in
> the ancestor.
>
> diff --git a/mercurial/merge.py b/mercurial/merge.py
> --- a/mercurial/merge.py
> +++ b/mercurial/merge.py
> @@ -421,9 +421,7 @@
>                      actions['m'].append((f, (f, f, fa, False, pa.node()),
>                                     "both renamed from " + fa))
>                  else:
> -                    # Note: f as ancestor is wrong - we can't really make a
> -                    # 3-way merge without an ancestor file.
> -                    actions['m'].append((f, (f, f, f, False, pa.node()),
> +                    actions['m'].append((f, (f, f, None, False, pa.node()),
>                                     "both created"))
>              else:
>                  a = ma[f]
> @@ -493,8 +491,7 @@
>              else:
>                  different = _checkunknownfile(repo, wctx, p2, f)
>                  if force and branchmerge and different:
> -                    # FIXME: This is wrong - f is not in ma ...
> -                    actions['m'].append((f, (f, f, f, False, pa.node()),
> +                    actions['m'].append((f, (f, f, None, False, pa.node()),
>                                      "remote differs from untracked local"))
>                  elif not force and different:
>                      aborts.append((f, 'ud'))
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - Nov. 25, 2014, 11:49 p.m.
On 11/25/2014 07:38 AM, Augie Fackler wrote:
> On Mon, Nov 24, 2014 at 09:22:52PM -0800, Martin von Zweigbergk wrote:
>> # HG changeset patch
>> # User Martin von Zweigbergk <martinvonz@google.com>
>> # Date 1416874622 28800
>> #      Mon Nov 24 16:17:02 2014 -0800
>> # Node ID 701701b84bba376ab63af472f3dc57730ba4c251
>> # Parent  9a17f0a1c28705f0d7fdf7732d94f27087892dad
>> merge: use None as filename for base in 'both created' conflicts
>
> Series looks good, but I think Pierre-Yves should review before this
> goes in.

This patches looks good to me.
Augie Fackler - Nov. 28, 2014, 3:59 p.m.
On Mon, Nov 24, 2014 at 09:22:52PM -0800, Martin von Zweigbergk wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1416874622 28800
> #      Mon Nov 24 16:17:02 2014 -0800
> # Node ID 701701b84bba376ab63af472f3dc57730ba4c251
> # Parent  9a17f0a1c28705f0d7fdf7732d94f27087892dad
> merge: use None as filename for base in 'both created' conflicts

queued per marmoute's review

>
> Instead of using a file that we know is not in the common ancestor's
> maniffest, let's use None. This is safe as the only place that cares
> about the value (applyupdates) already checks if the item exists in
> the ancestor.
>
> diff --git a/mercurial/merge.py b/mercurial/merge.py
> --- a/mercurial/merge.py
> +++ b/mercurial/merge.py
> @@ -421,9 +421,7 @@
>                      actions['m'].append((f, (f, f, fa, False, pa.node()),
>                                     "both renamed from " + fa))
>                  else:
> -                    # Note: f as ancestor is wrong - we can't really make a
> -                    # 3-way merge without an ancestor file.
> -                    actions['m'].append((f, (f, f, f, False, pa.node()),
> +                    actions['m'].append((f, (f, f, None, False, pa.node()),
>                                     "both created"))
>              else:
>                  a = ma[f]
> @@ -493,8 +491,7 @@
>              else:
>                  different = _checkunknownfile(repo, wctx, p2, f)
>                  if force and branchmerge and different:
> -                    # FIXME: This is wrong - f is not in ma ...
> -                    actions['m'].append((f, (f, f, f, False, pa.node()),
> +                    actions['m'].append((f, (f, f, None, False, pa.node()),
>                                      "remote differs from untracked local"))
>                  elif not force and different:
>                      aborts.append((f, 'ud'))
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -421,9 +421,7 @@ 
                     actions['m'].append((f, (f, f, fa, False, pa.node()),
                                    "both renamed from " + fa))
                 else:
-                    # Note: f as ancestor is wrong - we can't really make a
-                    # 3-way merge without an ancestor file.
-                    actions['m'].append((f, (f, f, f, False, pa.node()),
+                    actions['m'].append((f, (f, f, None, False, pa.node()),
                                    "both created"))
             else:
                 a = ma[f]
@@ -493,8 +491,7 @@ 
             else:
                 different = _checkunknownfile(repo, wctx, p2, f)
                 if force and branchmerge and different:
-                    # FIXME: This is wrong - f is not in ma ...
-                    actions['m'].append((f, (f, f, f, False, pa.node()),
+                    actions['m'].append((f, (f, f, None, False, pa.node()),
                                     "remote differs from untracked local"))
                 elif not force and different:
                     aborts.append((f, 'ud'))