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

login
register
mail settings
Submitter phabricator
Date July 8, 2019, 6:05 p.m.
Message ID <differential-rev-PHID-DREV-zfhso46f36oax42ad2jb-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/40819/
State Superseded
Headers show

Comments

phabricator - July 8, 2019, 6:05 p.m.
durin42 created this revision.
Herald added subscribers: mercurial-devel, mjpieters.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

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

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

CHANGE DETAILS




To: durin42, #hg-reviewers
Cc: mjpieters, mercurial-devel
Yuya Nishihara - July 10, 2019, 12:17 a.m.
>          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

No idea what to do, but maybe the process would be still running if
ValueError occurred.
phabricator - July 10, 2019, 12:18 a.m.
yuja added a comment.


  >   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
  
  No idea what to do, but maybe the process would be still running if
  ValueError occurred.

REPOSITORY
  rHG Mercurial

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

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

To: durin42, #hg-reviewers, pulkit
Cc: yuja, 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: