forked from kemitix/git-next
fix: new commit_log matches original from API request
The original was including a lot of extra commits, those are now trimmed to match the expected.
This commit is contained in:
parent
6cab8bb2ba
commit
3642b2cdd1
3 changed files with 39 additions and 17 deletions
|
@ -14,6 +14,7 @@ pub async fn validate_positions(
|
||||||
) -> git::validation::Result {
|
) -> git::validation::Result {
|
||||||
let repo_details = &forge.repo_details;
|
let repo_details = &forge.repo_details;
|
||||||
// Collect Commit Histories for `main`, `next` and `dev` branches
|
// Collect Commit Histories for `main`, `next` and `dev` branches
|
||||||
|
repository.fetch()?;
|
||||||
let commit_histories =
|
let commit_histories =
|
||||||
get_commit_histories(repository, repo_details, &repo_config, &forge.net).await;
|
get_commit_histories(repository, repo_details, &repo_config, &forge.net).await;
|
||||||
let commit_histories = match commit_histories {
|
let commit_histories = match commit_histories {
|
||||||
|
@ -23,7 +24,6 @@ pub async fn validate_positions(
|
||||||
return Err(git::validation::Error::Network(Box::new(err)));
|
return Err(git::validation::Error::Network(Box::new(err)));
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
// Validations
|
// Validations
|
||||||
let Some(main) = commit_histories.main.first().cloned() else {
|
let Some(main) = commit_histories.main.first().cloned() else {
|
||||||
warn!(
|
warn!(
|
||||||
|
@ -60,7 +60,6 @@ pub async fn validate_positions(
|
||||||
let next_is_ancestor_of_dev = commit_histories.dev.iter().any(|dev| dev == &next);
|
let next_is_ancestor_of_dev = commit_histories.dev.iter().any(|dev| dev == &next);
|
||||||
if !next_is_ancestor_of_dev {
|
if !next_is_ancestor_of_dev {
|
||||||
info!("Next is not an ancestor of dev - resetting next to main");
|
info!("Next is not an ancestor of dev - resetting next to main");
|
||||||
|
|
||||||
if let Err(err) = forge.branch_reset(
|
if let Err(err) = forge.branch_reset(
|
||||||
repository,
|
repository,
|
||||||
repo_config.branches().next(),
|
repo_config.branches().next(),
|
||||||
|
@ -77,11 +76,12 @@ pub async fn validate_positions(
|
||||||
repo_config.branches().next(),
|
repo_config.branches().next(),
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
|
||||||
let next_commits = commit_histories
|
let next_commits = commit_histories
|
||||||
.next
|
.next
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.take(2)
|
.take(2) // next should never be more than one commit ahead of main, so this should be
|
||||||
|
// either be next and main on two adjacent commits, or next and main on the same commit,
|
||||||
|
// plus the parent of main.
|
||||||
.collect::<Vec<_>>();
|
.collect::<Vec<_>>();
|
||||||
if !next_commits.contains(&main) {
|
if !next_commits.contains(&main) {
|
||||||
warn!(
|
warn!(
|
||||||
|
@ -128,7 +128,6 @@ pub async fn validate_positions(
|
||||||
repo_config.branches().next(),
|
repo_config.branches().next(),
|
||||||
)); // dev is not based on next
|
)); // dev is not based on next
|
||||||
}
|
}
|
||||||
|
|
||||||
let Some(dev) = commit_histories.dev.first().cloned() else {
|
let Some(dev) = commit_histories.dev.first().cloned() else {
|
||||||
warn!(
|
warn!(
|
||||||
"No commits on dev branch '{}'",
|
"No commits on dev branch '{}'",
|
||||||
|
@ -138,7 +137,6 @@ pub async fn validate_positions(
|
||||||
repo_config.branches().dev(),
|
repo_config.branches().dev(),
|
||||||
));
|
));
|
||||||
};
|
};
|
||||||
|
|
||||||
Ok(git::validation::Positions {
|
Ok(git::validation::Positions {
|
||||||
main,
|
main,
|
||||||
next,
|
next,
|
||||||
|
@ -170,12 +168,11 @@ async fn get_commit_histories(
|
||||||
net,
|
net,
|
||||||
)
|
)
|
||||||
.await)?;
|
.await)?;
|
||||||
let next_head = next[0].clone();
|
|
||||||
let dev = (get_commit_history(
|
let dev = (get_commit_history(
|
||||||
repository,
|
repository,
|
||||||
repo_details,
|
repo_details,
|
||||||
&repo_config.branches().dev(),
|
&repo_config.branches().dev(),
|
||||||
&[next_head, main_head],
|
&[main_head],
|
||||||
net,
|
net,
|
||||||
)
|
)
|
||||||
.await)?;
|
.await)?;
|
||||||
|
@ -189,6 +186,7 @@ async fn get_commit_histories(
|
||||||
Ok(histories)
|
Ok(histories)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tracing::instrument(skip_all, fields(%branch_name, ?find_commits))]
|
||||||
async fn get_commit_history(
|
async fn get_commit_history(
|
||||||
repository: &git::repository::OpenRepository,
|
repository: &git::repository::OpenRepository,
|
||||||
repo_details: &git::RepoDetails,
|
repo_details: &git::RepoDetails,
|
||||||
|
@ -204,7 +202,7 @@ async fn get_commit_history(
|
||||||
info!("new version matches");
|
info!("new version matches");
|
||||||
Ok(updated)
|
Ok(updated)
|
||||||
} else {
|
} else {
|
||||||
error!(?updated, ?original, "new version doesn't match original");
|
error!("new version doesn't match original");
|
||||||
Ok(original)
|
Ok(original)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -269,8 +267,13 @@ async fn original_get_commit_history(
|
||||||
|| find_commits
|
|| find_commits
|
||||||
.iter()
|
.iter()
|
||||||
.any(|find_commit| commits.iter().any(|commit| commit == find_commit));
|
.any(|find_commit| commits.iter().any(|commit| commit == find_commit));
|
||||||
let at_end = commits.len() < limit;
|
let at_end = commits.len() < limit; // i.e. less than the number of commits requested
|
||||||
all_commits.extend(commits);
|
for commit in commits {
|
||||||
|
all_commits.push(commit.clone());
|
||||||
|
if find_commits.contains(&commit) {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
if found || at_end {
|
if found || at_end {
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
|
@ -22,6 +22,7 @@ impl super::OpenRepositoryLike for RealOpenRepository {
|
||||||
remote.url(direction.into()).map(Into::into)
|
remote.url(direction.into()).map(Into::into)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tracing::instrument(skip_all)]
|
||||||
fn fetch(&self) -> Result<(), git::fetch::Error> {
|
fn fetch(&self) -> Result<(), git::fetch::Error> {
|
||||||
let Ok(repository) = self.0.lock() else {
|
let Ok(repository) = self.0.lock() else {
|
||||||
#[cfg(not(tarpaulin_include))] // don't test mutex lock failure
|
#[cfg(not(tarpaulin_include))] // don't test mutex lock failure
|
||||||
|
@ -44,12 +45,13 @@ impl super::OpenRepositoryLike for RealOpenRepository {
|
||||||
}
|
}
|
||||||
Err(e) => Err(git::fetch::Error::Fetch(e.to_string()))?,
|
Err(e) => Err(git::fetch::Error::Fetch(e.to_string()))?,
|
||||||
}
|
}
|
||||||
|
info!("Fetch okay");
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
// TODO: (#72) reimplement using `gix`
|
// TODO: (#72) reimplement using `gix`
|
||||||
#[cfg(not(tarpaulin_include))] // would require writing to external service
|
#[cfg(not(tarpaulin_include))] // would require writing to external service
|
||||||
|
#[tracing::instrument(skip_all)]
|
||||||
fn push(
|
fn push(
|
||||||
&self,
|
&self,
|
||||||
repo_details: &git::RepoDetails,
|
repo_details: &git::RepoDetails,
|
||||||
|
@ -71,13 +73,11 @@ impl super::OpenRepositoryLike for RealOpenRepository {
|
||||||
origin.expose_secret()
|
origin.expose_secret()
|
||||||
)
|
)
|
||||||
.into();
|
.into();
|
||||||
|
|
||||||
let git_dir = self
|
let git_dir = self
|
||||||
.0
|
.0
|
||||||
.lock()
|
.lock()
|
||||||
.map_err(|_| git::push::Error::Lock)
|
.map_err(|_| git::push::Error::Lock)
|
||||||
.map(|r| r.git_dir().to_path_buf())?;
|
.map(|r| r.git_dir().to_path_buf())?;
|
||||||
|
|
||||||
let ctx = gix::diff::command::Context {
|
let ctx = gix::diff::command::Context {
|
||||||
git_dir: Some(git_dir.clone()),
|
git_dir: Some(git_dir.clone()),
|
||||||
..Default::default()
|
..Default::default()
|
||||||
|
@ -91,7 +91,7 @@ impl super::OpenRepositoryLike for RealOpenRepository {
|
||||||
{
|
{
|
||||||
Ok(mut child) => match child.wait() {
|
Ok(mut child) => match child.wait() {
|
||||||
Ok(_) => {
|
Ok(_) => {
|
||||||
info!("Branch updated");
|
info!("Push okay");
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
Err(err) => {
|
Err(err) => {
|
||||||
|
@ -111,11 +111,15 @@ impl super::OpenRepositoryLike for RealOpenRepository {
|
||||||
branch_name: &config::BranchName,
|
branch_name: &config::BranchName,
|
||||||
find_commits: &[git::Commit],
|
find_commits: &[git::Commit],
|
||||||
) -> Result<Vec<crate::Commit>, git::commit::log::Error> {
|
) -> Result<Vec<crate::Commit>, git::commit::log::Error> {
|
||||||
|
let limit = match find_commits.is_empty() {
|
||||||
|
true => 1,
|
||||||
|
false => 50,
|
||||||
|
};
|
||||||
self.0
|
self.0
|
||||||
.lock()
|
.lock()
|
||||||
.map_err(|_| git::commit::log::Error::Lock)
|
.map_err(|_| git::commit::log::Error::Lock)
|
||||||
.map(|repo| {
|
.map(|repo| {
|
||||||
let branch_name = branch_name.to_string();
|
let branch_name = format!("remotes/origin/{branch_name}");
|
||||||
let branch_name = BStr::new(&branch_name);
|
let branch_name = BStr::new(&branch_name);
|
||||||
let branch_head = repo
|
let branch_head = repo
|
||||||
.rev_parse_single(branch_name)
|
.rev_parse_single(branch_name)
|
||||||
|
@ -127,7 +131,7 @@ impl super::OpenRepositoryLike for RealOpenRepository {
|
||||||
.all()
|
.all()
|
||||||
.map_err(|e| e.to_string())?;
|
.map_err(|e| e.to_string())?;
|
||||||
let mut commits = vec![];
|
let mut commits = vec![];
|
||||||
for item in walk {
|
for item in walk.take(limit) {
|
||||||
let item = item.map_err(|e| e.to_string())?;
|
let item = item.map_err(|e| e.to_string())?;
|
||||||
let commit = item.object().map_err(|e| e.to_string())?;
|
let commit = item.object().map_err(|e| e.to_string())?;
|
||||||
let id = commit.id().to_string();
|
let id = commit.id().to_string();
|
||||||
|
@ -136,12 +140,15 @@ impl super::OpenRepositoryLike for RealOpenRepository {
|
||||||
git::commit::Sha::new(id),
|
git::commit::Sha::new(id),
|
||||||
git::commit::Message::new(message),
|
git::commit::Message::new(message),
|
||||||
);
|
);
|
||||||
|
info!(?commit, "found");
|
||||||
if find_commits.contains(&commit) {
|
if find_commits.contains(&commit) {
|
||||||
|
info!("Is in find_commits");
|
||||||
commits.push(commit);
|
commits.push(commit);
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
commits.push(commit);
|
commits.push(commit);
|
||||||
}
|
}
|
||||||
|
info!("finished walkfing");
|
||||||
Ok(commits)
|
Ok(commits)
|
||||||
})?
|
})?
|
||||||
}
|
}
|
||||||
|
|
|
@ -13,13 +13,25 @@ pub struct Positions {
|
||||||
#[derive(Debug, derive_more::Display)]
|
#[derive(Debug, derive_more::Display)]
|
||||||
pub enum Error {
|
pub enum Error {
|
||||||
Network(Box<kxio::network::NetworkError>),
|
Network(Box<kxio::network::NetworkError>),
|
||||||
|
|
||||||
|
Fetch(git::fetch::Error),
|
||||||
|
|
||||||
#[display("Failed to Reset Branch {branch} to {commit}")]
|
#[display("Failed to Reset Branch {branch} to {commit}")]
|
||||||
FailedToResetBranch {
|
FailedToResetBranch {
|
||||||
branch: BranchName,
|
branch: BranchName,
|
||||||
commit: git::Commit,
|
commit: git::Commit,
|
||||||
},
|
},
|
||||||
|
|
||||||
BranchReset(BranchName),
|
BranchReset(BranchName),
|
||||||
|
|
||||||
BranchHasNoCommits(BranchName),
|
BranchHasNoCommits(BranchName),
|
||||||
|
|
||||||
DevBranchNotBasedOn(BranchName),
|
DevBranchNotBasedOn(BranchName),
|
||||||
}
|
}
|
||||||
impl std::error::Error for Error {}
|
impl std::error::Error for Error {}
|
||||||
|
|
||||||
|
impl From<git::fetch::Error> for Error {
|
||||||
|
fn from(value: git::fetch::Error) -> Self {
|
||||||
|
Self::Fetch(value)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
Loading…
Reference in a new issue