chore: remove unnecessary optimization (#20)

Reviewed-on: #20

crates/signed_state/Cargo.toml

@@ -17,6 +17,7 @@ nostr-connect.workspace = true
 
 bitcoin_hashes = "1"
 
+gix.workspace = true
 gpui.workspace = true
 flume.workspace = true
 futures.workspace = true

crates/signed_state/src/backend.rs

@@ -5,7 +5,7 @@ use std::time::{Duration, Instant};
 
 use anyhow::{Error, anyhow, bail};
 use bitcoin_hashes::sha1::Hash as Sha1Hash;
-use gpui::{App, AppContext, Context, Entity, EventEmitter, Global, Task};
+use gpui::{App, AppContext, BackgroundExecutor, Context, Entity, EventEmitter, Global, Task};
 use nostr::event::IntoEventBuilder;
 use nostr_connect::prelude::*;
 use nostr_sdk::client::SyncSummary;
@@ -13,7 +13,7 @@ use nostr_sdk::prelude::*;
 use signed_core::{Announcement, RepoAddr, build_state, filters, identifier_from_name};
 use signed_nostr::{SignedAuthUrlHandler, UniversalSigner, Update};
 
-use crate::git_store::GitStore;
+use crate::git_store::repo_mirror_path;
 use crate::inbox::Inbox;
 use crate::repos::RepoListStore;
 
@@ -531,6 +531,7 @@ impl Backend {
                 let repo_id = repo_id.clone();
                 let servers = servers.clone();
                 let refs = refs.clone();
+                let executor = cx.background_executor().clone();
                 async move {
                     push_staged_to_grasps(
                         &client,
@@ -541,6 +542,7 @@ impl Backend {
                         &destination,
                         &owner,
                         &servers,
+                        &executor,
                         signed_git::push_main,
                     )
                     .await
@@ -686,6 +688,7 @@ impl Backend {
                     let servers = servers.clone();
                     let refs = refs.clone();
                     let head = head.clone();
+                    let executor = cx.background_executor().clone();
                     async move {
                         push_staged_to_grasps(
                             &client,
@@ -696,6 +699,7 @@ impl Backend {
                             &path,
                             &owner,
                             &servers,
+                            &executor,
                             signed_git::push_all,
                         )
                         .await
@@ -752,8 +756,7 @@ impl Backend {
         announcement: Announcement,
         cx: &mut Context<Self>,
     ) -> Task<Result<PushOutcome, Error>> {
-        let cache = GitStore::global(cx).cache().clone();
-        let path = cache.repo_path(&announcement.addr());
+        let path = repo_mirror_path(&announcement.addr());
         self.push_repo_from(announcement, path, None, cx)
     }
 
@@ -854,6 +857,7 @@ impl Backend {
                     let relays = relays.clone();
                     let refs = refs.clone();
                     let head = head.clone();
+                    let executor = cx.background_executor().clone();
                     async move {
                         push_staged_to_grasps(
                             &client,
@@ -864,6 +868,7 @@ impl Backend {
                             &path,
                             &owner,
                             &relays,
+                            &executor,
                             signed_git::push_all,
                         )
                         .await
@@ -1086,10 +1091,6 @@ impl Backend {
         self.signer.clone()
     }
 
-    pub fn pushing_repos(&self) -> Entity<HashSet<RepoAddr>> {
-        self.pushing_repos.clone()
-    }
-
     pub fn inbox(&self) -> Entity<Inbox> {
         self.inbox.clone()
     }
@@ -1102,10 +1103,6 @@ impl Backend {
         self.passphrase_required
     }
 
-    pub fn emit_error(&mut self, message: impl Into<String>, cx: &mut Context<Self>) {
-        cx.emit(BackendEvent::error(message));
-    }
-
     fn sync_inbox(&mut self, cx: &mut Context<Self>) {
         let client = self.client.clone();
         let me = self.current_user;
@@ -1498,24 +1495,21 @@ const GRASP_RETRY_DELAY: Duration = Duration::from_secs(1);
 #[derive(Debug, Clone)]
 pub struct GraspServerResult {
     pub relay: RelayUrl,
-    pub git_url: String,
     /// `None` when the server accepted the data, the reason otherwise.
     pub reason: Option<String>,
 }
 
 impl GraspServerResult {
-    fn ok(relay: RelayUrl, git_url: String) -> Self {
+    fn ok(relay: RelayUrl) -> Self {
         Self {
             relay,
-            git_url,
             reason: None,
         }
     }
 
-    fn failed(relay: RelayUrl, git_url: String, reason: impl Into<String>) -> Self {
+    fn failed(relay: RelayUrl, reason: impl Into<String>) -> Self {
         Self {
             relay,
-            git_url,
             reason: Some(reason.into()),
         }
     }
@@ -1711,21 +1705,16 @@ async fn push_staged_to_grasps(
     path: &Path,
     owner: &str,
     servers: &[RelayUrl],
+    executor: &BackgroundExecutor,
     push: fn(&Path, &str, &str, &str) -> Result<(), Error>,
 ) -> PushOutcome {
     let mut outcome = PushOutcome::default();
 
-    if refs.is_empty() {
-        return outcome;
-    }
-
     for relay in servers {
         let Some(base) = grasp_base_url(relay) else {
-            outcome.servers.push(GraspServerResult::failed(
-                relay.clone(),
-                relay.to_string(),
-                "no domain",
-            ));
+            outcome
+                .servers
+                .push(GraspServerResult::failed(relay.clone(), "no domain"));
             continue;
         };
         let git_url = format!("{base}/{owner}/{repo_id}.git");
@@ -1739,7 +1728,7 @@ async fn push_staged_to_grasps(
         'server: for attempt in 1..=GRASP_PUSH_ATTEMPTS {
             if attempt > 1 {
                 // Give the server's ingest a moment before re-staging.
-                std::thread::sleep(GRASP_RETRY_DELAY);
+                executor.timer(GRASP_RETRY_DELAY).await;
             }
 
             let (event, created_at) =
@@ -1806,11 +1795,9 @@ async fn push_staged_to_grasps(
                 log::warn!("grasp push failed: {relay}: {reason}");
                 outcome
                     .servers
-                    .push(GraspServerResult::failed(relay.clone(), git_url, reason));
+                    .push(GraspServerResult::failed(relay.clone(), reason));
             }
-            None => outcome
-                .servers
-                .push(GraspServerResult::ok(relay.clone(), git_url)),
+            None => outcome.servers.push(GraspServerResult::ok(relay.clone())),
         }
     }
 
@@ -1968,13 +1955,9 @@ mod tests {
     fn push_outcome_reports_partial_failures() {
         let outcome = PushOutcome {
             servers: vec![
-                GraspServerResult::ok(
-                    RelayUrl::parse("wss://gitnostr.com").expect("url"),
-                    "https://gitnostr.com/npub1owner/repo.git".to_owned(),
-                ),
+                GraspServerResult::ok(RelayUrl::parse("wss://gitnostr.com").expect("url")),
                 GraspServerResult::failed(
                     RelayUrl::parse("wss://relay.ngit.dev").expect("url"),
-                    "https://relay.ngit.dev/npub1owner/repo.git".to_owned(),
                     "remote: ERR authorisation failed: No state events in purgatory\nfatal: ...",
                 ),
             ],

crates/signed_state/src/checkouts.rs

@@ -9,7 +9,7 @@ use settings::{CheckoutRecord, SettingsStore};
 use signed_core::{Announcement, RepoAddr};
 
 use crate::backend::{Backend, BackendEvent};
-use crate::git_store::GitStore;
+use crate::git_store::repo_mirror_root;
 use crate::refresh::{RefreshGate, RefreshRequest};
 use crate::repos::{LocalReposStore, RepoListStore};
 
@@ -93,6 +93,8 @@ pub struct CheckoutsStore {
     /// A recompute defaults the base the same way.
     requested_head: HashMap<RepoAddr, Option<String>>,
     refresh: RefreshGate,
+    /// True while the timer between a scheduled refresh and its run is pending.
+    debounce_pending: bool,
     local_pending: bool,
     /// When the last full pass (with a remote refresh) completed.
     ///
@@ -163,6 +165,7 @@ impl CheckoutsStore {
             push_statuses: HashMap::new(),
             requested_head: HashMap::new(),
             refresh: RefreshGate::default(),
+            debounce_pending: false,
             local_pending: false,
             last_full_sync: None,
             _subscriptions: subscriptions,
@@ -279,12 +282,15 @@ impl CheckoutsStore {
 
     /// Re-resolve the associations and the requested statuses.
     ///
-    /// Requests arriving while a pass runs fold into a follow-up.
+    /// Requests arriving while a pass runs fold into a follow-up, requests
+    /// arriving while the debounce timer is pending are dropped.
     pub fn refresh(&mut self, cx: &mut Context<Self>) {
-        if self.refresh.request() != RefreshRequest::Schedule {
+        if self.debounce_pending || self.refresh.request() != RefreshRequest::Schedule {
             return;
         }
 
+        self.debounce_pending = true;
+
         cx.spawn(async move |this, cx| {
             cx.background_executor().timer(REFRESH_DEBOUNCE).await;
             this.update(cx, |this, cx| this.run_refresh(cx))
@@ -300,6 +306,7 @@ impl CheckoutsStore {
     /// remote reconciliation cadence ([`Self::local_tick`]); they also restart
     /// the fast local pass.
     fn run_refresh(&mut self, cx: &mut Context<Self>) {
+        self.debounce_pending = false;
         self.refresh.begin();
 
         let records = {
@@ -321,7 +328,7 @@ impl CheckoutsStore {
 
         let announcements = RepoListStore::global(cx).read(cx).announcements.clone();
         let scanned = LocalReposStore::global(cx).read(cx).repos.clone();
-        let cache_root = GitStore::global(cx).cache().root().canonicalize().ok();
+        let cache_root = repo_mirror_root().canonicalize().ok();
 
         let requested: Vec<(RepoAddr, Option<String>)> = self
             .status_requested
@@ -453,7 +460,7 @@ impl CheckoutsStore {
         }
 
         // A full pass or a fresh request covers this tick, skip it.
-        if self.refresh.running() || self.refresh.debouncing() {
+        if self.refresh.running() || self.debounce_pending {
             self.schedule_local_pass(cx);
             return;
         }
@@ -515,7 +522,7 @@ impl CheckoutsStore {
             this.update(cx, |this, cx| {
                 // A full pass or a fresh request will apply fresher data
                 // (the tracking refs move only when a full pass fetches).
-                if this.refresh.running() || this.refresh.debouncing() {
+                if this.refresh.running() || this.debounce_pending {
                     return;
                 }
 

crates/signed_state/src/git_store.rs

@@ -1,32 +1,41 @@
 use std::path::PathBuf;
+use std::sync::OnceLock;
 
-use gpui::{App, Global};
+use anyhow::Result;
+use gix::Repository;
+use signed_core::RepoAddr;
 use signed_git::GitCache;
 
-struct GlobalGitStore(GitCache);
+static GIT_CACHE: OnceLock<GitCache> = OnceLock::new();
 
-impl Global for GlobalGitStore {}
+fn git_cache() -> &'static GitCache {
+    GIT_CACHE
+        .get()
+        .expect("git cache is initialized by signed_state::init")
+}
 
-/// Global access to the on-disk git clone cache, the grasp mirrors.
-#[derive(Debug, Clone)]
-pub struct GitStore(GitCache);
+/// The root directory of the repository mirrors.
+pub(crate) fn repo_mirror_root() -> PathBuf {
+    git_cache().root().to_path_buf()
+}
 
-impl GitStore {
-    pub fn set_global(root: impl Into<PathBuf>, cx: &mut App) -> Self {
-        let store = Self::new(root);
-        cx.set_global(GlobalGitStore(store.0.clone()));
-        store
-    }
+/// The on-disk path of the mirror of `addr`.
+pub fn repo_mirror_path(addr: &RepoAddr) -> PathBuf {
+    git_cache().repo_path(addr)
+}
 
-    pub fn global(cx: &App) -> Self {
-        Self(cx.global::<GlobalGitStore>().0.clone())
-    }
+/// Open the mirror of `addr`, if it has been cloned.
+pub fn open_repo_mirror(addr: &RepoAddr) -> Result<Option<Repository>> {
+    git_cache().open(addr)
+}
 
-    fn new(root: impl Into<PathBuf>) -> Self {
-        Self(GitCache::new(root.into()))
-    }
+/// Open the mirror of `addr`, cloning it first when it does not exist yet.
+pub fn ensure_repo_mirror<U: AsRef<str>>(addr: &RepoAddr, clone_urls: &[U]) -> Result<Repository> {
+    git_cache().ensure_clone(addr, clone_urls)
+}
 
-    pub fn cache(&self) -> &GitCache {
-        &self.0
+pub(crate) fn set_git_cache(root: impl Into<PathBuf>) {
+    if GIT_CACHE.set(GitCache::new(root.into())).is_err() {
+        log::warn!("git cache root is already set, keeping the first one");
     }
 }

crates/signed_state/src/lib.rs

@@ -11,7 +11,8 @@ use std::path::{Path, PathBuf};
 
 pub use backend::{Backend, BackendEvent, user_grasp_list_servers};
 pub use checkouts::{CheckoutStatus, CheckoutsStore, pr_proposes_checkout};
-pub use git_store::GitStore;
+use git_store::set_git_cache;
+pub use git_store::{ensure_repo_mirror, open_repo_mirror, repo_mirror_path};
 use gpui::{App, AppContext};
 pub use inbox::{Inbox, query_inbox};
 pub use nostr_sdk::prelude::Timestamp;
@@ -31,6 +32,7 @@ pub fn init(
     // rustls uses the `aws_lc_rs` provider by default.
     let _ = rustls::crypto::aws_lc_rs::default_provider().install_default();
 
+    // Initialize the nostr client and signer
     let (client, signer) = cx.foreground_executor().block_on(async move {
         let path = db_path.as_ref().to_path_buf();
         new_backend(path)
@@ -38,11 +40,22 @@ pub fn init(
             .expect("failed to initialize nostr backend")
     });
 
+    // Set Git cache for the repos root
+    set_git_cache(repos_root);
+
+    // Set global stores for the backend
     Backend::set_global(cx.new(|cx| Backend::new(client, signer, cx)), cx);
+
+    // Set global stores for the profile
     ProfileStore::set_global(cx.new(ProfileStore::new), cx);
+
+    // Set global stores for the repo list and local repos
     RepoListStore::set_global(cx.new(RepoListStore::new), cx);
-    GitStore::set_global(repos_root, cx);
+
+    // Set global stores for the local repos
     LocalReposStore::set_global(cx.new(|cx| LocalReposStore::new(scan_paths, cx)), cx);
+
+    // Set global stores for the checkouts
     CheckoutsStore::set_global(cx.new(CheckoutsStore::new), cx);
 }
 
@@ -50,10 +63,10 @@ pub fn init(
 #[cfg(target_arch = "wasm32")]
 pub fn init(cx: &mut App) {
     let (client, signer) = new_backend().expect("failed to initialize nostr backend");
+    set_git_cache(PathBuf::new());
     Backend::set_global(cx.new(|cx| Backend::new(client, signer, cx)), cx);
     ProfileStore::set_global(cx.new(ProfileStore::new), cx);
     RepoListStore::set_global(cx.new(RepoListStore::new), cx);
-    GitStore::set_global(PathBuf::new(), cx);
     LocalReposStore::set_global(cx.new(|cx| LocalReposStore::new(Vec::new(), cx)), cx);
     CheckoutsStore::set_global(cx.new(|cx| CheckoutsStore::new(cx)), cx);
 }

crates/signed_state/src/profile.rs

@@ -50,7 +50,7 @@ impl Profile {
             return SharedString::from(name.trim().to_owned());
         }
 
-        SharedString::from(shorten_pubkey(self.public_key, 4))
+        SharedString::from(shorten_pubkey(self.public_key))
     }
 
     pub fn picture(&self) -> Option<SharedString> {

crates/signed_state/src/refresh.rs

@@ -3,14 +3,13 @@
 pub struct RefreshGate {
     running: bool,
     dirty: bool,
-    debouncing: bool,
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum RefreshRequest {
-    /// No run or timer covers the request, start the debounce timer.
+    /// No run covers the request, start one now.
     Schedule,
-    /// A run or pending timer already covers the request.
+    /// A run is in flight and covers the request, fold it into a follow-up.
     Fold,
 }
 
@@ -19,28 +18,20 @@ impl RefreshGate {
         self.running
     }
 
-    pub fn debouncing(&self) -> bool {
-        self.debouncing
-    }
-
     /// A new refresh request arrived.
     ///
-    /// Folded into a follow-up run while one is in flight, dropped while the
-    /// debounce timer is pending, otherwise starts the timer.
+    /// Folded into a follow-up run while one is in flight, otherwise the
+    /// caller starts the run itself.
     pub fn request(&mut self) -> RefreshRequest {
         if self.running {
             self.dirty = true;
             RefreshRequest::Fold
-        } else if self.debouncing {
-            RefreshRequest::Fold
         } else {
-            self.debouncing = true;
             RefreshRequest::Schedule
         }
     }
 
     pub fn begin(&mut self) {
-        self.debouncing = false;
         self.running = true;
     }
 
@@ -55,3 +46,48 @@ impl RefreshGate {
         self.running = false;
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn a_request_while_running_folds_into_a_follow_up() {
+        let mut gate = RefreshGate::default();
+        gate.begin();
+
+        assert_eq!(gate.request(), RefreshRequest::Fold);
+        assert!(gate.finish());
+    }
+
+    #[test]
+    fn a_request_without_a_run_schedules() {
+        let mut gate = RefreshGate::default();
+
+        assert_eq!(gate.request(), RefreshRequest::Schedule);
+        assert!(!gate.running());
+    }
+
+    #[test]
+    fn a_request_after_a_run_schedules_again() {
+        let mut gate = RefreshGate::default();
+        gate.begin();
+        assert_eq!(gate.request(), RefreshRequest::Fold);
+        assert!(gate.finish());
+
+        assert_eq!(gate.request(), RefreshRequest::Schedule);
+    }
+
+    #[test]
+    fn abort_keeps_the_pending_request() {
+        let mut gate = RefreshGate::default();
+        gate.begin();
+        assert_eq!(gate.request(), RefreshRequest::Fold);
+
+        gate.abort();
+        assert!(!gate.running());
+
+        gate.begin();
+        assert!(gate.finish());
+    }
+}

crates/signed_state/src/repo.rs

@@ -17,7 +17,7 @@ use crate::backend::{
     user_grasp_list_servers,
 };
 use crate::checkouts::CheckoutsStore;
-use crate::git_store::GitStore;
+use crate::git_store::ensure_repo_mirror;
 use crate::refresh::{RefreshGate, RefreshRequest};
 use crate::repos::RepoListStore;
 
@@ -56,10 +56,6 @@ pub struct RepoStore {
     /// Computed with [`Self::status_by_root`] on every refresh.
     open_issue_count: usize,
     open_pr_count: usize,
-    /// Incremented on every applied refresh.
-    ///
-    /// Views key their derived-data caches to it instead of recomputing on every render.
-    version: u64,
     pub last_error: Option<String>,
     /// Non-fatal warning of the last action, if any.
     ///
@@ -125,7 +121,6 @@ impl RepoStore {
             status_by_root: HashMap::new(),
             open_issue_count: 0,
             open_pr_count: 0,
-            version: 0,
             last_error: None,
             last_warning: None,
             last_push_warning: None,
@@ -153,7 +148,6 @@ impl RepoStore {
             status_by_root: HashMap::new(),
             open_issue_count: 0,
             open_pr_count: 0,
-            version: 0,
             last_error: None,
             last_warning: None,
             last_push_warning: None,
@@ -401,9 +395,6 @@ impl RepoStore {
                 }
             }
 
-            // Cover notes, 1624, and label events, 1985, carry no `a` tag.
-            // Query them per root like comments and statuses.
-            // The events are only stored for interop and nothing displays them.
             sort_newest_first(&mut issues);
             sort_newest_first(&mut patches);
             sort_newest_first(&mut pull_requests);
@@ -522,7 +513,6 @@ impl RepoStore {
                 this.open_issue_count = open_issue_count;
                 this.open_pr_count = open_pr_count;
                 this.loaded = true;
-                this.version = this.version.wrapping_add(1);
 
                 // Comments and statuses without an `a` tag.
                 // None are addressed to the repository.
@@ -580,10 +570,6 @@ impl RepoStore {
         status_of(&self.status_by_root, root)
     }
 
-    pub fn version(&self) -> u64 {
-        self.version
-    }
-
     /// Number of open issues.
     ///
     /// Issues whose resolved status is [`RepoStatus::Open`].
@@ -1210,8 +1196,6 @@ impl RepoStore {
             return;
         }
 
-        let cache = GitStore::global(cx).cache().clone();
-
         let clone_urls: Vec<Url> = self
             .announcement
             .as_ref()
@@ -1237,7 +1221,7 @@ impl RepoStore {
         let root = root.clone();
 
         let apply = cx.background_spawn(async move {
-            let repo = cache.ensure_clone(&addr, &clone_urls)?;
+            let repo = ensure_repo_mirror(&addr, &clone_urls)?;
             let workdir = repo
                 .workdir()
                 .ok_or_else(|| anyhow::anyhow!("repository has no worktree"))?