Skip to content

Commit 32d356b

Browse files
manicminermanicminer
committed
Always create temporary source/target branches for closed MRs, otherwise we will inadvertently try to create a PR against current trunk branch (which will already have the changes)
1 parent dc3195d commit 32d356b

File tree

1 file changed

+100
-103
lines changed

1 file changed

+100
-103
lines changed

‎main.go‎

Lines changed: 100 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -808,131 +808,128 @@ func migratePullRequests(ctx context.Context, githubPath, gitlabPath []string, p
808808
if pullRequest == nil && !strings.EqualFold(mergeRequest.State, "opened") {
809809
logger.Trace("searching for existing branch for closed/merged merge request", "name", gitlabPath[1], "group", gitlabPath[0], "project_id", project.ID, "merge_request_id", mergeRequest.IID, "source_branch", mergeRequest.SourceBranch)
810810

811-
// Only create temporary branches if the source branch has been deleted
812-
if _, err = repo.Reference(plumbing.ReferenceName(mergeRequest.SourceBranch), false); err != nil {
813-
// Create a worktree
814-
worktree, err := repo.Worktree()
815-
if err != nil {
816-
sendErr(fmt.Errorf("creating worktree: %v", err))
817-
failureCount++
818-
continue
819-
}
820-
821-
// Generate temporary branch names
822-
mergeRequest.SourceBranch = fmt.Sprintf("migration-source-%d/%s", mergeRequest.IID, mergeRequest.SourceBranch)
823-
mergeRequest.TargetBranch = fmt.Sprintf("migration-target-%d/%s", mergeRequest.IID, mergeRequest.TargetBranch)
811+
// Create a worktree
812+
worktree, err := repo.Worktree()
813+
if err != nil {
814+
sendErr(fmt.Errorf("creating worktree: %v", err))
815+
failureCount++
816+
continue
817+
}
824818

825-
logger.Trace("retrieving commits for merge request", "name", gitlabPath[1], "group", gitlabPath[0], "project_id", project.ID, "merge_request_id", mergeRequest.IID)
826-
mergeRequestCommits, _, err := gl.MergeRequests.GetMergeRequestCommits(project.ID, mergeRequest.IID, &gitlab.GetMergeRequestCommitsOptions{OrderBy: "created_at", Sort: "asc"})
827-
if err != nil {
828-
sendErr(fmt.Errorf("retrieving merge request commits: %v", err))
829-
failureCount++
830-
continue
831-
}
819+
// Generate temporary branch names
820+
mergeRequest.SourceBranch = fmt.Sprintf("migration-source-%d/%s", mergeRequest.IID, mergeRequest.SourceBranch)
821+
mergeRequest.TargetBranch = fmt.Sprintf("migration-target-%d/%s", mergeRequest.IID, mergeRequest.TargetBranch)
832822

833-
// Some merge requests have no commits, disregard these
834-
if len(mergeRequestCommits) == 0 {
835-
continue
836-
}
823+
logger.Trace("retrieving commits for merge request", "name", gitlabPath[1], "group", gitlabPath[0], "project_id", project.ID, "merge_request_id", mergeRequest.IID)
824+
mergeRequestCommits, _, err := gl.MergeRequests.GetMergeRequestCommits(project.ID, mergeRequest.IID, &gitlab.GetMergeRequestCommitsOptions{OrderBy: "created_at", Sort: "asc"})
825+
if err != nil {
826+
sendErr(fmt.Errorf("retrieving merge request commits: %v", err))
827+
failureCount++
828+
continue
829+
}
837830

838-
// API is buggy, ordering is not respected, so we'll reorder by commit datestamp
839-
sort.Slice(mergeRequestCommits, func(i, j int) bool {
840-
return mergeRequestCommits[i].CommittedDate.Before(*mergeRequestCommits[j].CommittedDate)
841-
})
831+
// Some merge requests have no commits, disregard these
832+
if len(mergeRequestCommits) == 0 {
833+
continue
834+
}
842835

843-
if mergeRequestCommits[0] == nil {
844-
sendErr(fmt.Errorf("start commit for merge request %d is nil", mergeRequest.IID))
845-
failureCount++
846-
continue
847-
}
848-
if mergeRequestCommits[len(mergeRequestCommits)-1] == nil {
849-
sendErr(fmt.Errorf("end commit for merge request %d is nil", mergeRequest.IID))
850-
failureCount++
851-
continue
852-
}
836+
// API is buggy, ordering is not respected, so we'll reorder by commit datestamp
837+
sort.Slice(mergeRequestCommits, func(i, j int) bool {
838+
return mergeRequestCommits[i].CommittedDate.Before(*mergeRequestCommits[j].CommittedDate)
839+
})
853840

854-
logger.Trace("inspecting start commit", "name", gitlabPath[1], "group", gitlabPath[0], "project_id", project.ID, "merge_request_id", mergeRequest.IID, "sha", mergeRequestCommits[0].ShortID)
855-
startCommit, err := object.GetCommit(repo.Storer, plumbing.NewHash(mergeRequestCommits[0].ID))
856-
if err != nil {
857-
sendErr(fmt.Errorf("loading start commit: %v", err))
858-
failureCount++
859-
continue
860-
}
841+
if mergeRequestCommits[0] == nil {
842+
sendErr(fmt.Errorf("start commit for merge request %d is nil", mergeRequest.IID))
843+
failureCount++
844+
continue
845+
}
846+
if mergeRequestCommits[len(mergeRequestCommits)-1] == nil {
847+
sendErr(fmt.Errorf("end commit for merge request %d is nil", mergeRequest.IID))
848+
failureCount++
849+
continue
850+
}
861851

862-
if startCommit.NumParents() == 0 {
863-
// Orphaned commit, start with an empty branch
864-
// TODO: this isn't working as hoped, try to figure this out. in the meantime, we'll skip MRs from orphaned branches
865-
//if err = repo.Storer.SetReference(plumbing.NewSymbolicReference("HEAD", plumbing.ReferenceName(fmt.Sprintf("refs/heads/%s", mergeRequest.TargetBranch)))); err != nil {
866-
// return fmt.Errorf("creating empty branch: %s", err)
867-
//}
868-
sendErr(fmt.Errorf("start commit %s for merge request %d has no parents", mergeRequestCommits[0].ShortID, mergeRequest.IID))
869-
continue
870-
} else {
871-
// Sometimes we will be starting from a merge commit, so look for a suitable parent commit to branch out from
872-
var startCommitParent *object.Commit
873-
for i := 0; i < startCommit.NumParents(); i++ {
874-
logger.Trace("inspecting start commit parent", "name", gitlabPath[1], "group", gitlabPath[0], "project_id", project.ID, "merge_request_id", mergeRequest.IID, "sha", mergeRequestCommits[0].ShortID)
875-
startCommitParent, err = startCommit.Parent(0)
876-
if err != nil {
877-
sendErr(fmt.Errorf("loading parent commit: %s", err))
878-
}
852+
logger.Trace("inspecting start commit", "name", gitlabPath[1], "group", gitlabPath[0], "project_id", project.ID, "merge_request_id", mergeRequest.IID, "sha", mergeRequestCommits[0].ShortID)
853+
startCommit, err := object.GetCommit(repo.Storer, plumbing.NewHash(mergeRequestCommits[0].ID))
854+
if err != nil {
855+
sendErr(fmt.Errorf("loading start commit: %v", err))
856+
failureCount++
857+
continue
858+
}
879859

880-
continue
860+
if startCommit.NumParents() == 0 {
861+
// Orphaned commit, start with an empty branch
862+
// TODO: this isn't working as hoped, try to figure this out. in the meantime, we'll skip MRs from orphaned branches
863+
//if err = repo.Storer.SetReference(plumbing.NewSymbolicReference("HEAD", plumbing.ReferenceName(fmt.Sprintf("refs/heads/%s", mergeRequest.TargetBranch)))); err != nil {
864+
// return fmt.Errorf("creating empty branch: %s", err)
865+
//}
866+
sendErr(fmt.Errorf("start commit %s for merge request %d has no parents", mergeRequestCommits[0].ShortID, mergeRequest.IID))
867+
continue
868+
} else {
869+
// Sometimes we will be starting from a merge commit, so look for a suitable parent commit to branch out from
870+
var startCommitParent *object.Commit
871+
for i := 0; i < startCommit.NumParents(); i++ {
872+
logger.Trace("inspecting start commit parent", "name", gitlabPath[1], "group", gitlabPath[0], "project_id", project.ID, "merge_request_id", mergeRequest.IID, "sha", mergeRequestCommits[0].ShortID)
873+
startCommitParent, err = startCommit.Parent(0)
874+
if err != nil {
875+
sendErr(fmt.Errorf("loading parent commit: %s", err))
881876
}
882877

883-
if startCommitParent == nil {
884-
sendErr(fmt.Errorf("identifying suitable parent of start commit %s for merge request %d", mergeRequestCommits[0].ShortID, mergeRequest.IID))
885-
failureCount++
886-
}
878+
continue
879+
}
887880

888-
logger.Trace("creating target branch for merged/closed merge request", "name", gitlabPath[1], "group", gitlabPath[0], "project_id", project.ID, "merge_request_id", mergeRequest.IID, "branch", mergeRequest.TargetBranch, "sha", startCommitParent.Hash)
889-
if err = worktree.Checkout(&git.CheckoutOptions{
890-
Create: true,
891-
Force: true,
892-
Branch: plumbing.NewBranchReferenceName(mergeRequest.TargetBranch),
893-
Hash: startCommitParent.Hash,
894-
}); err != nil {
895-
sendErr(fmt.Errorf("checking out temporary target branch: %v", err))
896-
failureCount++
897-
continue
898-
}
881+
if startCommitParent == nil {
882+
sendErr(fmt.Errorf("identifying suitable parent of start commit %s for merge request %d", mergeRequestCommits[0].ShortID, mergeRequest.IID))
883+
failureCount++
899884
}
900885

901-
endHash := plumbing.NewHash(mergeRequestCommits[len(mergeRequestCommits)-1].ID)
902-
logger.Trace("creating source branch for merged/closed merge request", "name", gitlabPath[1], "group", gitlabPath[0], "project_id", project.ID, "merge_request_id", mergeRequest.IID, "branch", mergeRequest.SourceBranch, "sha", endHash)
886+
logger.Trace("creating target branch for merged/closed merge request", "name", gitlabPath[1], "group", gitlabPath[0], "project_id", project.ID, "merge_request_id", mergeRequest.IID, "branch", mergeRequest.TargetBranch, "sha", startCommitParent.Hash)
903887
if err = worktree.Checkout(&git.CheckoutOptions{
904888
Create: true,
905889
Force: true,
906-
Branch: plumbing.NewBranchReferenceName(mergeRequest.SourceBranch),
907-
Hash: endHash,
890+
Branch: plumbing.NewBranchReferenceName(mergeRequest.TargetBranch),
891+
Hash: startCommitParent.Hash,
908892
}); err != nil {
909-
sendErr(fmt.Errorf("checking out temporary source branch: %v", err))
893+
sendErr(fmt.Errorf("checking out temporary target branch: %v", err))
910894
failureCount++
911895
continue
912896
}
897+
}
913898

914-
logger.Debug("pushing branches for merged/closed merge request", "owner", githubPath[0], "repo", githubPath[1], "source_branch", mergeRequest.SourceBranch, "target_branch", mergeRequest.TargetBranch)
915-
if err = repo.PushContext(ctx, &git.PushOptions{
916-
RemoteName: "github",
917-
RefSpecs: []config.RefSpec{
918-
config.RefSpec(fmt.Sprintf("refs/heads/%[1]s:refs/heads/%[1]s", mergeRequest.SourceBranch)),
919-
config.RefSpec(fmt.Sprintf("refs/heads/%[1]s:refs/heads/%[1]s", mergeRequest.TargetBranch)),
920-
},
921-
Force: true,
922-
}); err != nil {
923-
upToDateError := errors.New("already up-to-date")
924-
if errors.As(err, &upToDateError) {
925-
logger.Trace("branch already exists and is up-to-date on GitHub", "owner", githubPath[0], "repo", githubPath[1], "source_branch", mergeRequest.SourceBranch, "target_branch", mergeRequest.TargetBranch)
926-
} else {
927-
sendErr(fmt.Errorf("pushing temporary branches to github: %v", err))
928-
failureCount++
929-
continue
930-
}
931-
}
899+
endHash := plumbing.NewHash(mergeRequestCommits[len(mergeRequestCommits)-1].ID)
900+
logger.Trace("creating source branch for merged/closed merge request", "name", gitlabPath[1], "group", gitlabPath[0], "project_id", project.ID, "merge_request_id", mergeRequest.IID, "branch", mergeRequest.SourceBranch, "sha", endHash)
901+
if err = worktree.Checkout(&git.CheckoutOptions{
902+
Create: true,
903+
Force: true,
904+
Branch: plumbing.NewBranchReferenceName(mergeRequest.SourceBranch),
905+
Hash: endHash,
906+
}); err != nil {
907+
sendErr(fmt.Errorf("checking out temporary source branch: %v", err))
908+
failureCount++
909+
continue
910+
}
932911

933-
// We will clean up these temporary branches after configuring and closing the pull request
934-
cleanUpBranch = true
912+
logger.Debug("pushing branches for merged/closed merge request", "owner", githubPath[0], "repo", githubPath[1], "source_branch", mergeRequest.SourceBranch, "target_branch", mergeRequest.TargetBranch)
913+
if err = repo.PushContext(ctx, &git.PushOptions{
914+
RemoteName: "github",
915+
RefSpecs: []config.RefSpec{
916+
config.RefSpec(fmt.Sprintf("refs/heads/%[1]s:refs/heads/%[1]s", mergeRequest.SourceBranch)),
917+
config.RefSpec(fmt.Sprintf("refs/heads/%[1]s:refs/heads/%[1]s", mergeRequest.TargetBranch)),
918+
},
919+
Force: true,
920+
}); err != nil {
921+
upToDateError := errors.New("already up-to-date")
922+
if errors.As(err, &upToDateError) {
923+
logger.Trace("branch already exists and is up-to-date on GitHub", "owner", githubPath[0], "repo", githubPath[1], "source_branch", mergeRequest.SourceBranch, "target_branch", mergeRequest.TargetBranch)
924+
} else {
925+
sendErr(fmt.Errorf("pushing temporary branches to github: %v", err))
926+
failureCount++
927+
continue
928+
}
935929
}
930+
931+
// We will clean up these temporary branches after configuring and closing the pull request
932+
cleanUpBranch = true
936933
}
937934

938935
if renameMasterToMain && mergeRequest.TargetBranch == "master" {

0 commit comments

Comments
 (0)