Base for iterating over git history

I wanted to do some statistics over the history of a Git repository. At first I tried using GitPython, but it wasn't as trivial as I imagined it to be. In the end, I just call the necessary git commands with the submodule module.

Can this code be considered clean and readable or does it have some style issues?

import argparse
import os
import subprocess

def main(args):
if not os.path.isdir(args.path):
print "not a directory"
return
if ".git" not in os.listdir(args.path):
print "not a repo"
return

os.chdir(args.path)

commitIDs = []
start = 0       #load commits in batches, to avoid too long hangs
max_count = 100
while True:
text = subprocess.check_output(["git", "rev-list", args.branch, "--max-count=%s" % max_count, "--skip=%s" % start])
start += max_count
for line in text.splitlines():
commitIDs.append(line)
if len(text.splitlines()) != max_count:
break

print commitID
#do something with the commit
#for example:
#devnull = open(os.devnull, 'w')
#subprocess.call(["git", "checkout", commitID], stdout=devnull)

if __name__ == '__main__':
parser = argparse.ArgumentParser()
parser.add_argument('--skip', '-s', type=int, default=1, help='use every n-th commit')
args = parser.parse_args()

main(args)


Update:

import argparse
import os
import subprocess
import sys
import git

def main(args):
try:
repo = git.Repo(args.path)
except:
sys.exit("no such repo")

try:
text = repo.git.rev_list(args.branch).splitlines()
except:
sys.exit("no such branch")

commit_ids = text[::args.skip]

print "loaded %s commits" % len(commit_ids)

print commit_id
#do something with the commit
#for example:
#repo.git.checkout(commit_id)

if __name__ == '__main__':
parser = argparse.ArgumentParser()
parser.add_argument('--skip', '-s', type=int, default=1, help='use every n-th commit')
args = parser.parse_args()

main(args)
sys.exit(0)


Great, glad you got rid of that loop: some more things I'd like to point out. As this is essentially another question now I'll add another answer

1. I don't think a function should call sys.exit(). I'd rather you raised an exception instead and call sys.exit() in __main__

try:
repo = git.Repo(args.path)
except:
sys.exit("no such repo")


I would prefer:

from git import Repo, InvalidGitRepositoryError
#...

try:
repo = git.Repo(args.path)
except InvalidGitRepositoryError, e:
raise InvalidGitRepositoryError("%s is not a git repository" % (str(e)))


This enables other Python classes to use your main function (which is a terrible name) in a more predictable way (Python doesn't close calling your function).

2. I would also rather you check for the branch than assume if rev-list gives an error that the branch doesn't exist. There may be some other case that causes it to throw an exception.

class InvalidBranchError(Exception):
pass

if args.branch not in repo.branches:
raise InvalidBranchError("Branch does not exist: %s" % (args.branch))

3. Instead of main taking an arguments parser, I would prefer you used regular arguments (again so other python modules can use your code more readily):

def main(path=".", branch="master"):
pass

main(path=args.path, branch=args.branch)

• Thanks, I will work on it tomorrow. On the note of the main function, I just thought that passing args object to the function isn't a good idea too as it makes it harder to call it manually. – Adrian17 Jun 1 '14 at 22:00
• And about the first point, I'm not sure if I get it, your suggested code raises pretty much the same error, so what's the difference between not handling it at all and seeing git.exc.InvalidGitRepositoryError: C:\  and handling to see git.exc.InvalidGitRepositoryError: C:\ is not a git repository  ? – Adrian17 Jun 1 '14 at 22:09
• I figured in __main__ you would catch exceptions and do sys.exit( str(exception) ) so adding some details may be useful if you decided to use that approach – megawac Jun 1 '14 at 23:15

You actually can access the commit_id with GitPython methods via the hexsha attribute. Short example:

import git

def main():
repo = git.Repo('.')
for commit in repo.iter_commits('master'):
# do something with the commit, for example:
repo.git.diff(commit.parents[0].hexsha, commit.hexsha)
# commit.committed_datetime.strftime("%a %d. %b %Y")
# commit.message.rstrip()

if __name__ == "__main__":
main()

• The goal of the site is to provide value by reviewing the code in the question. Can you explain how your answer improves upon the original question? – forsvarir Jul 20 '16 at 10:24
• Thanks for clarification, I wasn't aware that. Basically this is a very short answer to following sentence form the original question: 'At first I tried using GitPython, but it wasn't as trivial as I imagined it to be.' I didn't have the time to fully rewrite the code, but thought might profit if I share my 'solution'. – Andi Jul 26 '16 at 14:00

• I would suggest to use exit codes (sys.exit()), it will help you if you are planning to you your script together with other scripts (chaining it, or use from shell scripts)
• I would consider that subprocess.check_output(["git", "rev-list... may return not a list of commits (but error for example). In this case you may go to infinite loop.
• Usually you can execute git commands from any subfolder of a git repo. In your case you are limiting it to be a root folder by using this condition if ".git" not in os.listdir(args.path)
• Instead of for line in text.splitlines(): commitIDs.append(line), do commitIDs.extend(text.splitlines())
• You execute text.splitlines() twice, consider creating a temporary variable for it
• You are constructing a huge list first and use args.skip on it. Instead you can apply args.skip inside your while loop. It will limit amound of memory you need.
• commitIDs => commit_ids
• they don't necessary conflict commits = text.splitlines()[::args.skip], commitIDs.extend(commits) – RomanI Jun 1 '14 at 17:59