Certain commit makes barkeep explode

[cespare]

(From Mike)

The commit <barkeep url>/commits/uploader/707533b291bc6871655425746dae75e28fc14717 produces a 500:

undefined method `+' for nil:NilClass
/Users/caleb/Projects/barkeep/lib/git_helper.rb:195:in `block in tag_diff'
/Users/caleb/Projects/barkeep/lib/git_helper.rb:157:in `each'
/Users/caleb/Projects/barkeep/lib/git_helper.rb:157:in `tag_diff'
/Users/caleb/Projects/barkeep/lib/git_helper.rb:122:in `tag_file'
/Users/caleb/Projects/barkeep/lib/git_helper.rb:99:in `block in get_tagged_commit_diffs'
/Users/caleb/Projects/barkeep/lib/git_helper.rb:75:in `map'
/Users/caleb/Projects/barkeep/lib/git_helper.rb:75:in `get_tagged_commit_diffs'
/Users/caleb/Projects/barkeep/app.rb:161:in `block in <class:Barkeep>'

[mstyer]

Looks like it happens on the first commit of any repo; also happens on this one:

http://barkeep.sv2/commits/ingestion-testing/cde12e52685dc8fe3d6ca4d9af1f0a9c5ed2446d

which is the first commit in the ingestion-testing repo.

[bkad]

I'll take a look tonight

[bkad]

i've come to the conclusion that i have no business parsing unified diffs.
usually new files are of the chunk form @@ -0 +1,:diff_length @@, but in this commit, it is of the chunk form @@ -0,0 +1 @@, so our diff parser thinks its a removed file instead of a new one and fails.

It's possible it has something to do with this diff being only one line long

[cespare]

This did not fix the issue for me. I get a different stack trace, but there's still a 500 from the same file.

[bkad]

1461d6d fixes the exception, but causes the diff to lose information.

the (second) cause of the bug is trailing newlines in diffs getting eaten by String.split("\n"). The fix is to pass a negative max splits argument to .split.

[bkad]

fixed in fb46059

[cespare]

Cool, LGTM.