[PATCH v3] 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

---
changes in v2:
  prettier code
changes in v3:
  process handling inserted in try-except block -> if user does not have
  git installed, it will throw OSError exception
---
Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;

Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
---
 lnst/Common/Config.py | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/lnst/Common/Config.py b/lnst/Common/Config.py
index 5362da1..d47f3e2 100644
--- a/lnst/Common/Config.py
+++ b/lnst/Common/Config.py
@@ -34,19 +34,26 @@ 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:
+            cmd = ['git', 'rev-parse', 'HEAD']
+            try:
+                proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=null)
+                data = proc.communicate()
+            except:
+                os.chdir(cwd)
+                return LNSTMajorVersion
+            # git command passed
+            if data[0] != '':
+                version = data[0].strip()
+            # git command failed
+            else:
+                version = LNSTMajorVersion
+        os.chdir(cwd)
+        return version
 
     def controller_init(self):
         self._options['environment'] = dict()
-- 
2.4.11

[Ondrej Lichtner]

On Tue, Jul 19, 2016 at 11:11:12AM +0200, Jiri Prochazka 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
>  
>  ---
>  changes in v2:
>    prettier code
>  changes in v3:
>    process handling inserted in try-except block -> if user does not have
>    git installed, it will throw OSError exception
>  ---
>  Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
>  
>  Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
>  --- 
The Signed-of-by line should be part of the commit message - before the
first '---' line, otherwise it won't be part of the commit after
applying.

But I can fix that manually when pushing so no need for a v4 because of
that.

Other than that I think everything looks fine.

Unless anyone else has any comments then I can Ack this and push it to
master.

Acked-by: Ondrej Lichtner <olichtne(a)redhat.com&gt;

[Jan Tluka]

Wed, Jul 20, 2016 at 10:45:31AM CEST, olichtne(a)redhat.com wrote:
> On Tue, Jul 19, 2016 at 11:11:12AM +0200, Jiri Prochazka 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
> > 
> > ---
> > changes in v2:
> >   prettier code
> > changes in v3:
> >   process handling inserted in try-except block -> if user does not have
> >   git installed, it will throw OSError exception
> > ---
> > Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
> > 
> > Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
> > ---
>
> The Signed-of-by line should be part of the commit message - before the
> first '---' line, otherwise it won't be part of the commit after
> applying.
>
> But I can fix that manually when pushing so no need for a v4 because of
> that.
>
> Other than that I think everything looks fine.
>
> Unless anyone else has any comments then I can Ack this and push it to
> master.
>
> Acked-by: Ondrej Lichtner <olichtne(a)redhat.com&gt;
> _______________________________________________
> LNST-developers mailing list
> lnst-developers(a)lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/[e-mail]... 
I'm fine with this.

Acked-by: Jan Tluka <jtluka(a)redhat.com&gt;

-Jan

[Ondrej Lichtner]

On Tue, Jul 19, 2016 at 11:11:12AM +0200, Jiri Prochazka 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
>  
>  ---
>  changes in v2:
>    prettier code
>  changes in v3:
>    process handling inserted in try-except block -> if user does not have
>    git installed, it will throw OSError exception
>  ---
>  Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
>  
>  Signed-off-by: Jiri Prochazka <jprochaz(a)redhat.com&gt;
>  ---
>   lnst/Common/Config.py | 33 ++++++++++++++++++++-------------
>   1 file changed, 20 insertions(+), 13 deletions(-)
>  
>  diff --git a/lnst/Common/Config.py b/lnst/Common/Config.py
>  index 5362da1..d47f3e2 100644
>  --- a/lnst/Common/Config.py
>  +++ b/lnst/Common/Config.py
>  @@ -34,19 +34,26 @@ 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:
>  +            cmd = ['git', 'rev-parse', 'HEAD']
>  +            try:
>  +                proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=null)
>  +                data = proc.communicate()
>  +            except:
>  +                os.chdir(cwd)
>  +                return LNSTMajorVersion
>  +            # git command passed
>  +            if data[0] != '':
>  +                version = data[0].strip()
>  +            # git command failed
>  +            else:
>  +                version = LNSTMajorVersion
>  +        os.chdir(cwd)
>  +        return version
>   
>       def controller_init(self):
>           self._options['environment'] = dict()
>  -- 
>  2.4.11
>  _______________________________________________
>  LNST-developers mailing list
>  lnst-developers(a)lists.fedorahosted.org
>  https://lists.fedorahosted.org/admin/lists/[e-mail]... 
pushed to master, thanks

-Ondrej