<div dir="ltr">On Tue, Feb 12, 2013 at 3:02 AM, Bryan O'Sullivan <<a href="mailto:bos@serpentine.com">bos@serpentine.com</a>> wrote:<br>><br>> On Sun, Feb 10, 2013 at 3:25 AM, Idan Kamara <<a href="mailto:idankk86@gmail.com">idankk86@gmail.com</a>> wrote:<br>
>><br>>> No, he didn't. And had he given me the chance to reply to his last<br>>> response, I would have shown it:<br>><br>><br>> Yep, that's a bug. But it's just a bug - I fixed another one in the new<br>
> parallel code just a little while ago.<div><br></div><div>So we should exit on all exceptions, but we can't print a traceback</div><div>since that's not desirable when not running from the command line</div><div>
(and it wasn't readable anyway since they got interleaved in one</div><div>another).</div><div><br></div><div>Maybe we should serialize the exceptions to the master and have</div><div>him decide what to do (e.g. reraise)?</div>
<div><br></div><div>>  <br>>><br>>> And the only reason the sys.exit I had a problem with is<br>>> 'working' is thanks to this:<br>>><br>>> <a href="http://selenic.com/repo/hg/file/0027a5cec9d0/mercurial/dispatch.py#l201">http://selenic.com/repo/hg/file/0027a5cec9d0/mercurial/dispatch.py#l201</a><br>
><br>><br>> Correct, and that's deliberate. Perhaps you could tell me what you'd<br>> rather see, because I obviously think the use of sys.exit in this context is<br>> reasonable based on what I currently know.<br>
<br><div>I think raising util.Abort instead should do it.</div></div><div><br></div><div>I wrote this test to check these things:</div><div><br></div><div><div>import unittest, silenttestrunner, os</div><div><br></div><div>
from mercurial import worker, ui, util</div><div><br></div><div>class testworker(unittest.TestCase):</div><div>    def testchilderror(self):</div><div>        def workerfunc(x):</div><div>            raise util.Abort</div>
<div><br></div><div>        g = worker._platformworker(ui.ui(), workerfunc, (), [1])</div><div>        self.assertRaises(util.Abort, g.next)</div><div><br></div><div>if __name__ == '__main__':</div><div>    silenttestrunner.main(__name__)</div>
</div><div><br></div><div>It will currently fail on a StopIteration because the child doesn't exit,</div><div>making it call g.next on a finished generator. So doing this fixes it:</div><div><br></div><div><div>diff --git a/mercurial/worker.py b/mercurial/worker.py</div>
<div>--- a/mercurial/worker.py</div><div>+++ b/mercurial/worker.py</div><div>@@ -83,7 +83,7 @@</div><div>                 for i, item in func(*(staticargs + (pargs,))):</div><div>                     os.write(wfd, '%d %s\n' % (i, item))</div>
<div>                 os._exit(0)</div><div>-            except KeyboardInterrupt:</div><div>+            except:</div><div>                 os._exit(255)</div><div>     os.close(wfd)</div><div>     fp = os.fdopen(rfd, 'rb', 0)</div>
<div>@@ -96,7 +96,7 @@</div><div>         for i in xrange(workers):</div><div>             problems |= os.wait()[1]</div><div>         if problems:</div><div>-            sys.exit(1)</div><div>+            raise util.Abort(_('child error'))</div>
<div>     try:</div><div>         for line in fp:</div><div>             l = line.split(' ', 1)</div></div></div>