From 6f7be11fa1dfb7538af54ffafd1cc644d3975a1d Mon Sep 17 00:00:00 2001
From: Dmitry Vyukov <dvyukov@google.com>
Date: Wed, 27 Nov 2019 14:05:14 +0100
Subject: dashboard/app: don't report bisections pointing to release commits

They should have been detected by "same binary" logic.
But the problem is that we may use different compilers
for different commits and they switch exactly at release commits.
So we can build the release with a differnet compiler than the
rest of commits and then obviously it won't be "same binary".
Detect release commits separately.

Update #1271
---
 pkg/bisect/bisect.go      |  7 +++++++
 pkg/bisect/bisect_test.go | 14 ++++++++++++++
 pkg/vcs/git.go            | 12 ++++++++++++
 pkg/vcs/vcs.go            |  2 ++
 4 files changed, 35 insertions(+)

(limited to 'pkg')

diff --git a/pkg/bisect/bisect.go b/pkg/bisect/bisect.go
index 4fbeeec4f..1f3d8ea61 100644
--- a/pkg/bisect/bisect.go
+++ b/pkg/bisect/bisect.go
@@ -71,6 +71,7 @@ const NumTests = 10 // number of tests we do per commit
 //    - Commit is nil
 //    - NoopChange is set if the commit did not cause any change in the kernel binary
 //      (bisection result it most likely wrong)
+//    - Bisected to a release commit
 //  - if bisection is inconclusive, range of potential cause/fix commits in Commits
 //    - report is nil in such case
 //    - Commit is nil
@@ -83,6 +84,7 @@ type Result struct {
 	Report     *report.Report
 	Commit     *vcs.Commit
 	NoopChange bool
+	IsRelease  bool
 }
 
 // Run does the bisection and returns either the Result,
@@ -221,6 +223,11 @@ func (env *env) bisect() (*Result, error) {
 	}
 	if len(commits) == 1 {
 		com := commits[0]
+		isRelease, err := env.bisecter.IsRelease(com.Hash)
+		if err != nil {
+			env.log("failed to detect release: %v", err)
+		}
+		res.IsRelease = isRelease
 		if testRes := results[com.Hash]; testRes != nil {
 			res.Report = testRes.rep
 			if testRes.kernelSign != "" && len(com.Parents) == 1 {
diff --git a/pkg/bisect/bisect_test.go b/pkg/bisect/bisect_test.go
index 01dbfd44e..56e5e5ccb 100644
--- a/pkg/bisect/bisect_test.go
+++ b/pkg/bisect/bisect_test.go
@@ -127,6 +127,7 @@ type BisectionTest struct {
 	expectErr    bool
 	expectRep    bool
 	noopChange   bool
+	isRelease    bool
 	commitLen    int
 	oldestLatest int
 	// input and output
@@ -177,6 +178,7 @@ func TestBisectionResults(t *testing.T) {
 			startCommit: 400,
 			commitLen:   1,
 			culprit:     500,
+			isRelease:   true,
 		},
 		// Tests that fix bisection returns error when crash does not reproduce
 		// on the original commit.
@@ -254,6 +256,7 @@ func TestBisectionResults(t *testing.T) {
 			sameBinaryStart: 405,
 			sameBinaryEnd:   500,
 			noopChange:      true,
+			isRelease:       true,
 		},
 		{
 			name:            "cause-same-binary-release2",
@@ -285,6 +288,14 @@ func TestBisectionResults(t *testing.T) {
 			sameBinaryEnd:   905,
 			noopChange:      true,
 		},
+		{
+			name:        "fix-release",
+			fix:         true,
+			startCommit: 400,
+			commitLen:   1,
+			culprit:     900,
+			isRelease:   true,
+		},
 	}
 	for _, test := range tests {
 		test := test
@@ -328,6 +339,9 @@ func TestBisectionResults(t *testing.T) {
 			if res.NoopChange != test.noopChange {
 				t.Fatalf("got noop change: %v, want: %v", res.NoopChange, test.noopChange)
 			}
+			if res.IsRelease != test.isRelease {
+				t.Fatalf("got release change: %v, want: %v", res.IsRelease, test.isRelease)
+			}
 			if test.oldestLatest != 0 && fmt.Sprint(test.oldestLatest) != res.Commit.Title ||
 				test.oldestLatest == 0 && res.Commit != nil {
 				t.Fatalf("expected latest/oldest: %v got '%v'",
diff --git a/pkg/vcs/git.go b/pkg/vcs/git.go
index 5a6de4b3f..a42f12edc 100644
--- a/pkg/vcs/git.go
+++ b/pkg/vcs/git.go
@@ -476,3 +476,15 @@ func (git *git) previousReleaseTags(commit string, self bool) ([]string, error)
 	tags = append(tags, tags1...)
 	return tags, nil
 }
+
+func (git *git) IsRelease(commit string) (bool, error) {
+	tags1, err := git.previousReleaseTags(commit, true)
+	if err != nil {
+		return false, err
+	}
+	tags2, err := git.previousReleaseTags(commit, false)
+	if err != nil {
+		return false, err
+	}
+	return len(tags1) != len(tags2), nil
+}
diff --git a/pkg/vcs/vcs.go b/pkg/vcs/vcs.go
index 777f0f7e7..ea4e1d0ad 100644
--- a/pkg/vcs/vcs.go
+++ b/pkg/vcs/vcs.go
@@ -64,6 +64,8 @@ type Bisecter interface {
 	// PreviousReleaseTags returns list of preceding release tags that are reachable from the given commit.
 	PreviousReleaseTags(commit string) ([]string, error)
 
+	IsRelease(commit string) (bool, error)
+
 	EnvForCommit(binDir, commit string, kernelConfig []byte) (*BisectEnv, error)
 }
 
-- 
cgit mrf-deployment