On 10/8/2016 11:48 PM, Pierre-Yves David wrote:
On 10/08/2016 07:06 PM, Mads Kiilerich wrote:
On 10/08/2016 06:11 PM, Ryan McElroy wrote:
# HG changeset patch
# User Ryan McElroy <rmcel...@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.
+1 we should keep the same erro flow than for other patch error
(especially because I'm not sure how --partial deal with this patch.
(The iterhunks docstring seems wrong, for example regarding 'file'
entries and firsthunk. Let's go find pmezard!)
TGVs for Brest leave from Gare Montparnasse every hour.
Cheers,
1/ I can't tell if this is actually still queued or not because of the
follow-up comments. Do I need to send a V2 that keeps the TODO/FIXME line?
2/ I think not crashing is a very good start. I'm okay opening a
follow-up issue to move on to even better behavior, but not crashing
seems imperative.
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
