chore: refactor the backend #17
@@ -44,7 +44,11 @@ impl GitCache {
|
||||
}
|
||||
|
||||
/// Open the existing clone, fetching it first.
|
||||
pub fn ensure_clone(&self, addr: &RepoAddr, clone_urls: &[String]) -> Result<gix::Repository> {
|
||||
pub fn ensure_clone<U: AsRef<str>>(
|
||||
&self,
|
||||
addr: &RepoAddr,
|
||||
clone_urls: &[U],
|
||||
) -> Result<gix::Repository> {
|
||||
let path = self.repo_path(addr);
|
||||
|
||||
if let Some(repo) = self.open(addr)? {
|
||||
@@ -122,7 +126,7 @@ pub fn find_git_repos(root: &Path) -> Vec<PathBuf> {
|
||||
/// Clone into `path` from the first working URL in `clone_urls`.
|
||||
///
|
||||
/// Unlike [`GitCache::ensure_clone`], the clone is not kept in any cache.
|
||||
pub fn clone_repo(clone_urls: &[String], path: &Path) -> Result<()> {
|
||||
pub fn clone_repo<U: AsRef<str>>(clone_urls: &[U], path: &Path) -> Result<()> {
|
||||
if path.exists() {
|
||||
bail!("destination {} already exists", path.display());
|
||||
}
|
||||
@@ -347,14 +351,14 @@ fn transport_url(url: &str) -> String {
|
||||
///
|
||||
/// Returns the last error wrapped in `failed to {verb} from any mirror`,
|
||||
/// or `no clone URLs provided` when the list is empty.
|
||||
fn try_each_url<F>(urls: &[String], verb: &str, mut attempt: F) -> Result<()>
|
||||
fn try_each_url<U: AsRef<str>, F>(urls: &[U], verb: &str, mut attempt: F) -> Result<()>
|
||||
where
|
||||
F: FnMut(&str) -> Result<()>,
|
||||
{
|
||||
let mut last_err: Option<anyhow::Error> = None;
|
||||
|
||||
for url in urls {
|
||||
match attempt(url) {
|
||||
match attempt(url.as_ref()) {
|
||||
Ok(()) => return Ok(()),
|
||||
Err(e) => last_err = Some(e),
|
||||
}
|
||||
@@ -698,7 +702,7 @@ fn edit_local_config(
|
||||
/// When no URL works, the last error is returned.
|
||||
///
|
||||
/// Never touches the checked-out refs or the worktree.
|
||||
pub fn fetch_repo_refs(repo_path: &Path, urls: &[String], refspec: &str) -> Result<()> {
|
||||
pub fn fetch_repo_refs<U: AsRef<str>>(repo_path: &Path, urls: &[U], refspec: &str) -> Result<()> {
|
||||
let repo = gix::open(repo_path)?;
|
||||
let refspec = gix::refspec::parse(
|
||||
gix::bstr::BStr::new(refspec),
|
||||
@@ -2950,9 +2954,10 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn working_copy_cloned_from_the_mirror_matches_head_and_origin() {
|
||||
// The mirror is a freshly initialized repository.
|
||||
// Its `origin` points at the grasp server.
|
||||
// `Backend::create_repository` leaves it in the GitCache.
|
||||
// The mirror is a freshly initialized repository, standing in for
|
||||
// the grasp server. Its `origin` points at the (fake) grasp server.
|
||||
// `GitCache::ensure_clone` lazily clones from a URL shaped like this
|
||||
// the first time a repository is opened.
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let mirror = dir.path().join("mirror");
|
||||
let commit = init_repository(&mirror, "My Repo", "Does things.").expect("init");
|
||||
@@ -3166,7 +3171,8 @@ mod tests {
|
||||
assert!(err.to_string().contains("failed to fetch"));
|
||||
|
||||
// Without any URL there is nothing to try.
|
||||
let err = fetch_repo_refs(dir, &[], "+refs/heads/*:refs/fork/x/*").expect_err("no URLs");
|
||||
let err = fetch_repo_refs(dir, &[] as &[String], "+refs/heads/*:refs/fork/x/*")
|
||||
.expect_err("no URLs");
|
||||
assert!(err.to_string().contains("no clone URLs"));
|
||||
}
|
||||
|
||||
|
||||
@@ -5,14 +5,14 @@ use std::str::FromStr;
|
||||
use std::sync::{Arc, Mutex};
|
||||
use std::time::Duration;
|
||||
|
||||
use anyhow::{Context as AnyhowContext, Error, anyhow, bail};
|
||||
use anyhow::{Error, anyhow, bail};
|
||||
use bitcoin_hashes::sha1::Hash as Sha1Hash;
|
||||
use gpui::{App, AppContext, Context, Entity, EventEmitter, Global, Task};
|
||||
use nostr::event::IntoEventBuilder;
|
||||
use nostr_connect::prelude::*;
|
||||
use nostr_sdk::client::SyncSummary;
|
||||
use nostr_sdk::prelude::*;
|
||||
use signed_core::{Announcement, RepoAddr, build_state, filters, identifier_from_name, repo_addr};
|
||||
use signed_core::{Announcement, RepoAddr, build_state, filters, identifier_from_name};
|
||||
use signed_nostr::{SignedAuthUrlHandler, UniversalSigner, Update};
|
||||
|
||||
use crate::git_store::GitStore;
|
||||
@@ -420,41 +420,14 @@ impl Backend {
|
||||
)));
|
||||
}
|
||||
|
||||
let addr = repo_addr(public_key, repo_id.clone());
|
||||
let cache = GitStore::global(cx).cache().clone();
|
||||
let path = cache.repo_path(&addr);
|
||||
let owner = public_key.to_bech32().unwrap();
|
||||
let servers = grasp_servers.clone();
|
||||
let client = self.client.clone();
|
||||
|
||||
cx.spawn(async move |this, cx| {
|
||||
// Initialize the local clone and create the user's working copy from it.
|
||||
let work = cx.background_spawn({
|
||||
let path = path.clone();
|
||||
let folder = folder.clone();
|
||||
let name = name.clone();
|
||||
let description = description.clone();
|
||||
let owner = owner.clone();
|
||||
let repo_id = repo_id.clone();
|
||||
let servers = servers.clone();
|
||||
|
||||
async move {
|
||||
let parent = path
|
||||
.parent()
|
||||
.ok_or_else(|| anyhow!("invalid repository path"))?;
|
||||
std::fs::create_dir_all(parent)?;
|
||||
let commit = signed_git::init_repository(&path, &name, &description)?;
|
||||
|
||||
// Point `origin` at the first grasp server.
|
||||
// Later fetches and pushes have a target, like ngit.
|
||||
if let Some(base) = servers.first().and_then(grasp_base_url) {
|
||||
let url = format!("{base}/{owner}/{repo_id}.git");
|
||||
signed_git::ensure_origin(&path, &url).ok();
|
||||
}
|
||||
|
||||
// A working copy at `<folder>/<name>`, like the header's Clone action.
|
||||
// Cloned from the mirror above so it shares the announced history.
|
||||
// `origin` is set to the first grasp server, not the mirror path.
|
||||
// Initialize directly at the user's chosen destination.
|
||||
// No mirror is pre-populated: `GitCache::ensure_clone` lazily clones
|
||||
// from the grasp server the first time the repo detail view needs it,
|
||||
// exactly like every other repository.
|
||||
let destination = {
|
||||
let dir_name = signed_git::sanitize_path_component(&name);
|
||||
let dir_name = if dir_name.is_empty() {
|
||||
@@ -465,27 +438,32 @@ impl Backend {
|
||||
folder.join(dir_name)
|
||||
};
|
||||
|
||||
let mirror_url = Url::from_file_path(&path)
|
||||
.map_err(|_| anyhow!("invalid mirror path"))?
|
||||
.to_string();
|
||||
cx.spawn(async move |this, cx| {
|
||||
let work = cx.background_spawn({
|
||||
let destination = destination.clone();
|
||||
let name = name.clone();
|
||||
let description = description.clone();
|
||||
let owner = owner.clone();
|
||||
let repo_id = repo_id.clone();
|
||||
let servers = servers.clone();
|
||||
|
||||
signed_git::clone_repo(&[mirror_url], &destination).with_context(|| {
|
||||
format!(
|
||||
"failed to create the working copy at {}",
|
||||
destination.display()
|
||||
)
|
||||
})?;
|
||||
async move {
|
||||
if destination.exists() {
|
||||
bail!("destination {} already exists", destination.display());
|
||||
}
|
||||
|
||||
let commit = signed_git::init_repository(&destination, &name, &description)?;
|
||||
|
||||
if let Some(base) = servers.first().and_then(grasp_base_url) {
|
||||
let url = format!("{base}/{owner}/{repo_id}.git");
|
||||
signed_git::set_origin(&destination, &url)?;
|
||||
}
|
||||
|
||||
Ok::<_, Error>((commit, destination))
|
||||
Ok::<_, Error>(commit)
|
||||
}
|
||||
});
|
||||
|
||||
let (commit, checkout_path) = work.await?;
|
||||
let commit = work.await?;
|
||||
let commit_sha = Sha1Hash::from_str(&commit).map_err(|_| anyhow!("invalid id"))?;
|
||||
|
||||
// The nostr client queues events until each relay is connected.
|
||||
@@ -525,7 +503,7 @@ impl Backend {
|
||||
let push = cx.background_spawn({
|
||||
let client = client.clone();
|
||||
let signer = signer.clone();
|
||||
let path = path.clone();
|
||||
let destination = destination.clone();
|
||||
let owner = owner.clone();
|
||||
let repo_id = repo_id.clone();
|
||||
let servers = servers.clone();
|
||||
@@ -537,7 +515,7 @@ impl Backend {
|
||||
&repo_id,
|
||||
&refs,
|
||||
Some("main"),
|
||||
&path,
|
||||
&destination,
|
||||
&owner,
|
||||
&servers,
|
||||
signed_git::push_main,
|
||||
@@ -577,7 +555,7 @@ impl Backend {
|
||||
let announcement = Announcement::from_event(&event)
|
||||
.ok_or_else(|| anyhow!("failed to parse announcement"))?;
|
||||
|
||||
Ok((announcement, checkout_path))
|
||||
Ok((announcement, destination))
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -999,10 +999,10 @@ impl RepoStore {
|
||||
let cache = GitStore::global(cx).cache().clone();
|
||||
let addr = self.addr.clone();
|
||||
|
||||
let clone_urls: Vec<String> = self
|
||||
let clone_urls: Vec<Url> = self
|
||||
.announcement
|
||||
.as_ref()
|
||||
.map(|a| a.clone.iter().map(ToString::to_string).collect())
|
||||
.map(|a| a.clone.clone())
|
||||
.unwrap_or_default();
|
||||
|
||||
let patch = pull_request_patch(root, self.patches.iter());
|
||||
@@ -1222,7 +1222,7 @@ impl RepoStore {
|
||||
return self.action_error("Repository announcement is not loaded yet", cx);
|
||||
};
|
||||
|
||||
let clone_urls: Vec<String> = announcement.clone.iter().map(ToString::to_string).collect();
|
||||
let clone_urls = announcement.clone.clone();
|
||||
let addr = self.addr.clone();
|
||||
|
||||
self.cloning = true;
|
||||
|
||||
@@ -24,7 +24,7 @@ use gpui_component::{
|
||||
ActiveTheme, Colorize, Icon, IconName, Sizable, StyledExt, ThemeStyled,
|
||||
VirtualListScrollHandle, h_flex, v_flex,
|
||||
};
|
||||
use nostr::prelude::{RelayUrl, ToBech32};
|
||||
use nostr::prelude::{RelayUrl, ToBech32, Url};
|
||||
use signed_core::{Announcement, RepoAddr, RepoStatus, filters};
|
||||
use signed_git::{CommitList, FileCommit};
|
||||
use signed_state::{
|
||||
@@ -390,7 +390,7 @@ impl RepoDetailView {
|
||||
|
||||
let cache = GitStore::global(cx).cache().clone();
|
||||
let addr = initial.addr();
|
||||
let clone_urls: Vec<String> = initial.clone.iter().map(ToString::to_string).collect();
|
||||
let clone_urls: Vec<Url> = initial.clone.clone();
|
||||
|
||||
// Captured before the loads start.
|
||||
// A branch/tag switch bumps the generation, discarding the refresh below.
|
||||
|
||||
@@ -591,14 +591,14 @@ impl NewPullRequestView {
|
||||
let cache = GitStore::global(cx).cache().clone();
|
||||
let mirror_path = cache.repo_path(&base);
|
||||
let namespace = fork_namespace(&announcement);
|
||||
let clone_urls: Vec<String> = announcement.clone.iter().map(ToString::to_string).collect();
|
||||
let clone_urls = announcement.clone.clone();
|
||||
|
||||
let base_clone_urls: Vec<String> = self
|
||||
let base_clone_urls: Vec<Url> = self
|
||||
.store
|
||||
.read(cx)
|
||||
.announcement
|
||||
.as_ref()
|
||||
.map(|a| a.clone.iter().map(ToString::to_string).collect())
|
||||
.map(|a| a.clone.clone())
|
||||
.unwrap_or_default();
|
||||
|
||||
// Keep the current compare and base when the fork is already applied.
|
||||
|
||||
@@ -20,7 +20,7 @@ use gpui_component::{
|
||||
ActiveTheme, Sizable, StyledExt, VirtualListScrollHandle, WindowExt, h_flex, v_flex,
|
||||
v_virtual_list,
|
||||
};
|
||||
use nostr::prelude::{Event, EventId, Kind, Nip34Tag};
|
||||
use nostr::prelude::{Event, EventId, Kind, Nip34Tag, Url};
|
||||
use signed_core::{activity_subject, pull_request_patch};
|
||||
use signed_git::{FileCommit, patch_commits, patch_diffs};
|
||||
use signed_state::{Backend, GitStore, ProfileStore, RepoStore};
|
||||
@@ -139,12 +139,8 @@ impl PullRequestDetailView {
|
||||
.and_then(merge_base_of)
|
||||
.or_else(|| merge_base_of(root));
|
||||
|
||||
let clone_urls = clone_urls_of(root).or_else(|| {
|
||||
store
|
||||
.announcement
|
||||
.as_ref()
|
||||
.map(|a| a.clone.iter().map(ToString::to_string).collect())
|
||||
});
|
||||
let clone_urls = clone_urls_of(root)
|
||||
.or_else(|| store.announcement.as_ref().map(|a| a.clone.clone()));
|
||||
|
||||
(
|
||||
root.content.clone(),
|
||||
@@ -730,12 +726,12 @@ fn merge_base_of(event: &Event) -> Option<String> {
|
||||
/// The `clone` tag of a PR event.
|
||||
///
|
||||
/// URLs where the proposed branch can be fetched, or `None` if the PR has none.
|
||||
fn clone_urls_of(event: &Event) -> Option<Vec<String>> {
|
||||
fn clone_urls_of(event: &Event) -> Option<Vec<Url>> {
|
||||
event
|
||||
.tags
|
||||
.iter()
|
||||
.find_map(|tag| match Nip34Tag::parse(tag.as_slice()) {
|
||||
Ok(Nip34Tag::Clone(urls)) => Some(urls.iter().map(ToString::to_string).collect()),
|
||||
Ok(Nip34Tag::Clone(urls)) => Some(urls),
|
||||
_ => None,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -691,6 +691,24 @@ Delete `Backend::add_relays` entirely once both call sites are inlined.
|
||||
|
||||
## 9. `create_repository`'s flow is backwards: it inits a mirror, then clones it into the real destination
|
||||
|
||||
> **Status: done.** `Backend::create_repository` now computes `destination`
|
||||
> (`folder.join(dir_name)`) up front and calls `signed_git::init_repository`
|
||||
> directly on it — no mirror path, no `Url::from_file_path`, no `clone_repo`
|
||||
> call, no double `origin` setup. An explicit `destination.exists()` check
|
||||
> (mirroring what `clone_repo` used to guard for free) replaces the removed
|
||||
> clone step's own guard. The push at the end now runs against `destination`
|
||||
> instead of the mirror path, and the task returns `(announcement,
|
||||
> destination)` as before — no caller-visible signature change. This also
|
||||
> dropped `GitStore`/`GitCache::repo_path`/`repo_addr` usage from the
|
||||
> function entirely, since no mirror is created there anymore; `repo_addr`
|
||||
> and `Context as AnyhowContext` became unused imports in `backend.rs` and
|
||||
> were removed. Updated a stale comment in `signed_git`'s
|
||||
> `working_copy_cloned_from_the_mirror_matches_head_and_origin` test, which
|
||||
> referenced this flow by name even though it's a generic `clone_repo`
|
||||
> fixture unrelated to `Backend::create_repository`. `cargo check --workspace`,
|
||||
> `cargo clippy --workspace`, and `cargo test --workspace` (signed_git 67,
|
||||
> signed_state 24, workspace 14) all pass.
|
||||
|
||||
This is a real business-logic flaw, not just a style issue. Today
|
||||
(`backend.rs:437-501`):
|
||||
|
||||
@@ -1046,6 +1064,21 @@ already exists once in the same crate.
|
||||
|
||||
## 15. `Vec<Url>` → `Vec<String>` conversion sprawl — fix the 3 `signed_git` signatures, not the 8 call sites
|
||||
|
||||
> **Status: done.** `try_each_url`, `clone_repo`, `GitCache::ensure_clone`
|
||||
> and `fetch_repo_refs` are now generic over `U: AsRef<str>`. All 7 call
|
||||
> sites (`repo.rs::merge_pull_request`/`clone_to_folder`,
|
||||
> `repo_detail/mod.rs::load_repo`, `new_pull_request.rs::choose_fork` x2,
|
||||
> `pull_request_detail.rs::load`/`clone_urls_of`) now pass the
|
||||
> `Vec<Url>`/`Vec<Url>`-derived value straight through with a plain
|
||||
> `.clone()` of the field, no `.iter().map(ToString::to_string).collect()`
|
||||
> anywhere left in non-test code. One test in `signed_git` passed an empty
|
||||
> untyped `&[]` literal to `fetch_repo_refs`, which lost its type-inference
|
||||
> anchor once the function went generic — fixed with an explicit
|
||||
> `&[] as &[String]` annotation. `cargo check --workspace`,
|
||||
> `cargo clippy -p signed_state -p workspace -p signed_git --all-targets`,
|
||||
> and `cargo test --workspace` (signed_git 67, signed_state 24, workspace 14)
|
||||
> all pass.
|
||||
|
||||
`Announcement::clone` is `Vec<Url>` (`signed_core/src/model.rs`, `Url` being
|
||||
`nostr`'s re-export of the `url` crate's `Url`, `nostr/src/types/url.rs:15`,
|
||||
`pub use url::*;`). Every call site that needs to hand those URLs to
|
||||
@@ -1300,17 +1333,23 @@ method.
|
||||
`latest_grasp_list_servers`.
|
||||
|
||||
Done: see §1. `fetch_events` no longer appears anywhere in the workspace.
|
||||
5. **Generalize the 3 `signed_git` URL-list signatures** to `&[impl AsRef<str>]`
|
||||
5. ✅ **Generalize the 3 `signed_git` URL-list signatures** to `&[impl AsRef<str>]`
|
||||
(§15), then delete the now-redundant `.map(ToString::to_string).collect()`
|
||||
at all 7 call sites. Self-contained to `signed_git`'s public API plus a
|
||||
one-line change per call site; re-run `signed_git`'s existing tests
|
||||
(`clone_repo`/`fetch_repo_refs` already have coverage).
|
||||
6. **Fix `create_repository`'s init/clone ordering** (§9): initialize and
|
||||
|
||||
Done: see §15 for the full list of call sites and verification notes.
|
||||
6. ✅ **Fix `create_repository`'s init/clone ordering** (§9): initialize and
|
||||
push directly at the user's chosen destination, drop the mirror
|
||||
pre-population entirely and let `ensure_clone` populate it lazily like
|
||||
every other repo. Self-contained to one function; verify against this
|
||||
crate's existing `init_repository`/push tests plus a manual
|
||||
create-repository-then-open-detail-view pass.
|
||||
|
||||
Done: see §9. Manual create-repository-then-open-detail-view pass still
|
||||
recommended before shipping, since it depends on the grasp push actually
|
||||
succeeding end-to-end against a live server.
|
||||
7. **Merge `local_repos.rs` and `repo_list.rs` into one file** (§13),
|
||||
keeping both stores as independent entities. Purely organizational, zero
|
||||
call-site changes, safe to do any time.
|
||||
|
||||
Reference in New Issue
Block a user