[PATCH] import: abort instead of crashing when copy source does not exist (issue5375)
Mads Kiilerich
mads at kiilerich.com
Sat Oct 8 13:06:15 EDT 2016
On 10/08/2016 06:11 PM, Ryan McElroy wrote:
> # HG changeset patch
> # User Ryan McElroy <rmcelroy at 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.
>
> 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]
Hmm.
For a patch modifying a non-existing file we already get:
unable to find 'f' for patching
1 out of 1 hunks FAILED -- saving rejects to file f.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
$ cat f.rej
--- f
+++ f
@@ -1,1 +1,2 @@
+f
In the tested case, I would thus also expect it to leave a .rej file
with the failing rename "hunk" while applying the rest of the patch
(even though a pure rename arguably *doesn't* have any hunks).
BUT the logic around this check seems wrong. A rename or copy of a
missing file should be handled exactly the same, no matter if it is a
bare rename/copy or if it also modifies the file (= has a first hunk).
I don't know if it is better to give a not-entirely-correct abort than
to fail with an assertion error, but I think it still deserves a
FIXME/TODO.
(The iterhunks docstring seems wrong, for example regarding 'file'
entries and firsthunk. Let's go find pmezard!)
/Mads
More information about the Mercurial-devel
mailing list