This is an archive of the discontinued Mercurial Phabricator instance.

dispatch: ignore failure to flush ui
ClosedPublic

Authored by martinvonz on Oct 11 2021, 7:30 PM.

Details

Summary

When the pager dies, we get a SIGPIPE. That causes
error.SignalInterrupt to be raised (from ui._catchterm()`). Any
further writes or flushes will cause further SIGPIPEs and furhter
error.SignalInterrupt. If we write or flush outside of the
try/except that handle KeyboardInterrupt (which
error.SignalInterrupt is a subclass of), then control will escape
from the dispatch module. Let's fix that by ignoring errors from
flushing the ui.

I would have rather fixed this by restoring the stdout and stderr
streams when the pager dies, but it gets complicated because of
multiple ui instances (ui/lui) and different pager setups between
regular hg and chg.

This changes a test in test-pager.t, but I don't understand why. I
would have thought that all the output from the command should have
gone to the broken pager.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Oct 11 2021, 7:30 PM
pulkit accepted this revision.Oct 12 2021, 9:21 AM
This revision is now accepted and ready to land.Oct 12 2021, 9:21 AM
This revision was automatically updated to reflect the committed changes.

This breaks CI on Python 2: https://foss.heptapod.net/mercurial/mercurial-devel/-/jobs/254041

We unfortunately have to support Python 2 until the release in a few days, so this should get fixed.

This breaks CI on Python 2: https://foss.heptapod.net/mercurial/mercurial-devel/-/jobs/254041
We unfortunately have to support Python 2 until the release in a few days, so this should get fixed.

It doesn't reproduce for me. Does it for you? I wonder what I need to do to make it reproduce. I have /dev/full.

This breaks CI on Python 2: https://foss.heptapod.net/mercurial/mercurial-devel/-/jobs/254041
We unfortunately have to support Python 2 until the release in a few days, so this should get fixed.

It doesn't reproduce for me. Does it for you? I wonder what I need to do to make it reproduce. I have /dev/full.

It does, I'm on Linux if somehow this is OS specific.

$ HGMODULEPOLICY=py python2 ./tests/run-tests.py -j6 tests/test-basic.t
running 1 tests using 1 parallel processes

--- /home/alphare/workspace/octobus/mercurial-devel/tests/test-basic.t
+++ /home/alphare/workspace/octobus/mercurial-devel/tests/test-basic.t.err
@@ -40,13 +40,10 @@
   A a

   $ hg status >/dev/full
-  abort: No space left on device
-  [255]
 #endif

 #if devfull
   $ hg status >/dev/full 2>&1
-  [255]

   $ hg status ENOENT 2>/dev/full
   [255]

ERROR: test-basic.t output changed
!
Failed test-basic.t: output changed
# Ran 1 tests, 0 skipped, 1 failed.
python hash seed: 454035856

This breaks CI on Python 2: https://foss.heptapod.net/mercurial/mercurial-devel/-/jobs/254041
We unfortunately have to support Python 2 until the release in a few days, so this should get fixed.

It doesn't reproduce for me. Does it for you? I wonder what I need to do to make it reproduce. I have /dev/full.

It does, I'm on Linux if somehow this is OS specific.

$ HGMODULEPOLICY=py python2 ./tests/run-tests.py -j6 tests/test-basic.t
running 1 tests using 1 parallel processes
--- /home/alphare/workspace/octobus/mercurial-devel/tests/test-basic.t
+++ /home/alphare/workspace/octobus/mercurial-devel/tests/test-basic.t.err
@@ -40,13 +40,10 @@
   A a
   $ hg status >/dev/full
-  abort: No space left on device
-  [255]
 #endif
 #if devfull
   $ hg status >/dev/full 2>&1
-  [255]
   $ hg status ENOENT 2>/dev/full
   [255]
ERROR: test-basic.t output changed
!
Failed test-basic.t: output changed
# Ran 1 tests, 0 skipped, 1 failed.
python hash seed: 454035856

Oh, it does reproduce for me too now. Sorry, I had forgotten that I updated to another commit while trying to figure out how to run the python2 test. I'll see if I can figure out what's happening there. Worst case we roll it back and wait until after we don't care about py2 anymore.

Oh, it does reproduce for me too now. Sorry, I had forgotten that I updated to another commit while trying to figure out how to run the python2 test. I'll see if I can figure out what's happening there. Worst case we roll it back and wait until after we don't care about py2 anymore.

Thanks! That option also works for me. :)