Patchwork import: abort instead of crashing when copy source does not exist (issue5375)

login
register
mail settings
Submitter Ryan McElroy
Date Oct. 8, 2016, 4:11 p.m.
Message ID <961569a2cbeebfd54c93.1475943098@devbig314.prn1.facebook.com>
Download mbox | patch
Permalink /patch/16941/
State Accepted
Headers show

Comments

Ryan McElroy - Oct. 8, 2016, 4:11 p.m.
# HG changeset patch
# User Ryan McElroy <rmcelroy@fb.com>
# Date 1475929618 25200
#      Sat Oct 08 05:26:58 2016 -0700
# Node ID 961569a2cbeebfd54c9369c6e36d03d4938aef38
# Parent  dbcef8918bbdd8a64d9f79a37bcfa284a26f3a39
import: abort instead of crashing when copy source does not exist (issue5375)

Previously, when a patch contained a move or copy from a source that did not
exist, `hg import` would crash. This patch changes import to raise a PatchError
with an explanantion of what is wrong with the patch to avoid the stack trace
and bad user experience.
Augie Fackler - Oct. 8, 2016, 4:31 p.m.
On Sat, Oct 08, 2016 at 09:11:38AM -0700, Ryan McElroy wrote:
> # HG changeset patch
> # User Ryan McElroy <rmcelroy@fb.com>
> # Date 1475929618 25200
> #      Sat Oct 08 05:26:58 2016 -0700
> # Node ID 961569a2cbeebfd54c9369c6e36d03d4938aef38
> # Parent  dbcef8918bbdd8a64d9f79a37bcfa284a26f3a39
> import: abort instead of crashing when copy source does not exist (issue5375)

Queued, thanks

>
> Previously, when a patch contained a move or copy from a source that did not
> exist, `hg import` would crash. This patch changes import to raise a PatchError
> with an explanantion of what is wrong with the patch to avoid the stack trace
> and bad user experience.
>
> diff --git a/mercurial/patch.py b/mercurial/patch.py
> --- a/mercurial/patch.py
> +++ b/mercurial/patch.py
> @@ -1952,8 +1952,10 @@ def _applydiff(ui, fp, patcher, backend,
>                  data, mode = None, None
>                  if gp.op in ('RENAME', 'COPY'):
>                      data, mode = store.getfile(gp.oldpath)[:2]
> -                    # FIXME: failing getfile has never been handled here
> -                    assert data is not None
> +                    if data is None:
> +                        # This means that the old path does not exist
> +                        raise PatchError(_("source file '%s' does not exist")
> +                                           % gp.oldpath)
>                  if gp.mode:
>                      mode = gp.mode
>                      if gp.op == 'ADD':
> diff --git a/tests/test-import.t b/tests/test-import.t
> --- a/tests/test-import.t
> +++ b/tests/test-import.t
> @@ -1793,3 +1793,13 @@ repository when file not found for patch
>    1 out of 1 hunks FAILED -- saving rejects to file file1.rej
>    abort: patch failed to apply
>    [255]
> +
> +test import crash (issue5375)
> +  $ cd ..
> +  $ hg init repo
> +  $ cd repo
> +  $ printf "diff --git a/a b/b\nrename from a\nrename to b" | hg import -
> +  applying patch from stdin
> +  a not tracked!
> +  abort: source file 'a' does not exist
> +  [255]
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/patch.py b/mercurial/patch.py
--- a/mercurial/patch.py
+++ b/mercurial/patch.py
@@ -1952,8 +1952,10 @@  def _applydiff(ui, fp, patcher, backend,
                 data, mode = None, None
                 if gp.op in ('RENAME', 'COPY'):
                     data, mode = store.getfile(gp.oldpath)[:2]
-                    # FIXME: failing getfile has never been handled here
-                    assert data is not None
+                    if data is None:
+                        # This means that the old path does not exist
+                        raise PatchError(_("source file '%s' does not exist")
+                                           % gp.oldpath)
                 if gp.mode:
                     mode = gp.mode
                     if gp.op == 'ADD':
diff --git a/tests/test-import.t b/tests/test-import.t
--- a/tests/test-import.t
+++ b/tests/test-import.t
@@ -1793,3 +1793,13 @@  repository when file not found for patch
   1 out of 1 hunks FAILED -- saving rejects to file file1.rej
   abort: patch failed to apply
   [255]
+
+test import crash (issue5375)
+  $ cd ..
+  $ hg init repo
+  $ cd repo
+  $ printf "diff --git a/a b/b\nrename from a\nrename to b" | hg import -
+  applying patch from stdin
+  a not tracked!
+  abort: source file 'a' does not exist
+  [255]