Patchwork D11824: errors: return more detailed errors when failing to parse or apply patch

login
register
mail settings
Submitter phabricator
Date Nov. 29, 2021, 10 p.m.
Message ID <differential-rev-PHID-DREV-hdohar5p3yuikbr2kec4-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/50156/
State Superseded
Headers show

Comments

phabricator - Nov. 29, 2021, 10 p.m.
martinvonz created this revision.
Herald added a reviewer: durin42.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  This patch adds subclasses of `PatchError` so we can distinguish
  between failure to parse a patch from failure to apply it. It updates
  the callers to raise either `InputError` or `StateError` depending on
  which type of error occurred.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D11824

AFFECTED FILES
  mercurial/cmdutil.py
  mercurial/error.py
  mercurial/patch.py
  tests/test-commit-interactive.t
  tests/test-import-bypass.t
  tests/test-import-git.t
  tests/test-import-unknown.t
  tests/test-import.t
  tests/test-narrow-expanddirstate.t

CHANGE DETAILS




To: martinvonz, durin42, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/tests/test-narrow-expanddirstate.t b/tests/test-narrow-expanddirstate.t
--- a/tests/test-narrow-expanddirstate.t
+++ b/tests/test-narrow-expanddirstate.t
@@ -142,7 +142,7 @@ 
   Hunk #1 FAILED at 0
   1 out of 1 hunks FAILED -- saving rejects to file patchdir/f3.rej
   abort: patch failed to apply
-  [255]
+  [20]
   $ hg tracked | grep patchdir
   [1]
   $ hg files | grep patchdir > /dev/null
diff --git a/tests/test-import.t b/tests/test-import.t
--- a/tests/test-import.t
+++ b/tests/test-import.t
@@ -234,7 +234,7 @@ 
   $ hg --cwd b import -mpatch ../broken.patch
   applying ../broken.patch
   abort: bad hunk #1
-  [255]
+  [10]
   $ rm -r b
 
 hg -R repo import
@@ -834,7 +834,7 @@ 
   Hunk #1 FAILED at 0
   1 out of 1 hunks FAILED -- saving rejects to file a.rej
   abort: patch failed to apply
-  [255]
+  [20]
   $ hg import --no-commit -v fuzzy-tip.patch
   applying fuzzy-tip.patch
   patching file a
@@ -853,7 +853,7 @@ 
   Hunk #1 FAILED at 0
   1 out of 1 hunks FAILED -- saving rejects to file a.rej
   abort: patch failed to apply
-  [255]
+  [20]
   $ hg up -qC
   $ hg import --config patch.fuzz=2 --exact fuzzy-reparent.patch
   applying fuzzy-reparent.patch
@@ -2054,7 +2054,7 @@ 
   (use '--prefix' to apply patch relative to the current directory)
   1 out of 1 hunks FAILED -- saving rejects to file file1.rej
   abort: patch failed to apply
-  [255]
+  [20]
 
 test import crash (issue5375)
   $ cd ..
@@ -2064,7 +2064,7 @@ 
   applying patch from stdin
   a not tracked!
   abort: source file 'a' does not exist
-  [255]
+  [20]
 
 test immature end of hunk
 
@@ -2076,7 +2076,7 @@ 
   > EOF
   applying patch from stdin
   abort: bad hunk #1: incomplete hunk
-  [255]
+  [10]
 
   $ hg import - <<'EOF'
   > diff --git a/foo b/foo
@@ -2087,4 +2087,4 @@ 
   > EOF
   applying patch from stdin
   abort: bad hunk #1: incomplete hunk
-  [255]
+  [10]
diff --git a/tests/test-import-unknown.t b/tests/test-import-unknown.t
--- a/tests/test-import-unknown.t
+++ b/tests/test-import-unknown.t
@@ -29,7 +29,7 @@ 
   file added already exists
   1 out of 1 hunks FAILED -- saving rejects to file added.rej
   abort: patch failed to apply
-  [255]
+  [20]
 
 Test modifying an unknown file
 
@@ -41,7 +41,7 @@ 
   $ hg import --no-commit ../unknown.diff
   applying ../unknown.diff
   abort: cannot patch changed: file is not tracked
-  [255]
+  [20]
 
 Test removing an unknown file
 
@@ -54,7 +54,7 @@ 
   $ hg import --no-commit ../unknown.diff
   applying ../unknown.diff
   abort: cannot patch removed: file is not tracked
-  [255]
+  [20]
 
 Test copying onto an unknown file
 
@@ -64,6 +64,6 @@ 
   $ hg import --no-commit ../unknown.diff
   applying ../unknown.diff
   abort: cannot create copied: destination already exists
-  [255]
+  [20]
 
   $ cd ..
diff --git a/tests/test-import-git.t b/tests/test-import-git.t
--- a/tests/test-import-git.t
+++ b/tests/test-import-git.t
@@ -519,7 +519,7 @@ 
   > EOF
   applying patch from stdin
   abort: could not decode "binary2" binary patch: bad base85 character at position 6
-  [255]
+  [10]
 
   $ hg revert -aq
   $ hg import -d "1000000 0" -m rename-as-binary - <<"EOF"
@@ -534,7 +534,7 @@ 
   > EOF
   applying patch from stdin
   abort: "binary2" length is 5 bytes, should be 6
-  [255]
+  [10]
 
   $ hg revert -aq
   $ hg import -d "1000000 0" -m rename-as-binary - <<"EOF"
@@ -548,7 +548,7 @@ 
   > EOF
   applying patch from stdin
   abort: could not extract "binary2" binary data
-  [255]
+  [10]
 
 Simulate a copy/paste turning LF into CRLF (issue2870)
 
@@ -748,7 +748,7 @@ 
   > EOF
   applying patch from stdin
   abort: cannot create b: destination already exists
-  [255]
+  [20]
   $ cat b
   b
 
@@ -768,7 +768,7 @@ 
   cannot create b: destination already exists
   1 out of 1 hunks FAILED -- saving rejects to file b.rej
   abort: patch failed to apply
-  [255]
+  [20]
   $ cat b
   b
 
@@ -791,7 +791,7 @@ 
   Hunk #1 FAILED at 0
   1 out of 1 hunks FAILED -- saving rejects to file linkb.rej
   abort: patch failed to apply
-  [255]
+  [20]
   $ hg st
   ? b.rej
   ? linkb.rej
diff --git a/tests/test-import-bypass.t b/tests/test-import-bypass.t
--- a/tests/test-import-bypass.t
+++ b/tests/test-import-bypass.t
@@ -43,7 +43,7 @@ 
   unable to find 'a' for patching
   (use '--prefix' to apply patch relative to the current directory)
   abort: patch failed to apply
-  [255]
+  [20]
   $ hg st
   $ shortlog
   o  1:4e322f7ce8e3 test 0 0 - foo - changea
@@ -234,7 +234,7 @@ 
   patching file a
   Hunk #1 FAILED at 0
   abort: patch failed to apply
-  [255]
+  [20]
   $ hg --config patch.eol=auto import -d '0 0' -m 'test patch.eol' --bypass ../test.diff
   applying ../test.diff
   $ shortlog
diff --git a/tests/test-commit-interactive.t b/tests/test-commit-interactive.t
--- a/tests/test-commit-interactive.t
+++ b/tests/test-commit-interactive.t
@@ -1494,7 +1494,7 @@ 
   Hunk #1 FAILED at 0
   1 out of 1 hunks FAILED -- saving rejects to file editedfile.rej
   abort: patch failed to apply
-  [10]
+  [20]
   $ cat editedfile
   This change will not be committed
   This is the second line
diff --git a/mercurial/patch.py b/mercurial/patch.py
--- a/mercurial/patch.py
+++ b/mercurial/patch.py
@@ -55,6 +55,8 @@ 
 )
 
 PatchError = error.PatchError
+PatchParseError = error.PatchParseError
+PatchApplicationError = error.PatchApplicationError
 
 # public functions
 
@@ -553,7 +555,9 @@ 
         if not self.repo.dirstate.get_entry(fname).any_tracked and self.exists(
             fname
         ):
-            raise PatchError(_(b'cannot patch %s: file is not tracked') % fname)
+            raise PatchApplicationError(
+                _(b'cannot patch %s: file is not tracked') % fname
+            )
 
     def setfile(self, fname, data, mode, copysource):
         self._checkknown(fname)
@@ -637,7 +641,9 @@ 
 
     def _checkknown(self, fname):
         if fname not in self.ctx:
-            raise PatchError(_(b'cannot patch %s: file is not tracked') % fname)
+            raise PatchApplicationError(
+                _(b'cannot patch %s: file is not tracked') % fname
+            )
 
     def getfile(self, fname):
         try:
@@ -793,7 +799,7 @@ 
 
     def apply(self, h):
         if not h.complete():
-            raise PatchError(
+            raise PatchParseError(
                 _(b"bad hunk #%d %s (%d %d %d %d)")
                 % (h.number, h.desc, len(h.a), h.lena, len(h.b), h.lenb)
             )
@@ -1388,7 +1394,7 @@ 
     def read_unified_hunk(self, lr):
         m = unidesc.match(self.desc)
         if not m:
-            raise PatchError(_(b"bad hunk #%d") % self.number)
+            raise PatchParseError(_(b"bad hunk #%d") % self.number)
         self.starta, self.lena, self.startb, self.lenb = m.groups()
         if self.lena is None:
             self.lena = 1
@@ -1405,7 +1411,7 @@ 
                 lr, self.hunk, self.lena, self.lenb, self.a, self.b
             )
         except error.ParseError as e:
-            raise PatchError(_(b"bad hunk #%d: %s") % (self.number, e))
+            raise PatchParseError(_(b"bad hunk #%d: %s") % (self.number, e))
         # if we hit eof before finishing out the hunk, the last line will
         # be zero length.  Lets try to fix it up.
         while len(self.hunk[-1]) == 0:
@@ -1420,7 +1426,7 @@ 
         self.desc = lr.readline()
         m = contextdesc.match(self.desc)
         if not m:
-            raise PatchError(_(b"bad hunk #%d") % self.number)
+            raise PatchParseError(_(b"bad hunk #%d") % self.number)
         self.starta, aend = m.groups()
         self.starta = int(self.starta)
         if aend is None:
@@ -1440,7 +1446,7 @@ 
             elif l.startswith(b'  '):
                 u = b' ' + s
             else:
-                raise PatchError(
+                raise PatchParseError(
                     _(b"bad hunk #%d old text line %d") % (self.number, x)
                 )
             self.a.append(u)
@@ -1454,7 +1460,7 @@ 
             l = lr.readline()
         m = contextdesc.match(l)
         if not m:
-            raise PatchError(_(b"bad hunk #%d") % self.number)
+            raise PatchParseError(_(b"bad hunk #%d") % self.number)
         self.startb, bend = m.groups()
         self.startb = int(self.startb)
         if bend is None:
@@ -1487,7 +1493,7 @@ 
                 lr.push(l)
                 break
             else:
-                raise PatchError(
+                raise PatchParseError(
                     _(b"bad hunk #%d old text line %d") % (self.number, x)
                 )
             self.b.append(s)
@@ -1601,7 +1607,7 @@ 
         while True:
             line = getline(lr, self.hunk)
             if not line:
-                raise PatchError(
+                raise PatchParseError(
                     _(b'could not extract "%s" binary data') % self._fname
                 )
             if line.startswith(b'literal '):
@@ -1622,14 +1628,14 @@ 
             try:
                 dec.append(util.b85decode(line[1:])[:l])
             except ValueError as e:
-                raise PatchError(
+                raise PatchParseError(
                     _(b'could not decode "%s" binary patch: %s')
                     % (self._fname, stringutil.forcebytestr(e))
                 )
             line = getline(lr, self.hunk)
         text = zlib.decompress(b''.join(dec))
         if len(text) != size:
-            raise PatchError(
+            raise PatchParseError(
                 _(b'"%s" length is %d bytes, should be %d')
                 % (self._fname, len(text), size)
             )
@@ -1847,7 +1853,7 @@ 
         try:
             p.transitions[state][newstate](p, data)
         except KeyError:
-            raise PatchError(
+            raise PatchParseError(
                 b'unhandled transition: %s -> %s' % (state, newstate)
             )
         state = newstate
@@ -1874,7 +1880,7 @@ 
     ('a//b/', 'd/e/c')
     >>> pathtransform(b'a/b/c', 3, b'')
     Traceback (most recent call last):
-    PatchError: unable to strip away 1 of 3 dirs from a/b/c
+    PatchApplicationError: unable to strip away 1 of 3 dirs from a/b/c
     """
     pathlen = len(path)
     i = 0
@@ -1884,7 +1890,7 @@ 
     while count > 0:
         i = path.find(b'/', i)
         if i == -1:
-            raise PatchError(
+            raise PatchApplicationError(
                 _(b"unable to strip away %d of %d dirs from %s")
                 % (count, strip, path)
             )
@@ -1947,7 +1953,7 @@ 
         elif not nulla:
             fname = afile
         else:
-            raise PatchError(_(b"undefined source and destination files"))
+            raise PatchParseError(_(b"undefined source and destination files"))
 
     gp = patchmeta(fname)
     if create:
@@ -2097,7 +2103,7 @@ 
                     gp.copy(),
                 )
             if not gitpatches:
-                raise PatchError(
+                raise PatchParseError(
                     _(b'failed to synchronize metadata for "%s"') % afile[2:]
                 )
             newfile = True
@@ -2193,7 +2199,7 @@ 
             out += binchunk[i:offset_end]
             i += cmd
         else:
-            raise PatchError(_(b'unexpected delta opcode 0'))
+            raise PatchApplicationError(_(b'unexpected delta opcode 0'))
     return out
 
 
@@ -2270,7 +2276,7 @@ 
                     data, mode = store.getfile(gp.oldpath)[:2]
                     if data is None:
                         # This means that the old path does not exist
-                        raise PatchError(
+                        raise PatchApplicationError(
                             _(b"source file '%s' does not exist") % gp.oldpath
                         )
                 if gp.mode:
@@ -2283,7 +2289,7 @@ 
                     if gp.op in (b'ADD', b'RENAME', b'COPY') and backend.exists(
                         gp.path
                     ):
-                        raise PatchError(
+                        raise PatchApplicationError(
                             _(
                                 b"cannot create %s: destination "
                                 b"already exists"
@@ -2365,7 +2371,7 @@ 
             scmutil.marktouched(repo, files, similarity)
     code = fp.close()
     if code:
-        raise PatchError(
+        raise PatchApplicationError(
             _(b"patch command failed: %s") % procutil.explainexit(code)
         )
     return fuzz
@@ -2397,7 +2403,7 @@ 
         files.update(backend.close())
         store.close()
     if ret < 0:
-        raise PatchError(_(b'patch failed to apply'))
+        raise PatchApplicationError(_(b'patch failed to apply'))
     return ret > 0
 
 
diff --git a/mercurial/error.py b/mercurial/error.py
--- a/mercurial/error.py
+++ b/mercurial/error.py
@@ -388,6 +388,14 @@ 
     __bytes__ = _tobytes
 
 
+class PatchParseError(PatchError):
+    __bytes__ = _tobytes
+
+
+class PatchApplicationError(PatchError):
+    __bytes__ = _tobytes
+
+
 def getsimilar(symbols, value):
     # type: (Iterable[bytes], bytes) -> List[bytes]
     sim = lambda x: difflib.SequenceMatcher(None, value, x).ratio()
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -522,8 +522,10 @@ 
         # 1. filter patch, since we are intending to apply subset of it
         try:
             chunks, newopts = filterfn(ui, original_headers, match)
-        except error.PatchError as err:
+        except error.PatchParseError as err:
             raise error.InputError(_(b'error parsing patch: %s') % err)
+        except error.PatchApplicationError as err:
+            raise error.StateError(_(b'error applying patch: %s') % err)
         opts.update(newopts)
 
         # We need to keep a backup of files that have been newly added and
@@ -608,8 +610,10 @@ 
                     ui.debug(b'applying patch\n')
                     ui.debug(fp.getvalue())
                     patch.internalpatch(ui, repo, fp, 1, eolmode=None)
-                except error.PatchError as err:
+                except error.PatchParseError as err:
                     raise error.InputError(pycompat.bytestr(err))
+                except error.PatchApplicationError as err:
+                    raise error.StateError(pycompat.bytestr(err))
             del fp
 
             # 4. We prepared working directory according to filtered
@@ -2020,9 +2024,11 @@ 
                 eolmode=None,
                 similarity=sim / 100.0,
             )
-        except error.PatchError as e:
+        except error.PatchParseError as e:
+            raise error.InputError(pycompat.bytestr(e))
+        except error.PatchApplicationError as e:
             if not partial:
-                raise error.Abort(pycompat.bytestr(e))
+                raise error.StateError(pycompat.bytestr(e))
             if partial:
                 rejects = True
 
@@ -2079,8 +2085,10 @@ 
                     files,
                     eolmode=None,
                 )
-            except error.PatchError as e:
-                raise error.Abort(stringutil.forcebytestr(e))
+            except error.PatchParseError as e:
+                raise error.InputError(stringutil.forcebytestr(e))
+            except error.PatchApplicationError as e:
+                raise error.StateError(stringutil.forcebytestr(e))
             if opts.get(b'exact'):
                 editor = None
             else:
@@ -3674,8 +3682,10 @@ 
             if operation == b'discard':
                 chunks = patch.reversehunks(chunks)
 
-        except error.PatchError as err:
-            raise error.Abort(_(b'error parsing patch: %s') % err)
+        except error.PatchParseError as err:
+            raise error.InputError(_(b'error parsing patch: %s') % err)
+        except error.PatchApplicationError as err:
+            raise error.StateError(_(b'error applying patch: %s') % err)
 
         # FIXME: when doing an interactive revert of a copy, there's no way of
         # performing a partial revert of the added file, the only option is
@@ -3710,8 +3720,10 @@ 
         if dopatch:
             try:
                 patch.internalpatch(repo.ui, repo, fp, 1, eolmode=None)
-            except error.PatchError as err:
-                raise error.Abort(pycompat.bytestr(err))
+            except error.PatchParseError as err:
+                raise error.InputError(pycompat.bytestr(err))
+            except error.PatchApplicationError as err:
+                raise error.StateError(pycompat.bytestr(err))
         del fp
     else:
         for f in actions[b'revert'][0]: