Popen should use a sequence of command line arguments, not a string.

[choksi81]

We are using subprocess.Popen() with a string based argument in many places. Suppose we want to run the command: ls -l, we commonly call this like:

subprocess.Popen('ls -l')

This actually causes us problems when the command or arguments may have spaces. To avoid any potential problems, we should instead use the 'sequence' form. This looks like the following:

subprocess.Popen('-l')

This seems to be the root cause of #505

[choksi81]

Author: justinc
This ticket requires a deeper understanding of Popen's behavior than I originally thought. This is not as easy as changing the strings to lists! There are a lot of places where we use Popen with a series of commands involving pipes or redirection. Someone needs to take the time to fully understand how to do this the correct way and then translate all of the calls over.

The parts of this ticket that are known to impact us are resolved in r2444 and the fix for #505.

[choksi81]

Author: cemeyer
This ticket is something I had noticed and wanted to improve for a while.

[choksi81]

Author: cemeyer
The fix for all of the Popen-using files in repy is in r2461. I have tested every replacement made in that diff on the platform it is supposed to run on, to make sure it has the correct behavior.

Note: there is one place in windows_api.py where I couldn't find a way to use the sequence-style Popen invocation because of the way subprocess works on Windows.

[choksi81]

Author: cemeyer
I'll look through the other components for Popen usage when I get a chance.

[choksi81]

Author: cemeyer
Ok, so I've mostly finished the changes needed to nodemanager -- there's still a minor issue, but I'll send you an email about that. (I haven't committed anything yet.)

It looks like softwareupdater already uses Popen correctly.

---

There are a few uses of the depricated form of popen, but none of them are in nodemanager/softwareupdater/repy:

• deploymentscripts/reinstall.py uses it in a couple of places
• dist/install.py uses it once
• autograder/emulab/sshxmlrpc.py uses it
• integrationtests/pushsoftwareupdate/pushsoftwareupdate.mix uses it
• dist/linux/scripts/uninstall.py
• dist/mac/scripts/uninstall.py

There are several uses of os.system, again none of which are in "critical" places (repy, nodemanager, softwareupdater):

• www/deploy_state_transitions.py uses it
• autograder/emulab/sshxmlrpc.py uses os.system as well as os.popen!
• deploymentscripts/tests/testsDeployRun.py
• Several tests in integrationtests/deployment_scripts/
• integrationtests/downloadandinstallseattle/downloadandinstallseattle.py
• www/keygen/genkeys.py
• www/remote_installer/remote_client.py
• www/remote_installer/remote_server.py
• autograder/www/upload/views.py
• www/geni/download/views.py
• www/web_installers/customized_installer/views.py

[choksi81]

Author: cemeyer
Fixed in r2493.

[choksi81]

Attachments:
https://github.com/SeattleTestbed/attic/blob/master/TICKET_ATTACHMENTS/nodemanager_popen_fixes.diff