Message6972

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.