Issue1842

classification
Title: Add IBM i support to Jython
Type: Severity: normal
Components: Library Versions: Jython 2.5
process
Status: open Resolution: accepted
Dependencies: Superseder:
Assigned To: zyasoft Nosy List: amak, fwierzbicki, ryanwatkins, zyasoft
Priority: low Keywords: patch

Created on 2012-02-21.21:49:10 by ryanwatkins, last changed 2014-10-05.16:48:38 by zyasoft.

Files
File name Uploaded Description Edit Remove
ibmi_jython_support.diff ryanwatkins, 2012-02-21.21:49:10
Messages
msg6783 (view) Author: Ryan Watkins (ryanwatkins) Date: 2012-02-21.21:49:09
I would like to get the attached patch incorporated into Jython if at all possible.  The patch adds support for the IBM i platform.

Only two source files from the existing Jython code base were updated.  Here are the file names and a brief description of their updates:

subprocess.py (2 lines inserted/2 lines modified) - Add special handling for environment variable initialization for the IBM i platform. 

os.py (4 lines inserted) - Add code to Jython to automatically detect it is running on the IBM i platform and then map the os name and shell to the appropriate values.
msg6893 (view) Author: Alan Kennedy (amak) Date: 2012-03-19.22:06:42
It would be most useful if you could also supply tests for this patch.

Testing with IBM OSes is complex for us.

http://bugs.jython.org/issue550200
msg6970 (view) Author: Ryan Watkins (ryanwatkins) Date: 2012-03-28.18:39:30
Hi Alan, is there any specific test cases you'd like me to test?  Personally I have verified the submitted changes work on IBM i through the use of some IBM code we have that uses Jython 2.5.1 on the IBM i platform.
msg6971 (view) Author: Ryan Watkins (ryanwatkins) Date: 2012-03-28.18:43:46
I should also add that prior to these changes, we had encountered some ebcdic/ascii encoding issues.  With the changes, things are working correctly.
msg6972 (view) Author: Alan Kennedy (amak) Date: 2012-03-28.19:54:24
A couple of questions.

1. Is the patch against the current head? The line numbers don't seem right.

The diff against subprocess.py starts at line 1243. But the line that is changed is on line 1319 in my copy of subprocess.py from the latest checkout.

2. You mention you " .. had encountered some ebcdic/ascii encoding issues". Please can you elaborate on those issues a little?

Were the issues related to encoding of keys or values in the environment? If yes, we might be better off addressing the issues at source rather than papering over them in the subprocess module.

3. This construct is confusing and doesn't read well: too many negatives.

ostype = os.get_os_type ()
if ostype != 'ibmi' or env is not None:
    # os.environ may be inherited for compatibility with CPython
    self._setup_env(dict(os.environ if env is None else env), ..)

The "or env is not None" is confusing when combined with "dict(os.environ if env is None else env)". Do you expect "env" to always be "not None" on ibmi platforms?

I'd prefer to see it written this way

process_env = os.environ if env is None else env
if os.get_os_type() == 'ibmi':
    process_env = ??????????
self._setup_env(dict(process_env), ..)

4. The location given for the shell on ibmi, i.e.

+               ibmi=[
+        ['OS/400'],
+        [['/QOpenSys/usr/bin/sh', '-c']]
+        ],

Is that "/QOpenSys/..." path system specific? Or is that true of all "ibmi" systems? Under what circumstances is it true?

It would be great if you supply a documentation link, as a comment, that verifies that.
msg6976 (view) Author: Frank Wierzbicki (fwierzbicki) Date: 2012-03-29.18:08:38
Does this patch address #550200? I'd love to close that one - it is one of the oldest bugs in this tracker.
msg6989 (view) Author: Ryan Watkins (ryanwatkins) Date: 2012-03-30.19:11:37
fyi...it might take me a bit to get back to your questions.  I need to check with a colleague who originally made the changes.
msg7042 (view) Author: Ryan Watkins (ryanwatkins) Date: 2012-04-10.00:09:07
Hi Alan,

Here are replies to your questions.  The answers to 2 & 3 were provided to me by a colleague who made the actual code changes in Jython for IBM i:

1) I did the diff against the Jython 2.5.1 download jar file's Lib/subprocess.py file.

2) IBM i has processes that run in either EBCIDC or ASCII character encodings.  Java always runs with Unicode and has its stdin, stdout, stderr streams as ASCII streams.   Depending on the shell interpreter, shell scripts will either run with EBCDIC stdin, stdout, stderr streams or ASCII streams.   The native EXEC module in Java that handles calling sub processes does inline stream conversion between EBCDIC and ASCII for sub process streams.  Whether conversion happens, which streams are converted, and how the conversion settings are propagated down the sub process chain is controlled by complex logic in that uses a set of IBM i specific environment variables.   To avoid having streams incorrectly converted and mangling characters and data, we've addressed the issue in Jython at the time we launch sub-processes to allow IBM i Java and native code to handle the process environment inheritance.

3) The original block of code was: 
           self._setup_env(dict(os.environ if env is None else env),
                            builder.environment())

In this block of code the 'env' variable is a caller provided environment.  The os.environ will get the current set of environment variables from Java.  On IBM i we only want to call self._setup_env(...) if the caller explicitly passes an environment variable map in 'env'.  If they don't explicitly pass an environment we want Java and the shell environment to handle the environment inheritance which correctly handles complex logic for inheriting and changing values of the ASCII/EBCDIC stream process environment variables.

As it was, the code was using os.enviorn to set the child process environment which was overriding the Java/shell process logic and setting the ASCII/EBCDIC stream handling/converting environment variables incorrectly.

4) /QOpenSys/usr/bin/sh is the default shell on all IBM i systems. Here is a link stating so: 
 
http://publib.boulder.ibm.com/infocenter/iseries/v7r1m0/index.jsp?topic=%2Frzalc%2Frzalcpase.htm
msg7072 (view) Author: Alan Kennedy (amak) Date: 2012-04-28.17:37:14
Thanks Ryan, those are good answers, and I'm happy to apply the changes.

I plan to apply the changes in the next few days.

Frank, is there a deadline for the release of a 2.5.3?
msg7073 (view) Author: Frank Wierzbicki (fwierzbicki) Date: 2012-04-28.22:30:56
Alan: we'll need to do at least one more beta due to http://bugs.jython.org/issue1871 which I haven't fixed yet (it's on my todo for soon - I hope to get it resolved next week). So a final is at least 2 weeks off (given a minimum of a week for the last beta).
msg7075 (view) Author: Alan Kennedy (amak) Date: 2012-05-01.16:41:49
OK, we have a problem.

The patch is against a version of jython that is nearly three years old. The code has changed significantly since then. We are not going to be able to produce a branch of 2.5.1: the changes should be checked into the main branch and released as part of 2.5.3.

The recognition of OS and setting of command shell now happens in this code

http://hg.python.org/jython/file/d53002b5f3d0/src/org/python/modules/posix/OS.java

I think I should be able to adapt your patch to the new style, but need to know the following.

What is the value for sys.registry.getProperty("python.os") and java.lang.System.getProperty("os.name") on ibmi systems. I.e. what does the following code output?

>>> import sys
>>> print sys.registry.getProperty("python.os")
>>> import java
>>> print java.lang.System.getProperty("os.name")
msg7078 (view) Author: Ryan Watkins (ryanwatkins) Date: 2012-05-03.02:54:50
This is the output on IBM i for the following:

import sys
print sys.registry.getProperty("python.os")

===>   None   <===

import java
print java.lang.System.getProperty("os.name")

===>   OS/400  <===
msg7082 (view) Author: Alan Kennedy (amak) Date: 2012-05-05.15:22:53
OK, I've checked in the beginnings of OS/400 support.

http://hg.python.org/jython/rev/c62b1a8fa99b

It is quite different to the support proposed in the patch, primarily because the structure of jython code involved has changed significantly.

I'm only checking this into the 2.5 branch for now: once we know it's working, I'll merge it forward to the head.

It would be great if someone with access to OS/400 could download the latest 2.5 branch and try it out.

Primarily it's about testing the os module to see if the new support works. I have made the assumption that java on OS/400 behaves the same way as standard posix.

>>> import os
>>> os._name
'ibmi'
>>> import os.path
>>> os.path.dirname("/QOpenSys/usr/bin/sh")
'/QOpenSys/usr/bin/'

If we know this support is working, we can then address the environment issues, perhaps more cleanly than in the original patch.
msg7175 (view) Author: Ryan Watkins (ryanwatkins) Date: 2012-05-31.18:48:30
Hi Alan, Assuming I am testing this correctly I get the following result for both

os.path.dirname("/QOpenSys/usr/bin/sh")
-and-
os._name

File "/QOpenSys/home/rwatkin/temp/lib/jython/Lib/os.py", line 132, in <module>
       raise ImportError, 'no os specific module found'                           

FYI...this is the contents of _names in os.py:

('_systemrestart', '_ast', 'itertools', 'exceptions', 'zipimport', '_csv', 'math', 'thread', 'imp', 'time', 'sys', 'synchronize',
    'struct', '_collections', '_functools', 'array', 'gc', '_py_compile', '_weakref', 'cPickle', '_marshal', '_random', '__builtin__
   ', 'operator', 'errno', '_sre', 'binascii', 'cmath', 'ucnhash', 'jarray', '_hashlib', 'cStringIO', '_codecs')   

just prior to the error being raised.
msg7176 (view) Author: Ryan Watkins (ryanwatkins) Date: 2012-05-31.18:52:19
Note I am a Python newbie but these were the two test .py files I ran:

import os.path
message = "os.path.dirname="
message += str(os.path.dirname("/QOpenSys/usr/bin/sh"))
print message                                          

-and-

import sys, errno
print sys.builtin_module_names

import os
import java
message = "os._name="
message += str(java.lang.System.getProperty("os._name"))
print message
msg8330 (view) Author: Jim Baker (zyasoft) Date: 2014-05-04.20:22:52
Is the OP still interested in this support for 2.7?
msg8386 (view) Author: Ryan Watkins (ryanwatkins) Date: 2014-05-12.22:33:52
Jim, I haven't looked at this stuff for awhile but would be willing to help test this for IBM i if you incorporated the patch into Jython 2.7.

My phython/jython skills are almost non-existent but I would be willing to help run some tests if you'd be willing to guide me a little bit.   

In general I think incorporating this patch would be great for the IBM i platform.
msg8393 (view) Author: Jim Baker (zyasoft) Date: 2014-05-13.19:00:14
Diff looks straightforward, but needs to be updated. I should be able to rebuild manually and build a jar you can then test. If that works, we will add to our beta 4 release.
msg8394 (view) Author: Jim Baker (zyasoft) Date: 2014-05-13.19:00:42
You = Ryan Watkins in my previous message :)
msg9073 (view) Author: Jim Baker (zyasoft) Date: 2014-10-05.16:48:38
Deferring to 2.7.1 or later
History
Date User Action Args
2014-10-05 16:48:38zyasoftsetpriority: low
messages: + msg9073
2014-05-13 19:00:43zyasoftsetmessages: + msg8394
2014-05-13 19:00:14zyasoftsetassignee: amak -> zyasoft
resolution: accepted
messages: + msg8393
2014-05-12 22:33:52ryanwatkinssetmessages: + msg8386
2014-05-04 20:22:52zyasoftsetnosy: + zyasoft
messages: + msg8330
title: Add IBM i support to Jython 2.5.1 -> Add IBM i support to Jython
2013-02-19 21:51:37fwierzbickisetversions: + Jython 2.5, - 2.5.3b2
2012-05-31 18:52:19ryanwatkinssetmessages: + msg7176
2012-05-31 18:48:31ryanwatkinssetmessages: + msg7175
2012-05-05 15:22:54amaksetmessages: + msg7082
2012-05-03 02:54:51ryanwatkinssetmessages: + msg7078
2012-05-01 16:41:50amaksetmessages: + msg7075
versions: + 2.5.3b2, - 2.5.1
2012-04-28 22:30:56fwierzbickisetmessages: + msg7073
2012-04-28 17:37:14amaksetmessages: + msg7072
2012-04-10 00:09:07ryanwatkinssetmessages: + msg7042
2012-03-30 19:11:37ryanwatkinssetmessages: + msg6989
2012-03-29 18:08:38fwierzbickisetmessages: + msg6976
2012-03-28 19:54:24amaksetassignee: amak
messages: + msg6972
2012-03-28 18:43:46ryanwatkinssetmessages: + msg6971
2012-03-28 18:39:30ryanwatkinssetmessages: + msg6970
2012-03-20 04:31:17fwierzbickisetnosy: + fwierzbicki
2012-03-19 22:06:42amaksetnosy: + amak
messages: + msg6893
2012-02-21 21:49:10ryanwatkinscreate