Re: Clearcase Popen problem on buggy python 2.7.2

[Christian Hammond]

Hi Bart,

That change to add shell= was to fix a bug when installed on Windows 7 where a console window would pop up every time cleartool was invoked. It seemed pretty bad, but I don't know if there was a better way to tackle it.

I'm not sure what the best solution is, given both the problem you're hitting and the Windows 7 issue. Any thoughts?

Christian

--
Christian Hammond - chi...@chipx86.com
Review Board - http://www.reviewboard.org
VMware, Inc. - http://www.vmware.com

On Fri, Feb 1, 2013 at 12:49 PM, Bart Celary <bce...@gmail.com> wrote:

Good day!

I have recently upgraded to 1.7.4 our review server and after solving the issue I am writing about I must admit there's a bunch of changes I love! Great work!

Now back to the issue.

We are using an old and buggy (especially in respect to mod_wsgi, see: http://stackoverflow.com/a/8310359/15165 / http://bugs.python.org/issue13156) version of Python. Why? It is actually a case of not having enough time to spend on re-installing RB on newer Python. The problem was with the subprocess.Popen which did not work well in clearcase module (stdout/err were empty). On 1.6.4 everything worked fine so I've checked the difference and it appears that removing the Shell option from the Popen invocations fixed the issue on 1.7.4. I have not created a patch for this change but it is rather straightforward. I believe the Popen calls do not require running the commands through a shell. I wonder if Jan K. could comment on that and check if it is possible to remove this. It would make future upgrades less painful for me and people who are stuck on that buggy version of Python.

Regards,

Bart

--
 
---
You received this message because you are subscribed to the Google Groups "reviewboard-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to reviewboard-d...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Bart Celary]

It looks like the shell= fix is more important in this case. I will have to live with it. The only possible option would be to detect we are on Windows and keep this as a flag for Popen calls. Not sure if that would be OK? If so, I can send a patch for review.

Bart

[Christian Hammond]

Provided it's heavily documented in the code (description of the Windows bug, description of the Python bug, version numbers you know are affected), I'm fine with conditionally setting shell.

Is this not a problem for other processes that Review Board calls out to? Or are you using this exclusively with clearcase?

Christian

-- 
Christian Hammond - chi...@chipx86.com
Review Board - http://www.reviewboard.org
VMware, Inc. - http://www.vmware.com

[Bart Celary]

Yes. This problem affects any other process that calls a command through shell... However only clearcase.py uses shell=True option so the workaround would be limited to clearcase.py.

Bart

[Christian Hammond]

What I'm curious about, and we'd need someone to test this, is if this solves the problem on Windows (first answer):

http://stackoverflow.com/questions/7006238/how-do-i-hide-the-console-when-i-use-os-system-or-subprocess-call

Tao, would you be able to test this?

That way, we wouldn't have to pass the shell parameter, but still prevent a console window from opening.

Christian

-- 
Christian Hammond - chi...@chipx86.com
Review Board - http://www.reviewboard.org
VMware, Inc. - http://www.vmware.com

[nan...@gmail.com]

All,

please find a proposal to fix this regression at http://reviews.reviewboard.org/r/3994/

Nicolas

Le samedi 23 février 2013 13:22:19 UTC+1, nan...@gmail.com a écrit :

Hi everyone,

I'm found such issue (popen +shell=true) on ubuntu 12.04 python 2.7.3

When I look at python 2.x or 3.x documentation, it is written

If passing a single string, either shell must be True (see below) or else the string must simply name the program to be executed without specifying any arguments.

http://docs.python.org/2/library/subprocess.html#frequently-used-arguments
http://docs.python.org/3/library/subprocess.html#frequently-used-arguments

So shell=true is more a workaround for windows 7 system only because all calls to popen are using multiple string

Could this change be reverted please until someone propose a conditionnal to enable sheel=true only for windows7 ?

I have no windows 7 to test but some conditionnals proposed here can do this job
http://stackoverflow.com/questions/196930/how-to-check-if-os-is-vista-in-python

best regards,

Nicolas

[nan...@gmail.com]

Hi David,
Regarding previous changes made and problem analysis, I've just try to adapt workaround to set shell=true only when environment meets context of happened issue.
I hope that original fixer can test my update and confirm it does not introduce any regression on his side

I've not tested this fix in clearcase context but Windows 7 and python 2.7.3 + a test script and check conditionnals were rights

Does this answer match your question?

Nicolas