8
\$\begingroup\$

I wrote a little Python script to replace some files in usr/bin with symlinks to files in a different location.

I wouldn't mind anyone telling me where I could do better.

#!/usr/bin/env python -tt

"""Xcode4 installs and expects to find a git installation at /usr/bin.
This is a pain if you want to control your own installation of git that
might be installed elsewhere. This script replaces the git files in
/usr/bin with symlinks to the preferred installation.

Update the 'src_directory' to the location of your preferred git installation.
"""
import sys
import os

#- Configuration  -----------------------------------------------------------------
src_directory = '/usr/local/git/bin/' # preferred installation
#----------------------------------------------------------------------------------

dest_directory = '/usr/bin/'
files = ('git','git-cvsserver','git-receive-pack','git-shell','git-upload-archive','git-upload-pack','gitk')

def main():
    if os.getuid():
        print "This script needs to be run as 'sudo python update_git.py'"
        sys.exit(1)

    for a_file in files:
        src_file = os.path.join(src_directory, a_file)
        dest_file = os.path.join(dest_directory, a_file)

        if os.path.exists(dest_file):
            os.remove(dest_file)

        os.symlink(src_file, dest_file)

if __name__ == '__main__':
    main()
\$\endgroup\$

1 Answer 1

5
\$\begingroup\$
#!/usr/bin/env python -tt

"""Xcode4 installs and expects to find a git installation at /usr/bin.
This is a pain if you want to control your own installation of git that
might be installed elsewhere. This script replaces the git files in
/usr/bin with symlinks to the preferred installation.

Update the 'src_directory' to the location of your preferred git installation.
"""
import sys
import os

#- Configuration  -----------------------------------------------------------------
src_directory = '/usr/local/git/bin/' # preferred installation
#----------------------------------------------------------------------------------

dest_directory = '/usr/bin/'
files = ('git','git-cvsserver','git-receive-pack','git-shell','git-upload-archive','git-upload-pack','gitk')

The official python style guide recommends using ALL_CAPS to name global constants. Additionally, some of these constants might be better as command line arguments to the script. That way you don't need to modify the script to install to a different location.

def main():
    if os.getuid():

I suggest using if os.getuid() != 0 because I think its better to be explicit. The code will run the same, but this way I think its clearer that you are checking for zero rather then an actual boolean value.

        print "This script needs to be run as 'sudo python update_git.py'"
        sys.exit(1)

    for a_file in files:

a_file is kinda ugly. I'm guessing you are using it to avoid replace the builtin file. I suggest filename.

        src_file = os.path.join(src_directory, a_file)
        dest_file = os.path.join(dest_directory, a_file)

        if os.path.exists(dest_file):
            os.remove(dest_file)

What if someone deletes the file between your script checking if it exists and deleting it? Also checking if the file exists duplicates effort that remove will have to do. You could rewrite the code to try remove, and then catching the doesn't exist exception. I wouldn't bother here though.

        os.symlink(src_file, dest_file)

if __name__ == '__main__':
    main()

You don't catch any exceptions. In a script this simple that might be okay. But it might be a good idea to try/catch IOError/OSError and print them out for the user hopefully with enough detail that the user can tell whats going wrong. If you don't the exception will be dumped, but the user will also see a stack trace which might be scary.

\$\endgroup\$
1
  • \$\begingroup\$ Wow! That's a lot of good points. Thanks very much! \$\endgroup\$
    – Abizern
    Commented May 28, 2011 at 16:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.