Re: [PATCH] Config: replace subprocess.check_output with python2.6 compatible equivalent

[Jan Tluka]

Tue, Jul 19, 2016 at 10:38:14AM CEST, jprochaz(a)redhat.com wrote:
> 2016-07-19 10:11 GMT+02:00 Jan Tluka <jtluka(a)redhat.com&gt;:
>
> > Mon, Jul 18, 2016 at 06:44:07PM CEST, jprochaz(a)redhat.com wrote:
> > >Method check_output used for getting hash of HEAD in git repo was
> > >introduced in Python2.7 and it does not work for Python2.6 which is
> > >default Python version for RHEL6.x.
> > >
> > >This patch replaces method check_output with Popen which is available in
> > >both 2.6 and 2.7.
> > >
> > >Fixes #174
> > >
> > >Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
> > >---
> > > lnst/Common/Config.py | 27 ++++++++++++++-------------
> > > 1 file changed, 14 insertions(+), 13 deletions(-)
> > >
> > >diff --git a/lnst/Common/Config.py b/lnst/Common/Config.py
> > >index 5362da1..29c2f16 100644
> > >--- a/lnst/Common/Config.py
> > >+++ b/lnst/Common/Config.py
> > >@@ -34,19 +34,20 @@ class Config():
> > >
> > >     def _get_version(self):
> > >         # Check if I'm in git
> > >-        try:
> > >-            cwd = os.getcwd()
> > >-            abspath = os.path.abspath(__file__)
> > >-            dname = os.path.dirname(abspath)
> > >-            os.chdir(dname)
> > >-            with open(os.devnull, 'w') as null:
> > >-                head = subprocess.check_output(['git',
> 'rev-parse',
> > 'HEAD'],
> > >-                                               stderr=null).strip()
> > >-            return head
> > >-        except subprocess.CalledProcessError:
> > >-            return LNSTMajorVersion
> > >-        finally:
> > >-            os.chdir(cwd)
> > >+        cwd = os.getcwd()
> > >+        abspath = os.path.abspath(__file__)
> > >+        dname = os.path.dirname(abspath)
> > >+        os.chdir(dname)
> > >+        with open(os.devnull, 'w') as null:
> > >+            proc = subprocess.Popen(['git', 'rev-parse',
> 'HEAD'],
> > >+                                           stdout=subprocess.PIPE,
> > >+                                           stderr=null).communicate()
> >                                                          ^^^^
> > That looks ugly and is hard to read!
> >
> > How about exception handling?
> > https://docs.python.org/2.6/library/subprocess.html#exceptions
>
>
>
> ​Unlike check_output, neither Popen constructor or communicate() method
> throws exception when execution of the command failed.
>
>  
Yes it does,

Traceback (most recent call last):
  File "./popentest.py", line 6, in <module>
    stdout=subprocess.PIPE)
  File "/usr/lib64/python2.6/subprocess.py", line 642, in __init__
    errread, errwrite)
  File "/usr/lib64/python2.6/subprocess.py", line 1238, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory

cat popentest.py
#!/usr/bin/python

import subprocess

proc = subprocess.Popen(['glit', 'rev-parse', 'HEAD'],
                        stdout=subprocess.PIPE)

proc.communicate()

Have you checked Python2.6 docs?

One more thing, seems that null is not defined in 2.6 as well.
i.e. stderr=null is not valid. At least it did not work on my rhel6
guest.

>>
>> >+            if proc[0] != '':
>> >+                version = proc[0].strip()
>> >+            else:
>> >+                version = LNSTMajorVersion
>> >+        os.chdir(cwd)
>> >+        return version
>> >
>> >     def controller_init(self):
>> >         self._options['environment'] = dict()
>> >--
>> >2.4.11
>> >_______________________________________________
>> >LNST-developers mailing list
>> &gt;lnst-developers(a)lists.fedorahosted.org
>> >
>>
https://lists.fedorahosted.org/admin/lists/[e-mail]...
>>