diff options
| author | DanConwayDev <DanConwayDev@protonmail.com> | 2024-09-23 10:46:26 +0100 |
|---|---|---|
| committer | DanConwayDev <DanConwayDev@protonmail.com> | 2024-09-23 13:36:53 +0100 |
| commit | 2cdbb8c7ac6c98af6e36c4458c08bef2299794e1 (patch) | |
| tree | fcf7c56d2f5eda0776643f8a52090965f1de06a8 /src/lib | |
| parent | 51358320c50afece31fc25945a09e3d7aac8f39c (diff) | |
fix(remote): add resilience to `prs`
make poorly formatted patches fail silently. we stop trusting that the
`commit` tag in the latest patch can be produced by apply the patches.
to achieve this we must recreate the commit during the list command,
which require fetching the parent oids.
support patches without optional `commit` and `parent-commit` tags.
Diffstat (limited to 'src/lib')
| -rw-r--r-- | src/lib/git/mod.rs | 82 |
1 files changed, 55 insertions, 27 deletions
diff --git a/src/lib/git/mod.rs b/src/lib/git/mod.rs index 7ebdefd..05186b0 100644 --- a/src/lib/git/mod.rs +++ b/src/lib/git/mod.rs | |||
| @@ -79,7 +79,11 @@ pub trait RepoActions { | |||
| 79 | branch_name: &str, | 79 | branch_name: &str, |
| 80 | patch_and_ancestors: Vec<nostr::Event>, | 80 | patch_and_ancestors: Vec<nostr::Event>, |
| 81 | ) -> Result<Vec<nostr::Event>>; | 81 | ) -> Result<Vec<nostr::Event>>; |
| 82 | fn create_commit_from_patch(&self, patch: &nostr::Event) -> Result<Oid>; | 82 | fn create_commit_from_patch( |
| 83 | &self, | ||
| 84 | patch: &nostr::Event, | ||
| 85 | parent_commit_id_override: Option<String>, | ||
| 86 | ) -> Result<Oid>; | ||
| 83 | fn parse_starting_commits(&self, starting_commits: &str) -> Result<Vec<Sha1Hash>>; | 87 | fn parse_starting_commits(&self, starting_commits: &str) -> Result<Vec<Sha1Hash>>; |
| 84 | fn ancestor_of(&self, decendant: &Sha1Hash, ancestor: &Sha1Hash) -> Result<bool>; | 88 | fn ancestor_of(&self, decendant: &Sha1Hash, ancestor: &Sha1Hash) -> Result<bool>; |
| 85 | fn get_git_config_item(&self, item: &str, global: Option<bool>) -> Result<Option<String>>; | 89 | fn get_git_config_item(&self, item: &str, global: Option<bool>) -> Result<Option<String>>; |
| @@ -540,19 +544,30 @@ impl RepoActions for Repo { | |||
| 540 | let commit_id = get_commit_id_from_patch(patch)?; | 544 | let commit_id = get_commit_id_from_patch(patch)?; |
| 541 | // only create new commits - otherwise make them the tip | 545 | // only create new commits - otherwise make them the tip |
| 542 | if !self.does_commit_exist(&commit_id)? { | 546 | if !self.does_commit_exist(&commit_id)? { |
| 543 | self.create_commit_from_patch(patch)?; | 547 | self.create_commit_from_patch(patch, None)?; |
| 544 | } | 548 | } |
| 545 | self.create_branch_at_commit(branch_name, &commit_id)?; | 549 | self.create_branch_at_commit(branch_name, &commit_id)?; |
| 546 | self.checkout(branch_name)?; | 550 | self.checkout(branch_name)?; |
| 547 | } | 551 | } |
| 548 | Ok(patches_to_apply) | 552 | Ok(patches_to_apply) |
| 549 | } | 553 | } |
| 550 | fn create_commit_from_patch(&self, patch: &nostr::Event) -> Result<Oid> { | 554 | fn create_commit_from_patch( |
| 551 | let commit_id = get_commit_id_from_patch(patch)?; | 555 | &self, |
| 552 | if self.does_commit_exist(&commit_id)? { | 556 | patch: &nostr::Event, |
| 553 | return Ok(Oid::from_str(&commit_id)?); | 557 | parent_commit_id_override: Option<String>, |
| 558 | ) -> Result<Oid> { | ||
| 559 | let commit_id = get_commit_id_from_patch(patch); | ||
| 560 | if let Ok(commit_id) = &commit_id { | ||
| 561 | if self.does_commit_exist(commit_id).unwrap_or(false) { | ||
| 562 | return Ok(Oid::from_str(commit_id)?); | ||
| 563 | } | ||
| 554 | } | 564 | } |
| 555 | let parent_commit_id = tag_value(patch, "parent-commit")?; | 565 | |
| 566 | let parent_commit_id = if let Some(commit_id) = parent_commit_id_override.clone() { | ||
| 567 | commit_id | ||
| 568 | } else { | ||
| 569 | tag_value(patch, "parent-commit")? | ||
| 570 | }; | ||
| 556 | 571 | ||
| 557 | let parent_commit = self | 572 | let parent_commit = self |
| 558 | .git_repo | 573 | .git_repo |
| @@ -604,25 +619,38 @@ impl RepoActions for Repo { | |||
| 604 | // were identical when in a scenario when they should be different but I dont | 619 | // were identical when in a scenario when they should be different but I dont |
| 605 | // think we have a test case for it. surely we should be using the | 620 | // think we have a test case for it. surely we should be using the |
| 606 | // extract_sig_from_patch_tags outputs to address this? | 621 | // extract_sig_from_patch_tags outputs to address this? |
| 607 | if !applied_oid.to_string().eq(&commit_id) { | 622 | let custom_parent = if let Some(ovderride_parent) = parent_commit_id_override { |
| 608 | let commit = self.git_repo.find_commit(applied_oid)?; | 623 | if let Ok(tag_parent) = tag_value(patch, "parent-commit") { |
| 609 | applied_oid = commit | 624 | ovderride_parent != tag_parent |
| 610 | .amend( | 625 | } else { |
| 611 | None, | 626 | true |
| 612 | Some(&commit.author()), | 627 | } |
| 613 | Some(&commit.committer()), | 628 | } else { |
| 614 | None, | 629 | false |
| 615 | None, | 630 | }; |
| 616 | None, | 631 | if !custom_parent { |
| 617 | ) | 632 | if let Ok(commit_id) = &commit_id { |
| 618 | .context("cannot amend commit to produce new oid")?; | 633 | if !applied_oid.to_string().eq(commit_id) { |
| 619 | } | 634 | let commit = self.git_repo.find_commit(applied_oid)?; |
| 620 | if !applied_oid.to_string().eq(&commit_id) { | 635 | applied_oid = commit |
| 621 | bail!( | 636 | .amend( |
| 622 | "when applied the patch commit id ({}) doesn't match the one specified in the event tag ({})", | 637 | None, |
| 623 | applied_oid.to_string(), | 638 | Some(&commit.author()), |
| 624 | get_commit_id_from_patch(patch)?, | 639 | Some(&commit.committer()), |
| 625 | ); | 640 | None, |
| 641 | None, | ||
| 642 | None, | ||
| 643 | ) | ||
| 644 | .context("cannot amend commit to produce new oid")?; | ||
| 645 | } | ||
| 646 | if !applied_oid.to_string().eq(commit_id) { | ||
| 647 | bail!( | ||
| 648 | "when applied the patch commit id ({}) doesn't match the one specified in the event tag ({})", | ||
| 649 | applied_oid.to_string(), | ||
| 650 | get_commit_id_from_patch(patch)?, | ||
| 651 | ); | ||
| 652 | } | ||
| 653 | } | ||
| 626 | } | 654 | } |
| 627 | self.git_repo.set_index(&mut existing_index)?; | 655 | self.git_repo.set_index(&mut existing_index)?; |
| 628 | Ok(applied_oid) | 656 | Ok(applied_oid) |
| @@ -1651,7 +1679,7 @@ mod tests { | |||
| 1651 | test_repo.populate()?; | 1679 | test_repo.populate()?; |
| 1652 | let git_repo = Repo::from_path(&test_repo.dir)?; | 1680 | let git_repo = Repo::from_path(&test_repo.dir)?; |
| 1653 | println!("{:?}", &patch_event); | 1681 | println!("{:?}", &patch_event); |
| 1654 | git_repo.create_commit_from_patch(&patch_event)?; | 1682 | git_repo.create_commit_from_patch(&patch_event, None)?; |
| 1655 | let commit_id = tag_value(&patch_event, "commit")?; | 1683 | let commit_id = tag_value(&patch_event, "commit")?; |
| 1656 | // does commit with id exist? | 1684 | // does commit with id exist? |
| 1657 | assert!(git_repo.does_commit_exist(&commit_id)?); | 1685 | assert!(git_repo.does_commit_exist(&commit_id)?); |