This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9152

hook: ignore EPIPE when flushing stdout/stderr
ClosedPublic

Authored by mplamann on Oct 5 2020, 6:37 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHGb3e8d8e4a40d: hook: ignore EPIPE when flushing stdout/stderr
Summary

This fixes the bug described in the parent commit.
test-transaction-rollback-on-sigpipe.t is updated to show the new
behavior.

Diff Detail

Repository
rHG Mercurial
Branch
stable
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

mplamann created this revision.Oct 5 2020, 6:37 PM
pulkit added a subscriber: pulkit.Oct 9 2020, 5:19 AM
pulkit added inline comments.
mercurial/hook.py
266

unrelated change

mplamann updated this revision to Diff 23207.Oct 14 2020, 12:33 PM
pulkit accepted this revision.Oct 16 2020, 3:32 AM
This revision is now accepted and ready to land.Oct 16 2020, 3:32 AM
mplamann added a commit: rHGb3e8d8e4a40d: hook: ignore EPIPE when flushing stdout/stderr.Oct 16 2020, 3:48 AM
Closed by commit rHGb3e8d8e4a40d: hook: ignore EPIPE when flushing stdout/stderr (authored by mplamann). · Explain Why
This revision was automatically updated to reflect the committed changes.
martinvonz added a subscriber: martinvonz.Oct 20 2020, 1:06 PM

The test case fails on both py2 and py3 for me. This CI pipeline agrees that it's failing: https://foss.heptapod.net/octobus/mercurial-devel/-/pipelines/11695. Can you send a fix?

martinvonz added a comment.Oct 22 2020, 12:04 PM
In D9152#138736, @martinvonz wrote:

The test case fails on both py2 and py3 for me. This CI pipeline agrees that it's failing: https://foss.heptapod.net/octobus/mercurial-devel/-/pipelines/11695. Can you send a fix?

The test is still failing. Any chance you can fix it?

mplamann added a comment.Oct 22 2020, 12:45 PM
In D9152#138786, @martinvonz wrote:
In D9152#138736, @martinvonz wrote:

The test case fails on both py2 and py3 for me. This CI pipeline agrees that it's failing: https://foss.heptapod.net/octobus/mercurial-devel/-/pipelines/11695. Can you send a fix?

The test is still failing. Any chance you can fix it?

I've pushed a commit that I believe should fix this problem: https://phab.mercurial-scm.org/D9239

mercurial/hook.py
266

Thanks, reverted that.

Revision Contents
Changeset List

Diff 23207

mercurial/hook.py

# hook.py - hook support for mercurial # hook.py - hook support for mercurial
# #
# Copyright 2007 Matt Mackall <mpm@selenic.com> # Copyright 2007 Matt Mackall <mpm@selenic.com>
# #
# This software may be used and distributed according to the terms of the # This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version. # GNU General Public License version 2 or any later version.
from __future__ import absolute_import from __future__ import absolute_import
import contextlib import contextlib
import errno
import os import os
import sys import sys
from .i18n import _ from .i18n import _
from .pycompat import getattr from .pycompat import getattr
from . import ( from . import (
demandimport, demandimport,
encoding, encoding,
hooks.append((hname, cmd)) hooks.append((hname, cmd))
res = runhooks(ui, repo, htype, hooks, throw=throw, **args) res = runhooks(ui, repo, htype, hooks, throw=throw, **args)
r = False r = False
for hname, cmd in hooks: for hname, cmd in hooks:
r = res[hname][0] or r r = res[hname][0] or r
return r return r
pulkitUnsubmitted
Not Done

unrelated change

pulkit: unrelated change
mplamannAuthorUnsubmitted
Done

Thanks, reverted that.

mplamann: Thanks, reverted that.
@contextlib.contextmanager @contextlib.contextmanager
def redirect_stdio(): def redirect_stdio():
"""Redirects stdout to stderr, if possible.""" """Redirects stdout to stderr, if possible."""
oldstdout = -1 oldstdout = -1
try: try:
if _redirect: if _redirect:
try: try:
pass pass
yield yield
finally: finally:
# The stderr is fully buffered on Windows when connected to a pipe. # The stderr is fully buffered on Windows when connected to a pipe.
# A forcible flush is required to make small stderr data in the # A forcible flush is required to make small stderr data in the
# remote side available to the client immediately. # remote side available to the client immediately.
try:
procutil.stderr.flush() procutil.stderr.flush()
except IOError as err:
if err.errno not in (errno.EPIPE, errno.EIO, errno.EBADF):
raise error.StdioError(err)
if _redirect and oldstdout >= 0: if _redirect and oldstdout >= 0:
try:
procutil.stdout.flush() # write hook output to stderr fd procutil.stdout.flush() # write hook output to stderr fd
except IOError as err:
if err.errno not in (errno.EPIPE, errno.EIO, errno.EBADF):
raise error.StdioError(err)
os.dup2(oldstdout, stdoutno) os.dup2(oldstdout, stdoutno)
os.close(oldstdout) os.close(oldstdout)
def runhooks(ui, repo, htype, hooks, throw=False, **args): def runhooks(ui, repo, htype, hooks, throw=False, **args):
args = pycompat.byteskwargs(args) args = pycompat.byteskwargs(args)
res = {} res = {}

tests/test-transaction-rollback-on-sigpipe.t

pushing to ssh://user@dummy/$TESTTMP/remote pushing to ssh://user@dummy/$TESTTMP/remote
searching for changes searching for changes
remote: adding changesets remote: adding changesets
remote: adding manifests remote: adding manifests
remote: adding file changes remote: adding file changes
abort: stream ended unexpectedly (got 0 bytes, expected 4) abort: stream ended unexpectedly (got 0 bytes, expected 4)
$ check_for_abandoned_transaction $ check_for_abandoned_transaction
Abandoned transaction! [1]