Cleaning multiple build directories

Question

I'm cleaning build directories produced by GNOME build tool, JHBuild. This tool either downloads tarballs or clones git repositories, depending on set-up. After that, it proceeds to compilation (and then installation). Once in a while, something gets screwed up, and I need to clean the build directories so that I can start from scratch (because I don't know how to fix some of these problems).

Here's how I did it, and I'd like you to tell me if it can be improved:

import os
import subprocess
top_level = os.path.expanduser("~/src/gnome")
for filename in os.listdir(top_level):
 full_path = "{}/{}".format(top_level, filename)
 if os.path.isdir(full_path):
 cmd = "cd ~/src/gnome/{} && git clean -dfx".format(filename)
 if subprocess.call(cmd, shell=True) != 0:
 cmd = "cd ~/src/gnome/{} && make distclean".format(filename)
 if subprocess.call(cmd, shell=True) != 0:
 cmd = "cd ~/src/gnome/{} && make clean".format(filename)
 subprocess.call(cmd, shell=True)

Answer 1

full_path = "{}/{}".format(top_level, filename)

You can use os.path.join(top_level, filename) for that. That way it will also work on any system which does not use / as a directory separator (that's not really a realistic concern in this case, but using os.path.join doesn't cost you anything and is a good practice to get used to).

cmd = "cd ~/src/gnome/{} && git clean -dfx".format(filename)

First of all spelling out ~/src/gnome again is bad practice. This way if you want to change it to a different directory you have to change it in all 4 places. You already have it in the top_level variable, so you should use that variable everywhere.

On second thought you should actually not use top_level here, because what you're doing here is you're joining top_level and filename. However you already did that with full_path. So you should just use full_path here.

You should also consider using os.chdir instead of cd in the shell. This way you only have to change the directory once per iteration instead of on every call. I.e. you can just do:

if os.path.isdir(full_path):
 os.chdir(full_path)
 if subprocess.call("git clean -dfx", shell=True) != 0: 
 if subprocess.call("make distclean", shell=True) != 0:
 subprocess.call("make clean", shell=True)

(Note that since full_path is an absolute path, it doesn't matter that you don't chdir back at the end of each iteration.)

Another good habit to get into is to not use shell=True, but instead pass in a list with command and the arguments, e.g. subprocess.call(["make", "clean"]). It doesn't make a difference in this case, but in cases where your parameters may contain spaces, it saves you the trouble of escaping them.

• tshepang: Regarding your very last comment, I'm tempted to change the subprocess to something like subprocess.call("git clean -dfx".split()). Is that kool?
• sepp2k: @Tshepang: The point of passing in an array instead of shell=True is that it will handle arguments with spaces or shell-metacharacters automatically correctly without you having to worry about. Using split circumvents this again (as far as spaces are concerned anyway), so there's little point in that. Since in this case you know that your arguments don't contain any funny characters, you can just keep using shell=True if you don't want to pass in a spelled-out array.
• tshepang: In such cases, won't shlex.split() be useful?
• sepp2k: @Tshepang: I don't see how. If you already know the arguments, subprocess.call(shlex.split('some_command "some filename"')) buys you nothing over subprocess.call(["some_command", "some filename"] and IMHO has a higher chance that you might forget the quotes. However if the filename is inside a variable, subprocess.call(["some_comand", filename]) still works no matter which special characters filename contains, while using shlex.split would require you to somehow escape the contents of filename first, in which case you could have just used shell=True in the first place.