| Submitter | Pierre-Yves David |
|---|---|
| Date | June 3, 2015, 8:26 p.m. |
| Message ID | <0521446e1c5934bf377d.1433363190@marginatus.alto.octopoid.net> |
| Download | mbox | patch |
| Permalink | /patch/9461/ |
| State | Superseded |
| Commit | c2ec81891502662ebe2d3ad987bd0ea60ab1b60f |
| Headers | show |
Comments
On Wed, 2015-06-03 at 13:26 -0700, Pierre-Yves David wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@fb.com> > # Date 1432162805 18000 > # Wed May 20 18:00:05 2015 -0500 > # Node ID 0521446e1c5934bf377d8c6bd8a48baad6b57ae1 > # Parent d3b8690c4ecc2a60b8edd9f632f77824284b6027 > util: add a simple poll utility > > We'll use it to detect when the a sshpeer have server output to be displayed. > > The implementation is super basic because all case support is not the focus of > this series. > > diff --git a/mercurial/util.py b/mercurial/util.py > --- a/mercurial/util.py > +++ b/mercurial/util.py > @@ -16,11 +16,11 @@ hide platform-specific details from the > import i18n > _ = i18n._ > import error, osutil, encoding, parsers > import errno, shutil, sys, tempfile, traceback > import re as remod > -import os, time, datetime, calendar, textwrap, signal, collections > +import os, time, datetime, calendar, textwrap, signal, collections, sys, select Hmm, is it really possible that util doesn't import sys? Very nearby signs point to no. > +def poll(fds): > + """block until something happened on any filedescriptors > + > + This is a generic helper that will check for any activity > + (read, write. exception). return the list of touched file. > + > + In unsupported case raise a NotImplementedError""" > + if os.name == 'nt': # we do not support windows yet. > + raise NotImplementedError() > + if any(sys.maxint <= f for f in fds): > + raise NotImplementedError() > + res = select.select(fds, fds, fds) > + return sorted(list(set(sum(res, [])))) > + Please use posix.py and windows.py, rather than putting conditionals in util.
On Wed, 2015-06-03 at 13:26 -0700, Pierre-Yves David wrote: > + if any(sys.maxint <= f for f in fds): > + raise NotImplementedError() Also, I'm not sure why you're testing against maxint. The actual limit is typically something like 2048, defined in FD_SETSIZE. And the reason for the limit is that every possible file descriptor up to that limit is represented as a bit in the input field. A bit vector with 2^64 bits is indeed too large. Modern interfaces like poll/epoll don't try to pass in all the descriptors at once and thus don't run into these limits.
On 06/03/2015 04:03 PM, Matt Mackall wrote: > On Wed, 2015-06-03 at 13:26 -0700, Pierre-Yves David wrote: >> + if any(sys.maxint <= f for f in fds): >> + raise NotImplementedError() > > Also, I'm not sure why you're testing against maxint. The actual limit > is typically something like 2048, defined in FD_SETSIZE. And the reason > for the limit is that every possible file descriptor up to that limit is > represented as a bit in the input field. A bit vector with 2^64 bits is > indeed too large. Neat, Augie and I were looking for such variable with no luck. Python store this in TYPES.FD_SETSIZE. But is also turn out that Python raise a ValueError if you give a too large file descriptor to select. So we can just catch that. V2 on the way.
Patch
diff --git a/mercurial/util.py b/mercurial/util.py --- a/mercurial/util.py +++ b/mercurial/util.py @@ -16,11 +16,11 @@ hide platform-specific details from the import i18n _ = i18n._ import error, osutil, encoding, parsers import errno, shutil, sys, tempfile, traceback import re as remod -import os, time, datetime, calendar, textwrap, signal, collections +import os, time, datetime, calendar, textwrap, signal, collections, sys, select import imp, socket, urllib import gc if os.name == 'nt': import windows as platform @@ -327,10 +327,24 @@ class bufferedinputpipe(object): self._eof = True else: # inefficient add self._buffer.append(data) +def poll(fds): + """block until something happened on any filedescriptors + + This is a generic helper that will check for any activity + (read, write. exception). return the list of touched file. + + In unsupported case raise a NotImplementedError""" + if os.name == 'nt': # we do not support windows yet. + raise NotImplementedError() + if any(sys.maxint <= f for f in fds): + raise NotImplementedError() + res = select.select(fds, fds, fds) + return sorted(list(set(sum(res, [])))) + def popen2(cmd, env=None, newlines=False): # Setting bufsize to -1 lets the system decide the buffer size. # The default for bufsize is 0, meaning unbuffered. This leads to # poor performance on Mac OS X: http://bugs.python.org/issue4194 p = subprocess.Popen(cmd, shell=True, bufsize=-1,