From 8d917f890863716503189ff5aaca998448b49b7f Mon Sep 17 00:00:00 2001 From: Aleksandr Pak Date: Thu, 30 Jun 2022 19:49:08 +0300 Subject: [PATCH 1/2] [#10] Add rebase on top of default branch Problem: Currently when there are human commits in update branch, update bot reports an error. Solution: Rebase current changes in remote update branch on top of remote default branch if possible. Otherwise report an error. --- src/git.rs | 109 +++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 90 insertions(+), 19 deletions(-) diff --git a/src/git.rs b/src/git.rs index 98c5a02..7234eb7 100644 --- a/src/git.rs +++ b/src/git.rs @@ -178,14 +178,34 @@ pub enum SetupUpdateBranchError { FindDefaultBranch(git2::Error), #[error("Error peeling to default branch commit: {0}")] PeelDefaultBranchCommit(git2::Error), - #[error("Error peeling to update branch commit: {0}")] - PeelUpdateBranchCommit(git2::Error), #[error("Error creating a new branch pointing to default branch commit: {0}")] BranchToUpdateBranchWithDefault(git2::Error), #[error("Error setting head to update branch: {0}")] SetUpdateBranchHead(git2::Error), - #[error("There are human commits in the update branch")] - HumanCommitsInUpdateBranch, + #[error("Error initializing rebase: {0}")] + InitializeRebase(git2::Error), + #[error("Error peeling to local update branch commit: {0}")] + PeelLocalUpdateBranchCommit(git2::Error), + #[error("Error finding annotated commit for update branch commit: {0}")] + FindAnnotatedUpdateBranchCommit(git2::Error), + #[error("Error creating signature for rebase commits: {0}")] + SigningForRebaseCommits(git2::Error), + #[error("Error finding annotated commit for default branch commit: {0}")] + FindAnnotatedDefaultBranchCommit(git2::Error), + #[error("Error getting next rebase patch: {0}")] + RebaseNext(git2::Error), + #[error("Error committing rebase patch, probably because of rebase conflicts: {0}")] + RebaseCommit(git2::Error), + #[error("Error finishing rebase: {0}")] + FinishRebase(git2::Error), + #[error("Error getting HEAD: {0}")] + GetHead(git2::Error), + #[error("Error peeling HEAD to commit: {0}")] + PeelHead(git2::Error), + #[error("Error creating a new branch pointing to remote update branch: {0}")] + BranchToUpdateBranchWithRemoteBranch(git2::Error), + #[error("Error aborting rebase: {0}")] + AbortRebase(git2::Error), } pub fn setup_update_branch( @@ -197,18 +217,6 @@ pub fn setup_update_branch( BranchType::Remote, ); - if let Ok(b) = update_branch { - if b.into_reference() - .peel_to_commit() - .map_err(SetupUpdateBranchError::PeelUpdateBranchCommit)? - .author() - .email() - != Some(&settings.author.email) - { - return Err(SetupUpdateBranchError::HumanCommitsInUpdateBranch); - } - } - let default_branch_commit = repo .find_branch( &format!("origin/{}", &settings.default_branch), @@ -219,9 +227,72 @@ pub fn setup_update_branch( .peel_to_commit() .map_err(SetupUpdateBranchError::PeelDefaultBranchCommit)?; - // TODO: handle errors we care about here? - repo.branch(&settings.update_branch, &default_branch_commit, true) - .map_err(SetupUpdateBranchError::BranchToUpdateBranchWithDefault)?; + match update_branch { + Err(_) => { + // no update branch exists, creating new one from default + // TODO: handle errors we care about here? + repo.branch(&settings.update_branch, &default_branch_commit, true) + .map_err(SetupUpdateBranchError::BranchToUpdateBranchWithDefault)?; + } + Ok(remote_update_branch) => { + // update branch exists, we should try to: + // 1. rebase update branch on top of default + + let update_branch_commit = &remote_update_branch + .into_reference() + .peel_to_commit() + .map_err(SetupUpdateBranchError::PeelLocalUpdateBranchCommit)?; + + let update_annotated_commit = repo + .find_annotated_commit(update_branch_commit.id()) + .map_err(SetupUpdateBranchError::FindAnnotatedUpdateBranchCommit)?; + + let default_annotated_commit = + repo.find_annotated_commit(default_branch_commit.id()) + .map_err(SetupUpdateBranchError::FindAnnotatedDefaultBranchCommit)?; + + let mut rebase = repo + .rebase( + Some(&update_annotated_commit), + None, + Some(&default_annotated_commit), + None, + ) + .map_err(SetupUpdateBranchError::InitializeRebase)?; + + let committer = Signature::now(&settings.author.name, &settings.author.email) + .map_err(SetupUpdateBranchError::SigningForRebaseCommits)?; + + while let Some(op) = rebase.next() { + op.map_err(SetupUpdateBranchError::RebaseNext)?; + match rebase.commit(None, &committer, None) { + Ok(_) => {} + Err(e) => { + // Potential conflicts, should abort the commit + rebase + .abort() + .map_err(SetupUpdateBranchError::AbortRebase)?; + return Err(SetupUpdateBranchError::RebaseCommit(e)); + } + } + } + + rebase + .finish(None) + .map_err(SetupUpdateBranchError::FinishRebase)?; + + let head = repo.head().map_err(SetupUpdateBranchError::GetHead)?; + + repo.branch( + &settings.update_branch, + &head + .peel_to_commit() + .map_err(SetupUpdateBranchError::PeelHead)?, + true, + ) + .map_err(SetupUpdateBranchError::BranchToUpdateBranchWithRemoteBranch)?; + } + }; repo.set_head(&format!("refs/heads/{}", settings.update_branch)) .map_err(SetupUpdateBranchError::SetUpdateBranchHead)?; From fa2a253db934d9c0d9a77133e082685092fd5fe7 Mon Sep 17 00:00:00 2001 From: Aleksandr Pak Date: Wed, 20 Jul 2022 14:56:15 +0300 Subject: [PATCH 2/2] [#10] Fix detached HEAD after rebase fail Problem: bot doesn't checkout to a sensible branch after a rebase fail. Solution: checkout to default branch. Note: currently an error "rebase patch already applied" occurs. Needs to be fixed. --- src/git.rs | 52 ++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/src/git.rs b/src/git.rs index 7234eb7..0144d3c 100644 --- a/src/git.rs +++ b/src/git.rs @@ -2,8 +2,8 @@ // // SPDX-License-Identifier: MPL-2.0 -use git2::RemoteCallbacks; use git2::{BranchType, FetchOptions, PushOptions, Repository, ResetType, Signature}; +use git2::{Rebase, RemoteCallbacks}; use std::collections::hash_map::DefaultHasher; use std::fs::{create_dir, remove_dir_all}; use std::hash::{Hash, Hasher}; @@ -182,6 +182,10 @@ pub enum SetupUpdateBranchError { BranchToUpdateBranchWithDefault(git2::Error), #[error("Error setting head to update branch: {0}")] SetUpdateBranchHead(git2::Error), + #[error("Error setting head to default branch: {0}")] + SetDefaultBranchHead(git2::Error), + #[error("Error resetting to default branch commit: {0}")] + ResetToDefaultBranchCommit(git2::Error), #[error("Error initializing rebase: {0}")] InitializeRebase(git2::Error), #[error("Error peeling to local update branch commit: {0}")] @@ -194,7 +198,7 @@ pub enum SetupUpdateBranchError { FindAnnotatedDefaultBranchCommit(git2::Error), #[error("Error getting next rebase patch: {0}")] RebaseNext(git2::Error), - #[error("Error committing rebase patch, probably because of rebase conflicts: {0}")] + #[error("Error committing rebase patch: {0}")] RebaseCommit(git2::Error), #[error("Error finishing rebase: {0}")] FinishRebase(git2::Error), @@ -204,8 +208,13 @@ pub enum SetupUpdateBranchError { PeelHead(git2::Error), #[error("Error creating a new branch pointing to remote update branch: {0}")] BranchToUpdateBranchWithRemoteBranch(git2::Error), - #[error("Error aborting rebase: {0}")] - AbortRebase(git2::Error), +} + +fn safe_abort(rebase: &mut Rebase) { + match rebase.abort() { + Err(e) => error!("Rebase abort failed: {}", e), + _ => {} + } } pub fn setup_update_branch( @@ -263,23 +272,42 @@ pub fn setup_update_branch( let committer = Signature::now(&settings.author.name, &settings.author.email) .map_err(SetupUpdateBranchError::SigningForRebaseCommits)?; + let checkout_to_default = || -> Result<(), SetupUpdateBranchError> { + repo.set_head(&format!("refs/heads/{}", settings.default_branch)) + .map_err(SetupUpdateBranchError::SetDefaultBranchHead)?; + + repo.reset(default_branch_commit.as_object(), ResetType::Hard, None) + .map_err(SetupUpdateBranchError::ResetToDefaultBranchCommit)?; + Ok(()) + }; + while let Some(op) = rebase.next() { - op.map_err(SetupUpdateBranchError::RebaseNext)?; + match op { + Ok(_) => {} + Err(e) => { + safe_abort(&mut rebase); + checkout_to_default()?; + return Err(SetupUpdateBranchError::RebaseNext(e)); + } + } + match rebase.commit(None, &committer, None) { Ok(_) => {} Err(e) => { - // Potential conflicts, should abort the commit - rebase - .abort() - .map_err(SetupUpdateBranchError::AbortRebase)?; + safe_abort(&mut rebase); + checkout_to_default()?; return Err(SetupUpdateBranchError::RebaseCommit(e)); } } } - rebase - .finish(None) - .map_err(SetupUpdateBranchError::FinishRebase)?; + match rebase.finish(None) { + Ok(_) => {} + Err(e) => { + checkout_to_default()?; + return Err(SetupUpdateBranchError::FinishRebase(e)); + } + } let head = repo.head().map_err(SetupUpdateBranchError::GetHead)?;