From dcfe4f80531a74f09fe6b702b5fca96d763fd921 Mon Sep 17 00:00:00 2001
From: "ojarjur@google.com" <ojarjur@google.com>
Date: Fri, 20 Nov 2015 14:51:29 -0800
Subject: [PATCH] Fixed a bug in reading reviews where we would not discount
 commented-upon commits that had been garbage collected, and as a result the
 tool would exit due to a git command failing unexpectedly

---
 repository/git.go       | 13 +++++++++++++
 repository/mock_repo.go |  8 ++++++++
 repository/repo.go      |  3 +++
 review/review.go        |  3 +++
 4 files changed, 27 insertions(+)

diff --git a/repository/git.go b/repository/git.go
index e7dba6c6..c17a33ed 100644
--- a/repository/git.go
+++ b/repository/git.go
@@ -111,6 +111,19 @@ func (repo *GitRepo) HasUncommittedChanges() bool {
 	return false
 }
 
+// VerifyCommit verifies that the supplied hash points to a known commit.
+func (repo *GitRepo) VerifyCommit(hash string) error {
+	out, err := repo.runGitCommand("cat-file", "-t", hash)
+	if err != nil {
+		return err
+	}
+	objectType := strings.TrimSpace(string(out))
+	if objectType != "commit" {
+		return fmt.Errorf("Hash %q points to a non-commit object of type %q", hash, objectType)
+	}
+	return nil
+}
+
 // VerifyGitRef verifies that the supplied ref points to a known commit.
 func (repo *GitRepo) VerifyGitRef(ref string) error {
 	_, err := repo.runGitCommand("show-ref", "--verify", ref)
diff --git a/repository/mock_repo.go b/repository/mock_repo.go
index 42d7b073..54f7e91d 100644
--- a/repository/mock_repo.go
+++ b/repository/mock_repo.go
@@ -186,6 +186,14 @@ func (r mockRepoForTest) resolveLocalRef(ref string) (string, error) {
 	return "", fmt.Errorf("The ref %q does not exist", ref)
 }
 
+// VerifyCommit verifies that the supplied hash points to a known commit.
+func (r mockRepoForTest) VerifyCommit(hash string) error {
+	if _, ok := r.Commits[hash]; !ok {
+		return fmt.Errorf("The given hash %q is not a known commit", hash)
+	}
+	return nil
+}
+
 // VerifyGitRef verifies that the supplied ref points to a known commit.
 func (r mockRepoForTest) VerifyGitRef(ref string) error {
 	_, err := r.resolveLocalRef(ref)
diff --git a/repository/repo.go b/repository/repo.go
index 7d975560..24ed2929 100644
--- a/repository/repo.go
+++ b/repository/repo.go
@@ -43,6 +43,9 @@ type Repo interface {
 	// HasUncommittedChanges returns true if there are local, uncommitted changes.
 	HasUncommittedChanges() bool
 
+	// VerifyCommit verifies that the supplied hash points to a known commit.
+	VerifyCommit(hash string) error
+
 	// VerifyGitRef verifies that the supplied ref points to a known commit.
 	VerifyGitRef(ref string) error
 
diff --git a/review/review.go b/review/review.go
index 50cacf71..82a9235d 100644
--- a/review/review.go
+++ b/review/review.go
@@ -300,6 +300,9 @@ func (r *Review) GetJson() (string, error) {
 // and all of the commits that are referenced in the given comment threads.
 func (r *Review) findLastCommit(latestCommit string, commentThreads []CommentThread) string {
 	isLater := func(commit string) bool {
+		if err := r.Repo.VerifyCommit(commit); err != nil {
+			return false
+		}
 		if r.Repo.IsAncestor(latestCommit, commit) {
 			return true
 		}