| Submitter | Yuya Nishihara |
|---|---|
| Date | March 4, 2022, 2:12 a.m. |
| Message ID | <3a9729bead90ef7caaf4.1646359929@lemosa> |
| Download | mbox | patch |
| Permalink | /patch/50679/ |
| State | New |
| Headers | show |
Comments
Queued, thanks! On 3/4/22 03:12, Yuya Nishihara wrote: > # HG changeset patch > # User Yuya Nishihara <yuya@tcha.org> > # Date 1646357326 -32400 > # Fri Mar 04 10:28:46 2022 +0900 > # Node ID 3a9729bead90ef7caaf4d25138680c83a888be78 > # Parent 46b3ecfb16e2781ede9752d972dc22f0e1dfea87 > chgserver: remove Python 2 file descriptor logic > > Follows up 0bb28b7736bc "chgserver: remove Python 2 support code." > > On Python 2, we had to close newfp prior to restoring the original file > description since "delete newfp" would otherwise close the file descriptor > shared with the long-lived fp: > > in attachio(): > newfp = os.fdopen(fp.fileno(), mode, bufsize) > in _restoreio(): > newfp.close() # temporarily close newfp.fileno() (= fp.fileno()) > os.dup2(fd, fp.fileno()) # reopen fp.fileno() with original fd > > On the other hand, we shouldn't call newfp.close() on Python 3 since > any function calls are proxied to the underlying file object by > procutil.LineBufferedWrapper. > > diff --git a/mercurial/chgserver.py b/mercurial/chgserver.py > --- a/mercurial/chgserver.py > +++ b/mercurial/chgserver.py > @@ -438,14 +438,8 @@ class chgcmdserver(commandserver.server) > nullfd = os.open(os.devnull, os.O_WRONLY) > ui = self.ui > for (ch, fp, fd), (cn, fn, mode) in zip(self._oldios, _iochannels): > - newfp = getattr(ui, fn) > - # On Python 3, newfp is just a wrapper around fp even if newfp is > - # not fp, so deleting newfp is safe. > - if newfp is not fp: > - newfp.close() > - # restore original fd: fp is open again > try: > - if newfp is fp and 'w' in mode: > + if 'w' in mode: > # Discard buffered data which couldn't be flushed because > # of EPIPE. The data should belong to the current session > # and should never persist. > > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
I installed @ and am now seeing the following with `alias hg=chg`:
$ hg version
Mercurial Distributed SCM (version 6.1+hg145.469b9ee336a6)
(see https://mercurial-scm.org for more information)
Copyright (C) 2005-2022 Olivia Mackall and others
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
gps@ubuntu-vm-main:~/src/hg (@)$ Traceback (most recent call last):
File "/home/gps/lib/python/mercurial/commandserver.py", line 509, in
_serverequest
sv.cleanup()
File "/home/gps/lib/python/mercurial/chgserver.py", line 382, in cleanup
self._restoreio()
File "/home/gps/lib/python/mercurial/chgserver.py", line 454, in
_restoreio
os.dup2(fd, fp.fileno())
ValueError: I/O operation on closed file
Traceback (most recent call last):
File "/home/gps/lib/python/mercurial/commandserver.py", line 693, in
_acceptnewconnection
self._runworker(conn)
File "/home/gps/lib/python/mercurial/commandserver.py", line 739, in
_runworker
_serverequest(
File "/home/gps/lib/python/mercurial/commandserver.py", line 509, in
_serverequest
sv.cleanup()
File "/home/gps/lib/python/mercurial/chgserver.py", line 382, in cleanup
self._restoreio()
File "/home/gps/lib/python/mercurial/chgserver.py", line 454, in
_restoreio
os.dup2(fd, fp.fileno())
ValueError: I/O operation on closed file
I didn't bisect. So the regression could be another patch. But it certainly
looks like chg isn't working properly on @.
On Wed, Mar 9, 2022 at 3:02 AM Raphaël Gomès <raphael.gomes@octobus.net>
wrote:
> Queued, thanks!
>
> On 3/4/22 03:12, Yuya Nishihara wrote:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1646357326 -32400
> > # Fri Mar 04 10:28:46 2022 +0900
> > # Node ID 3a9729bead90ef7caaf4d25138680c83a888be78
> > # Parent 46b3ecfb16e2781ede9752d972dc22f0e1dfea87
> > chgserver: remove Python 2 file descriptor logic
> >
> > Follows up 0bb28b7736bc "chgserver: remove Python 2 support code."
> >
> > On Python 2, we had to close newfp prior to restoring the original file
> > description since "delete newfp" would otherwise close the file
> descriptor
> > shared with the long-lived fp:
> >
> > in attachio():
> > newfp = os.fdopen(fp.fileno(), mode, bufsize)
> > in _restoreio():
> > newfp.close() # temporarily close newfp.fileno() (= fp.fileno())
> > os.dup2(fd, fp.fileno()) # reopen fp.fileno() with original fd
> >
> > On the other hand, we shouldn't call newfp.close() on Python 3 since
> > any function calls are proxied to the underlying file object by
> > procutil.LineBufferedWrapper.
> >
> > diff --git a/mercurial/chgserver.py b/mercurial/chgserver.py
> > --- a/mercurial/chgserver.py
> > +++ b/mercurial/chgserver.py
> > @@ -438,14 +438,8 @@ class chgcmdserver(commandserver.server)
> > nullfd = os.open(os.devnull, os.O_WRONLY)
> > ui = self.ui
> > for (ch, fp, fd), (cn, fn, mode) in zip(self._oldios,
> _iochannels):
> > - newfp = getattr(ui, fn)
> > - # On Python 3, newfp is just a wrapper around fp even if
> newfp is
> > - # not fp, so deleting newfp is safe.
> > - if newfp is not fp:
> > - newfp.close()
> > - # restore original fd: fp is open again
> > try:
> > - if newfp is fp and 'w' in mode:
> > + if 'w' in mode:
> > # Discard buffered data which couldn't be flushed
> because
> > # of EPIPE. The data should belong to the current
> session
> > # and should never persist.
> >
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Apparently I installed an older @, which wasn't working. After applying this patch, I _think_ chg is happy again. On Wed, Mar 9, 2022 at 7:13 PM Gregory Szorc <gregory.szorc@gmail.com> wrote: > I installed @ and am now seeing the following with `alias hg=chg`: > > $ hg version > Mercurial Distributed SCM (version 6.1+hg145.469b9ee336a6) > (see https://mercurial-scm.org for more information) > > Copyright (C) 2005-2022 Olivia Mackall and others > This is free software; see the source for copying conditions. There is NO > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > gps@ubuntu-vm-main:~/src/hg (@)$ Traceback (most recent call last): > File "/home/gps/lib/python/mercurial/commandserver.py", line 509, in > _serverequest > sv.cleanup() > File "/home/gps/lib/python/mercurial/chgserver.py", line 382, in cleanup > self._restoreio() > File "/home/gps/lib/python/mercurial/chgserver.py", line 454, in > _restoreio > os.dup2(fd, fp.fileno()) > ValueError: I/O operation on closed file > Traceback (most recent call last): > File "/home/gps/lib/python/mercurial/commandserver.py", line 693, in > _acceptnewconnection > self._runworker(conn) > File "/home/gps/lib/python/mercurial/commandserver.py", line 739, in > _runworker > _serverequest( > File "/home/gps/lib/python/mercurial/commandserver.py", line 509, in > _serverequest > sv.cleanup() > File "/home/gps/lib/python/mercurial/chgserver.py", line 382, in cleanup > self._restoreio() > File "/home/gps/lib/python/mercurial/chgserver.py", line 454, in > _restoreio > os.dup2(fd, fp.fileno()) > ValueError: I/O operation on closed file > > I didn't bisect. So the regression could be another patch. But it > certainly looks like chg isn't working properly on @. > > On Wed, Mar 9, 2022 at 3:02 AM Raphaël Gomès <raphael.gomes@octobus.net> > wrote: > >> Queued, thanks! >> >> On 3/4/22 03:12, Yuya Nishihara wrote: >> > # HG changeset patch >> > # User Yuya Nishihara <yuya@tcha.org> >> > # Date 1646357326 -32400 >> > # Fri Mar 04 10:28:46 2022 +0900 >> > # Node ID 3a9729bead90ef7caaf4d25138680c83a888be78 >> > # Parent 46b3ecfb16e2781ede9752d972dc22f0e1dfea87 >> > chgserver: remove Python 2 file descriptor logic >> > >> > Follows up 0bb28b7736bc "chgserver: remove Python 2 support code." >> > >> > On Python 2, we had to close newfp prior to restoring the original file >> > description since "delete newfp" would otherwise close the file >> descriptor >> > shared with the long-lived fp: >> > >> > in attachio(): >> > newfp = os.fdopen(fp.fileno(), mode, bufsize) >> > in _restoreio(): >> > newfp.close() # temporarily close newfp.fileno() (= fp.fileno()) >> > os.dup2(fd, fp.fileno()) # reopen fp.fileno() with original fd >> > >> > On the other hand, we shouldn't call newfp.close() on Python 3 since >> > any function calls are proxied to the underlying file object by >> > procutil.LineBufferedWrapper. >> > >> > diff --git a/mercurial/chgserver.py b/mercurial/chgserver.py >> > --- a/mercurial/chgserver.py >> > +++ b/mercurial/chgserver.py >> > @@ -438,14 +438,8 @@ class chgcmdserver(commandserver.server) >> > nullfd = os.open(os.devnull, os.O_WRONLY) >> > ui = self.ui >> > for (ch, fp, fd), (cn, fn, mode) in zip(self._oldios, >> _iochannels): >> > - newfp = getattr(ui, fn) >> > - # On Python 3, newfp is just a wrapper around fp even if >> newfp is >> > - # not fp, so deleting newfp is safe. >> > - if newfp is not fp: >> > - newfp.close() >> > - # restore original fd: fp is open again >> > try: >> > - if newfp is fp and 'w' in mode: >> > + if 'w' in mode: >> > # Discard buffered data which couldn't be flushed >> because >> > # of EPIPE. The data should belong to the current >> session >> > # and should never persist. >> > >> > _______________________________________________ >> > Mercurial-devel mailing list >> > Mercurial-devel@mercurial-scm.org >> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel >> _______________________________________________ >> Mercurial-devel mailing list >> Mercurial-devel@mercurial-scm.org >> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel >> >
Patch
diff --git a/mercurial/chgserver.py b/mercurial/chgserver.py --- a/mercurial/chgserver.py +++ b/mercurial/chgserver.py @@ -438,14 +438,8 @@ class chgcmdserver(commandserver.server) nullfd = os.open(os.devnull, os.O_WRONLY) ui = self.ui for (ch, fp, fd), (cn, fn, mode) in zip(self._oldios, _iochannels): - newfp = getattr(ui, fn) - # On Python 3, newfp is just a wrapper around fp even if newfp is - # not fp, so deleting newfp is safe. - if newfp is not fp: - newfp.close() - # restore original fd: fp is open again try: - if newfp is fp and 'w' in mode: + if 'w' in mode: # Discard buffered data which couldn't be flushed because # of EPIPE. The data should belong to the current session # and should never persist.
# HG changeset patch # User Yuya Nishihara <yuya@tcha.org> # Date 1646357326 -32400 # Fri Mar 04 10:28:46 2022 +0900 # Node ID 3a9729bead90ef7caaf4d25138680c83a888be78 # Parent 46b3ecfb16e2781ede9752d972dc22f0e1dfea87 chgserver: remove Python 2 file descriptor logic Follows up 0bb28b7736bc "chgserver: remove Python 2 support code." On Python 2, we had to close newfp prior to restoring the original file description since "delete newfp" would otherwise close the file descriptor shared with the long-lived fp: in attachio(): newfp = os.fdopen(fp.fileno(), mode, bufsize) in _restoreio(): newfp.close() # temporarily close newfp.fileno() (= fp.fileno()) os.dup2(fd, fp.fileno()) # reopen fp.fileno() with original fd On the other hand, we shouldn't call newfp.close() on Python 3 since any function calls are proxied to the underlying file object by procutil.LineBufferedWrapper.