Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Solaris 11, python2.6 fixes in ptyprocess.py #15

Closed
wants to merge 8 commits into from

Conversation

jquast
Copy link
Member

@jquast jquast commented Apr 25, 2015

  • termios.tcgetattr(fd)[6][VEOF] becomes value of 1(int), where we expect '\x04' (^D). seems to be a bug with the compilation of python2.6 that Sun provides with Solaris 11. This raises TypeError when ord(1) happens when trying to determine the byte for EOF.

    If _is_solaris is True, and the return value is an integer, just assume its a bork and manually set eof = 4(int)(^D).

  • Fix several "ValueError: zero length field name in format" errors for Python 2.6, which is what ships with Solaris 11. This caused an exception at a critical path in pty.fork(), where the parent would block on os.read of 'exec_err_pipe_read' indefinitely, as the child raised an exception trying to construct the 'tosend' value (though the exception could not be seen). Test runner just blocks indefinitely without such fixes.

  • In spawn(), allow errno.ENXIO when calling inst.setwindowsize(), in some cases, such as spawn(['/bin/true']), the child process may have exited so quickly as to no longer be able to accept any terminal driver calls. Reported by @reynir in Race condition in pexpect.spawn() on Solaris 11 pexpect#206

- termios.tcgetattr(fd)[6][VEOF] becomes value of 1(int), where we
  expect '\x04' (^D).  seems to be a bug with the compilation of
  python2.6 that Sun provides with Solaris 11.

  if _is_solairs is True, and the return value is an integer,
  just assume its a bork and manually set 4(int)(^D).

- Fix several "ValueError: zero length field name in format" errors
  for Python 2.6, which is what ships with Solaris 11.  This caused
  an exception at a critical path in pty.fork(), where the parent
  would block on os.read of 'exec_err_pipe_read' indefinitely, as
  the child raised an exception trying to construct the 'tosend'
  value.

- In spawn(), allow errno.ENXIO when calling inst.setwindowsize(),
  in some cases, such as spawn(['/bin/true']), the child process
  may have exited so quickly as to no longer be able to accept any
  terminal driver calls.
@jquast jquast changed the title Various Solaris 11 fixes in ptyprocess.py Various Solaris 11 and python2.6 fixes in ptyprocess.py Apr 25, 2015
@jquast jquast changed the title Various Solaris 11 and python2.6 fixes in ptyprocess.py Solaris 11, python2.6 fixes in ptyprocess.py Apr 25, 2015
@takluyver
Copy link
Member

I thought we had deliberately dropped Python 2.6 support in Pexpect? I don't mind support Solaris if it has a sufficiently modern Python, but if even Python 2.7 (from 2010) is not supported, I don't think it's necessary to add support in Ptyprocess, which is a much newer piece of software.

Relevant reading: Nick Coghlan's blog post Stop Supporting Python 2.6 (for free).

@jquast
Copy link
Member Author

jquast commented Apr 26, 2015

It just isn't very hard to support python2.6. This document says, "and this is proving to be a hassle for you" -- It isn't for me.. Travis-CI still tests python2.6 for free. It seemed silly to fix @reynir's bug report only to have it lock up/freeze for him on the next release -- he or someone else will just open another bug later. In such cases not supporting py26 might be more of a hassle: We should at least make setup.py assert that python version >= 2.7 is used to prevent them from installing it and discovering and filing bug reports for such behavior.

pexpect still works with python2.6, (and for tests, we need only replace unittest with unittest2 and we can stop backporting things like assertRaises in PexpectTestCase.py).

Anyway, I'll make a separate PR for only the errno.ENXIO fix and just assume the testing already done is sufficient for python2.7. Installing python2.7 on Solaris is no trivial task!

@jquast jquast closed this Apr 26, 2015
@jquast
Copy link
Member Author

jquast commented Apr 26, 2015

PR #16 prevents ptyprocess from being installed on python2.6, and #17 (presumably) fixes the race condition

@takluyver
Copy link
Member

I agree it's not technically very difficult, but it's one more thing to think about - I have to remember which features I can't use because of 2.6. And it seems perverse to be adding such support to new packages. I would be fine with ptyprocess' setup.py checking the Python version (sorry @reynir).

If installing a newer version of Python on Solaris is a hassle, then I think the best way forwards is for someone to work on making that process easier, rather than the whole ecosystem having to work around it.

Thanks, I'll go and review #16 and #17 now.

@jquast jquast deleted the solaris-spawn-race-issue branch April 26, 2015 02:55
@reynir
Copy link
Contributor

reynir commented May 21, 2015

It's ok, I've moved my code away from python and pexpect. I understand the motivation for not supporting 2.6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants