Patchwork D6616: extdata: avoid crashing inside subprocess when we get a revset parse error

login
register
mail settings
Submitter phabricator
Date July 8, 2019, 10:05 p.m.
Message ID <4ab5c2ffe86ab9bd40e043f49067092d@localhost.localdomain>
Download mbox | patch
Permalink /patch/40843/
State Not Applicable
Headers show

Comments

phabricator - July 8, 2019, 10:05 p.m.
Closed by commit rHGea6558db1011: extdata: avoid crashing inside subprocess when we get a revset parse error (authored by durin42).
This revision was automatically updated to reflect the committed changes.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D6616?vs=15800&id=15825

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6616/new/

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

AFFECTED FILES
  mercurial/scmutil.py
  tests/test-extdata.t

CHANGE DETAILS




To: durin42, #hg-reviewers, pulkit
Cc: mjpieters, mercurial-devel

Patch

diff --git a/tests/test-extdata.t b/tests/test-extdata.t
--- a/tests/test-extdata.t
+++ b/tests/test-extdata.t
@@ -66,9 +66,14 @@ 
   > 9de260b1e88e
   > EOF
 
-BUG: this should print the revset parse error
-  $ hg log -qr "extdata(badparse)" 2>&1 | grep ValueError
-  ValueError: Mixing iteration and read methods would lose data
+It might be nice if this error message mentioned where the bad string
+came from (eg line X of extdata source S), but the important thing is
+that we don't crash before we can print the parse error.
+  $ hg log -qr "extdata(badparse)"
+  hg: parse error at 0: not a prefix: +
+  (+---------------------------------------+
+   ^ here)
+  [255]
 
 test template support:
 
diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -1541,7 +1541,12 @@ 
                 pass # we ignore data for nodes that don't exist locally
     finally:
         if proc:
-            proc.communicate()
+            try:
+                proc.communicate()
+            except ValueError:
+                # This happens if we started iterating src and then
+                # get a parse error on a line. It should be safe to ignore.
+                pass
         if src:
             src.close()
     if proc and proc.returncode != 0: