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

[Jiri Prochazka]

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()
+            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

[Jan Tluka]

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

>+            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]

[Jiri Prochazka]

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.


>  >+            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]...
>

[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]...
>>