HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythongitMinor

Use git to get SHA-1 of repo

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
shagetgituserepo

Problem

I've created the function below to get the SHA-1 of the repository used by my CI server. The hash is then used to update the pass/fail status of the commit on GitHub.

I'm a python amateur. What stands out below?

def get_sha():
    root = os.environ["PWD"]
    repodir = os.listdir(root)[0]
    # /Library/Developer/XcodeServer/Integrations/Caches/6490b1f573dca4e4e0d988197ae6c225/Source/repo_name
    repo = os.path.join(root, repodir)
    sha = subprocess.check_output(['git', 'rev-parse', 'HEAD'], cwd=repo).decode('ascii').strip()
    return sha


Please advise on best practices, errors this might encounter, values I should be checking, etc.

EDIT:

Based on feedback I have revised it to the following:

def get_sha():
    for subdir in os.listdir('.'):
        if is_git_directory(subdir):
            return get_repo_sha(subdir)
    assert False

def is_git_directory(path = '.'):
    return subprocess.call(['git', '-C', path, 'status'], stderr=subprocess.STDOUT, stdout = open(os.devnull, 'w')) == 0    

def get_repo_sha(repo):
    sha = subprocess.check_output(['git', 'rev-parse', 'HEAD'], cwd=repo).decode('ascii').strip()
    return sha

Solution

It seems to me that this would be equivalent but shorter:

def get_sha():
    repo = os.listdir('.')[0]
    sha = subprocess.check_output(['git', 'rev-parse', 'HEAD'], cwd=repo).decode('ascii').strip()
    return sha


Because, . is essentially the same as os.environ['PWD'],
with the difference that it's not an absolute path.
Since subprocess.check_output(..., cwd=...) works fine with relative paths too,
I don't see the reason to use an absolute path, and so no need for PWD.

But more importantly,
I find it a bit odd that this script assumes that the first entry in the directory listing is a Git repository.
This setup seems a bit fragile.
It would be better to pass the repo path as a parameter to this function:

def get_sha(repo):
    sha = subprocess.check_output(['git', 'rev-parse', 'HEAD'], cwd=repo).decode('ascii').strip()
    return sha


Then you'd have something simple, clear, no questions asked.

Code Snippets

def get_sha():
    repo = os.listdir('.')[0]
    sha = subprocess.check_output(['git', 'rev-parse', 'HEAD'], cwd=repo).decode('ascii').strip()
    return sha
def get_sha(repo):
    sha = subprocess.check_output(['git', 'rev-parse', 'HEAD'], cwd=repo).decode('ascii').strip()
    return sha

Context

StackExchange Code Review Q#82630, answer score: 7

Revisions (0)

No revisions yet.