Opened 3 years ago

Last modified 12 months ago

#3180 new defect

Fix call to subprocess.Popen in git_buildbot.py

Reported by: verm Owned by:
Priority: minor Milestone: 0.9.+
Version: master Keywords: git, simple
Cc:

Description

[3add6d54] changed git_buildbot.py to use subprocess.Popen. On Windows I believe it is always shell=True which is why passing the arguments as a string works. On Unix it must be a list (as far as I know). I do not know if it will cause any issues on Windows.

skelly on IRC mentioned this may have been meant as os.popen instead of subprocess.Popen

See git_buildbot.py:242.

This is on FreeBSD 10.1 under Python 2.7.

Change History (3)

comment:1 follow-up: Changed 3 years ago by dustin

  • Keywords simple added
  • Milestone changed from undecided to 0.9.+
  • Priority changed from major to minor

I don't think it *must* be a list -- https://docs.python.org/2/library/subprocess.html#subprocess.Popen

Perhaps we could just set shell=True?

Some tests would be great, too.

I'm setting this to minor since it's a contrib script the functionality for which is also available in Buildbot proper.

comment:2 in reply to: ↑ 1 Changed 3 years ago by verm

Replying to dustin:

I don't think it *must* be a list -- https://docs.python.org/2/library/subprocess.html#subprocess.Popen

It doesn't need to be a list if it's a single command with no args eg ls but you need to do ['ls', '-l'] for args otherwise it runs "ls -l" and you get a no such file or directory error.

Perhaps we could just set shell=True?

Not sure what the ramifications of this are if none it makes sense.

Some tests would be great, too.

I will see about that and a patch.

I'm setting this to minor since it's a contrib script the functionality for which is also available in Buildbot proper.

OK.

Note: See TracTickets for help on using tickets.