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/bin/git_remote_nostr/list.rs | |
| 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/bin/git_remote_nostr/list.rs')
| -rw-r--r-- | src/bin/git_remote_nostr/list.rs | 92 |
1 files changed, 68 insertions, 24 deletions
diff --git a/src/bin/git_remote_nostr/list.rs b/src/bin/git_remote_nostr/list.rs index 959b8c8..378a124 100644 --- a/src/bin/git_remote_nostr/list.rs +++ b/src/bin/git_remote_nostr/list.rs | |||
| @@ -5,14 +5,13 @@ use anyhow::{anyhow, Context, Result}; | |||
| 5 | use auth_git2::GitAuthenticator; | 5 | use auth_git2::GitAuthenticator; |
| 6 | use client::get_state_from_cache; | 6 | use client::get_state_from_cache; |
| 7 | use git::RepoActions; | 7 | use git::RepoActions; |
| 8 | use git_events::{event_to_cover_letter, get_commit_id_from_patch}; | ||
| 9 | use ngit::{ | 8 | use ngit::{ |
| 10 | client, | 9 | client, |
| 11 | git::{ | 10 | git::{ |
| 12 | self, | 11 | self, |
| 13 | nostr_url::{CloneUrl, NostrUrlDecoded, ServerProtocol}, | 12 | nostr_url::{CloneUrl, NostrUrlDecoded, ServerProtocol}, |
| 14 | }, | 13 | }, |
| 15 | git_events, | 14 | git_events::event_to_cover_letter, |
| 16 | login::get_curent_user, | 15 | login::get_curent_user, |
| 17 | repo_ref, | 16 | repo_ref, |
| 18 | }; | 17 | }; |
| @@ -20,6 +19,7 @@ use nostr_sdk::hashes::sha1::Hash as Sha1Hash; | |||
| 20 | use repo_ref::RepoRef; | 19 | use repo_ref::RepoRef; |
| 21 | 20 | ||
| 22 | use crate::{ | 21 | use crate::{ |
| 22 | fetch::{fetch_from_git_server, make_commits_for_proposal}, | ||
| 23 | git::Repo, | 23 | git::Repo, |
| 24 | utils::{ | 24 | utils::{ |
| 25 | fetch_or_list_error_is_not_authentication_failure, get_open_proposals, | 25 | fetch_or_list_error_is_not_authentication_failure, get_open_proposals, |
| @@ -88,6 +88,61 @@ pub async fn run_list( | |||
| 88 | 88 | ||
| 89 | state.retain(|k, _| !k.starts_with("refs/heads/pr/")); | 89 | state.retain(|k, _| !k.starts_with("refs/heads/pr/")); |
| 90 | 90 | ||
| 91 | let proposals_state = | ||
| 92 | get_open_proposals_state(&term, git_repo, repo_ref, decoded_nostr_url, &remote_states) | ||
| 93 | .await?; | ||
| 94 | |||
| 95 | state.extend(proposals_state); | ||
| 96 | |||
| 97 | // TODO 'for push' should we check with the git servers to see if any of them | ||
| 98 | // allow push from the user? | ||
| 99 | for (name, value) in state { | ||
| 100 | if value.starts_with("ref: ") { | ||
| 101 | if !for_push { | ||
| 102 | println!("{} {name}", value.replace("ref: ", "@")); | ||
| 103 | } | ||
| 104 | } else { | ||
| 105 | println!("{value} {name}"); | ||
| 106 | } | ||
| 107 | } | ||
| 108 | |||
| 109 | println!(); | ||
| 110 | Ok(remote_states) | ||
| 111 | } | ||
| 112 | |||
| 113 | async fn get_open_proposals_state( | ||
| 114 | term: &console::Term, | ||
| 115 | git_repo: &Repo, | ||
| 116 | repo_ref: &RepoRef, | ||
| 117 | decoded_nostr_url: &NostrUrlDecoded, | ||
| 118 | remote_states: &HashMap<String, HashMap<String, String>>, | ||
| 119 | ) -> Result<HashMap<String, String>> { | ||
| 120 | // we cannot use commit_id in the latest patch in a proposal because: | ||
| 121 | // 1) the `commit` tag is optional | ||
| 122 | // 2) if the commit tag is wrong, it will cause errors which stop clone from | ||
| 123 | // working | ||
| 124 | |||
| 125 | // without trusting commit_id we must apply each patch which requires the oid of | ||
| 126 | // the parent so we much do a fetch | ||
| 127 | for (git_server_url, oids_from_git_servers) in remote_states { | ||
| 128 | if fetch_from_git_server( | ||
| 129 | git_repo, | ||
| 130 | &oids_from_git_servers | ||
| 131 | .values() | ||
| 132 | .filter(|v| !v.starts_with("ref: ")) | ||
| 133 | .cloned() | ||
| 134 | .collect::<Vec<String>>(), | ||
| 135 | git_server_url, | ||
| 136 | decoded_nostr_url, | ||
| 137 | term, | ||
| 138 | ) | ||
| 139 | .is_ok() | ||
| 140 | { | ||
| 141 | break; | ||
| 142 | } | ||
| 143 | } | ||
| 144 | |||
| 145 | let mut state = HashMap::new(); | ||
| 91 | let open_proposals = get_open_proposals(git_repo, repo_ref).await?; | 146 | let open_proposals = get_open_proposals(git_repo, repo_ref).await?; |
| 92 | let current_user = get_curent_user(git_repo)?; | 147 | let current_user = get_curent_user(git_repo)?; |
| 93 | for (_, (proposal, patches)) in open_proposals { | 148 | for (_, (proposal, patches)) in open_proposals { |
| @@ -102,32 +157,21 @@ pub async fn run_list( | |||
| 102 | } else { | 157 | } else { |
| 103 | branch_name | 158 | branch_name |
| 104 | }; | 159 | }; |
| 105 | if let Some(patch) = patches.first() { | 160 | match make_commits_for_proposal(git_repo, repo_ref, &patches) { |
| 106 | // TODO this isn't resilient because the commit id stated may not be correct | 161 | Ok(tip) => { |
| 107 | // we will need to check whether the commit id exists in the repo or apply the | 162 | state.insert(format!("refs/heads/{branch_name}"), tip); |
| 108 | // proposal and each patch to check | ||
| 109 | if let Ok(commit_id) = get_commit_id_from_patch(patch) { | ||
| 110 | state.insert(format!("refs/heads/{branch_name}"), commit_id); | ||
| 111 | } | 163 | } |
| 112 | } | 164 | Err(error) => { |
| 113 | } | 165 | let _ = term.write_line( |
| 114 | } | 166 | format!("WARNING: cannot fetch branch {branch_name} error: {error}") |
| 115 | } | 167 | .as_str(), |
| 116 | 168 | ); | |
| 117 | // TODO 'for push' should we check with the git servers to see if any of them | 169 | } |
| 118 | // allow push from the user? | 170 | }; |
| 119 | for (name, value) in state { | ||
| 120 | if value.starts_with("ref: ") { | ||
| 121 | if !for_push { | ||
| 122 | println!("{} {name}", value.replace("ref: ", "@")); | ||
| 123 | } | 171 | } |
| 124 | } else { | ||
| 125 | println!("{value} {name}"); | ||
| 126 | } | 172 | } |
| 127 | } | 173 | } |
| 128 | 174 | Ok(state) | |
| 129 | println!(); | ||
| 130 | Ok(remote_states) | ||
| 131 | } | 175 | } |
| 132 | 176 | ||
| 133 | pub fn list_from_remotes( | 177 | pub fn list_from_remotes( |