Skip to content

Commit fd03f3a

Browse files
authored
chore: deprecate cherry-picking on release (#186)
## Why this should be merged Our old strategy of only cherry-picking on release branches made the `main` branch incompatible with dependent repos. ## How this works The goal of the old strategy was to avoid cherry-picking on the same branch to which we would later merge the duplicated, upstream commit. This can instead be achieved by tracking the cherry-picked commits and reverting them as part of the geth sync. Since we already do this for our own changes and a single cherry-pick (see #128), there's no need to use the two approaches simultaneously. This PR deletes the cherry-picking mechanism and removes the release-branch test that enforced its proper usage. It will be followed up by a series of PRs, one per cherry-pick that would have otherwise been placed on release branches. ## How this was tested n/a
1 parent 7b6ff3e commit fd03f3a

File tree

4 files changed

+4
-210
lines changed

4 files changed

+4
-210
lines changed

libevm/tooling/go.mod

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ replace github.com/ava-labs/libevm => ../../
77
require (
88
github.com/ava-labs/libevm v0.0.0-00010101000000-000000000000
99
github.com/go-git/go-git/v5 v5.14.0
10-
github.com/google/go-cmp v0.7.0
1110
github.com/stretchr/testify v1.10.0
1211
)
1312

libevm/tooling/release/cherrypick.sh

Lines changed: 0 additions & 47 deletions
This file was deleted.

libevm/tooling/release/cherrypicks

Lines changed: 0 additions & 15 deletions
This file was deleted.

libevm/tooling/release/release_test.go

Lines changed: 4 additions & 147 deletions
Original file line numberDiff line numberDiff line change
@@ -22,100 +22,24 @@ import (
2222
"fmt"
2323
"os"
2424
"path/filepath"
25-
"regexp"
2625
"slices"
27-
"sort"
2826
"strings"
2927
"testing"
30-
"time"
3128

3229
"github.com/go-git/go-git/v5"
3330
"github.com/go-git/go-git/v5/plumbing"
3431
"github.com/go-git/go-git/v5/plumbing/object"
35-
"github.com/google/go-cmp/cmp"
3632
"github.com/stretchr/testify/assert"
3733
"github.com/stretchr/testify/require"
3834

3935
"github.com/ava-labs/libevm/params"
40-
41-
_ "embed"
4236
)
4337

4438
func TestMain(m *testing.M) {
4539
flag.Parse()
4640
os.Exit(m.Run())
4741
}
4842

49-
var (
50-
//go:embed cherrypicks
51-
cherryPicks string
52-
lineFormatRE = regexp.MustCompile(`^([a-fA-F0-9]{40}) # (.*)$`)
53-
)
54-
55-
type parsedLine struct {
56-
hash, commitMsg string
57-
}
58-
59-
func parseCherryPicks(t *testing.T) (rawLines []string, lines []parsedLine) {
60-
t.Helper()
61-
for i, line := range strings.Split(cherryPicks, "\n") {
62-
if line == "" || strings.HasPrefix(line, "#") {
63-
continue
64-
}
65-
66-
switch matches := lineFormatRE.FindStringSubmatch(line); len(matches) {
67-
case 3:
68-
rawLines = append(rawLines, line)
69-
lines = append(lines, parsedLine{
70-
hash: matches[1],
71-
commitMsg: matches[2],
72-
})
73-
74-
default:
75-
t.Errorf("Line %d is improperly formatted: %s", i, line)
76-
}
77-
}
78-
return rawLines, lines
79-
}
80-
81-
func TestCherryPicksFormat(t *testing.T) {
82-
rawLines, lines := parseCherryPicks(t)
83-
if t.Failed() {
84-
t.Fatalf("Required line regexp: %s", lineFormatRE.String())
85-
}
86-
87-
commits := make([]struct {
88-
obj *object.Commit
89-
line parsedLine
90-
}, len(lines))
91-
92-
repo := openGitRepo(t)
93-
for i, line := range lines {
94-
obj, err := repo.CommitObject(plumbing.NewHash(line.hash))
95-
require.NoErrorf(t, err, "%T.CommitObject(%q)", repo, line.hash)
96-
97-
commits[i].obj = obj
98-
commits[i].line = line
99-
}
100-
sort.Slice(commits, func(i, j int) bool {
101-
ci, cj := commits[i].obj, commits[j].obj
102-
return ci.Committer.When.Before(cj.Committer.When)
103-
})
104-
105-
var want []string
106-
for _, c := range commits {
107-
msg := strings.Split(c.obj.Message, "\n")[0]
108-
want = append(
109-
want,
110-
fmt.Sprintf("%s # %s", c.line.hash, msg),
111-
)
112-
}
113-
if diff := cmp.Diff(want, rawLines); diff != "" {
114-
t.Errorf("Commits in `cherrypicks` file out of order or have incorrect commit message(s);\n(-want +got):\n%s", diff)
115-
t.Logf("To fix, copy:\n%s", strings.Join(want, "\n"))
116-
}
117-
}
118-
11943
const (
12044
defaultBranch = "main"
12145
releaseBranchPrefix = "release/"
@@ -149,20 +73,8 @@ func TestBranchProperties(t *testing.T) {
14973
// 1. They are named release/v${libevm-version};
15074
// 2. The libevm version's [params.ReleaseType] is appropriate for a release
15175
// branch; and
152-
// 3. The commit history is a "linear fork" off the default branch, with only
153-
// certain allowable commits.
154-
//
155-
// We define a "linear fork" as there being a single ancestral commit at which
156-
// the release branch diverged from the default branch, with no merge commits
157-
// after this divergence:
158-
//
159-
// ______________ main
160-
// \___ release/*
161-
//
162-
// The commits in the release branch that are not in the default branch MUST be:
163-
//
164-
// 1. The cherry-pick commits embedded as [cherryPicks], in order; then
165-
// 2. A single, final commit to change the libevm version.
76+
// 3. The commit history since the default branch is only a single commit, to
77+
// change the version.
16678
//
16779
// testReleaseBranch assumes that the git HEAD currently points at either
16880
// `targetBranch` itself, or at a candidate (i.e. PR source) for said branch.
@@ -199,27 +111,10 @@ func testReleaseBranch(t *testing.T, targetBranch string) {
199111
newCommits := linearCommitsSince(t, history, fork)
200112
logCommits(t, "History since fork from default branch", newCommits)
201113

202-
t.Run("cherry_picked_commits", func(t *testing.T) {
203-
_, cherryPick := parseCherryPicks(t)
204-
wantCommits := commitsFromHashes(t, repo, cherryPick, fork)
205-
logCommits(t, "Expected cherry-picks", wantCommits)
206-
if got, want := len(newCommits), len(wantCommits)+1; got != want {
207-
t.Fatalf("Got %d commits since fork from default; want number to be cherry-picked plus one (%d)", got, want)
208-
}
209-
210-
opt := compareCherryPickedCommits()
211-
if diff := cmp.Diff(wantCommits, newCommits[:len(wantCommits)], opt); diff != "" {
212-
t.Fatalf("Cherry-picked commits for release branch (-want +got):\n%s", diff)
213-
}
214-
})
215-
216114
t.Run("final_commit", func(t *testing.T) {
217-
n := len(newCommits)
218-
last := newCommits[n-1]
115+
require.Len(t, newCommits, 1, "Single commit off default branch")
116+
last := newCommits[0]
219117
penultimate := fork
220-
if n >= 2 {
221-
penultimate = newCommits[n-2]
222-
}
223118

224119
lastCommitDiffs, err := object.DiffTree(
225120
treeFromCommit(t, last),
@@ -293,25 +188,6 @@ func linearCommitsSince(t *testing.T, iter object.CommitIter, since *object.Comm
293188
return commits
294189
}
295190

296-
func commitsFromHashes(t *testing.T, repo *git.Repository, lines []parsedLine, skipAncestorsOf *object.Commit) []*object.Commit {
297-
t.Helper()
298-
299-
var commits []*object.Commit
300-
for _, l := range lines {
301-
c, err := repo.CommitObject(plumbing.NewHash(l.hash))
302-
require.NoError(t, err)
303-
304-
skip, err := c.IsAncestor(skipAncestorsOf)
305-
require.NoError(t, err)
306-
if skip || c.Hash == skipAncestorsOf.Hash {
307-
continue
308-
}
309-
commits = append(commits, c)
310-
}
311-
312-
return commits
313-
}
314-
315191
func commitMsgFirstLine(c *object.Commit) string {
316192
return strings.Split(c.Message, "\n")[0]
317193
}
@@ -323,25 +199,6 @@ func logCommits(t *testing.T, header string, commits []*object.Commit) {
323199
}
324200
}
325201

326-
// compareCherryPickedCommits returns a [cmp.Transformer] that converts
327-
// [object.Commit] instances into structs carrying only the pertinent commit
328-
// properties that remain stable when cherry-picked. Note, however, that this
329-
// does not include the actual diffs induced by cherry-picking.
330-
func compareCherryPickedCommits() cmp.Option {
331-
type comparableCommit struct {
332-
MessageFirstLine, Author string
333-
Authored time.Time
334-
}
335-
336-
return cmp.Transformer("gitCommit", func(c *object.Commit) comparableCommit {
337-
return comparableCommit{
338-
MessageFirstLine: commitMsgFirstLine(c),
339-
Author: c.Author.String(),
340-
Authored: c.Author.When,
341-
}
342-
})
343-
}
344-
345202
func treeFromCommit(t *testing.T, c *object.Commit) *object.Tree {
346203
t.Helper()
347204
tree, err := c.Tree()

0 commit comments

Comments
 (0)