Skip to content

Commit 85b65cd

Browse files
committed
release: merge ER commits to RC branches
Previously, as a part of post-publishing tasks, we created merge commits to the main release branch. When we have an extraordinary release, there is a chance of having the next release's branch already cut. If the extraordinary release PRs are not backported to the next release's branch, we may have a regression, when the next version doesn't have the fix merged to the extraordinary release branch. This PR adds an additional check to verify that all changes from the extraordinary branch are backported to the next release branch if it's already cut. If there are changes, automation creates a merge PR to have all extraordinary changes in the next release branch. Additionally, add a check if there were no changes on the baking/staging branch after it was cut and skip the merge PR creation. Added a global flag pointing to the artifacts directory for cases we need to store some outputs. Fixes: RE-817 Release note: None
1 parent c6550af commit 85b65cd

File tree

4 files changed

+202
-21
lines changed

4 files changed

+202
-21
lines changed

build/teamcity/internal/cockroach/release/process/update_versions_impl.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,4 +37,5 @@ $(bazel info --config=crosslinux bazel-bin)/pkg/cmd/release/release_/release \
3737
3838
--smtp-host=smtp.gmail.com \
3939
--smtp-port=587 \
40+
--artifacts-dir=/artifacts \
4041
--to=$to

pkg/cmd/release/git.go

Lines changed: 91 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package main
77

88
import (
99
"context"
10+
"errors"
1011
"fmt"
1112
"log"
1213
"os/exec"
@@ -288,7 +289,7 @@ func listRemoteBranches(pattern string) ([]string, error) {
288289
if err != nil {
289290
return []string{}, fmt.Errorf("git ls-remote: %w", err)
290291
}
291-
log.Printf("git ls-remote returned: %s", out)
292+
log.Printf("git ls-remote for %s returned: %s", pattern, out)
292293
var remoteBranches []string
293294
// Example output:
294295
// $ git ls-remote origin "refs/heads/release-23.1*"
@@ -307,25 +308,102 @@ func listRemoteBranches(pattern string) ([]string, error) {
307308
return remoteBranches, nil
308309
}
309310

310-
// fileExistsInGit checks if a file exists in a local repository, assuming the remote name is `origin`.
311-
func fileExistsInGit(branch string, f string) (bool, error) {
312-
cmd := exec.Command("git", "ls-tree", remoteOrigin+"/"+branch, f)
311+
// fileContent uses `git cat-file -p ref:file` to get to the file contents without `git checkout`.
312+
func fileContent(ref string, f string) (string, error) {
313+
cmd := exec.Command("git", "cat-file", "-p", ref+":"+f)
313314
out, err := cmd.Output()
314315
if err != nil {
315-
return false, fmt.Errorf("git ls-tree: %s %s %w, `%s`", branch, f, err, out)
316+
return "", fmt.Errorf("git cat-file %s:%s: %w, `%s`", ref, f, err, out)
316317
}
317-
if len(out) == 0 {
318-
return false, nil
318+
return string(out), nil
319+
}
320+
321+
// isAncestor checks if ref1 is an ancestor of ref2.
322+
// Returns true if ref1 is an ancestor of ref2, false if not, and error if the command fails.
323+
func isAncestor(ref1, ref2 string) (bool, error) {
324+
cmd := exec.Command("git", "merge-base", "--is-ancestor", remoteOrigin+"/"+ref1, remoteOrigin+"/"+ref2)
325+
err := cmd.Run()
326+
if err != nil {
327+
// Treat exit code 1 as false, as it means that ref1 is not an ancestor of ref2.
328+
var exitErr *exec.ExitError
329+
if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 {
330+
return false, nil
331+
}
332+
return false, fmt.Errorf("checking ancestry relationship between %s and %s: %w", ref1, ref2, err)
319333
}
320334
return true, nil
321335
}
322336

323-
// fileContent uses `git cat-file -p ref:file` to get to the file contents without `git checkout`.
324-
func fileContent(ref string, f string) (string, error) {
325-
cmd := exec.Command("git", "cat-file", "-p", ref+":"+f)
326-
out, err := cmd.Output()
337+
// mergeCreatesContentChanges checks if a merge commit introduces changes to the branch.
338+
// Returns true if the merge commit introduces changes, false if not, and error if the command fails.
339+
func mergeCreatesContentChanges(branch, intoBranch string, ignoredPatterns []string) (bool, error) {
340+
// Make sure the working directory is clean
341+
statusCmd := exec.Command("git", "status", "--porcelain")
342+
if out, err := statusCmd.Output(); err != nil {
343+
return false, fmt.Errorf("checking git status: %w", err)
344+
} else if len(out) > 0 {
345+
return false, fmt.Errorf("working directory is not clean")
346+
}
347+
348+
// Get the current branch name before we start
349+
currentBranchCmd := exec.Command("git", "rev-parse", "--abbrev-ref", "HEAD")
350+
currentBranch, err := currentBranchCmd.Output()
327351
if err != nil {
328-
return "", fmt.Errorf("git cat-file %s:%s: %w, `%s`", ref, f, err, out)
352+
return false, fmt.Errorf("getting current branch: %w", err)
329353
}
330-
return string(out), nil
354+
originalBranch := strings.TrimSpace(string(currentBranch))
355+
356+
// Checkout the branch to merge into. Use a temporary branch to avoid
357+
// conflicts with the current branch name.
358+
tmpIntoBranch := intoBranch + "-tmp"
359+
checkoutCmd := exec.Command("git", "checkout", "-b", tmpIntoBranch, remoteOrigin+"/"+intoBranch)
360+
if err := checkoutCmd.Run(); err != nil {
361+
return false, fmt.Errorf("running checkout: %w", err)
362+
}
363+
364+
// Run the merge command without committing and without fast-forward. If
365+
// fast-forward is allowed and the current branch can fast forward, there
366+
// will be no merge commit, so the --no-commit option won't work.
367+
// We need to use the ours strategy to avoid conflicts. In the next step we
368+
// will checkout the ignored files from the current branch (like version.txt).
369+
mergeCmd := exec.Command("git", "merge", "--no-commit", "--no-ff", "--strategy=recursive", "-X", "ours", remoteOrigin+"/"+branch)
370+
if err := mergeCmd.Run(); err != nil {
371+
return false, fmt.Errorf("running merge: %w", err)
372+
}
373+
if len(ignoredPatterns) > 0 {
374+
coCmd := exec.Command("git", "checkout", tmpIntoBranch, "--")
375+
coCmd.Args = append(coCmd.Args, ignoredPatterns...)
376+
377+
if err := coCmd.Run(); err != nil {
378+
return false, fmt.Errorf("running checkout: %w", err)
379+
}
380+
}
381+
382+
// Check if there are any content changes. The exit code will be analyzed to
383+
// determine if there are changes after we clean up the current repo.
384+
diffCmd := exec.Command("git", "diff", "--staged", "--quiet")
385+
diffErr := diffCmd.Run()
386+
387+
// Always abort the merge attempt to clean up
388+
if err := exec.Command("git", "merge", "--abort").Run(); err != nil {
389+
return false, fmt.Errorf("aborting merge: %w", err)
390+
}
391+
if err := exec.Command("git", "checkout", originalBranch).Run(); err != nil {
392+
return false, fmt.Errorf("running original branch checkout: %w", err)
393+
}
394+
if err := exec.Command("git", "branch", "-D", tmpIntoBranch).Run(); err != nil {
395+
return false, fmt.Errorf("deleting tmp branch: %w", err)
396+
}
397+
398+
// If diff returns no error (exit code 0), there are no changes
399+
// If diff returns error with exit code 1, there are changes
400+
// Any other error is unexpected
401+
if diffErr == nil {
402+
return false, nil // No changes
403+
}
404+
var exitErr *exec.ExitError
405+
if errors.As(diffErr, &exitErr) && exitErr.ExitCode() == 1 {
406+
return true, nil // Has changes
407+
}
408+
return false, fmt.Errorf("checking diff: %w", diffErr)
331409
}

pkg/cmd/release/main.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ package main
88
import "github.com/spf13/cobra"
99

1010
var rootCmd = &cobra.Command{Use: "release"}
11+
var artifactsDir string
12+
13+
func init() {
14+
rootCmd.PersistentFlags().StringVar(&artifactsDir, "artifacts-dir", "", "artifacts directory")
15+
}
1116

1217
const (
1318
envSMTPUser = "SMTP_USER"

pkg/cmd/release/update_versions.go

Lines changed: 105 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,10 @@ type prRepo struct {
102102
workOnRepoError error
103103
}
104104

105+
type metadata struct {
106+
PRs []string `json:"prs"`
107+
}
108+
105109
func (r prRepo) String() string {
106110
return r.owner + "/" + r.repo + "@" + r.branch
107111
}
@@ -335,12 +339,32 @@ func updateVersions(_ *cobra.Command, _ []string) error {
335339
workOnRepoErrors = append(workOnRepoErrors, err)
336340
log.Printf("%s", err)
337341
}
342+
if artifactsDir != "" {
343+
if err := saveMetadata(artifactsDir, metadata{PRs: prs}); err != nil {
344+
err = fmt.Errorf("error saving metadata: %w", err)
345+
workOnRepoErrors = append(workOnRepoErrors, err)
346+
log.Printf("%s", err)
347+
}
348+
}
338349
if len(workOnRepoErrors) > 0 {
339350
return errors.Join(workOnRepoErrors...)
340351
}
341352
return nil
342353
}
343354

355+
func saveMetadata(dir string, meta metadata) error {
356+
dest := path.Join(dir, "prs.json")
357+
log.Printf("saving metadata to %s", dest)
358+
data, err := json.MarshalIndent(meta, "", " ")
359+
if err != nil {
360+
return fmt.Errorf("error marshaling PR metadata: %w", err)
361+
}
362+
if err := os.WriteFile(dest, data, 0o644); err != nil {
363+
return fmt.Errorf("error writing PR metadata file: %w", err)
364+
}
365+
return nil
366+
}
367+
344368
func sendPrReport(version *semver.Version, prs []string, smtpPassword string) error {
345369
log.Println("========================================================")
346370
log.Println("The following PRs are created:")
@@ -436,12 +460,8 @@ func generateRepoList(
436460
log.Printf("not bumping version on staging branch %s", branch)
437461
continue
438462
}
439-
ok, err := fileExistsInGit(branch, versionFile)
440-
if err != nil {
441-
return []prRepo{}, fmt.Errorf("checking version file: %w", err)
442-
}
443-
if !ok {
444-
log.Printf("skipping version bump on the %s branch, because %s does not exist on that branch", branch, versionFile)
463+
if branch == fmt.Sprintf("release-%s-rc", releasedVersion.String()) {
464+
log.Printf("not bumping version on the same branch %s", branch)
445465
continue
446466
}
447467
curVersion, err := fileContent(remoteOrigin+"/"+branch, versionFile)
@@ -522,7 +542,7 @@ func generateRepoList(
522542
// 5. Merge baking branch back to the release branch.
523543
maybeBakingbranches := []string{
524544
fmt.Sprintf("release-%s-rc", releasedVersion.String()), // e.g. release-23.1.17-rc
525-
fmt.Sprintf("staging-%s", releasedVersion.Original()), // e.g. staging-v23.1.17
545+
fmt.Sprintf("staging-v%s", releasedVersion.String()), // e.g. staging-v23.1.17
526546
}
527547
var bakingBranches []string
528548
for _, branch := range maybeBakingbranches {
@@ -535,8 +555,23 @@ func generateRepoList(
535555
if len(bakingBranches) > 1 {
536556
return []prRepo{}, fmt.Errorf("too many baking branches: %s", strings.Join(maybeBakingbranches, ", "))
537557
}
558+
// 6. Merge baking branch to the main release branch (e.g. release-25.1).
559+
// For pre-releases we may have no baking branches, thus we use `for` loop
560+
// to simplify the code.
538561
for _, mergeBranch := range bakingBranches {
539562
baseBranch := fmt.Sprintf("release-%d.%d", releasedVersion.Major(), releasedVersion.Minor())
563+
// Sometimes there are no changes on the baking/staging branches and a merge is not needed.
564+
alreadyOnBaseBranch, err := isAncestor(mergeBranch, baseBranch)
565+
if err != nil {
566+
return []prRepo{}, fmt.Errorf("checking if %s is ancestor of %s: %w", baseBranch, mergeBranch, err)
567+
}
568+
if alreadyOnBaseBranch {
569+
log.Printf("skipping merge of %s to %s, because %s is already an ancestor of %s", mergeBranch, baseBranch, mergeBranch, baseBranch)
570+
continue
571+
}
572+
// TODO: add a check to make sure the merge generates no unexpected
573+
// changes (we can ignore version.txt changes). The "ours" strategy
574+
// doesn't account for changes in the merge branch.
540575
repo := prRepo{
541576
owner: owner,
542577
repo: prefix + "cockroach",
@@ -545,7 +580,7 @@ func generateRepoList(
545580
githubUsername: "cockroach-teamcity",
546581
commitMessage: generateCommitMessage(fmt.Sprintf("merge %s to %s", mergeBranch, baseBranch), releasedVersion, nextVersion),
547582
fn: func(gitDir string) error {
548-
cmd := exec.Command("git", "merge", "-s", "ours", "--no-commit", "origin/"+mergeBranch)
583+
cmd := exec.Command("git", "merge", "-s", "ours", "--no-commit", remoteOrigin+"/"+mergeBranch)
549584
cmd.Dir = gitDir
550585
out, err := cmd.CombinedOutput()
551586
if err != nil {
@@ -557,6 +592,68 @@ func generateRepoList(
557592
}
558593
reposToWorkOn = append(reposToWorkOn, repo)
559594
}
595+
// 7. Merge staging branch to the next release RC branch if it is present.
596+
for _, mergeBranch := range bakingBranches {
597+
// When we have extraordinary releases, we may have next release RC
598+
// branches created. Make sure we merge this branch to the RC branch
599+
// only if there are changes.
600+
if !strings.HasPrefix(mergeBranch, "staging-") {
601+
log.Printf("skipping merge of %s, because it's not a staging branch", mergeBranch)
602+
continue
603+
}
604+
nextRCBranch := fmt.Sprintf("release-%s-rc", nextVersion.String())
605+
maybeNextReleaseRCBranches, err := listRemoteBranches(nextRCBranch)
606+
if err != nil {
607+
return []prRepo{}, fmt.Errorf("listing rc branch %s: %w", nextRCBranch, err)
608+
}
609+
if len(maybeNextReleaseRCBranches) < 1 {
610+
log.Printf("no next release RC branches found, skipping merge to %s", nextRCBranch)
611+
continue
612+
}
613+
alreadyOnRCBranch, err := isAncestor(mergeBranch, nextRCBranch)
614+
if err != nil {
615+
return []prRepo{}, fmt.Errorf("checking if %s is ancestor of %s: %w", mergeBranch, nextRCBranch, err)
616+
}
617+
if alreadyOnRCBranch {
618+
log.Printf("skipping merge of %s to %s, because %s is already an ancestor of %s", mergeBranch, nextRCBranch, mergeBranch, nextRCBranch)
619+
continue
620+
}
621+
// try to merge and see if anything is changed, ignore version.txt changes.
622+
createsMergeCommit, err := mergeCreatesContentChanges(mergeBranch, nextRCBranch, []string{versionFile})
623+
if err != nil {
624+
return []prRepo{}, fmt.Errorf("checking if merge creates content changes: %w", err)
625+
}
626+
if !createsMergeCommit {
627+
log.Printf("skipping merge of %s to %s, because the merge does not create content changes", mergeBranch, nextRCBranch)
628+
continue
629+
}
630+
repo := prRepo{
631+
owner: owner,
632+
repo: prefix + "cockroach",
633+
branch: nextRCBranch,
634+
prBranch: fmt.Sprintf("merge-%s-to-%s-%s", mergeBranch, nextRCBranch, randomString(4)),
635+
githubUsername: "cockroach-teamcity",
636+
commitMessage: generateCommitMessage(fmt.Sprintf("merge %s to %s", mergeBranch, nextRCBranch), releasedVersion, nextVersion),
637+
fn: func(gitDir string) error {
638+
cmd := exec.Command("git", "merge", "-X", "ours", "--strategy=recursive", "--no-commit", "--no-ff", remoteOrigin+"/"+mergeBranch)
639+
cmd.Dir = gitDir
640+
out, err := cmd.CombinedOutput()
641+
if err != nil {
642+
return fmt.Errorf("failed running '%s' with message '%s': %w", cmd.String(), string(out), err)
643+
}
644+
log.Printf("ran '%s': %s\n", cmd.String(), string(out))
645+
coCmd := exec.Command("git", "checkout", remoteOrigin+"/"+nextRCBranch, "--", versionFile)
646+
coCmd.Dir = gitDir
647+
out, err = coCmd.CombinedOutput()
648+
if err != nil {
649+
return fmt.Errorf("failed running '%s' with message '%s': %w", coCmd.String(), string(out), err)
650+
}
651+
log.Printf("ran '%s': %s\n", coCmd.String(), string(out))
652+
return nil
653+
},
654+
}
655+
reposToWorkOn = append(reposToWorkOn, repo)
656+
}
560657
return reposToWorkOn, nil
561658
}
562659

0 commit comments

Comments
 (0)